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
