lots of fatal build system bugs

[Ingo Schwarze]

Hi Branden,

after a (too) long time, i once more attempted to update my private
version of the OpenBSD groff port to the current git head, to avoid
a disaster when the next release finally appears - and got
completely stuck for several hours because there are large
numbers of portability issues that prevent building.

The current state is that i have a patch that barely makes it build.
I don't think the build is clean yet even with the patch, i have
seen indications of non-fatal issues at a few places, but did not yet
investigate these, let alone scrutinize the build logs as i usually do.
Also, i did not start any run time testing yet.

There are so many issues already now that it makes sense to get
these fixed before even starting a review for cleanliness and
before starting run time testing.

I'm sending this to you directly because you are a by far the
most active groff developer right now, so i hope we can agree
how to fix all this and then i can then push one or more patches
doing it.  This really doesn't feel like the stuff to shove under
the bugtracker carpet and let it rot there.

Please be assured that i'm *not* claiming you are responsible for
any of these regressions; that's *not* the reason why i'm sending
this to you.  Quite to the contrary, the reason is that i appreciate
your industriousness working on groff.

So, let's go through the patch appended at the end.
It's a minimal patch to make the software build.

 1. File Makefile.am, .version.tree mechanism
    There are several independent problems with this.

   1.1. .version.tree target line:
        automake parses this as a suffix rule and consequently
        adds .version and .tree to the .SUFFIXES pseudo-target.
        With the resulting ".SUFFIXES: .version .tree ..."  in the
        generated master Makefile, BSD make(1) no longer knows how to
        build the file ".version.tree" because the rule intended as
        a target rule is actually a suffix rule.  So that rule does
        explain how to build foo.tree from foo.version (for example,
        .version.tree from .version.version), but since there is no
        .version.version anywhere, BSD make dies complaining that it
        doesn't know how to build .version.tree.  GNU make somehow
        copes, but i have no idea why.
        So at the very least, the file .version.tree needs to be
        renamed such that its target rule does not look like a
        suffix rule, for example to ".version_tree".

   1.2. src/libs/libgroff/version.cpp: $(top_srcdir)/.version
        dependency in src/libs/libgroff/libgroff.am:
           In this respect, BSD make make does the following.
        When considering whether version.cpp needs to be rebuilt,
        it first considers whether $(top_srcdir)/.version is
        up to date.  Obviously, it is not, because it depends
        on the .PHONY target .version_tree, so every make runs the
        shell script snippet to rebuild $(top_srcdir)/.version.
        I find it quite logical that BSD make then goes ahead
        and also rebuilds version.cpp.  After all, that one had
        a dependency that was found to be out of date and a script
        was run to recreate it.  The end result is that during
        "make install", large swaths of the tree are rebuilt
        from scratch because lots of stuff depends on version.cpp.

        GNU make copes by being somewhat schizophrenic.
        First, it does believe that inspecting
        whether $(top_srcdir)/.version is up to date matters
        for deciding whether to rebuild version.cpp,
        finds it is out of date and runs the script to rebuild it.
        But then, it seems to forget all that it did so far,
        and adopts a new personality now assuming
        that $(top_srcdir)/.version is *not* out of date (even
        though a script was just run because it is).  Perfect
        double-think: GNU make firmly believes .version is out
        of date and is up to date at the same time.
        In any case, relying on that split personality is not
        portable.

   1.3. In any case, automatically regenerating .version
        from the Makefile defeats the whole purpose of the file,
        which is documented in build-aux/git-version-gen
        (not build-aux/git-gen-version) as follows:

# .version - present in a checked-out repository and in a distribution
#   tarball.  Usable in dependencies, particularly for files that don't
#   want to depend on config.h but do want to track version changes.
#   Delete this file prior to any autoconf run where you want to rebuild
#   files to pick up a version string change; and leave it stale to
#   minimize rebuild time after unrelated changes to configure sources.

        It says "and leave it stale" otherwise, so it is
        intentional and documented that it should *not* be
        automatically changed.  Also, build-aux/git-version-gen
        explicitly documents how the file is intended to be used:

# EXTRA_DIST = $(top_srcdir)/.version
# BUILT_SOURCES = $(top_srcdir)/.version
# $(top_srcdir)/.version:
#       echo $(VERSION) > $@-t && mv $@-t $@
# dist-hook:
#       echo $(VERSION) > $(distdir)/.tarball-version

        Second-guessing that and adding complicated, non-portable
        dependencies seems like a bad idea to me.  Certainly,
        just following what build-aux/git-version-gen recommends
        looks like the simplest and cleanest fix to me.

 2. contrib/sboxes/sboxes.am, 1st chunk (SBOXES_PROCESSEDEXAMPLEFILES)
    This one was hellish to figure out.  The problem is that it
    results in an empty rule
      contrib/sboxes/msboxes.ms contrib/sboxes/msboxes.pdf: yada yada
        no shell commands following
    whitout leading "./" whereas the later $(sboxes_builddir)/msboxes.ms
    target rule results in a rule
      ./contrib/sboxes/msboxes.ms: yada yada
         some shell commands here
    with a leading "./".  BSD make considers "contrib/sboxes/msboxes.ms"
    and "./contrib/sboxes/msboxes.ms" different targets and does *not*
    combine these rules, so when it comes to building the target,
    it uses the empty shell script and vonsequently the file never
    gets built.

    I believe the best fix is to always use $(sboxes_builddir)
    for the generated files rather than switching back and forth.

 3. contrib/sboxes/sboxes.am, 2nd chunk (msboxes.pdf)
    The DOC_GROFF mechanism is seriously complicated, and one of
    its many features is that it uses the variable $< which POSIX
    says can only be used in suffix rules, so this fails outright
    with POSIX make.
    On top of that, the whole point of DOC_GROFF is to pipe
    standard input into DOC_GROFF_ONLY which then calls groff.
    So it makes no sense whatsoever to add a file name argument
    (in this case, "$(sboxes_builddir)/msboxes.ms") after
    DOC_GROFF because that causes standard input to be discarded,
    neutering most of what DOC_GROFF did earlier.
    Fortunately, there is a simple way to fix both issues at the
    same time: change the rule to become a suffix rule
    and remove the bogus argument.
    Alternatively, one could keep the argument, change DOC_GROFF
    to DOC_GROFF_ONLY, and not switch to a suffix rule; i have
    no strong preference.

 4. doc/doc.am, doc/groff-man-pages.* rules:
    Again, we have $< in non-suffix rules, this time not only
    with one, but with many file name arguments.  Amazingly,
    GNU make somehow copes.  Not that what GNU make does makes
    any sense - it appears the $< picks the first dependency
    and silently discards all the others, which is obviously
    the perfect way to shove errors under the carpet - but at
    least it doesn't crash like POSIX make does!  ;-)
    Then again, all that resulted from $< is discarded anyway... :-(

    So here, i think the best fix is to just be explicit and
    switch from DOC_GROFF to DOC_GROFF_ONLY.

 5. doc/doc.am, doc/meintro.*, doc/meref.me rules:
    More $< GNUisms.
    The best fix here seems to replace $< by the POSIX $?.

 6. doc/doc.am, doc/meintro_fr.ps rule:
    Yet another $< GNUism.
    This is harder to fix because an ".me.ps" suffix rule
    lacking the -mfr already exists in doc/doc.am and clashes.
    But with the right "SUFFIXES +=" incantation to please automake,
    we can get a "_fr.me._fr.ps:" work in a portable way.
    Alternatively, we could switch to DOC_GROFF_ONLY instead;
    again, no strong preference.

 7. doc/doc.am, manual SUFFIXES definition:
    This is the only item on my list that isn't required to make
    it build, but i had to do so much wrestling with the .SUFFIXES
    pseudo-target anyway that i cleaned this up while there.
    The .xhtml is not used anywhere, for nothing whatsoever.
    All the others get automatically generated by automake,
    so there is no need to manually add them.
    In fact, maually adding them is detrimental in two ways:
    The manual list is doomed to get outdated (.xhtml proves
    the point) and it adds noise to the generated Makefile,
    hindering debugging (which is already difficult enough in
    such generated Makefiles without additional, pointless
    distractions).

So, here are my questions:

 I. Do you agree with all my points, or did i miss something,
    somewhere?

 II. When you apply my patch, do things still work for you on Linux?

 III. Do you prefer that i commit this in one single commit
      or separately?  If separately, do you want somme issues
      grouped together, for example
       * one commit for issue 1
       * one commit for issue 2
       * one commit for issue 3
       * one commit for issues 4-6
       * one commit for issue 7

 IV. I'm pretty sure these are all recent regressions and none
     of these problems existed in the last release.  Consequently,
     do you agree that ChangeLog entries are not needed?

Yours,
  Ingo

P.S.
Right now, i'm quite fed up because i had to do at least one bootstrap -
configure - build cyle for each of the issues, and some of them weren't
even identified until i did a complete bootstrap - configure - build -
make-dist - port-patch - port-configure - port-built - port-fake-install
cycle, and then i had to repeat all these cycles to develop and test
fixes, and in between, some required quite some staring at Makefiles
to understand.  At one point i even patched additional printf(3)
statements into our BSD make(1) implementation for debugging and stepped
through make(1) with gdb because at first i failed to understand from
mere "make -d m" debugging output what was really going on.  Ugh!

But i believe i'll get over it and do a full audit for cleanliness plus
run-time testing on some other day, once these nasty issues are out of
the way.
It would no doubt be better if i tested more often and did not let
so many issues accumulate.  :-/


diff --git a/Makefile.am b/Makefile.am
index 483efe46f..30180d774 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -921,20 +921,11 @@ SUFFIXES += .man
             $< \
             >$@
 
-# Version files - see script 'build-aux/git-gen-version'
+# Version files - see script 'build-aux/git-version-gen'
 EXTRA_DIST += $(top_srcdir)/.version
 BUILT_SOURCES += $(top_srcdir)/.version
-# Regenerate a temporary version string file on every build, but update
-# the real version file's mtime only if its contents change.
-.PHONY: .version.tree
-.version.tree:
-       @test -f $(distdir)/.tarball-verion || echo $(VERSION) > $@
-$(top_srcdir)/.version: .version.tree
-       @if ! test -f $(distdir)/.tarball-version; then \
-         test -f $@ || cp .version.tree $@; \
-         cmp -s .version.tree $@ || cp .version.tree $@; \
-       fi
-MOSTLYCLEANFILES += .version.tree
+$(top_srcdir)/.version:
+       echo $(VERSION) > $@-t && mv $@-t $@
 dist-hook:
        echo $(VERSION) > $(distdir)/.tarball-version
 
diff --git a/contrib/sboxes/sboxes.am b/contrib/sboxes/sboxes.am
index 61d814f7b..66afef382 100644
--- a/contrib/sboxes/sboxes.am
+++ b/contrib/sboxes/sboxes.am
@@ -36,8 +36,8 @@ SBOXES_EXAMPLEFILES = $(sboxes_srcdir)/msboxes.ms.in
 if BUILD_EXAMPLES
 sboxesexampledir = $(exampledir)/sboxes
 dist_sboxesexample_DATA = $(SBOXES_EXAMPLEFILES)
-SBOXES_PROCESSEDEXAMPLEFILES = contrib/sboxes/msboxes.ms \
-    contrib/sboxes/msboxes.pdf
+SBOXES_PROCESSEDEXAMPLEFILES = $(sboxes_builddir)/msboxes.ms \
+    $(sboxes_builddir)/msboxes.pdf
 sboxesprocessedexampledir = $(exampledir)/sboxes
 nodist_sboxesprocessedexample_DATA = $(SBOXES_PROCESSEDEXAMPLEFILES)
 else
@@ -62,9 +62,8 @@ $(sboxes_builddir)/msboxes.ms: $(SBOXES_EXAMPLEFILES) 
$(sboxesnotquine)
            $(SBOXES_EXAMPLEFILES) >> $@.tmp
        $(AM_V_GEN)mv $@.tmp $@
 
-$(sboxes_builddir)/msboxes.pdf: $(sboxes_builddir)/msboxes.ms
-       $(GROFF_V)$(DOC_GROFF) -M$(sboxes_srcdir) -ms -msboxes -Tpdf \
-         $(sboxes_builddir)/msboxes.ms > $@
+.ms.pdf:
+       $(GROFF_V)$(DOC_GROFF) -M$(sboxes_srcdir) -ms -msboxes -Tpdf > $@
 
 uninstall_groffdirs: uninstall_sboxes
 uninstall_sboxes:
diff --git a/doc/doc.am b/doc/doc.am
index 44f8c73e8..5f0103f3c 100644
--- a/doc/doc.am
+++ b/doc/doc.am
@@ -229,37 +229,36 @@ man-clean:
        $(RM) $(GROFF_MAN_PAGES_ALL)
 
 doc/groff-man-pages.pdf: $(GROFF_MAN_PAGES_ALL)
-       $(GROFF_V)$(DOC_GROFF) -Tpdf -P-e -mandoc -rC1 \
+       $(GROFF_V)$(DOC_GROFF_ONLY) -Tpdf -P-e -mandoc -rC1 \
          -rCHECKSTYLE=3 $(GROFF_MAN_PAGES1) \
          $(tmac_srcdir)/sv.tmac $(GROFF_MAN_PAGES2) \
          $(tmac_srcdir)/en.tmac $(GROFF_MAN_PAGES3) > $@
 
 doc/groff-man-pages.utf8.txt: $(GROFF_MAN_PAGES_ALL)
-       $(GROFF_V)$(DOC_GROFF) -Tutf8 -mandoc \
+       $(GROFF_V)$(DOC_GROFF_ONLY) -Tutf8 -mandoc \
          -rCHECKSTYLE=3 $(GROFF_MAN_PAGES1) \
          $(tmac_srcdir)/sv.tmac $(GROFF_MAN_PAGES2) \
          $(tmac_srcdir)/en.tmac $(GROFF_MAN_PAGES3) > $@
 
 doc/meintro.me: $(doc_srcdir)/meintro.me.in
        $(GROFF_V)$(MKDIR_P) `dirname $@` \
-       && $(DOC_SED) $< >$@
+       && $(DOC_SED) $? >$@
 
 doc/meintro_fr.me: $(doc_srcdir)/meintro_fr.me.in
        $(GROFF_V)$(MKDIR_P) `dirname $@` \
-       && $(DOC_SED) $< >$@
+       && $(DOC_SED) $? >$@
 
 doc/meref.me: $(doc_srcdir)/meref.me.in
        $(GROFF_V)$(MKDIR_P) `dirname $@` \
-       && $(DOC_SED) $< >$@
+       && $(DOC_SED) $? >$@
 
 # The me(7) intro French translation gets its own target rule because it
 # needs the "-mfr" option.
-doc/meintro_fr.ps: doc/meintro_fr.me
+SUFFIXES += _fr.me _fr.ps
+_fr.me._fr.ps:
        $(GROFF_V)$(MKDIR_P) `dirname $@` \
        && $(DOC_GROFF) -k -Tps -me -mfr >$@
 
-SUFFIXES += .me .ms .ps .html .txt .texi .dvi .pdf .xhtml
-
 # For simplicity, we always call preconv, grn, and eqn.
 .me.ps:
        $(GROFF_V)$(MKDIR_P) `dirname $@` \