Re: [PATCH] AC_CONFIG_MACRO_DIRS: improve tracing and add sanity checks

[Nick Bowler]

On 2012-11-12 12:42 -0700, Eric Blake wrote:
> On 11/12/2012 12:10 PM, Nick Bowler wrote:
> > On 2012-11-12 11:24 -0700, Eric Blake wrote:
[...]
> >> No, existing use of libtool was intended to honor only the first
> >> AC_CONFIG_MACRO_DIR, but due to a bug, actually honored only the last.
> >> It does NOT honor multiple directories, so anyone actually _using_
> >> AC_CONFIG_MACRO_DIR with existing libtool could only successfully use it
> >> once.  The new code in autoconf enforces that usage, since anything else
> >> is confusing.
> > 
> > Regardless of what was intended, what actually happened was that libtool
> > only looked at the last AC_CONFIG_MACRO_DIR.  As libtool never documented
> > its behaviour either way, we should strive to avoid breaking things by
> > not making arbitrary changes to longstanding behaviour.
> 
> That was a bug in libtool for using an under-documented autoconf macro,
> and not documenting its own further assumptions.

If this bug in libtool was a year old, nobody would care.  But as far as
I can see this has been the behaviour since day 1: almost 10 years ago.
Since nobody ever documented what this macro was actually supposed to
do, the observable behaviour of the only tool to ever "use" it is the
closest thing we've got to actual documentation.

> > Saying that, I wish to amend my original statement.
> > AC_CONFIG_MACRO_DIR_TRACE shouldn't be output in the reverse order of
> > AC_CONFIG_MACRO_DIR invocations: it should be output only for the last
> > such invocation.
> 
> If you can figure out how to write m4 code to do that, more power to
> you.  But I don't know how to easily prevent a call to
> AC_CONFIG_MACRO_DIR_TRACE on all but the last call to
> AC_CONFIG_MACRO_DIR.

A simple way would seem to be (totally untested):

  AC_DEFUN([AC_CONFIG_MACRO_DIR],
           [m4_defun([_AC_CONFIG_MACRO_DIR], [$1]))

then, in AC_OUTPUT:

  m4_ifdef([_AC_CONFIG_MACRO_DIR],
           [AC_CONFIG_MACRO_DIR_TRACE(m4_defn([_AC_CONFIG_MACRO_DIR]))])

But I still maintain that it is much easier to not make any changes
whatsoever to AC_CONFIG_MACRO_DIR: i.e., do nothing.

[...]
> >> Yes, it DOES do something - it tells libtool where to install files, and
> >> libtool needs to install files in the FIRST directory, not the last.  So
> >> this change is indeed intentional.
> > 
> > Sort of.  As far as I can tell it only tells libtoolize whether or not
> > to print a warning: not where to install files.  Libtoolize snarfs in
> > ACLOCAL_AMFLAGS regardless of whether or not AC_CONFIG_MACRO_DIR is
> > used.
> 
> And if I understand correctly, Gary has written patches for libtool.git
> to quit using ACLOCAL_AMFLAGS and instead start tracing configure.ac,
> precisely because automake hopes to get rid of ACLOCAL_AMFLAGS.
> Obviously, that means that libtool.git will need to honor
> AC_CONFIG_MACRO_DIRS, and properly honor the _first_ directory under
> that usage, but this time, the new macro has documented semantics that
> everyone will agree on.  So now it is just a question of what happens if
> you use older libtool with newer autoconf and automake.

I don't follow libtool development all that closely, so I can only
really talk about actual released versions that are in the hands of
users.  The latest is 2.4.2, at the time of this writing.

[...]
> >   (3) Add AC_CONFIG_MACRO_DIR([bar]) and AC_CONFIG_MACRO_DIR([foo]) to
> >       configure.ac, in that order.
> 
> 3 is wrong.  Just because libtool accidentally picked the last instance
> doesn't make it correct, and other tools such as aclocal were picking
> the first instance.

As far as I know, no released version of aclocal picks up the first
instance, or any instance for that matter.  Only libtoolize has ever
cared.  All released versions of aclocal require the search path to be
specified on the command line (or, in very recent versions, as an
environment variable).  Autoreconf snarfs ACLOCAL_AMFLAGS to pass the
relevant options to aclocal, and ignores AC_CONFIG_MACRO_DIR outright.

> > Of the three, option (2) is pretty unattractive since it requires
> > admitting that AC_CONFIG_MACRO_DIR is useless, so you may as well go
> > with (1) in that case.  That pretty much leaves (1) and (3) as viable
> > options.
> 
> 3 was never viable, because of conflicting interpretations.

Since only libtoolize has ever cared, there was never any conflicting
interpretation.  (3) works just fine today with the latest versions of
everything, and presumably has for the last 10 years.

> > And this is not hypothetical: this is exactly what I went through
> > with my own packages.  I ended up choosing option (1) and removed
> > AC_CONFIG_MACRO_DIR from everything, but I could see a reasonable
> > person choosing option (3) to silence the warnings.  All of these
> > options currently work: your change breaks (3) and I suspect (2)
> > will break later if we try to "help" and make AC_CONFIG_MACRO_DIR
> > useful.
> 
> 2 should still work.  In fact, the entire intent of my patch is that you
> can now use option 2 as follows:

Today, the following works -- both foo and bar are picked up by the
autotools, and libtoolize prints no warnings.  This was option (2):

  in configure.ac:
  AC_CONFIG_MACRO_DIR([foo])

  in Makefile.am:
  ACLOCAL_AMFLAGS = -I foo -I bar

I suspect (perhaps wrongly) that by making AC_CONFIG_MACRO_DIR "more
useful", this will break in the future because tools will start seeing
AC_CONFIG_MACRO_DIR and subsequently ignore the extra directory in
ACLOCAL_AMFLAGS.

Obviously it will be possible to modify configure.ac and/or Makefile.am
to support the new versions which is what you seem to be suggesting
here:

> # Declare the primary directory, where libtoolize and aclocal should
> # install things
> AC_CONFIG_MACRO_DIR([foo])
> # Declare secondary directories, where aclocal should also look, but
> # not touch
> AC_CONFIG_MACRO_DIRS([bar])
>
> Which works for both the new autotools (where AC_CONFIG_MACRO_DIR_TRACE
> properly sees foo first, then bar), and for the older libtool (which
> sees only one instance of AC_CONFIG_MACRO_DIR, with the desired result
> of 'foo' being the directory to modify during install).

but this suggestion fails with old versions of autoconf that don't
define AC_CONFIG_MACRO_DIRS!  If a maintainer is willing to throw away
compatibility with old autoconf, then it seems weird that they'd be
unwilling to throw away compatibility with old libtool.  (Keep in mind
that "compatibility with old libtool" here means "old libtoolize prints
less noise to standard error", not "the package won't autoreconf with
old libtool.")

Anyway, my solution to problems with multiple m4 directories was to
delete all uses of AC_CONFIG_MACRO_DIR, which has clearly proven to be
the correct course of action.  So my only worry is for packages whose
authors may have chosen a different path.  Maybe I'm the only person in
the world who ever ran into these issues...

Cheers,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)