* Stefano Lattarini wrote on Wed, Nov 03, 2010 at 06:48:16PM CET: > Hello Ralf. Again, just a couple of nits w.r.t. the test cases...
Thanks; but I didn't mean to actually commit the second patch (just in case that wasn't clear). That said, I'll reply to your comments inline. > On Monday 01 November 2010, Ralf Wildenhues wrote: > > --- /dev/null > > +++ b/tests/remakedry.test > > @@ -0,0 +1,82 @@ > > +# 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. But that requires me to think about the test again: was this really meant to test portable code? The rebuilding rules, esp. the am--refresh trick, rely on GNU make-specifics, and special-casing some test of it just because it may or may not happen to trigger the specifics doesn't seem robust. I don't want the next small patch to this test cause a failure for non-GNU make, a failure I'm really not interested in in this test. Does that make sense? > > +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. Definitely is. I want Makefile to be strictly newer so that ls -1t is guaranteed to work not because of locale ordering but because of different time stamps. > > +$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'? That would be a viable alternative that would also allow to ditch the second sleep, I guess (untested). > > +# 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'. What bug would you be trying to work around with it? > 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 > > + > > +: Cheers, Ralf