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

[Eric Blake]

On 11/12/2012 12:10 PM, Nick Bowler wrote:
> On 2012-11-12 11:24 -0700, Eric Blake wrote:
>> On 11/12/2012 08:42 AM, Nick Bowler wrote:
>>> On 2012-11-09 15:53 -0700, Eric Blake wrote:
>>> If the goal is compatibility with existing behaviour of libtool,
>>> then we must allow multiple invocations of AC_CONFIG_MACRO_DIR and
>>> they must emit AC_CONFIG_MACRO_DIR_TRACE in the reverse order.
>>
>> 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.

> 
> 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.  There comes a point where it is much more
practical to state that we simply _DO NOT_ support multiple calls to
AC_CONFIG_MACRO_DIR - we never have, and with autoconf 2.70, we now fail
up front if you attempt it, since different tools have had different
interpretations (purely by accident and not by specification) of what it
meant if you used it more than once.

We are changing from undefined behavior to explicitly defined behavior -
with the explicit definition being that we don't support multiple calls
to AC_CONFIG_MACRO_DIR.

>> 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.

> 
> Here's the thing: suppose a libtool-using package is using
> any currently-released versions of the autotools, and needs the
> equivalent of:
> 
>   ACLOCAL_AMFLAGS = -I foo -I bar
> 
> With this, libtoolize warns about missing AC_CONFIG_MACRO_DIR([foo]).
> Now the package maintainer essentially has three options:
> 
>   (1) Ignore the warning, and do not add any AC_CONFIG_MACRO_DIR to
>       configure.ac.
>   (2) Add AC_CONFIG_MACRO_DIR([foo]) to configure.ac, ignore bar.

1 and 2 are viable.

>   (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.

> 
> 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.

> 
> 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:

# 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).

> 
> Gratuitous fatal errors introduced by this change are worse than both
> silence and warnings.

They are not gratuitous errors, but intentional; we are actively
campaigning against the previous ambiguous semantic mess.  In my mind,
changing undefined behavior into a hard error is not classified as
introducing regressions - anyone previously using AC_CONFIG_MACRO_DIR
more than once was already treading on thin ice because there was no
documentation nor testsuite to guarantee the behavior that they happened
to get by accident.  If your package hits this error because you used
AC_CONFIG_MACRO_DIR more than once, then pick which directory is your
primary, make it first, and rename all the other uses to
AC_CONFIG_MACRO_DIRS.  Doing that will avoid the error with newer
autotools, and have no semantic difference with older autotools.

>  If all you want is to silence the libtool
> warnings with old versions, then I would actually suggest the following:
> 
>   dnl Shut up libtoolize
>   m4_ignore([AC_CONFIG_MACRO_DIR([m4])])

Yes, that is also a possibility for using option 1 while still avoiding
the libtool warning.  And if you do that, then you can _still_ mix it
with AC_CONFIG_MACRO_DIRS([foo bar]) for the benefit of newer automake.

> 
> which also works, and doesn't require any help from Autoconf.
> 

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature