Re: order of variables and rules in output

[Ralf Wildenhues]

Hi Ben,

a while ago on the automake list:

* Ben Pfaff wrote on Wed, Aug 25, 2010 at 07:22:58PM CEST:
> In a Makefile, the relative order of variable and rule
> definitions matters, because variables used in targets are
> expanded when rules are read.  If a variable is used in a target
> before the variable is changed, the variable's former expansion
> is used in the target, not the latter expansion.
> 
> Current Automake appears to reorder the Makefile.am so that all
> variable assignments precede all rules, so in a Makefile.am the
> relative order of variable and rule definitions does not matter.
> 
> Is this behavior documented?  Is it guaranteed to remain in the
> future?  I could not easily find mention of it in the manual, but
> it is sometimes convenient.

While looking for a good place to add this to the documentation, I
wondered whether we really consistently do this right now, and whether
we should be checking that somehow.

So here's a test: we shouldn't encounter a variable initialization after
the first rule, and a newline-tab sequence not escaped by a backslash is
a fairly good if heuristic indicator for being inside a rule.

The output contains variable initializations of the parallel-tests
driver part of check.am.  Somehow the parser thinks am__check_post is a
rule rather than a variable and fails to reorder it.  I'm replacing some
of the colons in that scriptlet, to avoid the failure for now.  Actually
fixing the parser would be better, if someone makes a safe fix.

The first patch is for maint, the maintainer-check coverage for master
only (since it depends on another feature in master).

Thanks,
Ralf

    Document and fix expansion of variables before rules.
    
    * doc/automake.texi (General Operation): Document that variables
    are expanded before rules.
    * lib/am/check.am (am__check_post): Reword a bit so it does not
    get matched as a rule.
    Suggestion by Ben Pfaff.

diff --git a/doc/automake.texi b/doc/automake.texi
index e3589d8..8848dcd 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -1763,7 +1763,8 @@ General Operation
 @trindex git-dist
 
 The variable definitions and rules in the @file{Makefile.am} are
-copied verbatim into the generated file.  This allows you to add
+copied mostly verbatim into the generated file, with all variable
+definitions preceding all rules.  This allows you to add almost
 arbitrary code into the generated @file{Makefile.in}.  For instance,
 the Automake distribution includes a non-standard rule for the
 @code{git-dist} target, which the Automake maintainer uses to make
diff --git a/lib/am/check.am b/lib/am/check.am
index c612b22..c953be8 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -128,13 +128,13 @@ case " $(XFAIL_TESTS) " in                                
\
   *[\ \        ]$$f[\ \        ]* | *[\ \      ]$$dir$$f[\ \   ]*) \
     xfailed=XFAIL;;                                    \
 esac;                                                  \
-case $$estatus:$$xfailed in                            \
-    0:XFAIL) col=$$red; res=XPASS;;                    \
-    0:*)     col=$$grn; res=PASS ;;                    \
-    77:*)    col=$$blu; res=SKIP ;;                    \
-    99:*)    col=$$red; res=FAIL ;;                    \
-    *:XFAIL) col=$$lgn; res=XFAIL;;                    \
-    *:*)     col=$$red; res=FAIL ;;                    \
+case $$estatus.$$xfailed in                            \
+    0.XFAIL) col=$$red; res=XPASS;;                    \
+    0.*)     col=$$grn; res=PASS ;;                    \
+    77.*)    col=$$blu; res=SKIP ;;                    \
+    99.*)    col=$$red; res=FAIL ;;                    \
+    *.XFAIL) col=$$lgn; res=XFAIL;;                    \
+    *.*)     col=$$red; res=FAIL ;;                    \
 esac;                                                  \
 echo "$${col}$$res$${std}: $$f";                       \
 echo "$$res: $$f (exit: $$estatus)" |                  \



    maintainer-check coverage for variables before rules.
    
    * Makefile.am (sc_ensure_testsuite_has_run): Suggest keeping
    around the test directories.
    (sc_tests_makefile_variable_order): New rule with a heuristic to
    catch ordering violations.

diff --git a/Makefile.am b/Makefile.am
index 8eb1bde..a5df109 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -195,6 +195,7 @@ sc_tests_plain_sleep \
 sc_tests_plain_egrep_fgrep \
 sc_tests_PATH_SEPARATOR \
 sc_tests_logs_duplicate_prefixes \
+sc_tests_makefile_variable_order \
 sc_mkdir_p \
 sc_perl_at_substs \
 sc_unquoted_DESTDIR \
@@ -467,9 +468,10 @@ sc_tests_plain_egrep_fgrep:
 ## Rule to ensure that the testsuite has been run before.  We don't depend on 
`check'
 ## here, because that would be very wasteful in the common case.  We could run
 ## `make check RECHECK_LOGS=' and avoid toplevel races with 
AM_RECURSIVE_TARGETS.
+## Suggest keeping test directories around for greppability of the Makefile.in 
files.
 sc_ensure_testsuite_has_run:
        @if test ! -f tests/test-suite.log; then \
-         echo "Run \`make check' before \`maintainer-check'" >&2; \
+         echo "Run \`env keep_testdirs=yes make check' before 
\`maintainer-check'" >&2; \
          exit 1; \
        fi
 .PHONY: sc_ensure_testsuite_has_run
@@ -482,6 +484,24 @@ sc_tests_logs_duplicate_prefixes: 
sc_ensure_testsuite_has_run
          exit 1; \
        fi
 
+## Ensure variables are listed before rules in Makefile.in files we generate.
+sc_tests_makefile_variable_order: sc_ensure_testsuite_has_run
+       @for file in `find tests -name Makefile.in -print`; do \
+         latevars=`sed -n \
+           -e :x -e 's/#.*//' \
+           -e '/\\\\$$/{' -e N -e 'b x' -e '}' \
+## Literal TAB.
+           -e '1,/^    /d' \
+## Allow @ so we match conditionals.
+           -e '/^ address@hidden,\} *=/p' $$file`; \
+         if test -n "$$latevars"; then \
+           echo 'Ensure variables are expanded before rules' >&2; \
+           echo "Variables are expanded too late in $$file:" >&2; \
+           echo "$$latevars" | sed 's/^/  /' >&2; \
+           exit 1; \
+         fi; \
+       done
+
 ## Using `:' as a PATH separator is not portable.
 sc_tests_PATH_SEPARATOR:
        @if grep -E '\bPATH=.*:.*' $(srcdir)/tests/*.test ; then \