Re: (2.59 bug) Extra whitespace in CFLAGS causes false "cache corrupt"

[Ralf Wildenhues]

Hi Paul,

* Paul Eggert wrote on Sun, Apr 30, 2006 at 09:32:56AM CEST:
> Ralf Wildenhues <address@hidden> writes:
> 
> > Proposed patch below.  Testsuite is still running, and I'd like someone
> > to look very closely over this.
> 
> Doesn't this fix mishandle the case where one of $ac_configure_args
> contains an apostrophe?  That is because the revised solution uses
> $ac_arg inside apostrophes.

Argh.  At least my sense of having overlooked something wasn't wrong
there.

Sigh.  We could actually use "$@" here and skip the eval step, except
that then, the output would suffer.  :-(

So here goes another try, with re-escaping once more.  Note we could
also do better separation of arguments the first time we parse, say
to fill variables $ac_sub_configure_args right away; but I'm pretty
sure there are packages out there that modify $ac_configure_args
behind Autoconf's back and will break if we do that.

I also noticed that, since we started to single-quote all arguments,
and not just those that contain active characters, those ominous shell
patterns can be considerably simplified.  (Hmm, we did this before 2.59;
the fact that this didn't raise bug reports yet may indicate that doing
all the parsing in the init step may not be such a bad idea after all..
I may get back to that post-2.60.)

I also folded the test into two existing tests.

What do you think now?  Before actually committing I'll test this on
some older shells..

Cheers, and thanks for the review!
Ralf

        * lib/autoconf/status.m4 (_AC_OUTPUT_SUBDIRS): Take care not to
        munge (multiple) white space and other oddities.
        * tests/torture.at (AT_CHECK_AC_ARG_VAR): Make sure to M4-escape
        single quotes in variable assignment.
        (AC_ARG_VAR, configure invocation): Adjust tests to expose this
        and similar failures by adding multiple spaces, tabs, and other
        special characters.
        Report and different test suggested by Francesco Romani
        <address@hidden> and Andrew Church <address@hidden>.

        * lib/autoconf/general.m4 (_AC_INIT_PREPARE): When we escape
        single quotes, we only need to search for single quotes; this
        both simplifies the search pattern, and makes us less
        susceptible to `echo' variations for arguments not containing
        single quotes.
        (_AC_ARG_VAR_VALIDATE): Likewise.

Index: lib/autoconf/general.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/autoconf/general.m4,v
retrieving revision 1.913
diff -u -r1.913 general.m4
--- lib/autoconf/general.m4     28 Apr 2006 04:17:51 -0000      1.913
+++ lib/autoconf/general.m4     30 Apr 2006 15:05:17 -0000
@@ -1156,9 +1156,7 @@
     -q | -quiet | --quiet | --quie | --qui | --qu | --q \
     | -silent | --silent | --silen | --sile | --sil)
       continue ;;
-dnl If you change this globbing pattern, test it on an old shell --
-dnl it's sensitive.  Putting any kind of quote in it causes syntax errors.
-  [  *" "*|*"  "*|*[\\\[\~\#\$\^\&\*\(\)\{\}\|\;\<\>\?\"\']*|*\]*)]
+    *\'*)
       ac_arg=`echo "$ac_arg" | sed "s/'/'\\\\\\\\''/g"` ;;
     esac
     case $ac_pass in
@@ -1500,10 +1498,7 @@
   # Pass precious variables to config.status.
   if test "$ac_new_set" = set; then
     case $ac_new_val in
-dnl If you change this globbing pattern, test it on an old shell --
-dnl it's sensitive.  Putting any kind of quote in it causes syntax errors.
-[    *" "*|*"  "*|*[\\\[\~\#\$\^\&\*\(\)\{\}\|\;\<\>\?\"\']*|*\]*)]
-      ac_arg=$ac_var=`echo "$ac_new_val" | sed "s/'/'\\\\\\\\''/g"` ;;
+    *\'*) ac_arg=$ac_var=`echo "$ac_new_val" | sed "s/'/'\\\\\\\\''/g"` ;;
     *) ac_arg=$ac_var=$ac_new_val ;;
     esac
     case " $ac_configure_args " in
Index: lib/autoconf/status.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/autoconf/status.m4,v
retrieving revision 1.98
diff -u -r1.98 status.m4
--- lib/autoconf/status.m4      24 Apr 2006 19:36:02 -0000      1.98
+++ lib/autoconf/status.m4      30 Apr 2006 15:05:17 -0000
@@ -915,7 +915,10 @@
   # Remove --cache-file and --srcdir arguments so they do not pile up.
   ac_sub_configure_args=
   ac_prev=
-  for ac_arg in $ac_configure_args; do
+  eval set x "$ac_configure_args"
+  shift
+  for ac_arg
+  do
     if test -n "$ac_prev"; then
       ac_prev=
       continue
@@ -938,7 +941,11 @@
       ac_prev=prefix ;;
     -prefix=* | --prefix=* | --prefi=* | --pref=* | --pre=* | --pr=* | --p=*)
       ;;
-    *) ac_sub_configure_args="$ac_sub_configure_args $ac_arg" ;;
+    *)
+      case $ac_arg in
+      *\'*) ac_arg=`echo "$ac_arg" | sed "s/'/'\\\\\\\\''/g"` ;;
+      esac
+      ac_sub_configure_args="$ac_sub_configure_args '$ac_arg'" ;;
     esac
   done
 
@@ -984,8 +991,8 @@
 
       AC_MSG_NOTICE([running $ac_sub_configure $ac_sub_configure_args 
--cache-file=$ac_sub_cache_file --srcdir=$ac_srcdir])
       # The eval makes quoting arguments work.
-      eval $ac_sub_configure $ac_sub_configure_args \
-          --cache-file=$ac_sub_cache_file --srcdir=$ac_srcdir ||
+      eval "$ac_sub_configure $ac_sub_configure_args \
+          --cache-file=$ac_sub_cache_file --srcdir=$ac_srcdir" ||
        AC_MSG_ERROR([$ac_sub_configure failed for $ac_dir])
     fi
 
Index: tests/torture.at
===================================================================
RCS file: /cvsroot/autoconf/autoconf/tests/torture.at,v
retrieving revision 1.59
diff -u -r1.59 torture.at
--- tests/torture.at    12 Apr 2006 20:40:22 -0000      1.59
+++ tests/torture.at    30 Apr 2006 15:25:43 -0000
@@ -56,8 +56,9 @@
 [rm -f config.cache
 
 # Initial value.
+m4_pattern_allow([^m4_bpatsubst$])
 m4_ifval([$1],
-        [precious='$1'; export precious],
+        [precious='m4_bpatsubst($1, [[']], ['\\''])'; export precious],
         [unset precious])
 AT_CHECK_CONFIGURE([--config-cache -q])
 AT_CHECK([cat file], [], [`$1'
@@ -119,6 +120,9 @@
 # Change a precious variable that contains braces
 AT_CHECK_AC_ARG_VAR([apple of my {eye}], [orange of my eye])
 
+# Change a precious variable that contains all kinds of fun
+AT_CHECK_AC_ARG_VAR(['p  r     ec"iou$], [orange of my eye])
+
 AT_CLEANUP
 
 
@@ -258,34 +262,51 @@
 # Check that `configure' and `config.status' honor their interface.
 #
 # We run `./configure one=val1 --enable-two=val2 --with-three=val3'
-# and verify that (i) `configure' correctly receives the arguments and
+# and verify that (i) `configure' correctly receives the arguments,
 # (ii) correctly passes them to `config.status', which we check by
-# running `config.status --recheck'.
+# running `config.status --recheck', and (iii) correctly passes them
+# to sub-configure scripts.
 
 AT_SETUP([configure invocation])
 
+mkdir sub
+
 AT_DATA([configure.ac],
 [[AC_INIT
+AC_CONFIG_SUBDIRS([sub])
+echo "result=$one$enable_two$with_three"
+AC_OUTPUT
+]])
+
+AT_DATA([sub/configure.ac],
+[[AC_INIT
 echo "result=$one$enable_two$with_three"
 AC_OUTPUT
 ]])
 
+echo fake install-sh script >install-sh
+
 AT_CHECK_AUTOCONF
+cd sub
+AT_CHECK_AUTOCONF
+cd ..
 
 AT_CHECK_CONFIGURE([one=one --enable-two=two --with-three=three |
                     sed -n -e 's/^result=//p'], 0,
-                  [onetwothree
-])
+                  [m4_do([onetwothree
+],                       [onetwothree
+])])
 AT_CHECK([./config.status --recheck | sed -n 's/^result=//p'], 0,
         [onetwothree
 ])
 
-AT_CHECK_CONFIGURE([one="\"'$ " --enable-two="\" ' $" --with-three=" \"'$"|
+AT_CHECK_CONFIGURE([one="\"'$ " --enable-two="\" '  $" --with-three="  \"'$"|
                      sed -n -e 's/^result=//p'], 0,
-                  ["'$ " ' $ "'$
-])
+                  [m4_do(["'$ " '  $   "'$
+],                       ["'$ " '  $   "'$
+])])
 AT_CHECK([./config.status --recheck | sed -n 's/^result=//p'], 0,
-        ["'$ " ' $ "'$
+        ["'$ " '  $    "'$
 ])
 
 AT_CLEANUP