automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [SIMPLE PATCH] {maint} Extend tests on `--help' and `--version' opti


From: Stefano Lattarini
Subject: Re: [SIMPLE PATCH] {maint} Extend tests on `--help' and `--version' options.
Date: Sun, 26 Sep 2010 11:39:38 +0200
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]