Re: [PATCH] Pass CONFIG_SHELL down if provided

[Eric Blake]

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

According to Paolo Bonzini on 11/12/2008 7:01 AM:
>> But who writes it to configure?  Right now, AC_INIT.  But it _should_ be
>> AS_INIT, otherwise, AS_INIT_GENERATED only works if you call AC_INIT
>> (which is the wrong dependency).
> 
> ... but (part of) the point of may patch was exactly to avoid the need
> to have that line in the top of M4sh scripts.  But yes, moving it to
> AS_INIT is also a possibility.

In the following patch, the test fails without the changes in lib/, but
passes afterwards.  An alternate patch would be to output
#!${CONFIG_SHELL-/bin/sh} rather than #!$SHELL in AS_INIT_GENERATED.
Which would you prefer?  I like the idea of making SHELL sane during any
m4sh script (ie. this patch variant), so that constructs like '$SHELL
$build_aux/config.sub' will work.

Should I go ahead and apply?

- --
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

iEYEARECAAYFAkkbUu4ACgkQ84KuGfSFAYDu9gCgqxuFyI8/BUgGhbTmUL4MfsFd
AXUAnRkIJktwE4lL+cKm7NqbfBbQpfsz
=b87Y
-----END PGP SIGNATURE-----

>From f527bbae6009fc065791633968b0697e90f754d4 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 12 Nov 2008 13:27:29 -0700
Subject: [PATCH] Make M4sh, not autoconf, guarantee sane $SHELL.

* lib/autoconf/general.m4 (_AC_INIT_DEFAULTS): Move setting of
SHELL...
* lib/m4sugar/m4sh.m4 (AS_INIT): ...to here.
* doc/autoconf.texi (Initialization Macros): Document the effect
on SHELL.
* tests/m4sh.at (AS@&address@hidden): New test.
Reported by Ralf Wildenhues.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog               |   11 +++++++++++
 doc/autoconf.texi       |    8 +++++++-
 lib/autoconf/general.m4 |    2 +-
 lib/m4sugar/m4sh.m4     |    3 ++-
 tests/m4sh.at           |   35 +++++++++++++++++++++++++++++++++++
 5 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 488d382..1370291 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2008-11-12  Eric Blake  <address@hidden>
+
+       Make M4sh, not autoconf, guarantee sane $SHELL.
+       * lib/autoconf/general.m4 (_AC_INIT_DEFAULTS): Move setting of
+       SHELL...
+       * lib/m4sugar/m4sh.m4 (AS_INIT): ...to here.
+       * doc/autoconf.texi (Initialization Macros): Document the effect
+       on SHELL.
+       * tests/m4sh.at (AS@&address@hidden): New test.
+       Reported by Ralf Wildenhues.
+
 2008-11-12  Ralf Wildenhues  <address@hidden>
 
        * tests/local.at (AT_COPYRIGHT): Bump copyright years.
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 92c224a..4081604 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -12313,10 +12313,15 @@ Initialization Macros
 
 @defmac AS_INIT
 @asindex{INIT}
address@hidden LC_ALL
address@hidden SHELL
 Initialize the M4sh environment.  This macro calls @code{m4_init}, then
 outputs the @code{#! /bin/sh} line, a notice about where the output was
 generated from, and code to sanitize the environment for the rest of the
-script.  Finally, it changes the current diversion to @code{BODY}.
+script.  Among other initializations, this sets @env{SHELL} to the shell
+chosen to run the script (@pxref{CONFIG_SHELL}), and @env{LC_ALL} to
+ensure the C locale.  Finally, it changes the current diversion to
address@hidden
 @end defmac
 
 @defmac AS_INIT_GENERATED (@var{file}, @ovar{comment})
@@ -19404,6 +19409,7 @@ config.status Invocation
 @file{config.status} checks several optional environment variables that
 can alter its behavior:
 
address@hidden
 @defvar CONFIG_SHELL
 @evindex CONFIG_SHELL
 The shell with which to run @command{configure} for the @option{--recheck}
diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
index 15377cb..1e9056a 100644
--- a/lib/autoconf/general.m4
+++ b/lib/autoconf/general.m4
@@ -417,7 +417,7 @@ cross_compiling=no
 subdirs=
 MFLAGS=
 MAKEFLAGS=
-AC_SUBST([SHELL], [${CONFIG_SHELL-/bin/sh}])dnl
+AC_SUBST([SHELL])dnl
 AC_SUBST([PATH_SEPARATOR])dnl
 
 # Identity of this package.
diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index e0b7c55..153c259 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -1908,7 +1908,8 @@ m4_pattern_forbid([^_?AS_])
 m4_divert_text([BINSH], address@hidden:@! /bin/sh])
 m4_divert_text([HEADER-COMMENT],
               address@hidden:@ Generated from __file__ by m4_PACKAGE_STRING.])
-m4_divert_text([M4SH-SANITIZE], [_AS_SHELL_SANITIZE])
+m4_divert_text([M4SH-SANITIZE], [_AS_SHELL_SANITIZE
+SHELL=${CONFIG_SHELL-/bin/sh}])
 m4_divert_text([M4SH-INIT-FN], [m4_text_box([M4sh Shell Functions.])])
 
 # Let's go!
diff --git a/tests/m4sh.at b/tests/m4sh.at
index 55363d8..214b555 100644
--- a/tests/m4sh.at
+++ b/tests/m4sh.at
@@ -1138,3 +1138,38 @@ cleanup 1
 ]])
 
 AT_CLEANUP
+
+
+## ------------------- ##
+## AS_INIT_GENERATED.  ##
+## ------------------- ##
+
+AT_SETUP([AS@&address@hidden)
+
+AT_DATA_M4SH([script.as], [[dnl
+m4_define([AS_MESSAGE_FD], [1])
+AS_INIT
+AS_INIT_GENERATED([child], [echo hello from child])
+cat >>child <<\EOF
+AS_ECHO(["SHELL=$SHELL"])
+EOF
+echo hello from parent
+AS_ECHO(["SHELL=$SHELL"])
+]])
+
+AT_CHECK_M4SH
+AT_CHECK([./script], [0], [stdout])
+AT_CHECK([grep 'SHELL=.' stdout], [0], [ignore])
+sed s/parent/child/ <stdout >expout
+AT_CHECK([./child], [0], [expout])
+SHELL=/bogus
+export SHELL
+cp stdout expout
+mv child child.bak
+AT_CHECK([./script], [0], [expout])
+AT_CHECK([cmp child child.bak])
+AT_CHECK([grep 'SHELL=.' stdout], [0], [ignore])
+sed s/parent/child/ <stdout >expout
+AT_CHECK([./child], [0], [expout])
+
+AT_CLEANUP
-- 
1.6.0.2