[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] Extended tests on AC_CONFIG_AUX_DIR.
From: |
Ralf Wildenhues |
Subject: |
Re: [PATCH 2/2] Extended tests on AC_CONFIG_AUX_DIR. |
Date: |
Tue, 14 Dec 2010 21:01:01 +0100 |
User-agent: |
Mutt/1.5.20 (2010-08-04) |
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
> +
> +:
Re: [PATCH 0/2] Tests initialization: put default definition of AC_CONFIG_AUX_DIR in the pre-populated configure.in., Ralf Wildenhues, 2010/12/13