Re: 2.63b autotest vs. bison testsuite

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 4/15/2009 2:05 PM:
>> I'm wondering whether s/AT_CHECK_NOESCAPE/AT_CHECK_EXPAND/g would be
>> good.  Even with that, the name makes me think that the macro would do
>> something different with its first argument, rather than arguments
>> number three and four.  Unfortunately, I don't have a good suggestion
>> to improve this.
> 
> Or even AT_CHECK_UNQUOTED, to match the existing naming of
> AC_DEFINE/AC_DEFINE_UNQUOTED.  Well, now's the time to make a name change,
> if any, as part of making it documented.

I'm thinking of applying this series; any problems?  The first does the
rename mentioned above.  The second patch fixes a couple of
inconsistencies from the previously undocumented interface, to make it
more like AC_DEFINE_UNQUOTED (there, we only do $, ``, and \ expansion,
but now " is literal, as if in an unquoted here-doc, rather than causing
the test to fall apart).  The third swaps the order of m4_expand and
AS_ESCAPE in AT_CHECK, so that even characters inside macro expansions
that need escaping are protected.

Of the three, only the last one presents any backwards incompatibilities,
but as the bison testsuite recently went through some serious hoops to try
to work around AT_CHECK failing to shell-escape macro contents, it seems
like it is a true bug fix as opposed to a gratuitous change in behavior.
I'm presently running several tests to see how many packages even try to
use macros expanding to \"`$ inside the AT_CHECK arguments, which would be
altered by this patch.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAknv6aYACgkQ84KuGfSFAYBa/wCfaihsPlYyI5NBFxPMVrc0bmUH
1a8An2V1Ss8IKwaVagwhqZzYYgjNrTN+
=Iihc
-----END PGP SIGNATURE-----

>From 009205aadf12e62813e7606ac3eba0c752478f66 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 22 Apr 2009 17:14:13 -0600
Subject: [PATCH 1/3] Rename AT_CHECK_NOESCAPE to AT_CHECK_UNQUOTED.

* lib/autotest/general.m4 (AT_CHECK_NOESCAPE): Deprecate, in favor
of new spelling...
(AT_CHECK_UNQUOTED): ...for consistency with AC_DEFINE_UNQUOTED.
* doc/autoconf.texi (Writing Testsuites) <AT_CHECK>: Document the
rename.
* NEWS: Likewise.
* tests/autotest.at (Binary output, Cleanup): Adjust tests.
* tests/torture.at (AC_CONFIG_FILES, HEADERS, LINKS and COMMANDS):
Likewise.
Reported by Ralf Wildenhues.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog               |   12 ++++++++++++
 NEWS                    |    3 ++-
 doc/autoconf.texi       |    6 +++---
 lib/autotest/general.m4 |   16 ++++++++++++----
 tests/autotest.at       |    8 ++++----
 tests/torture.at        |    4 ++--
 6 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d40e9c0..ea083fd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
 2009-04-22  Eric Blake  <address@hidden>

+       Rename AT_CHECK_NOESCAPE to AT_CHECK_UNQUOTED.
+       * lib/autotest/general.m4 (AT_CHECK_NOESCAPE): Deprecate, in favor
+       of new spelling...
+       (AT_CHECK_UNQUOTED): ...for consistency with AC_DEFINE_UNQUOTED.
+       * doc/autoconf.texi (Writing Testsuites) <AT_CHECK>: Document the
+       rename.
+       * NEWS: Likewise.
+       * tests/autotest.at (Binary output, Cleanup): Adjust tests.
+       * tests/torture.at (AC_CONFIG_FILES, HEADERS, LINKS and COMMANDS):
+       Likewise.
+       Reported by Ralf Wildenhues.
+
        Change FOO placeholder to use @var{text} instead.
        * doc/autoconf.texi (Configuration Actions): Rename AC_CONFIG_FOOS
        to address@hidden
diff --git a/NEWS b/NEWS
index 107a10c..2831d66 100644
--- a/NEWS
+++ b/NEWS
@@ -16,7 +16,8 @@ GNU Autoconf NEWS - User visible changes.
    of AT_XFAIL_IF.  It also understands the new directives
    ignore-nolog, stdout-nolog, and stderr-nolog.

-** The autotest macro AT_CHECK_NOESCAPE is now documented.
+** The following documented autotest macros are new:
+   AT_CHECK_UNQUOTED

 ** The following documented m4sugar macros are new:
    m4_argn  m4_default_nblank  m4_default_nblank_quoted  m4_ifblank
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 74b64d5..2eca82d 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -22403,10 +22403,10 @@ Writing Testsuites

 @defmac AT_CHECK (@var{commands}, @dvar{status, 0}, @ovar{stdout}, @
   @ovar{stderr}, @ovar{run-if-fail}, @ovar{run-if-pass})
address@hidden AT_CHECK_NOESCAPE (@var{commands}, @dvar{status, 0}, 
@ovar{stdout}, @
address@hidden AT_CHECK_UNQUOTED (@var{commands}, @dvar{status, 0}, 
@ovar{stdout}, @
   @ovar{stderr}, @ovar{run-if-fail}, @ovar{run-if-pass})
 @atindex{CHECK}
address@hidden
address@hidden
 Execute a test by performing given shell @var{commands}.  These commands
 should normally exit with @var{status}, while producing expected
 @var{stdout} and @var{stderr} contents.  If @var{commands} exit with
@@ -22433,7 +22433,7 @@ Writing Testsuites
 @var{commands} on standard out and standard error (including an empty
 parameter for no output); any differences are captured in the testsuite
 log and the test is failed.  The difference between @code{AT_CHECK} and
address@hidden is that only the latter performs shell
address@hidden is that only the latter performs shell
 expansions on comparison text given in the @var{stdout} and @var{stderr}
 arguments.

diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index 2e622a5..f959a41 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -332,7 +332,7 @@ at_fn_check_skip ()
 {
   case $[1] in
     99) echo 99 > "$at_status_file"; at_failed=:
-        AS_ECHO(["$[2]: hard failure"]); exit 99;;
+       AS_ECHO(["$[2]: hard failure"]); exit 99;;
     77) echo 77 > "$at_status_file"; exit 77;;
   esac
 }
@@ -350,7 +350,7 @@ dnl $? = 77 or $? = 99.
     $[1] ) ;;
     77) echo 77 > "$at_status_file"; exit 77;;
     99) echo 99 > "$at_status_file"; at_failed=:
-        AS_ECHO(["$[3]: hard failure"]); exit 99;;
+       AS_ECHO(["$[3]: hard failure"]); exit 99;;
     *) AS_ECHO(["$[3]: exit code was $[2], expected $[1]"])
       at_failed=:;;
   esac
@@ -1904,15 +1904,23 @@ _AT_DEFINE_SETUP([AT_CHECK],
 [_AT_CHECK(m4_expand([$1]), [$2], m4_expand([AS_ESCAPE([$3])]),
   m4_expand([AS_ESCAPE([$4])]), [$5], [$6])])

-# AT_CHECK_NOESCAPE(COMMANDS, [STATUS = 0], STDOUT, STDERR,
+# AT_CHECK_UNQUOTED(COMMANDS, [STATUS = 0], STDOUT, STDERR,
 #                   [RUN-IF-FAIL], [RUN-IF-PASS])
 # ---------------------------------------------------------
 # Like AT_CHECK, but do not AS_ESCAPE shell metacharacters in the STDOUT
 # and STDERR arguments before running the comparison.
-_AT_DEFINE_SETUP([AT_CHECK_NOESCAPE],
+_AT_DEFINE_SETUP([AT_CHECK_UNQUOTED],
 [_AT_CHECK(m4_expand([$1]), [$2], m4_expand([$3]),
   m4_expand([$4]), [$5], [$6])])

+# AT_CHECK_NOESCAPE(COMMANDS, [STATUS = 0], STDOUT, STDERR,
+#                   [RUN-IF-FAIL], [RUN-IF-PASS])
+# ---------------------------------------------------------
+# Obsolete spelling of AT_CHECK_UNQUOTED.
+m4_define([AT_CHECK_NOESCAPE],
+[m4_warn([obsolete], [use AT_CHECK_UNQUOTED instead of $0])]dnl
+[AT_CHECK_UNQUOTED($@)])
+

 # _AT_DECIDE_TRACEABLE(COMMANDS)
 # ------------------------------
diff --git a/tests/autotest.at b/tests/autotest.at
index 721925c..827808c 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -349,7 +349,7 @@ AT_CHECK_AT([Binary output],
     str=$str$str$str$str$str$str$str$str$str$str
     str=$str$str$str$str$str$str$str$str$str$str
     str=$str$str$str$str$str
-    AT_CHECK_NOESCAPE([echo $str], [0], [[$str]m4_newline])
+    AT_CHECK_UNQUOTED([echo $str], [0], [[$str]m4_newline])
     AT_CLEANUP
     AT_SETUP([fail: no trailing newline])
     AT_CHECK([printf short], [0], [stdout-nolog])
@@ -366,7 +366,7 @@ AT_CHECK_AT([Binary output],
     str=$str$str$str$str$str$str$str$str$str$str
     str=$str$str$str$str$str$str$str$str$str$str
     str=$str$str$str$str$str
-    AT_CHECK_NOESCAPE([echo x$str], [0], [[${str}x]m4_newline])
+    AT_CHECK_UNQUOTED([echo x$str], [0], [[${str}x]m4_newline])
     AT_CLEANUP
 ]], [], [0], [], [], [],
   [AT_CHECK([$CONFIG_SHELL ./micro-suite 4], [1], [ignore], [ignore])
@@ -376,10 +376,10 @@ AT_CHECK_AT([Binary output],

 AT_CHECK_AT_TEST([Cleanup],
   [AT_CHECK([test ! -f cleanup.success && test ! -f cleanup.failure])
-   AT_CHECK_NOESCAPE([exit $value], [ignore], [$output],
+   AT_CHECK_UNQUOTED([exit $value], [ignore], [$output],
      [], [touch cleanup.failure], [touch cleanup.success])],
   [], [], [], [],
-  [AT_KEYWORDS([AT@&address@hidden)
+  [AT_KEYWORDS([AT@&address@hidden)
     output=; export output],
   [AT_CHECK([test -d micro-suite.dir/1])
    AT_CHECK([test -f micro-suite.dir/1/cleanup.success])
diff --git a/tests/torture.at b/tests/torture.at
index 9d58e51..11cd05a 100644
--- a/tests/torture.at
+++ b/tests/torture.at
@@ -326,11 +326,11 @@ END
           [0], [ignore])
   # Run the same test a 2nd time to see that config.status does not recreate
   # the header (regression test)
-  AT_CHECK_NOESCAPE([./config.status "--header=$file:$file.in"],
+  AT_CHECK_UNQUOTED([./config.status "--header=$file:$file.in"],
                    [0], [config.status: creating $file
 config.status: $file is unchanged
 ])
-  AT_CHECK_NOESCAPE([grep ' & ' "$file"], [],
+  AT_CHECK_UNQUOTED([grep ' & ' "$file"], [],
 [/* $file.  Generated from $file.in by configure.  */
 ])
   AT_CHECK([$FGREP "$file" "$file"], [0], [ignore])
-- 
1.6.1.2


>From 87a645ff09c79ef0d4bf74e5909a82f5f94c965e Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 22 Apr 2009 21:38:29 -0600
Subject: [PATCH 2/3] Make AT_CHECK_UNQUOTED more like AC_DEFINE_UNQUOTED.

* lib/autotest/general.m4 (AT_CHECK_NOESCAPE): Keep older,
undocumented semantics, which mishandle ".
(AT_CHECK_UNQUOTED): Handle " correctly.
* tests/autotest.at (unquoted output): New test.
* doc/autoconf.texi (Writing Testsuites) <AT_CHECK>: Mention which
shell expansions are handled.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog               |    8 ++++++++
 doc/autoconf.texi       |   11 +++++++----
 lib/autotest/general.m4 |    9 +++++----
 tests/autotest.at       |    7 +++++++
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index ea083fd..c571bbd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-04-22  Eric Blake  <address@hidden>

+       Make AT_CHECK_UNQUOTED more like AC_DEFINE_UNQUOTED.
+       * lib/autotest/general.m4 (AT_CHECK_NOESCAPE): Keep older,
+       undocumented semantics, which mishandle ".
+       (AT_CHECK_UNQUOTED): Handle " correctly.
+       * tests/autotest.at (unquoted output): New test.
+       * doc/autoconf.texi (Writing Testsuites) <AT_CHECK>: Mention which
+       shell expansions are handled.
+
        Rename AT_CHECK_NOESCAPE to AT_CHECK_UNQUOTED.
        * lib/autotest/general.m4 (AT_CHECK_NOESCAPE): Deprecate, in favor
        of new spelling...
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 2eca82d..eab3065 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -8760,7 +8760,8 @@ Defining Symbols
 Like @code{AC_DEFINE}, but three shell expansions are
 performed---once---on @var{variable} and @var{value}: variable expansion
 (@samp{$}), command substitution (@samp{`}), and backslash escaping
-(@samp{\}).  Single and double quote characters in the value have no
+(@samp{\}), as if in an unquoted here-doc.  Single and double quote
+characters in the value have no
 special meaning.  Use this macro instead of @code{AC_DEFINE} when
 @var{variable} or @var{value} is a shell variable.  Examples:

@@ -22433,9 +22434,11 @@ Writing Testsuites
 @var{commands} on standard out and standard error (including an empty
 parameter for no output); any differences are captured in the testsuite
 log and the test is failed.  The difference between @code{AT_CHECK} and
address@hidden is that only the latter performs shell
-expansions on comparison text given in the @var{stdout} and @var{stderr}
-arguments.
address@hidden is that only the latter performs shell variable
+expansion (@samp{$}), command substitution (@samp{`}), and backslash
+escaping (@samp{\}) on comparison text given in the @var{stdout} and
address@hidden arguments; if the text includes a trailing newline, this
+would be the same as if it were specified via an unquoted here-doc.

 @table @samp
 @item ignore
diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index f959a41..a16224b 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -1910,16 +1910,17 @@ _AT_DEFINE_SETUP([AT_CHECK],
 # Like AT_CHECK, but do not AS_ESCAPE shell metacharacters in the STDOUT
 # and STDERR arguments before running the comparison.
 _AT_DEFINE_SETUP([AT_CHECK_UNQUOTED],
-[_AT_CHECK(m4_expand([$1]), [$2], m4_expand([$3]),
-  m4_expand([$4]), [$5], [$6])])
+[_AT_CHECK(m4_expand([$1]), [$2], AS_ESCAPE(m4_dquote(m4_expand([$3])), [""]),
+  AS_ESCAPE(m4_dquote(m4_expand([$4])), [""]), [$5], [$6])])

 # AT_CHECK_NOESCAPE(COMMANDS, [STATUS = 0], STDOUT, STDERR,
 #                   [RUN-IF-FAIL], [RUN-IF-PASS])
 # ---------------------------------------------------------
 # Obsolete spelling of AT_CHECK_UNQUOTED.
 m4_define([AT_CHECK_NOESCAPE],
-[m4_warn([obsolete], [use AT_CHECK_UNQUOTED instead of $0])]dnl
-[AT_CHECK_UNQUOTED($@)])
+[m4_warn([obsolete], [consider using AT_CHECK_UNQUOTED instead of $0])]dnl
+[_AT_CHECK(m4_expand([$1]), [$2], m4_expand([$3]),
+  m4_expand([$4]), [$5], [$6])])


 # _AT_DECIDE_TRACEABLE(COMMANDS)
diff --git a/tests/autotest.at b/tests/autotest.at
index 827808c..0fad78d 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -287,6 +287,13 @@ AT_CHECK_AT_TEST([errexit],
    AT_CHECK([grep "1 .* inhibited subsequent" stderr], [], [ignore])],
   [--errexit])

+AT_CHECK_AT_TEST([unquoted output],
+  [m4_define([backtick], [`])
+   a=a
+   AT_CHECK_UNQUOTED([echo 'a"b  backtick`'], [],
+     [${a}"`echo 'b  '`\`\backtick]m4_newline)],
+  [], [], [], [], [AT_KEYWORDS([AT@&address@hidden)])
+
 AT_CHECK_AT([Logging],
   [[AT_INIT([artificial test suite])
     dnl intentionally write failing tests, to see what gets logged
-- 
1.6.1.2


>From 3138e64081c2219cd28e9bb4faa8be9a3f27beb3 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 22 Apr 2009 22:06:04 -0600
Subject: [PATCH 3/3] Fix quoting of m4 macros in AT_CHECK.

* lib/autotest/general.m4 (AT_CHECK): Expand prior to adding
escapes, to avoid shell syntax errors caused by late macro
expansion.
* NEWS: Document this change.
* tests/autotest.at (Metacharacters in command from M4 expansion):
New test.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog               |    8 ++++++++
 NEWS                    |    3 +++
 lib/autotest/general.m4 |    4 ++--
 tests/autotest.at       |    6 ++++++
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c571bbd..9768a33 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-04-22  Eric Blake  <address@hidden>

+       Fix quoting of m4 macros in AT_CHECK.
+       * lib/autotest/general.m4 (AT_CHECK): Expand prior to adding
+       escapes, to avoid shell syntax errors caused by late macro
+       expansion.
+       * NEWS: Document this change.
+       * tests/autotest.at (Metacharacters in command from M4 expansion):
+       New test.
+
        Make AT_CHECK_UNQUOTED more like AC_DEFINE_UNQUOTED.
        * lib/autotest/general.m4 (AT_CHECK_NOESCAPE): Keep older,
        undocumented semantics, which mishandle ".
diff --git a/NEWS b/NEWS
index 2831d66..11466d6 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,9 @@ GNU Autoconf NEWS - User visible changes.
 ** Ensure AT_CHECK can support commands that include a # given with
    proper m4 quoting.  For shell comments, this is a new feature; for
    non-shell comments, this fixes a regression introduced in 2.63b.
+   Additionally, AT_CHECK correctly supplies shell escapes for
+   metacharacters occurring in m4 macro expansions within the expected
+   stdout and stderr parameters.

 ** The macro AT_CHECK now understands the concept of hard failure.  If
    a test exits with an unexpected status 99, cleanup actions for the
diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index a16224b..ce65397 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -1901,8 +1901,8 @@ $2[]_ATEOF
 # This may cause spurious failures when the test suite is run with `-x'.
 #
 _AT_DEFINE_SETUP([AT_CHECK],
-[_AT_CHECK(m4_expand([$1]), [$2], m4_expand([AS_ESCAPE([$3])]),
-  m4_expand([AS_ESCAPE([$4])]), [$5], [$6])])
+[_AT_CHECK(m4_expand([$1]), [$2], AS_ESCAPE(m4_dquote(m4_expand([$3]))),
+  AS_ESCAPE(m4_dquote(m4_expand([$4]))), [$5], [$6])])

 # AT_CHECK_UNQUOTED(COMMANDS, [STATUS = 0], STDOUT, STDERR,
 #                   [RUN-IF-FAIL], [RUN-IF-PASS])
diff --git a/tests/autotest.at b/tests/autotest.at
index 0fad78d..d86088c 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -491,6 +491,12 @@ bar'])
 ]], [])])


+AT_CHECK_AT_TEST([Metacharacters in command from M4 expansion],
+  [m4_define([GNU], [\"`])
+   AT_CHECK([echo '\"`' [GNU] 'GNU'], 0, [GNU [G][NU] [\"`
+]], [])])
+
+
 ## -------------------------------------- ##
 ## Backslash-<newline> in test commands.  ##
 ## -------------------------------------- ##
-- 
1.6.1.2