Re: [PATCH 2/2] Extended tests on AC_CONFIG_AUX_DIR.

[Ralf Wildenhues]

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
> +
> +: