bug-autoconf
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: another problem after updating autoconf-2.61 to 2.62


From: Eric Blake
Subject: Re: another problem after updating autoconf-2.61 to 2.62
Date: Thu, 5 Jun 2008 21:23:41 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

Karsten Hopp <karsten <at> redhat.com> writes:

> 
> People are starting to use this in Fedora and are filing bugreports. Here's 
another one:
> 
>    AT_KEYWORDS([m4_if([$1], [dummy], [], [$1 ])dummy])

I argue that this is a bug in the user's code; the documentation for 
AT_KEYWORDS is unchanged since 2.61:

 -- Macro: AT_KEYWORDS (KEYWORDS)
     Associate the space-separated list of KEYWORDS to the enclosing
     test group.

where the implication is the same for any other macro that takes a space-
separated list - passing a macro name rather than the expansion of a macro is 
problematic, because the algorithm won't know if the macro name expands into 
something that contains spaces.  The user should have written:

AT_KEYWORDS(m4_if([$1], [dummy], [], [$1 ])[dummy])

ie. expand m4_if PRIOR to passing it to AT_KEYWORDS.  


At any rate, the change in behavior occurred in this patch:
http://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=commitdiff;h=5b6f5

It used to be that AT_KEYWORDS merely appended its argument to the list, 
regardless of whether its argument could be further expanded in the final 
output file, but that can result in duplicate keywords.

In 2.62, the implementation switched to the new m4_append_uniq_w, such that 
duplicates are avoided, and anything you pass to AT_KEYWORDS is treated as 
literal whitespace-separated keywords with no further expansion; so you are 
treating "m4_if([$1]," as a keyword, "[]," as a keyword, and "[$1], [])dummy" 
as a keyword (the third keyword is an artifact of how m4_split handles 
splitting on whitespace nested inside layers of []).  This explains the 
complaint about unexpanded m4_if.


But I'm feeling nice today (maybe because it was my commit that exposed this 
bug in the user's file).  So even though the change only affects user code that 
wasn't compliant to the documentation, you are correct that it is an 
(unintended) change in behavior.  And it just happens to be possible to make 
AT_KEYWORDS support the old behavior by judiciously using m4_expand.  I'm 
committing this for the eventual 2.63 (and you can extract the one-liner code 
change in lib/autotest/general.m4 as a distro patch against 2.62):

>From 3ac7ceb0c2dfd023e0e9e944da6825b6bd676568 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 5 Jun 2008 15:18:11 -0600
Subject: [PATCH] Fix regression in AT_KEYWORDS([Macro]), from 2007-10-18.

* lib/autotest/general.m4 (AT_KEYWORDS): Expand argument prior to
converting it to lower case.
* tests/autotest.at (Keywords and ranges): Test this.
* NEWS: Document the fix.
* THANKS: Update.
Reported by Karsten Hopp.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog               |   10 ++++++++++
 NEWS                    |    4 ++++
 THANKS                  |    1 +
 lib/autotest/general.m4 |    5 +++--
 tests/autotest.at       |    4 ++--
 5 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 76340a6..ef3d04d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2008-06-05  Eric Blake  <address@hidden>
+
+       Fix regression in AT_KEYWORDS([Macro]), from 2007-10-18.
+       * lib/autotest/general.m4 (AT_KEYWORDS): Expand argument prior to
+       converting it to lower case.
+       * tests/autotest.at (Keywords and ranges): Test this.
+       * NEWS: Document the fix.
+       * THANKS: Update.
+       Reported by Karsten Hopp.
+
 2008-06-03  Eric Blake  <address@hidden>
 
        Fix 'make dist' regression from 2008-05-08.
diff --git a/NEWS b/NEWS
index 183d4f1..8b866ad 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,10 @@ GNU Autoconf NEWS - User visible changes.
 ** Two new quadrigraphs have been introduced: @{:@ for (, and @:}@ for ),
    allowing the output of unbalanced parantheses in more contexts.
 
+** AT_KEYWORDS once again performs expansion on its argument, such that
+   AT_KEYWORDS([m4_if([$1], [], [default])]) no longer complains about
+   the possibly unexpanded m4_if [regression introduced in 2.62].
+
 
 * Major changes in Autoconf 2.62 (2008-04-05) [stable]
   Released by Eric Blake, based on git versions 2.61a.*.
diff --git a/THANKS b/THANKS
index 5ec8921..224cdea 100644
--- a/THANKS
+++ b/THANKS
diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index 88d10a3..993dd9f 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -1476,9 +1476,10 @@ m4_case([$1],
 # AT_KEYWORDS(KEYWORDS)
 # ---------------------
 # Declare a list of keywords associated to the current test group.
-# The list is stored in lower case, since the -k option is case-insensitive.
+# Since the -k option is case-insensitive, the list is stored in lower case
+# to avoid duplicates that differ only by case.
 _AT_DEFINE_SETUP([AT_KEYWORDS],
-[m4_append_uniq_w([AT_keywords], m4_tolower([[$1]]))])
+[m4_append_uniq_w([AT_keywords], m4_tolower(m4_dquote(m4_expand([$1]))))])
 
 
 # AT_CAPTURE_FILE(FILE)
diff --git a/tests/autotest.at b/tests/autotest.at
index dc3cfd5..c4c0eda 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -690,7 +690,7 @@ AT_CHECK(:)
 AT_CLEANUP
 AT_SETUP(both) # 04
 AT_KEYWORDS([key1 key2])
-AT_KEYWORDS([key1])
+AT_KEYWORDS([m4@&address@hidden([Key1])])
 AT_CHECK(:)
 AT_CLEANUP
 AT_SETUP(test5) # 05
@@ -713,7 +713,7 @@ AT_CHECK(:)
 AT_CLEANUP
 ]])
 dnl check that AT_KEYWORDS does not duplicate words
-AT_CHECK([grep 'key1.*key1' k], [1])
+AT_CHECK([grep -i 'key1.*key1' k], [1])
 dnl check that -k requires an argument
 AT_CHECK([$CONFIG_SHELL ./k -k], [1], [], [ignore])
 
-- 
1.5.5.1







reply via email to

[Prev in Thread] Current Thread [Next in Thread]