automake-patches
[Top][All Lists]
Advanced

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

Re: [PATCH] New tests on obsoleted usages of automake/autoconf macros.


From: Ralf Wildenhues
Subject: Re: [PATCH] New tests on obsoleted usages of automake/autoconf macros.
Date: Sat, 6 Nov 2010 18:14:07 +0100
User-agent: Mutt/1.5.18 (2008-05-17)

* Stefano Lattarini wrote on Wed, Nov 03, 2010 at 07:12:27PM CET:
> Pinging this patch again, following this:
>  <http://lists.gnu.org/archive/html/automake-patches/2010-11/msg00003.html>
> 
> I've also re-based the patch off of latest maint, extended some checks a
> little bit, fixed a typo in comments, and fixed some very minor and theoretic
> portability problems (use of "test -z").
> The updated patch is attached.

The patch is ok except I don't think the PACKAGE_URL part will work with
Autoconf 2.62, it was added later only.  You can look at Automake's own
configure.ac for a workaround; the patch is OK with a fix to that end
that you deem suitable.

When you post updated patches, then to avoid having to re-review the
whole patch again you could additionally post an incremental patch.
You can usually get one with something like
  git diff address@hidden

right after committing the increment, or some
  git diff address@hidden address@hidden

for some suitable $a and $b.  Thanks.

> The only unresolved question about the patch is the following one:
> > At Thursday 17 June 2010, Ralf Wildenhues wrote:
> > > Stefano Lattarini wrote:
> > > > --- /dev/null
> > > > +++ b/tests/backcompat2.test
> > > >
> > > > @@ -0,0 +1,69 @@
> > > > +# A trick to make the test run muuuch faster, by avoiding repeated
> > > > +# runs of aclocal (one order of magnitude improvement in speed!).
> > > > +echo 'AC_INIT(x,0) AM_INIT_AUTOMAKE' >configure.in
> > > > +$ACLOCAL
> > 
> > > Hmm, does that mean this test completely relies on caching?
> > It should mean that the test relies on each autoconf call requiring an
> > aclocal.m4 that defines just AM_INIT_AUTOMAKE and the macros it requires,
> > and nothing more.  And this aclocal.m4 is created by the first $ACLOCAL
> > run.
> > Just to be sure, I amended the script to remove the autom4te.* stuff after
> > the call to $ACLOCAL (and only then).

OK.

> > > You probably need a $sleep or "rm -rf autom4te.cache" before
> > > each of the following edits to configure.in, to avoid autotools
> > > operating on old cached transformations of the file.
> > But then we would need to do that in each test where we modify and reuse
> > 'configure.in' (see e.g. acloca10.test, pr401.test, nodef.test, etc..).
> > Right?

Not if time stamps indicate updated files.

> > > Am I overlooking something here?
> > I'd like to ask you the same question :-)
> 
> So, OK to push as is, or should I address your concerns about autom4te
> caching in some way?

OK; let's see later when problems show up.

Thanks,
Ralf



reply via email to

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