On Wednesday 15 December 2010, Ralf Wildenhues wrote: > * Stefano Lattarini wrote on Wed, Dec 15, 2010 at 09:49:55PM CET: > > On Tuesday 14 December 2010, Ralf Wildenhues wrote: > > > * Stefano Lattarini wrote on Sat, Dec 11, 2010 at 03:00:34PM CET: > > > > +if mkdir aux; then > > > > > > What happens with this command on w32? I checked: > > > MinGW mkdir fails > > > DJGPP mkdir fails > > > Cygwin mkdir passes > > > > > > It seems that none of the systems actually cause harmful problems. > > > > > Good, because they shouldn't; > > Sure, but that's one of the things where I'm wary. We've had configure > tests in Libtool which froze a system, or required root access to fix, > on some of the older unices. > LOL, I didn't suspect that could really happen :-/
> I simply wasn't sure the above wasn't one of them. > > > this second, conditional check... > > > > > > + AUTOMAKE_fails > > > > + grep '^configure\.in:2:.*aux.*W32' stderr > > > > > > ... serves just to ensure that Automake fails with a "correct" error > > (i.e. about portability) even on platforms where the use of `aux' is > > valid, and even when an `aux' directory actually exist. > > Sure. > > > > > +nil=__no_such_program > > > > +unset NONESUCH || : # just to be sure > > > > > > "just to be sure" is a fairly meaningless comment. It actually made me > > > wonder whether you meant the "|| :" or the "unset" part with it. > > > > > I meant the "unset" part. > > > > > I'm not sure what you want to be sure of with the unset here. > > > > > That no `NONESUCH' variable is in the environment. > > > > I've gone with this now: > > > > # Make sure that we don't have a NONESUCH variable set > > # in the environment. > > unset NONESUCH || : > > Where I'd just remove the comment, because it's redundant: the code > explains what it does. ;-) > True enough. Comment removed. > > > > +out=out0 $MAKE test > > > > > > I'd write > > > env out=out0 ... > > > > > > here (and below), but only to make it clearer what is happening. No big > > > deal either way, so use whatever you prefer. (It makes a difference > > > when the command is a shell special one.) > > > > > Well, not using `env' means one less fork ;-) > > And I'm pretty confident that `make' is not going to become a shell > > builtin ;-) > > OK. > > > Subject: [PATCH] Extended tests on AC_CONFIG_AUX_DIR. > > > > * tests/auxdir.test: Enable `errexit' shell flag. Prefer `$me' > > over hard-coded test name. Use proper m4 quoting. Add trailing > > `:' command. > > * tests/auxdir2.test: Likewise. Try to call automake also with > > the `-a' option, so that it will not fail for spurious reasons. > > * tests/auxdir3.test: Add an explanatory comment and a trailing > > `:' command. > > * tests/auxdir4.test: Prefer `$me' over hard-coded test name. > > Make grepping of automake stderr slightly stricter. Also, now > > this just checks for unportable auxdir names (and it has been > > Generally, prefer active voice rather than passive in log entries: > "Check for unportable foo..." (this is documented in GCS). > > FWIW I'd write s/auxdir/auxiliary directory/ > OK, adjusted. Even if to be really precise we should call it at least "directory of auxiliary scripts" ;-) > > extended in this respect). Checks for non-existent auxdirs has > > also here (...ies) > Adjusted. > > been moved out to ... > > * tests/auxdir5.test: .. this new test. > > * tests/auxdir6.test: New test. > > * tests/auxdir7.test: Likewise. > > * tests/auxdir8.test: Likewise. > > * tests/auxdir9.test: Likewise. > > * tests/Makefile.am (TESTS): Updated. > I fixed also some other typos and grammaros in the ChangeLog entry; see the attached squash-in fore more info. > > --- a/tests/auxdir2.test > > +++ b/tests/auxdir2.test > > > +# Both these two invocations are meant. > > Comment writing is hard! :-) > The above still doesn't explain *why* both are done, > Well, before reading your latest reply, I could have written only "Both these two invocations are meant, because Ralf said so." ;-) > so while it does greater than zero information, it could still > be better. What about this? > > # Exercise both code paths concerning auxiliary files. > Better; added. > > +$AUTOMAKE -a > > $AUTOMAKE > > Patch is ok then. > I'll push it tomorrow if there are no further objections. Thanks, Stefano
diff --git a/ChangeLog b/ChangeLog index 60c80ca..f18c0ff 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,10 +10,11 @@ `:' command. * tests/auxdir4.test: Prefer `$me' over hard-coded test name. Make grepping of automake stderr slightly stricter. Also, now - this just checks for unportable auxdir names (and it has been - extended in this respect). Checks for non-existent auxdirs has - been moved out to ... - * tests/auxdir5.test: .. this new test. + this test just checks about Automake's reaction to unportable + auxiliary directory names (and it has been extended in this + respect). Moved the checks about non-existent auxiliary + directories to ... + * tests/auxdir5.test: ... this new test. * tests/auxdir6.test: New test. * tests/auxdir7.test: Likewise. * tests/auxdir8.test: Likewise. diff --git a/tests/auxdir2.test b/tests/auxdir2.test index 7936a2b..430abad 100755 --- a/tests/auxdir2.test +++ b/tests/auxdir2.test @@ -32,6 +32,7 @@ END $ACLOCAL # Both these two invocations are meant. +# They exercise both code paths concerning auxiliary files. $AUTOMAKE -a $AUTOMAKE diff --git a/tests/auxdir9.test b/tests/auxdir9.test index 3ee1dbc..db85ac1 100755 --- a/tests/auxdir9.test +++ b/tests/auxdir9.test @@ -22,8 +22,7 @@ set -e nil=__no_such_program -# Make sure that we don't have a NONESUCH variable set -# in the environment. + unset NONESUCH || : cat >>configure.in << END