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

[Stefano Lattarini]

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