Re: AC_BEFORE does not warn when it should

[Eric Blake]

On 05/31/2010 03:36 PM, Bruno Haible wrote:
> Hi,

Hi Bruno, and reviving an old thread.

> Here is a case where AC_BEFORE does not warn when it should.
>
> How to reproduce (with autoconf 2.65):
>    $ wget http://www.haible.de/bruno/gnu/autoconf-bug-20100601.tar.gz
>    $ tar xvfz autoconf-bug-20100601.tar.gz
>    $ cd autoconf-bug-20100601
>    $ autoconf
>
> No warning. Now look at the definition of gl_LIBUNISTRING
> (in file m4/libunistring.m4): It starts with
>
>    AC_BEFORE([gl_LIBUNISTRING], [gl_LIBUNISTRING_LIB_PREPARE])
>    # XXX gl_LIBUNISTRING starts here

Just to be clear, here you want autoconf to issue a warning if 
gl_LIBUNISTRING_LIB_PREPARE has already been expanded.  That is, you 
think gl_LIBUNISTRING should work well in isolation; and that it should 
work well with gl_LIBUNISTRING_LIB_PREPARE when gl_LIBUNISTRING is 
expanded first; but you have problems if both macros are called but 
gl_LIBUNISTRING is expanded second, thus wanting a warning to alert you 
to that fact.

> And the definition of gl_LIBUNISTRING_LIB_PREPARE (in file
> m4/libunistring-base.m4): It starts with
>
>    AC_BEFORE([gl_LIBUNISTRING], [gl_LIBUNISTRING_LIB_PREPARE])
>    # XXX gl_LIBUNISTRING_LIB_PREPARE starts here

As Gary already pointed out, AC_BEFORE([...], [$0]) is pointless (of 
course gl_LIBUNISTRING_LIB_PREPARE has not yet been expanded if we are 
now expanding it), not to mention that if it triggered, it would be 
printing a misleading error message (blaming the wrong macro name).  But 
it doesn't affect analysis; I've erased that line from your tarball and 
still see the issue you reported.

And, to make sure I still understand your intent: You are claiming that 
it is okay to run gl_LIBUNISTRING_LIB_PREPARE in isolation (that is, 
when gl_LIBUNISTRING is never used), which means that gl_LIBUNISTRING is 
not a pre-requisite of gl_LIBUNISTRING_LIB_PREPARE.  Had gl_LIBUNISTRING 
been a prerequisite, then adding the AC_REQUIRE([gl_LIBUNISTRING]) in 
gl_LIBUNISTRING_LIB_PREPARE would give you the warning message you 
desire, but it means that gl_LIBUNISTRING is unconditionally expanded, 
which goes counter to your argument that the two macros are 
independently useful.

If I'm mis-understanding your assumptions, let me know, although it 
doesn't affect my analysis of the autoconf limitation you stumbled upon. 
 And meanwhile, I think we can fix your particular usage case to work 
regardless of whether I can beef up autoconf to warn as documented, or 
whether you are still using an older autoconf that has that limitation 
of no warning.

> Both macros are defined with AC_DEFUN. Neither one requires nor invokes any
> other macro. In the final configure file, the "before" constraint is violated:
>
>    $ grep -n 'starts here' configure
>    12110:  # XXX gl_LIBUNISTRING_LIB_PREPARE starts here
>    14551:  # XXX gl_LIBUNISTRING starts here

In other words, you have proof that the expansion of gl_LIBUNISTRING 
occurs second, and that AC_BEFORE did not warn about 
gl_LIBUNISTRING_LIB_PREPARE even though that ended up first in the output.

> Yet autoconf has not warned. Why??

This is caused by the age-old nemesis of expansion before requiring. 
gl_LIBUNISTRING was indeed expanded first, which means that at the time 
AC_BEFORE was expanded, gl_LIBUNISTRING_LIB_PREPARE had not yet been 
seen.  In fact, using autoconf's --trace mechanism proves this point:

$ autoconf --trace gl_LIBUNISTRING --trace gl_LIBUNISTRING_LIB_PREPARE
configure.ac:63:gl_LIBUNISTRING:
configure.ac:63:gl_LIBUNISTRING_LIB_PREPARE:

So the problem boils down to understanding why the expansions are being 
reorganized, since the actual expansions happened in the correct order; 
and the culprit is AC_REQUIRE.

Both gl_LIBUNISTRING_LIBSOURCE and gl_LIBUNISTRING_LIBHEADER do an 
AC_REQUIRE of gl_LIBUNISTRING_LIB_PREPARE.  And gl_INIT (in 
gnulib-comp.m4) does a direct expansion of gl_LIBUNISTRING first, 
followed by a direct expansion of gl_LIBUNISTRING_LIBHEADER.  The end 
result is that the expansion of gl_LIBUNISTRING occurs inline in the 
body of gl_INIT, but the expansion of gl_LIBUNISTRING_LIB_PREPARE occurs 
prior to the body of gl_INIT.  So, even though 
gl_LIBUNISTRING_LIB_PREPARE was expanded second, it was expanded via 
AC_REQUIRE which resulted in it occurring first.

You can fix things by changing the gnulib modules to 
AC_REQUIRE([gl_LIBUNISTRING]) rather than directly expand it.  Or you 
can fix things by making macros like gl_LIBUNISTRING_LIBHEADER do an 
AC_REQUIRE of both gl_LIBUNISTRING and gl_LIBUNISTRING_LIB_PREPARE (back 
to the question of are the two macros really independently useful, with 
merely an ordering constraint to enforce if both are used, or are they 
dependent such that using one should always imply the use of the other). 
 I think you can also use AC_DEFUN_ONCE instead of AC_DEFUN to avoid 
the problem.

Meanwhile, I think I can fix the autoconf issue: if AC_BEFORE([a],[b]) 
tweaks the definition of [b] to inject an AC_REQUIRE([a]), then you can 
get the 'expanded before required' warning in the case where [a] is 
expanded then [b] required.  And it has limited impact: calling just [a] 
means you never expanded [b] to run into the newly injected AC_REQUIRE; 
calling just [b] means you never hit the AC_BEFORE of [a] to inject 
anything; directly calling or requiring both [a] and [b] in order means 
that you hit the AC_REQUIRE but [a] has already been expanded; and 
calling [b] before [a] means that you will still get the ordering 
constraint warning.  So, the only thing that changes is directly 
expanding [a] before requiring [b], where the injected AC_REQUIRE is 
sufficient to hook into the existing AC_REQUIRE 'expanded before 
required' logic to warn about the issue.

Indeed, with the patch below, I get this warning buried in the middle of 
various other warnings:

configure.ac:63: warning: AC_REQUIRE: `gl_LIBUNISTRING' was expanded 
before it was required
m4/libunistring-base.m4:67: gl_LIBUNISTRING_LIB_PREPARE is expanded from...
m4/libunistring-base.m4:51: gl_LIBUNISTRING_LIBHEADER is expanded from...

That is, the AC_BEFORE call in gl_LIBUNISTRING was sufficient to make 
gl_LIBUNISTRING_LIB_PREPARE behave as if it had done 
AC_REQUIRE([gl_LIBUNISTRING]), but without forcing the dependency if 
gl_LIBUNISTRING remains uncalled.

However, the list of warnings produced on your example when using my 
patched autoconf was rather large.  Some of the other warnings already 
existed when using autoconf.git without this patch, and have mostly been 
fixed in existing gnulib (such AC_LANG_CONFTEST without AC_LANG_SOURCE). 
 But there were other new warnings that cropped up because of this 
patch, such as AC_USE_SYSTEM_EXTENSIONS being required but not defun'd, 
so until I analyze those issues (whether it really is a dependency 
ordering bug in autoconf's definition, or whether there is a better way 
than the patch below for making AC_BEFORE reliably warn), I am claiming 
that this patch is not ready for prime time yet.  If you don't mind, I'd 
rather release autoconf 2.68 _without_ this patch, and look in more 
depth on how to issue the warning reliably and without false positives 
after 2.68 is released.

So, without further ado, here's a quickie proof-of-concept patch that 
makes AC_BEFORE warn in the cases where it would introduce an ordering 
constraint violation due to expanding the AC_BEFORE prior to 
AC_REQUIRE'ing the counterpart macro (deceptively short, huh?).

diff --git i/lib/m4sugar/m4sugar.m4 w/lib/m4sugar/m4sugar.m4
index d440127..724dade 100644
--- i/lib/m4sugar/m4sugar.m4
+++ w/lib/m4sugar/m4sugar.m4
@@ -1889,7 +1889,8 @@ m4_define([_m4_divert(GROW)],       10000)
 # by avoiding dnl and m4_defn overhead.
 m4_define([_m4_defun_pro],
 [m4_ifdef([_m4_expansion_stack], [], [_m4_defun_pro_outer([$1])])]dnl
-[m4_expansion_stack_push([$1])m4_pushdef([_m4_expanding($1)])])
+[m4_expansion_stack_push([$1])m4_pushdef([_m4_expanding($1)])]dnl
+[m4_ifdef([$0($1)], [m4_indir([$0($1)])])])

 m4_define([_m4_defun_pro_outer],
 [m4_set_delete([_m4_provide])]dnl
@@ -2022,7 +2023,9 @@ m4_define([m4_pattern_allow], [])
 # Issue a warning if CALLED-MACRO-NAME was called before THIS-MACRO-NAME.
 m4_define([m4_before],
 [m4_provide_if([$2],
-              [m4_warn([syntax], [$2 was called before $1])])])
+              [m4_warn([syntax], [$2 was called before $1])],
+[m4_append([_m4_defun_pro($2)],
+  m4_if([$0], [m4_before], [[m4_require]], [[AC_REQUIRE]])[([$1])])])])


 # m4_require(NAME-TO-CHECK, [BODY-TO-EXPAND = NAME-TO-CHECK])


--
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org