Re: [PATCH] aclocal: AC_CONFIG_MACRO_DIRS: work around autom4te option parsing bugs
Hi Eric, thanks for the quick feedback. First of all, I've noticed this squash-in is necessary to avoid a spurious testsuite failure: diff --git a/t/aclocal-acdir.sh b/t/aclocal-acdir.sh index 59182bb..944604b 100755 --- a/t/aclocal-acdir.sh +++ b/t/aclocal-acdir.sh @@ -21,6 +21,9 @@ . test-init.sh mkdir am sys +# FIXME: remove in Automake 1.14. +mkdir am/internal +: > am/internal/ac-config-macro-dirs.m4 cat >> configure.ac <<'END' MY_MACRO Now let's come to your nits ... On 11/15/2012 01:10 PM, Eric Blake wrote: > On 11/15/2012 04:52 AM, Stefano Lattarini wrote: >> On 11/15/2012 11:58 AM, Stefano Lattarini wrote: >>> >>> The below patch should allow our users to employ AC_CONFIG_MACRO_DIRS >>> with autoconf 2.69 as well. It still doesn't work with autoconf 2.68 >>> and earlier though, due to a bug in autom4te option parsing (fixed by >>> autoconf commit v2.68-120-gf4be358). That could be fixed by using an >>> external file rather than stdin to pass aclocal the contents of >>> '$ac_config_macro_dirs_fallback'; but I rather do so in a separate >>> patch, with a dedicated rationale. >>> >> Done with the patch below. I'll push it (together with the earlier one) >> by this evening (CET). >> > >> +++ b/m4/internal/ac-config-macro-dirs.m4 >> @@ -0,0 +1,16 @@ >> +# Support AC_CONFIG_MACRO_DIRS with older autoconf. -*- Autoconf -*- >> +# FIXME: To be removed in Automake 1.14, once we can assume autoconf >> +#2.70 or later. >> +# FIXME: keep ion sync with the contents of the variable > > s/ion/in/ > Fixed, thanks. >> +#'$ac_config_macro_dirs_fallback' in aclocal.in. >> + >> +# Copyright (C) 2012 Free Software Foundation, Inc. >> +# >> +# This file is free software; the Free Software Foundation >> +# gives unlimited permission to copy and/or distribute it, >> +# with or without modifications, as long as this notice is preserved. >> + >> +m4_ifndef([AC_CONFIG_MACRO_DIRS], [dnl >> + m4_defun([_AM_CONFIG_MACRO_DIRS], [])dnl >> + m4_defun([AC_CONFIG_MACRO_DIRS], [_AM_CONFIG_MACRO_DIRS($@)])dnl >> +]) > > Hmm - this version is slightly different than the perl version it is > replacing. Each use of AC_CONFIG_MACRO_DIRS now injects whitespace (6 > spaces) into the user's configure script, then ignores the rest of the > line (or worse, changes the rest of the line). > > That is, if the user writes: > > AC_CONFIG_MACRO_DIRS([a])AC_CONFIG_MACRO_DIRS([b]) > > the perl version transforms it into the empty string with two trace > calls, but this version transforms it into: > dnlAC_CONFIG_MACRO_DIRS([b]) > and only traces 'a'. Here's how I would fix it (using the style I use > in autoconf): > > m4_ifndef([AC_CONFIG_MACRO_DIRS], > [m4_defun([_AM_CONFIG_MACRO_DIRS])]dnl > [m4_defun([AC_CONFIG_MACRO_DIRS], [_AM_CONFIG_MACRO_DIRS($@)])]) > Thanks for catching this issue. I've amended my patch to follow your advice. I've also added: Helped-by: Eric Blake to the commit message. Best regards, Stefano
Re: [PATCH] aclocal: AC_CONFIG_MACRO_DIRS: work around autom4te option parsing bugs
On 11/15/2012 04:52 AM, Stefano Lattarini wrote: > On 11/15/2012 11:58 AM, Stefano Lattarini wrote: >> >> The below patch should allow our users to employ AC_CONFIG_MACRO_DIRS >> with autoconf 2.69 as well. It still doesn't work with autoconf 2.68 >> and earlier though, due to a bug in autom4te option parsing (fixed by >> autoconf commit v2.68-120-gf4be358). That could be fixed by using an >> external file rather than stdin to pass aclocal the contents of >> '$ac_config_macro_dirs_fallback'; but I rather do so in a separate >> patch, with a dedicated rationale. >> > Done with the patch below. I'll push it (together with the earlier one) > by this evening (CET). > > +++ b/m4/internal/ac-config-macro-dirs.m4 > @@ -0,0 +1,16 @@ > +# Support AC_CONFIG_MACRO_DIRS with older autoconf. -*- Autoconf -*- > +# FIXME: To be removed in Automake 1.14, once we can assume autoconf > +#2.70 or later. > +# FIXME: keep ion sync with the contents of the variable s/ion/in/ > +#'$ac_config_macro_dirs_fallback' in aclocal.in. > + > +# Copyright (C) 2012 Free Software Foundation, Inc. > +# > +# This file is free software; the Free Software Foundation > +# gives unlimited permission to copy and/or distribute it, > +# with or without modifications, as long as this notice is preserved. > + > +m4_ifndef([AC_CONFIG_MACRO_DIRS], [dnl > + m4_defun([_AM_CONFIG_MACRO_DIRS], [])dnl > + m4_defun([AC_CONFIG_MACRO_DIRS], [_AM_CONFIG_MACRO_DIRS($@)])dnl > +]) Hmm - this version is slightly different than the perl version it is replacing. Each use of AC_CONFIG_MACRO_DIRS now injects whitespace (6 spaces) into the user's configure script, then ignores the rest of the line (or worse, changes the rest of the line). That is, if the user writes: AC_CONFIG_MACRO_DIRS([a])AC_CONFIG_MACRO_DIRS([b]) the perl version transforms it into the empty string with two trace calls, but this version transforms it into: dnlAC_CONFIG_MACRO_DIRS([b]) and only traces 'a'. Here's how I would fix it (using the style I use in autoconf): m4_ifndef([AC_CONFIG_MACRO_DIRS], [m4_defun([_AM_CONFIG_MACRO_DIRS])]dnl [m4_defun([AC_CONFIG_MACRO_DIRS], [_AM_CONFIG_MACRO_DIRS($@)])]) -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[PATCH] aclocal: AC_CONFIG_MACRO_DIRS: work around autom4te option parsing bugs (was: Re: bug#12845: [PATCH] aclocal: tracing AC_CONFIG_MACRO_DIRS can work with older autoconf as well)
On 11/15/2012 11:58 AM, Stefano Lattarini wrote:
>
> The below patch should allow our users to employ AC_CONFIG_MACRO_DIRS
> with autoconf 2.69 as well. It still doesn't work with autoconf 2.68
> and earlier though, due to a bug in autom4te option parsing (fixed by
> autoconf commit v2.68-120-gf4be358). That could be fixed by using an
> external file rather than stdin to pass aclocal the contents of
> '$ac_config_macro_dirs_fallback'; but I rather do so in a separate
> patch, with a dedicated rationale.
>
Done with the patch below. I'll push it (together with the earlier one)
by this evening (CET).
Regards,
Stefano
8< 8< 8< 8< 8< 8< 8< 8< 8<
>From a5cddc48e23a650c75f270dcb2e63d57ea3aa07b Mon Sep 17 00:00:00 2001
Message-Id:
From: Stefano Lattarini
Date: Thu, 15 Nov 2012 12:24:27 +0100
Subject: [PATCH] aclocal: AC_CONFIG_MACRO_DIRS: work around autom4te option
parsing bugs
The autom4te program coming with autoconf 2.68 and earlier had a bug
which caused the "-" command line argument (with which we tell it to
read some input from from standard input) to aways be pushed at the
*end* of the command line, regardless of where the user specified it
(that bug was fixed by autoconf commit 'v2.68-120-gf4be358', "getopt:
new Autom4te::Getopt module").
This broken semantics conflict with our usage in aclocal, where we
need to pass some input to the invoked autom4te program early, and
have so far been using the stdin to do so. Now we start using an
external file instead.
* m4/internal/ac-config-macro-dirs.m4: New file, contain a fallback
definition of the AC_CONFIG_MACRO_DIRS macro for older autoconf
releases.
* aclocal.in (trace_used_macros): When invoking autom4te, use that
file instead of "abusing" standard input.
* Makefile.am (dist_automake_ac_DATA): Rename ...
(nobase_dist_automake_ac_DATA): ... like this.
Add 'm4/internal/ac-config-macro-dirs.m4' to it.
Signed-off-by: Stefano Lattarini
---
Makefile.am | 3 ++-
aclocal.in | 26 +++---
m4/internal/ac-config-macro-dirs.m4 | 16
3 files changed, 33 insertions(+), 12 deletions(-)
create mode 100644 m4/internal/ac-config-macro-dirs.m4
diff --git a/Makefile.am b/Makefile.am
index 065500f..34abc5a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -255,7 +255,8 @@ dist_am_DATA = \
## Automake-provided m4 macros. ##
## -- ##
-dist_automake_ac_DATA = \
+nobase_dist_automake_ac_DATA = \
+ m4/internal/ac-config-macro-dirs.m4 \
m4/amversion.m4 \
m4/ar-lib.m4 \
m4/as.m4 \
diff --git a/aclocal.in b/aclocal.in
index 3ee83c9..ce8ac5a 100644
--- a/aclocal.in
+++ b/aclocal.in
@@ -48,12 +48,12 @@ use File::Path ();
# Support AC_CONFIG_MACRO_DIRS also with older autoconf.
# FIXME: To be removed in Automake 1.14, once we can assume autoconf
#2.70 or later.
-# NOTE: This variable deliberately contain no newlines.
+# FIXME: keep in sync with 'internal/ac-config-macro-dirs.m4'.
my $ac_config_macro_dirs_fallback =
- "m4_ifndef([AC_CONFIG_MACRO_DIRS], [" .
-"m4_defun([_AM_CONFIG_MACRO_DIRS], [])" .
-"m4_defun([AC_CONFIG_MACRO_DIRS], [_AM_CONFIG_MACRO_DIRS(\$@)])" .
- "])";
+ 'm4_ifndef([AC_CONFIG_MACRO_DIRS], [' .
+'m4_defun([_AM_CONFIG_MACRO_DIRS], [])' .
+'m4_defun([AC_CONFIG_MACRO_DIRS], [_AM_CONFIG_MACRO_DIRS($@)])' .
+ '])';
# We do not operate in threaded mode.
$perl_threads = 0;
@@ -726,23 +726,27 @@ sub trace_used_macros ()
my %files = map { $map{$_} => 1 } keys %macro_seen;
%files = strip_redundant_includes %files;
- my $early_m4_code = "";
# When AC_CONFIG_MACRO_DIRS is used, avoid possible spurious warnings
# from autom4te about macros being "m4_require'd but not m4_defun'd";
# for more background, see:
# http://lists.gnu.org/archive/html/autoconf-patches/2012-11/msg4.html
# as well as autoconf commit 'v2.69-44-g1ed0548', "warn: allow aclocal
# to silence m4_require warnings".
- $early_m4_code .= "m4_define([m4_require_silent_probe], [-])";
- # Support AC_CONFIG_MACRO_DIRS also with older autoconf.
- # FIXME: To be removed in Automake 1.14, once we can assume autoconf
- #2.70 or later.
- $early_m4_code .= $ac_config_macro_dirs_fallback;
+ my $early_m4_code .= "m4_define([m4_require_silent_probe], [-])";
my $traces = ($ENV{AUTOM4TE} || '@am_AUTOM4TE@');
$traces .= " --language Autoconf-without-aclocal-m4 ";
$traces = "echo '$early_m4_code' | $traces - ";
+ # Support AC_CONFIG_MACRO_DIRS also with older autoconf.
+ # Note that we can't use '$ac_config_macro_dirs
