Re: m4_defn overhead

[Eric Blake]

Eric Blake <ebb9 <at> byu.net> writes:

> According to Ralf Wildenhues on 10/11/2007 11:21 AM:
> >>    (m4_append): Likewise, and quote the separator.
> > 
> > This patch causes a regression with CVS Libtool: its configure script is
> > unusably botched with this.  More specifically, when I revert the hunk
> > below, things look better, but I haven't analysed this further.  I can't
> > even tell whether it's Libtool to blame (but even if it is, we may want
> > to be a bit careful anyway).  If it's not, then some Autoconf testsuite
> > addition would seem to be in order.
> 
> Before the patch, m4_append and m4_append_uniq disagreed as to whether
> separator should be quoted.  I went with the latter, apparently libtool
> wants the former.

On further examination, I've decided the problem in libtool.m4:

m4_define([_LT_DECL],
[lt_if_append_uniq([lt_decl_varnames], [$2], [[, ]],...

where lt_if_append_uniq defers to m4_append.  Notice that the separator is the 
overquoted string `[, ]'; which prior to my m4sugar patch was being expanded by 
m4_append to strip a layer of quoting, but now it is being used as-is.  
Furthermore, this line in libtool.m4 ONLY appears on libtool HEAD, which has 
not been released yet, so I hope very few others are being bitten by this bug.

Here, I think I like the new m4sugar semantics better (if STRING is treated as 
a quoted string, then SEP should be as well - both or none, not half-and-half); 
and it would be trivial to fix lt_if_append_uniq to assume these semantics and 
write lt_append instead of relying on m4_append, so that libtool can work with 
autoconf < 2.62.  (Or we could stick with m4_append and use AC_PREREQ(2.61b), 
making libtool 2.0 require new autoconf, but I don't think that is 
appropriate...).

So, I would like to patch autoconf to add a testsuite to m4sugar.at, testing 
the NEW semantics; add a NEWS item that describes the change in semantics, but 
not alter m4sugar.m4.  I will follow up with a patch to the libtool list that 
fixes the libtool usage, regardless of autoconf version it is run with.

From: Eric Blake <address@hidden>
Date: Thu, 11 Oct 2007 14:46:40 -0600
Subject: [PATCH] Document effect of recent m4_append change on libtool HEAD.

* tests/m4sugar.at (m4@&address@hidden): New test.
* NEWS: Document semantics change.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog        |    6 ++++++
 NEWS             |    8 ++++++++
 tests/m4sugar.at |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1ccbd06..c67566d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2007-10-11  Eric Blake  <address@hidden>
+
+       Document effect of recent m4_append change on libtool HEAD.
+       * tests/m4sugar.at (m4@&address@hidden): New test.
+       * NEWS: Document semantics change.
+
 2007-10-11  Ralf Wildenhues  <address@hidden>
 
        * .gitignore: Ignore tags and TAGS files.
diff --git a/NEWS b/NEWS
index 5c0e3c5..2a59f84 100644
--- a/NEWS
+++ b/NEWS
@@ -77,6 +77,14 @@ GNU Autoconf NEWS - User visible changes.
    Packages using the undocumented m4sugar macro m4_PACKAGE_VERSION
    should consider using the new AC_VERSION instead.
 
+** The m4sugar macro m4_append has been fixed to properly quote its
+   separator argument.  Prior to this fix, m4_append_uniq would
+   mistakenly duplicate entries if the separator was active.  However,
+   it means that programs previously using
+     m4_append([name], [string], [[, ]])
+   are now using a four-character separator instead of the intended
+   comma and space.
+
 ** Warnings are now generated by default when an installer invokes
    'configure' with an unknown --enable-* or --with-* option.
    These warnings can be disabled with the new AC_DISABLE_OPTION_CHECKING
diff --git a/tests/m4sugar.at b/tests/m4sugar.at
index 81e7919..ecf961a 100644
--- a/tests/m4sugar.at
+++ b/tests/m4sugar.at
@@ -42,6 +42,8 @@ AT_CHECK_M4SUGAR([-o-],, [$2], [$3])
 #
 # - m4_split
 #
+# - m4_append
+#
 # - m4_text_wrap
 # uses m4_split code.
 
@@ -182,6 +184,53 @@ m4_split([foo='' bar=``])
 AT_CLEANUP
 
 
+## ----------- ##
+## m4_append.  ##
+## ----------- ##
+
+AT_SETUP([m4@&address@hidden)
+
+AT_CHECK_M4SUGAR_TEXT(
+[[m4_define([active], [ACTIVE])dnl
+m4_append([sentence], [This is an])dnl
+m4_append([sentence], [ active ])dnl
+m4_append([sentence], [symbol.])dnl
+sentence
+m4_undefine([active])dnl
+sentence
+m4_define([active], [ACTIVE])dnl
+m4_append([hooks], [m4_define([act1], [act2])])dnl
+m4_append([hooks], [m4_define([act2], [active])])dnl
+m4_undefine([active])dnl
+act1
+hooks
+act1
+dnl Test bug fix in 2.62 when separator is active.
+m4_define([a], [A])dnl
+m4_append_uniq([foo], [-], [a])dnl
+m4_append_uniq([foo], [-], [a])dnl
+m4_append_uniq([bar], [-], [a])dnl
+m4_append_uniq([bar], [~], [a])dnl
+m4_append_uniq([bar], [-], [a])dnl
+m4_defn([foo])
+m4_defn([bar])
+foo
+bar
+]],
+[[This is an ACTIVE symbol.
+This is an active symbol.
+act1
+
+active
+-
+-a~
+-
+-A~
+]])
+
+AT_CLEANUP
+
+
 ## -------------- ##
 ## m4_text_wrap.  ##
 ## -------------- ##
-- 
1.5.3.2