Hello Ralf. Again, just a couple of nits w.r.t. the test cases... On Monday 01 November 2010, Ralf Wildenhues wrote: > * Ralf Wildenhues wrote on Mon, Nov 01, 2010 at 10:18:55PM CET: > > 3) The rules to update Makefile, but also those to update and > > Makefile.in, are broken in some circumstances, too. > [...] > > 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. > > Here it is. > > Cheers, > Ralf > > Fix GNU `make -n Makefile' in subdirs to not touch the tree. > > * lib/am/configure.am (%MAKEFILE%): Split rule in nonrecursive > and possibly-recursive parts, for GNU `make -n'. > * tests/remakedry.test: New test. > * tests/Makefile.am (TESTS): Update. > * NEWS: Update. > > diff --git a/NEWS b/NEWS > index 6bc0d6f..76e6543 100644 > --- a/NEWS > +++ b/NEWS > @@ -48,7 +48,7 @@ Bugs fixed in 1.11.0a: > > - 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. > + Yacc source files and the rules to update config.h and a subdir Makefile. > > New in 1.11: > > diff --git a/lib/am/configure.am b/lib/am/configure.am > index e9299d6..c10b377 100644 > --- a/lib/am/configure.am > +++ b/lib/am/configure.am > @@ -1,5 +1,5 @@ > ## automake - create Makefile.in from Makefile.am > -## Copyright (C) 2001, 2002, 2003, 2004, 2006, 2007, 2008, 2009 Free > +## Copyright (C) 2001, 2002, 2003, 2004, 2006, 2007, 2008, 2009, 2010 Free > ## Software Foundation, Inc. > > ## This program is free software; you can redistribute it and/or modify > @@ -75,12 +75,16 @@ endif %?TOPDIR_P% > ?TOPDIR_P? echo ' $(SHELL) ./config.status'; \ > ?TOPDIR_P? $(SHELL) ./config.status;; \ > ?!TOPDIR_P? cd $(top_builddir) && $(MAKE) $(AM_MAKEFLAGS) am--refresh;; > \ > + esac > +## Split the nonrecursive part off, so GNU `make -n' will not execute it. > + @case '$?' in \ > + *config.status*) ;;\ > *) \ > ## FIXME: $(am__depfiles_maybe) lets us re-run the rule to create the > ## .P files. Ideally we wouldn't have to do this by hand. > echo ' cd $(top_builddir) && $(SHELL) ./config.status > %CONFIG-MAKEFILE% $(am__depfiles_maybe)'; \ > cd $(top_builddir) && $(SHELL) ./config.status %CONFIG-MAKEFILE% > $(am__depfiles_maybe);; \ > - esac; > + esac > > DIST_COMMON += %MAKEFILE-AM% > > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 9c81564..a9aca1e 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -619,6 +619,7 @@ remake4.test \ > remake5.test \ > remake6.test \ > remake7.test \ > +remakedry.test \ > regex.test \ > req.test \ > reqd.test \ > diff --git a/tests/remakedry.test b/tests/remakedry.test > new file mode 100755 > index 0000000..dab46f3 > --- /dev/null > +++ b/tests/remakedry.test > @@ -0,0 +1,82 @@ > +#! /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/>. > + > +# Make sure GNU `make -n' doesn't trigger file updates when Makefile > +# is out of date. > + > +# The subdir rebuilding rules rely on GNU make automatically updating > +# the makefile and its prerequisites (through am--refresh). > +required=GNUmake Why requiring GNU make here? After all, if a make implementation does not implement automatic Makefile-rebuild rules, it should trivially pass this test, and if implements them, it makes sense to run this test on it too. > +. ./defs || Exit 1 > + > +set -e > + > +cat >> configure.in << 'END' > +AC_CONFIG_FILES([sub/Makefile]) > +AC_OUTPUT > +END > + > +cat > Makefile.am <<'END' > +SUBDIRS = sub > +END > +mkdir sub > +: > sub/Makefile.am > + > +$ACLOCAL > +$AUTOMAKE > +$AUTOCONF > +./configure > +$MAKE > + > +# The toplevel rule is not problematic, as it is not recursive. > +$sleep > +touch Makefile.in > +$sleep Is this second sleep really needed? More instances below. > +$MAKE -n Makefile > +test `ls -1t Makefile Makefile.in | sed 1q` = Makefile.in Waht about using here the `is_newest' function provided by `tests/defs'? More instances below. > +$MAKE Makefile > +test `ls -1t Makefile Makefile.in | sed 1q` = Makefile > + > +$sleep > +touch config.status > +$sleep > +$MAKE -n Makefile > +test `ls -1t Makefile config.status | sed 1q` = config.status > +$MAKE Makefile > +test `ls -1t Makefile config.status | sed 1q` = Makefile > + > +# The subdir rules are fixed by separating the recursive part from the rest. > +cd sub > +$sleep > +touch Makefile.in > +$sleep > +$MAKE -n Makefile > +test `ls -1t Makefile Makefile.in | sed 1q` = Makefile.in > +$MAKE Makefile > +test `ls -1t Makefile Makefile.in | sed 1q` = Makefile > +cd .. > + > +$sleep > +touch config.status > +$sleep > +# Go through contortions to avoid GNU make's first, internal "rebuild > Makefile > +# even in dry mode" cycle in the recursive (am--refresh) part. > +(cd sub && $MAKE -n Makefile AM_MAKEFLAGS="-n Makefile") I'd add a "|| Exit 1" here, just to be sure w.r.t. shells with buggy `set -e'. One more intance below. > +test `ls -1t sub/Makefile config.status | sed 1q` = config.status > +(cd sub && $MAKE Makefile) > +test `ls -1t sub/Makefile config.status | sed 1q` = sub/Makefile > + > +:
Thanks, Stefano
