Hi Stefano, * Stefano Lattarini wrote on Sat, Dec 11, 2010 at 03:00:34PM CET: > * tests/auxdir.test: Refactored and made less hackish. Improved > heading comments. > * tests/auxdir2.test: Call automake with the `-a' option, so > that automake won't fail for spurious reasons. Add trailing > `:' command. > * tests/auxdir3.test: Add an explanatory comment and a trailing > `:' command. > * tests/auxdir4.test: 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. > * tests/auxdir6.test: New test. > * tests/auxdir7.test: Likewise. > * tests/auxdir8.test: Likewise. > * tests/Makefile.am (TESTS): Updated.
Can you update this patch to not require the previous 1/2? I have a couple more nits below. The patch is OK after addressing the issues. Thanks, Ralf > --- a/tests/auxdir.test > +++ b/tests/auxdir.test > @@ -16,20 +16,37 @@ > # along with this program. If not, see <http://www.gnu.org/licenses/>. > > # Test to make sure AC_CONFIG_AUX_DIR works correctly. > +# This test tries without an explicit call to AC_CONFIG_AUX_DIR; > +# the config auxdir should be implicitly defined to `.' since > +# the install-sh, mkinstalldirs, etc., scripts are in the top-level > +# directory. > +# Keep this in sync with sister tests auxdir6.test and auxdir7.test. > > -# The "./." is here so we don't have to mess with subdirs. > -config_auxdir=./. > +config_auxdir=NONE > . ./defs || Exit 1 > > +set -e > + > +cat >> configure.in << 'END' > +AC_CONFIG_FILES([subdir/Makefile]) > +END > + > +mkdir subdir > + > cat > Makefile.am << 'END' > pkgdata_DATA = > END > > -cp "$testsrcdir/../lib/mkinstalldirs" . > +cp Makefile.am subdir/Makefile.am > + > +: > mkinstalldirs > +: > install-sh > +: > missing > + > +$ACLOCAL > +$AUTOMAKE > > -# The "././" prefix confuses Automake into thinking it is doing a > -# subdir build. Yes, this is hacky. (This comment should be retained, along with the usage below.) > -$ACLOCAL || Exit 1 > -$AUTOMAKE ././Makefile || Exit 1 > +$FGREP '$(top_srcdir)/mkinstalldirs' Makefile.in > +$FGREP '$(top_srcdir)/mkinstalldirs' subdir/Makefile.in > > -grep '/\./\./mkinstalldirs' Makefile.in > +: > --- a/tests/auxdir2.test > +++ b/tests/auxdir2.test > @@ -25,4 +25,6 @@ set -e > : > Makefile.am > > $ACLOCAL > -$AUTOMAKE > +$AUTOMAKE -a This changes the code paths exercised (i.e., potentially removes coverage); please use either $AUTOMAKE -a $AUTOMAKE so that both are covered, or have one test with and one without -a. The former is more efficient. > --- a/tests/auxdir4.test > +++ b/tests/auxdir4.test > @@ -14,7 +14,7 @@ > # 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 we diagnose dangerous AC_CONFIG_AUX_DIR names. > +# Make sure we diagnose unportable AC_CONFIG_AUX_DIR names. > > config_auxdir=aux > . ./defs || Exit 1 > @@ -25,5 +25,11 @@ set -e > > $ACLOCAL > AUTOMAKE_fails > -grep 'configure.in:2:.*aux.*does not exist' stderr > -grep 'configure.in:2:.*aux.*W32' stderr > +grep '^configure\.in:2:.*aux.*W32' stderr > + > +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. > + AUTOMAKE_fails > + grep '^configure\.in:2:.*aux.*W32' stderr > +fi > + > +: > index 0000000..61b2720 > --- /dev/null > +++ b/tests/auxdir5.test > @@ -0,0 +1,30 @@ > +# Make sure we diagnose dangerous non-existent AC_CONFIG_AUX_DIR names. Why dangerous? s/dangerous// ? > +config_auxdir=nonesuch > +. ./defs || Exit 1 > + > +set -e > + > +: > Makefile.am > + > +$ACLOCAL > +AUTOMAKE_fails > +grep '^configure\.in:2:.*nonesuch.* not exist' stderr I wonder whether we had a PR before suggesting to just create the directory. I guess I agree though that not creating it is safer, if a bit less convenient for the user. > --- /dev/null > +++ b/tests/auxdir8.test > @@ -0,0 +1,132 @@ > +# Make sure that, if AC_CONFIG_AUX_DIR is not specified, Automake tries > +# to use `.', `..' and `../..', in precisely that order. Hmm. This is an Autoconf feature actually, rather than an Automake one. But we also document it in automake.texi. Thus it is ok to have a test, but it would not really be necessary to test the Autoconf part of this. Hmm, but it seems you need to use/rely on both to do a full test. OK. > +config_auxdir=NONE > +. ./defs || Exit 1 > + > +set -e > + > +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'm not sure what you want to be sure of with the unset here. > +cat >>configure.in << END > +AM_MISSING_PROG([NONESUCH],[$nil]) > +AC_OUTPUT > +END > + > +mkdir d3 > +mkdir d3/d2 > +mkdir d3/d2/d1 > +mkdir d3/d2/d1/d0 > + > +echo 'echo %%d3%% $*' > d3/missing > +chmod +x d3/missing > +echo 'echo %%d2%% $*' > d3/d2/missing > +chmod +x d3/d2/missing > +echo 'echo %%d1%% $*' > d3/d2/d1/missing > +chmod +x d3/d2/d1/missing > +echo 'echo %%d0%% $*' > d3/d2/d1/d0/missing > +chmod +x d3/d2/d1/d0/missing > + > +mv configure.in d3/d2/d1/d0/ > + > +cd d3/d2/d1/d0 > + > +cat > Makefile.am << 'EOF' > +.PHONY: test > +test: > + $(NONESUCH) >$(out) > +EOF > + > +$ACLOCAL > +$AUTOCONF > + > +# ------------------------------------------- # > +: We must end up with AC_CONFIG_AUX_DIR = . # > +# ------------------------------------------- # > + > +: > install-sh > +$AUTOMAKE > +./configure > +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.) > +cat out0 > +grep "%%d0%%.*$nil" out0 > +grep '%%d[123]' out0 && Exit 1 > + > +rm -f missing install-sh > + > +# -------------------------------------------- # > +: We must end up with AC_CONFIG_AUX_DIR = .. # > +# -------------------------------------------- # > + > +# Automake finds `install-sh' in `.', so it assumes that auxdir is `.'; > +# but it won't find `missing' in `.', so it will fail. > +: > install-sh > +AUTOMAKE_fails > +grep 'required file.*[^.]\./missing.*not found' stderr > +rm -f install-sh > + > +# Now things should work. > +: > ../install-sh > +$AUTOMAKE > +./configure > +out=out1 $MAKE test > +cat out1 > +grep "%%d1%%.*$nil" out1 > +grep '%%d[023]' out1 && Exit 1 > + > +rm -f ../missing ../install-sh > + > +# ----------------------------------------------- # > +: We must end up with AC_CONFIG_AUX_DIR = ../.. # > +# ----------------------------------------------- # > + > +# Automake finds `install-sh' in `.', so it assumes that auxdir is `.'; > +# but it won't find `missing' in `.', so it will fail. > +: > install-sh > +AUTOMAKE_fails > +grep 'required file.*[^.]\./missing.*not found' stderr > +rm -f install-sh > + > +# Automake finds `install-sh' in `..', so it assumes that auxdir is `..'; > +# but it won't find `missing' in `.', so it will fail. > +: > ../install-sh > +AUTOMAKE_fails > +grep 'required file.*[^.]\.\./missing.*not found' stderr > +rm -f ../install-sh > + > +# Now things should work. > +: > ../../install-sh > +$AUTOMAKE > +./configure > +out=out2 $MAKE test > +cat out2 > +grep "%%d2%%.*$nil" out2 > +grep '%%d[013]' out2 && Exit 1 > + > +rm -f ../../missing ../../install-sh > + > +# --------------------------------------------------------- # > +: AC_CONFIG_AUX_DIR will not be found: automake must fail # > +# --------------------------------------------------------- # > + > +AUTOMAKE_fails > +grep 'required file.*missing.*not found' stderr > + > +:
