On Sunday 26 September 2010, Ralf Wildenhues wrote:
> 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".
There's a pending patch (for "tests-init" branch) to rename it ;-).
> 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.
Yes, it's not clear even to me now why I used $curdir. Will remove it
consistently.
> 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.
But I'm using the wrappers, and that's fine (and necessary); what I
want to avoid are the extra arguments in $AUTOMAKE and $ACLOCAL.
> If you are desperately trying to omit extra arguments, then plain
> automake-$APIVERSION would be the right thing to use instead.
True. That's what I'll use.
> > # 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.
No, it avoids the use of arguments with $AUTOMAKE and $ACLOCAL, and
escape literal dots in grep searches. Should I drop this chunk
anyway?
> Not needed, AFAICS.
>
> > --- /dev/null
> > +++ b/tests/help3.test
> >
> > +# Make sure --help and --version takes precedence on other
> > options.
>
> s/on/over/
Fixed.
> > +cat > configure.in <<END
> > +AC_INIT([$me], [1.0])
> > +AC_CONFIG_AUX_DIR([.]) dnl prevent automake fro looking into
> > '..'
> from
Ditto.
>
> > +AM_INIT_AUTOMAKE([foreign])
> > +AC_CONFIG_FILES([Makefile])
> > +END
>
> Rest is fine with me, thanks.
>
> Cheers,
> Ralf
Thanks,
Stefano
