more lenient precious variable checks (was: [Fwd: A question regarding recent changes to CFLAGS_FOR_TARGET])

[Ralf Wildenhues]

Hello,

a while ago, Paolo suggested off-list a patch that would help GCC:

* Paolo Bonzini wrote on Sun, Mar 02, 2008:
>
> configure: loading cache .././config.cache
> configure: error: `CFLAGS' has changed since the previous run:
> configure:   former value:  -g -O2
> configure:   current value: -g -O2
> configure: error: changes in the environment can compromise the build

by ignoring some whitespace changes in precious variables:

> --- a/lib/autoconf/general.m4
> +++ b/lib/autoconf/general.m4
> @@ -1546,6 +1546,8 @@ for ac_var in $ac_precious_vars; do
>        ac_cache_corrupted=: ;;
>      ,);;
>      *)
> +      ac_old_val=`echo $ac_old_val | sed 's,^[       ],,;s,[         
> ]$,,;s,[        ][      ]*, ,g' `
> +      ac_new_val=`echo $ac_new_val | sed 's,^[       ],,;s,[         
> ]$,,;s,[        ][      ]*, ,g' `
>        if test "x$ac_old_val" != "x$ac_new_val"; then
>       AS_MESSAGE([error: `$ac_var' has changed since the previous run:], 2)
>       AS_MESSAGE([  former value:  $ac_old_val], 2)

Back then I said that this was unacceptable, for example for
  CPPFLAGS='-Dstring=" some  string with significant whitespace"'

and other, possibly user-defined precious variables for which leading
and trailing whitespace may be significant.

My thinking is not quite so clear-cut any more, although I still feel
that GCC build is being a bit lax.  However, maybe we can degrade the
error into a warning if all we see are differences in whitespace runs?

That however leaves open the question which value to use later; in the
patch below I chose the old, cached value, for consistency with the
environment in which the cache was filled.

What do you think about this patch?  Note that the sed script from above
patch is not needed, as the omission of double quotes around $ac_old_val
takes care of killing all the whitespace differences.  Also, note that
I ugly-quoted (using `..') the values so leading and trailing whitespace
is more visible to the reader.

OK?

Thanks,
Ralf

2008-03-25  Ralf Wildenhues  <address@hidden>

        Warn, not fail on whitespace-only precious variable differences.
        * lib/autoconf/general.m4 (_AC_ARG_VAR_VALIDATE): Output
        precious variable differences less ambiguous with `ugly-quotes'.
        If their settings differ only in whitespace, do not fail, but
        reuse the old value.
        * tests/torture.at (AT_CHECK_AC_ARG_VAR): Extend macro to allow
        an optional status and expected-warning argument.  Fix m4
        quotation for initial value.
        (AC_ARG_VAR): Also test for whitespace-only differences, and
        that the old value is retained in this case.
        * doc/autoconf.texi (Setting Output Variables): Document this.
        * NEWS: Update.
        Report and initial patch by Paolo Bonzini.

diff --git a/NEWS b/NEWS
index 79d72f1..3d24e69 100644
--- a/NEWS
+++ b/NEWS
@@ -76,6 +76,9 @@ GNU Autoconf NEWS - User visible changes.
 ** `configure --help=recursive' now works in read-only trees and from
    unconfigured build trees.
 
+** If precious variables differ only in whitespace, then the cache consistency
+   check warns instead of fails, and reuses the old value.
+
 ** AT_BANNER is now documented.
 
 ** AT_SETUP now handles macro expansions properly when calculating line
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index a32e779..bb9dc0b 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -8792,7 +8792,8 @@ configure: error: run `make distclean' and/or \
 
 @noindent
 and similarly if the variable is unset, or if its content is changed.
-
+If the content has white space changes only, then the error is degraded
+to a warning only, but the old value is reused.
 
 @item
 @var{variable} is kept during automatic reconfiguration
diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
index 645108e..058e193 100644
--- a/lib/autoconf/general.m4
+++ b/lib/autoconf/general.m4
@@ -1547,10 +1547,18 @@ for ac_var in $ac_precious_vars; do
     ,);;
     *)
       if test "x$ac_old_val" != "x$ac_new_val"; then
-       AS_MESSAGE([error: `$ac_var' has changed since the previous run:], 2)
-       AS_MESSAGE([  former value:  $ac_old_val], 2)
-       AS_MESSAGE([  current value: $ac_new_val], 2)
-       ac_cache_corrupted=:
+       # differences in whitespace do not lead to failure.
+       ac_old_val_w=`echo x $ac_old_val`
+       ac_new_val_w=`echo x $ac_new_val`
+       if test "$ac_old_val_w" != "$ac_new_val_w"; then
+         AS_MESSAGE([error: `$ac_var' has changed since the previous run:], 2)
+         ac_cache_corrupted=:
+       else
+         AS_MESSAGE([warning: ignoring whitespace changes in `$ac_var' since 
the previous run:], 2)
+         eval $ac_var=\$ac_old_val
+       fi
+       AS_MESSAGE([  former value:  `$ac_old_val'], 2)
+       AS_MESSAGE([  current value: `$ac_new_val'], 2)
       fi;;
   esac
   # Pass precious variables to config.status.
diff --git a/tests/torture.at b/tests/torture.at
index 0d7509e..a9e9dc1 100644
--- a/tests/torture.at
+++ b/tests/torture.at
@@ -46,16 +46,18 @@ AT_CLEANUP
 ## AC_ARG_VAR.  ##
 ## ------------ ##
 
-# AT_CHECK_AC_ARG_VAR(FIRST-VALUE, SECOND-VALUE)
-# ----------------------------------------------
+# AT_CHECK_AC_ARG_VAR(FIRST-VALUE, SECOND-VALUE, [STATUS = 1])
+# ------------------------------------------------------------
 # Check that AC_ARG_VAR caches the latest values, diagnoses
-# inconsistencies, and arms config.status.
+# inconsistencies, and arms config.status.  Check that recheck
+# returns STATUS, save configure output in files 'stdout' and 'stderr'
+# for further inspection.
 m4_define([AT_CHECK_AC_ARG_VAR],
 [rm -f config.cache
 
 # Initial value.
 m4_ifval([$1],
-        [precious='m4_bpatsubst($1, [[']], ['\\''])'; export precious],
+        [precious='m4_bpatsubst([$1], [[']], ['\\''])'; export precious],
         [unset precious])
 AT_CHECK_CONFIGURE([--config-cache -q])
 AT_CHECK([cat file], [], [`$1'
@@ -75,7 +77,7 @@ AT_CHECK([cat file], [], [`$1'
 m4_ifval([$2],
         [precious='$2'; export precious],
         [unset precious])
-AT_CHECK_CONFIGURE([--config-cache], [1], [], [ignore])
+AT_CHECK_CONFIGURE([--config-cache], [m4_default([$3], [1])], [stdout], 
[stderr])
 
 ])# AT_CHECK_AC_ARG_VAR
 
@@ -121,6 +123,13 @@ AT_CHECK_AC_ARG_VAR([apple of my {eye}], [orange of my 
eye])
 AT_CHECK_AC_ARG_VAR(['p  r     ec"iou$], [orange of my eye])
 dnl restore font-lock: "
 
+# Warn (but do not fail) about a whitespace-only change
+AT_CHECK_AC_ARG_VAR([   apple  of       my eye  ], [apple of my eye],
+                   [0], ["has whitespace changes"])
+mv stdout configure-output
+AT_CHECK([grep "ignoring whitespace changes" stderr], [], [ignore])
+AT_CHECK([grep "precious:       apple" configure-output], [], [ignore])
+
 AT_CLEANUP