Hi Stefano,
* Stefano Lattarini wrote on Sat, Sep 25, 2010 at 05:35:31PM CEST:
> OK for maint?
Well the patch surely makes it clear why $curdir was a badly chosen
name: it is not clear at which time this directory was "current".
I have nits below.
> Extend tests on `--help' and `--version' options.
>
> * tests/help.test: Create a new empty directory and chdir into
> it, rather than removing already present files. Run the aclocal
> and automake wrapper scripts directly, instead of relying on
> $AUTOMAKE and $ACLOCAL. Be sure to correctly match literal dots
> in aclocal's and automake's stderr. Add a trailing `:' command.
> * tests/help2.test: New test, checking that options `--help' and
> `--version' works in directories with broken `configure.in'.
> * tests/help3.test: New test, checking that options `--help' and
> `--version' take precedence on the other options.
> * tests/help4.test: New test, checking that the first among the
> `--help' and `--version' options to be specified on the command
> line wins.
> * tests/Makefile.am (TESTS): Updated.
> --- a/tests/help.test
> +++ b/tests/help.test
> @@ -21,21 +21,23 @@
>
> set -e
>
> -# Ensure we are run from the right directory.
> -# (The last thing we want is to delete some random user files.)
> -test -f ../defs
> -rm -f *
> +# Ensure we run in an empty directory.
> +mkdir emptydir
> +cd emptydir
>
> -$ACLOCAL --version
> -$ACLOCAL --help
> -$AUTOMAKE --version
> -$AUTOMAKE --help
> +"$curdir/aclocal-$APIVERSION" --version
> +"$curdir/aclocal-$APIVERSION" --help
> +"$curdir/automake-$APIVERSION" --version
> +"$curdir/automake-$APIVERSION" --help
I don't see the "$curdir/" prefixing should be needed. The relevant
programs are in $PATH. Likewise for all instances below and in the new
tets.
Not prepending a directory component is *on purpose*: the commands
should look as much like those you'd type manually, in your own package.
I don't understand why you wouldn't want to use the wrappers. If you
are desperately trying to omit extra arguments, then plain
automake-$APIVERSION would be the right thing to use instead.
> # aclocal and automake cannot work without configure.ac or configure.in
> -$ACLOCAL 2>stderr && { cat stderr >&2; Exit 1; }
> +"$curdir/aclocal-$APIVERSION" 2>stderr && { cat stderr >&2; Exit 1; }
> cat stderr >&2
> -grep configure.ac stderr
> -grep configure.in stderr
> -AUTOMAKE_fails
> -grep configure.ac stderr
> -grep configure.in stderr
> +$FGREP configure.ac stderr
> +$FGREP configure.in stderr
> +"$curdir/automake-$APIVERSION" 2>stderr && { cat stderr >&2; Exit 1; }
> +cat stderr >&2
> +$FGREP configure.ac stderr
> +$FGREP configure.in stderr
This all looks like change for the purpose of changing. Not needed,
AFAICS.
> --- /dev/null
> +++ b/tests/help3.test
> +# Make sure --help and --version takes precedence on other options.
s/on/over/
> +cat > configure.in <<END
> +AC_INIT([$me], [1.0])
> +AC_CONFIG_AUX_DIR([.]) dnl prevent automake fro looking into '..'
from
> +AM_INIT_AUTOMAKE([foreign])
> +AC_CONFIG_FILES([Makefile])
> +END
Rest is fine with me, thanks.
Cheers,
Ralf
