More problems with `make -n' in automake-generated rules.

[Ralf Wildenhues]

I noticed more issues with automake-generated rules and `make -n':

1) The solutions documented in the `Multiple Outputs' node are not safe
for use with `make -n'.

2) Consequently, the lisp rules are broken, but also the Yacc, Vala, and
config.h rules in some cases.

3) The rules to update Makefile, but also those to update and
Makefile.in, are broken in some circumstances, too.

The patch below fixes the documentation and addresses (2).  The patch
doesn't add testsuite coverage for Vala because there are several
changes pending for this; I'm guessing the code will work differently
afterwards anyway.

I'm not sure how useful it is to fix (3).  It is not easy as a user to
get GNU make to not update any of the dependencies of the Makefile file,
thanks to its remaking feature (info make "Remaking Makefiles").  I'll
reply with a patch for the 'Makefile' rule, but in order to expose that
bug, you need to use something like this in a subdirectory of a package:
  make -n Makefile AM_MAKEFLAGS="-n Makefile"

I don't think users go to this extent just to have `make -n' work, and
they definitely won't get the above right on the first try; but then the
rebuild will already have kicked in, making the issue moot for the
second try.

Before applying this (to maint, probably) I would appreciate if someone
could look over it to make sure the patch looks sane.  Thanks.

Thanks,
Ralf

    Fix and document rules to not touch the tree with `make -n'.
    
    * doc/automake.texi (Multiple Outputs): Document the problem of
    modifications during dry-run execution, propose solution.
    * NEWS: Update.
    * automake.in (lang_vala_finish_target): Split recipe so the
    stamp file is not removed with GNU `make -n'.
    (lang_yacc_target_hook): Separate removal of parser output file
    and header remaking.
    * lib/am/lisp.am ($(am__ELCFILES)): Determine whether -n was
    passed to make, take care not to remove any files in that case.
    * lib/am/remake-hdr.am (%CONFIG_H%): Separate removal of
    %STAMP% file from induced remaking of config header.
    * tests/autohdrdry.test, tests/lispdry.test, tests/yaccdry.test:
    New tests.
    * tests/Makefile.am (TESTS): Update.

diff --git a/NEWS b/NEWS
index 64afd8c..7ce106a 100644
--- a/NEWS
+++ b/NEWS
@@ -99,6 +99,10 @@ Bugs fixed in 1.11a:
   - The order of Yacc and Lex flags is fixed to be consistent with other
     languages: $(AM_YFLAGS) comes before $(YFLAGS), and $(AM_LFLAGS) before
     $(LFLAGS), so that the user variables override the developer variables.
+
+  - Rules generated by Automake now try harder to not change any files when
+    `make -n' is invoked.  Fixes include compilation of Emacs Lisp, Vala, or
+    Yacc source files and the rule to update config.h.
 
 New in 1.11:
 
diff --git a/automake.in b/automake.in
index cb5fe24..42eff2b 100644
--- a/automake.in
+++ b/automake.in
@@ -6050,11 +6050,11 @@ sub lang_vala_finish_target ($$)
     {
       foreach my $file ($var->value_as_list_recursive)
         {
-          $output_rules .= "\$(srcdir)/$file: 
\$(srcdir)/${derived}_vala.stamp\n".
-            "address@hidden test -f \$@; then :; else \\\n".
-            "\t  rm -f \$(srcdir)/${derived}_vala.stamp; \\\n".
-            "\t  \$(am__cd) \$(srcdir) && \$(MAKE) \$(AM_MAKEFLAGS) 
${derived}_vala.stamp; \\\n".
-            "\tfi\n"
+          $output_rules .= "\$(srcdir)/$file: 
\$(srcdir)/${derived}_vala.stamp\n"
+            . "address@hidden test -f \$@; then :; else rm -f 
\$(srcdir)/${derived}_vala.stamp; fi\n"
+            . "address@hidden test -f \$@; then :; else \\\n"
+            . "\t  \$(am__cd) \$(srcdir) && \$(MAKE) \$(AM_MAKEFLAGS) 
${derived}_vala.stamp; \\\n"
+           . "\tfi\n"
             if $file =~ s/(.*)\.vala$/$1.c/;
         }
     }
@@ -6070,11 +6070,11 @@ sub lang_vala_finish_target ($$)
                                  '--vapi', '--internal-vapi', '--gir')))
            {
              my $headerfile = $flag;
-             $output_rules .= "\$(srcdir)/$headerfile: 
\$(srcdir)/${derived}_vala.stamp\n".
-               "address@hidden test -f \$@; then :; else \\\n".
-               "\t  rm -f \$(srcdir)/${derived}_vala.stamp; \\\n".
-               "\t  \$(am__cd) \$(srcdir) && \$(MAKE) \$(AM_MAKEFLAGS) 
${derived}_vala.stamp; \\\n".
-               "\tfi\n";
+             $output_rules .= "\$(srcdir)/$headerfile: 
\$(srcdir)/${derived}_vala.stamp\n"
+               . "address@hidden test -f \$@; then :; else rm -f 
\$(srcdir)/${derived}_vala.stamp; \n"
+               . "address@hidden test -f \$@; then :; else \\\n"
+               . "\t  \$(am__cd) \$(srcdir) && \$(MAKE) \$(AM_MAKEFLAGS) 
${derived}_vala.stamp; \\\n"
+               . "\tfi\n";
 
              # valac is not used when building from dist tarballs
              # distribute the generated files
@@ -6169,10 +6169,8 @@ sub lang_yacc_target_hook
            $output_rules .=
              "$condstr${header}: $output\n"
              # Recover from removal of $header
-             . "address@hidden test ! -f \$@; then \\\n"
-             . "$condstr\t  rm -f $output; \\\n"
-             . "$condstr\t  \$(MAKE) \$(AM_MAKEFLAGS) $output; \\\n"
-             . "$condstr\telse :; fi\n";
+             . "address@hidden test ! -f \$@; then rm -f $output; else :; fi\n"
+             . "address@hidden test ! -f \$@; then \$(MAKE) \$(AM_MAKEFLAGS) 
$output; else :; fi\n";
          }
        # Distribute the generated file, unless its .y source was
        # listed in a nodist_ variable.  (&handle_source_transform
diff --git a/doc/automake.texi b/doc/automake.texi
index d1cc4de..e107e2c 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -11421,11 +11421,15 @@ data.h data.w data.x: data.c
         fi
 @end example
 
-However there are now two minor problems in this setup.  One is related
+However there are now three minor problems in this setup.  One is related
 to the timestamp ordering of @file{data.h}, @file{data.w},
address@hidden, and @file{data.c}.  The other one is a race condition
address@hidden, and @file{data.c}.  Another one is a race condition
 if a parallel @command{make} attempts to run multiple instances of the
-recover block at once.
+recover block at once.  Finally, the recursive rule breaks @samp{make -n}
+when run with GNU @command{make} (as well as some other @command{make}
+implementations), as it may remove @file{data.h} even when it should not
+(@pxref{MAKE Variable, , How the @code{MAKE} Variable Works, make,
+The GNU Make Manual}).
 
 Let us deal with the first problem.  @command{foo} outputs four files,
 but we do not know in which order these files are created.  Suppose
@@ -11533,8 +11537,8 @@ elc-stamp: $(ELFILES)
         @@mv -f elc-temp $@@
 
 $(ELCFILES): elc-stamp
-## Recover from the removal of $@@
         @@if test -f $@@; then :; else \
+## Recover from the removal of $@@
           trap 'rm -rf elc-lock elc-stamp' 1 2 13 15; \
           if mkdir elc-lock 2>/dev/null; then \
 ## This code is being executed by the first process.
@@ -11547,10 +11551,58 @@ $(ELCFILES): elc-stamp
             while test -d elc-lock; do sleep 1; done; \
 ## Succeed if and only if the first process succeeded.
             test -f elc-stamp; exit $$?; \
address@hidden $$
           fi; \
         fi
 @end example
+
+These solutions all still suffer from the third problem, namely that
+they break the promise that @samp{make -n} should not cause any actual
+changes to the tree.  For those solutions that do not create lock files,
+it is possible to split the recover rules into two separate recipe
+commands, one of which does all work but the recursion, and the
+other invokes the recursive @samp{$(MAKE)}.  The solutions involving
+locking could act upon the contents of the @samp{MAKEFLAGS} variable,
+but parsing that portably is not easy (@pxref{The Make Macro MAKEFLAGS,,,
+autoconf, The Autoconf Manual}).  Here is an example:
+
address@hidden
+ELFILES = one.el two.el three.el @dots{}
+ELCFILES = $(ELFILES:=c)
+
+elc-stamp: $(ELFILES)
+        @@rm -f elc-temp
+        @@touch elc-temp
+        $(elisp_comp) $(ELFILES)
+        @@mv -f elc-temp $@@
+
+$(ELCFILES): elc-stamp
+## Recover from the removal of $@@
+        @@dry=; for f in x $$MAKEFLAGS; do \
+          case $$f in \
+            *=*|--*);; \
+            *n*) dry=:;; \
+          esac; \
+        done; \
+        if test -f $@@; then :; else \
+          $$dry trap 'rm -rf elc-lock elc-stamp' 1 2 13 15; \
+          if $$dry mkdir elc-lock 2>/dev/null; then \
+## This code is being executed by the first process.
+            $$dry rm -f elc-stamp; \
+            $(MAKE) $(AM_MAKEFLAGS) elc-stamp; \
+            $$dry rmdir elc-lock; \
+          else \
+## This code is being executed by the follower processes.
+## Wait until the first process is done.
+            while test -d elc-lock && test -z "$$dry"; do \
 @c $$
+              sleep 1; \
+            done; \
+## Succeed if and only if the first process succeeded.
+            $$dry test -f elc-stamp; exit $$?; \
+          fi; \
+        fi
address@hidden example
 
 For completeness it should be noted that GNU @command{make} is able to
 express rules with multiple output files using pattern rules
diff --git a/lib/am/lisp.am b/lib/am/lisp.am
index 3449d18..ab45b30 100644
--- a/lib/am/lisp.am
+++ b/lib/am/lisp.am
@@ -1,6 +1,6 @@
 ## automake - create Makefile.in from Makefile.am
 ## Copyright (C) 1996, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006,
-## 2007, 2008, 2009 Free Software Foundation, Inc.
+## 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
 
 ## This program is free software; you can redistribute it and/or modify
 ## it under the terms of the GNU General Public License as published by
@@ -49,7 +49,16 @@ $(am__ELCFILES): elc-stamp
 ##
 ## Do not call `make elc-stamp' if emacs is not available, because it would
 ## be useless.
-       @if test "$(EMACS)" != no && test ! -f $@; then \
+##
+## If `make -n' is called, do not execute any command in the recipe that
+## changes the tree; however, invoke the recursive make for debuggability.
+       @dry=; for f in x $$MAKEFLAGS; do \
+         case $$f in \
+           *=*|--*);; \
+           *n*) dry=:;; \
+         esac; \
+       done; \
+       if test "$(EMACS)" != no && test ! -f $@; then \
 ## If `make -j' is used and more than one file has been erased, several
 ## processes can execute this block.  We have to make sure that only
 ## the first one will run `$(MAKE) $(AM_MAKEFLAGS) elc-stamp', and the
@@ -58,18 +67,18 @@ $(am__ELCFILES): elc-stamp
 ## There is a race here if only one child of make receive a signal.
 ## In that case the build may fail.  We remove elc-stamp when we receive
 ## a signal so we are sure the build will succeed the next time.
-         trap 'rm -rf elc-lock elc-stamp' 1 2 13 15; \
-         if mkdir elc-lock 2>/dev/null; then \
+         $$dry trap 'rm -rf elc-lock elc-stamp' 1 2 13 15; \
+         if $$dry mkdir elc-lock 2>/dev/null; then \
 ## This code is being executed by the first process.
-           rm -f elc-stamp; \
+           $$dry rm -f elc-stamp; \
            $(MAKE) $(AM_MAKEFLAGS) elc-stamp; \
-           rmdir elc-lock; \
+           $$dry rmdir elc-lock; \
          else \
 ## This code is being executed by the follower processes.
 ## Wait until the first process is done.
-           while test -d elc-lock; do sleep 1; done; \
+           while test -d elc-lock && test -z "$$dry"; do sleep 1; done; \
 ## Succeed if and only if the first process succeeded.
-           test -f elc-stamp; exit $$?; \
+           $$dry test -f elc-stamp; exit $$?; \
          fi; \
        else : ; fi
 
diff --git a/lib/am/remake-hdr.am b/lib/am/remake-hdr.am
index 5077be2..35f4a46 100644
--- a/lib/am/remake-hdr.am
+++ b/lib/am/remake-hdr.am
@@ -1,6 +1,6 @@
 ## automake - create Makefile.in from Makefile.am
 ## Copyright (C) 1994, 1995, 1996, 1997, 1998, 1999, 2001, 2003, 2004, 2005,
-## 2008, 2009 Free Software Foundation, Inc.
+## 2008, 2009, 2010 Free Software Foundation, Inc.
 
 ## This program is free software; you can redistribute it and/or modify
 ## it under the terms of the GNU General Public License as published by
@@ -20,10 +20,8 @@
 
 %CONFIG_H%: %STAMP%
 ## Recover from removal of CONFIG_HEADER
-       @if test ! -f $@; then \
-         rm -f %STAMP%; \
-         $(MAKE) $(AM_MAKEFLAGS) %STAMP%; \
-       else :; fi
+       @if test ! -f $@; then rm -f %STAMP%; else :; fi
+       @if test ! -f $@; then $(MAKE) $(AM_MAKEFLAGS) %STAMP%; else :; fi
 
 
 %STAMP%: %CONFIG_H_DEPS% $(top_builddir)/config.status
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 66accd4..bc2917f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -118,6 +118,7 @@ autohdr.test \
 autohdr2.test \
 autohdr3.test \
 autohdr4.test \
+autohdrdry.test \
 automake.test \
 auxdir.test \
 auxdir2.test \
@@ -495,6 +496,7 @@ lisp5.test \
 lisp6.test \
 lisp7.test \
 lisp8.test \
+lispdry.test \
 listval.test \
 location.test \
 longline.test \
@@ -839,6 +841,7 @@ yacc5.test \
 yacc6.test \
 yacc7.test \
 yacc8.test \
+yaccdry.test \
 yaccflags.test \
 yaccpp.test \
 yaccvpath.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 30425b0..b61383d 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -356,6 +356,7 @@ autohdr.test \
 autohdr2.test \
 autohdr3.test \
 autohdr4.test \
+autohdrdry.test \
 automake.test \
 auxdir.test \
 auxdir2.test \
@@ -733,6 +734,7 @@ lisp5.test \
 lisp6.test \
 lisp7.test \
 lisp8.test \
+lispdry.test \
 listval.test \
 location.test \
 longline.test \
@@ -1077,6 +1079,7 @@ yacc5.test \
 yacc6.test \
 yacc7.test \
 yacc8.test \
+yaccdry.test \
 yaccflags.test \
 yaccpp.test \
 yaccvpath.test \
diff --git a/tests/autohdrdry.test b/tests/autohdrdry.test
new file mode 100755
index 0000000..1631005
--- /dev/null
+++ b/tests/autohdrdry.test
@@ -0,0 +1,42 @@
+#!/bin/sh
+# Copyright (C) 2010 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Removal recovery rules for AC_CONFIG_HEADERS should not remove files
+# with `make -n'.
+
+. ./defs
+
+set -e
+
+cat >>configure.in <<'EOF'
+AC_PROG_CC
+AC_CONFIG_HEADERS([config.h])
+AC_OUTPUT
+EOF
+
+: >Makefile.am
+
+$ACLOCAL
+$AUTOCONF
+$AUTOHEADER
+$AUTOMAKE
+
+./configure
+$MAKE
+
+rm -f config.h
+$MAKE -n
+test -f stamp-h1
diff --git a/tests/lispdry.test b/tests/lispdry.test
new file mode 100755
index 0000000..9eea752
--- /dev/null
+++ b/tests/lispdry.test
@@ -0,0 +1,62 @@
+#! /bin/sh
+# Copyright (C) 2005, 2008, 2010 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GNU Automake; see the file COPYING.  If not, write to
+
+# Check that `make -n' works with the lisp_LISP recover rule.
+
+required='emacs non-root'
+. ./defs || Exit 1
+
+set -e
+
+cat > Makefile.am << 'EOF'
+dist_lisp_LISP = am-one.el am-two.el am-three.el
+EOF
+
+cat >> configure.in << 'EOF'
+AM_PATH_LISPDIR
+AC_OUTPUT
+EOF
+
+echo "(require 'am-two)" > am-one.el
+echo "(require 'am-three) (provide 'am-two)" > am-two.el
+echo "(provide 'am-three)" > am-three.el
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE --add-missing
+./configure
+
+$MAKE
+
+test -f am-one.elc
+test -f am-two.elc
+test -f am-three.elc
+test -f elc-stamp
+
+rm -f am-*.elc elc-stamp
+
+chmod a-w .
+
+$MAKE -n
+
+test ! -f am-one.elc
+test ! -f am-two.elc
+test ! -f am-three.elc
+test ! -f elc-stamp
+
+chmod u+w .
+
+:
diff --git a/tests/yaccdry.test b/tests/yaccdry.test
new file mode 100755
index 0000000..aab43a1
--- /dev/null
+++ b/tests/yaccdry.test
@@ -0,0 +1,58 @@
+#! /bin/sh
+# Copyright (C) 2010 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Removal recovery rules for headers should not remove files with `make -n'.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_PROG_CC
+AC_PROG_YACC
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+AM_YFLAGS = -d
+bin_PROGRAMS = foo
+foo_SOURCES = foo.c parse.y
+END
+
+cat > foo.c << 'END'
+int main () { return 0; }
+END
+
+cat > parse.y << 'END'
+%{
+int yylex () {return 0;}
+void yyerror (char *s) {}
+%}
+%%
+foobar : 'f' 'o' 'o' 'b' 'a' 'r' {};
+END
+
+$ACLOCAL
+$AUTOMAKE --add-missing
+$AUTOCONF
+./configure
+$MAKE
+
+rm -f parse.h
+$MAKE -n parse.h
+test -f parse.c
+
+: