autoconf-patches
[Top][All Lists]
Advanced

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

Re: [PATCH] Generic mechanism to detect shell features, take 2


From: Eric Sunshine
Subject: Re: [PATCH] Generic mechanism to detect shell features, take 2
Date: Tue, 13 Jan 2004 18:27:05 -0500

Hello,

On Tue, 13 Jan 2004 18:10:17 +0100, Paolo Bonzini wrote:
> This addresses the flaws that Eric found. Apart from logic errors, the main
> difference is that the execution of the tests is factored out in _AS_RUN,
> which uses eval to test the current shell (as I suggested, Paul confirmed,
> and Eric tested).

This is looking very, very nice.  There are only a few fatal flaws this time  
and a couple simple improvements, all of which are easily addressed.   
Commentary follows.

> +m4_define([_AS_RUN],
> +[m4_ifval([$2],
> +[($2 <<\_ASEOF
> +_AS_BOURNE_COMPATIBLE
> +$1
> +_ASEOF
> +)],

Optimization: Since TEST is already running under SHELL, we can drop the (  
and ) subshell and replace them with { and } as a slight optimization.

> +[eval "AS_ESCAPE(m4_quote($1))"])])

Fatal bug: The required and suggested feature tests bomb out with AS_EXIT(1)  
if the test fails.  Unfortunately, this kills the configure script itself,  
rather than merely signalling test failure, because the `eval' is executed in  
the context of the running script.  This is easily fixed by invoking `eval'  
in a subshell like this:

  [(eval "AS_ESCAPE(m4_quote($1))")]

> +m4_define([_AS_DETECT_REQUIRED_BODY], [:])

Fatal bug: The first required test is appended immediately after the `:',  
which results in a syntax error.  This is easily fixed by ensuring that there  
is a newline after the `:'.

> +m4_define([_AS_DETECT_SUGGESTED_BODY], [])

Bug: We need to use [:] here rather than [], just like we do in  
_AS_DETECT_REQUIRED_BODY, in order to ensure that this macro returns true  
when there are no suggested tests.

> +  AS_IF([_AS_RUN([_AS_DETECT_REQUIRED_BODY]) 2>/dev/null],
> +        [as_have_required=yes],
> +     [as_have_required=no])
> +  AS_IF([test $as_have_required = yes && dnl
> +      _AS_RUN([_AS_DETECT_SUGGESTED_BODY]) 2> /dev/null],
> +    [],
> +    [_AS_PATH_WALK([...],
> +      [case $as_dir in
> +      /*)
> +        for as_base in sh bash zsh ksh sh5; do

Improvement: The current logic is to test the currently running shell, and  
then search for other shells (sh bash zsh ksh sh5), however it would be a  
very good idea to also check SHELL before doing the exhaustive search, since  
SHELL might point at a valid and useful shell.

> +    [_AS_PATH_WALK([...],
> +      [case $as_dir in
> +      /*)
> +        for as_base in sh bash zsh ksh sh5; do
> +         ...
> +        done
> +       esac
> +
> +      if test "x$CONFIG_SHELL" != x; then
> +        AS_UNSET([ENV])
> +         ...
> +        exec "$CONFIG_SHELL" "$as_myself" ${1+"address@hidden"}
> +      fi])

Fatal bug: There is a quoting problem here.  The "x$CONFIG_SHELL" test is  
inside the _AS_PATH_WALK() iterator, therefore, it might attempt to do the  
`exec' after checking only the first path given to _AS_PATH_WALK(), and then  
ignore all the other paths given to _AS_PATH_WALK().  (My installation  
triggered this bug, for instance.)  The "x$CONFIG_SHELL" check needs to be  
outside the _AS_PATH_WALK() iterator.

> +    [_AS_PATH_WALK([...],
> +        for as_base in sh bash zsh ksh sh5; do
> +          AS_IF([...])],
> +                [CONFIG_SHELL=$as_dir/$as_base
> +                as_have_required=yes
> +                AS_IF([...])],
> +                      [break])])

Fatal bug: The `break' statement needs to break out from two `for' loops,  
the one established by _AS_PATH_WALK(), and the one which iterates over the  
possible shells.  This can be fixed by using `break 2', or by slightly  
re-organizing the code.

> +        for as_base in sh bash zsh ksh sh5; do

Fatal problem: This expanded shell list results in severe errors on NextStep  
because it locates the ancient zsh 2.5.02 shipped with NextStep.  Although  
this shell passes the function and LINENO tests, it crashes and burns big  
time when it tries running the configure script itself.  Here is a just a  
small sample of the errors emitted by zsh 2.5.02:

  ./configure: parse error near `fi' [923]
  ./configure: no matches found: conftest* [1332]
  ./configure: command not found: test -f [1550]
  TRAPEXIT: command not found: PATH_SEPARATOR [1252]
  TRAPEXIT: no matches found: *.core [1252]

Furthermore, the entire script simply hangs part way through when running  
under zsh 2.5.02, and ctrl-c is needed to get past the hang.  For this  
reason, I think it is better to use Autoconf 2.59's list of possible shells  
(sh bash ksh sh5), rather than introducing zsh at this time.  In the future,  
we may want to write a suitable test to identify and ignore this old zsh, but  
for now we need to get things working, so we should stick with the Autoconf  
2.59 shell list which does not mention zsh.

> +      if test "x$CONFIG_SHELL" != x; then
> +    if test $as_have_required = no; then

Suggestion: We may as well use AS_IF() for these tests for consistency.

Because the above issues are so easily addressed with very minor changes, I  
went ahead and modified Paolo's patch, and have included the new patch in  
this email.  This patch works perfectly in my test cases, and I believe that  
it is now suitable for inclusion in the CVS repository.  Since my changes to  
Paolo's submission are so minor, they should not require any paperwork.

Eric


2003-01-12 Paolo Bonzini <address@hidden>
           Eric Sunshine <address@hidden>
           Paul Eggert <address@hidden>

        * lib/m4sugar/m4sh.m4 (M4SH-SANITIZE): New diversion.
        (AS_INIT): Output shell initialization there. Removed optional
        parameter. Expand _AS_SHELL_FN_SPY.
        (AS_INIT_WITH_SHELL_FN): Removed.
        (_AS_SHELL_FN_SPY): New macro.
        (AS_DETECT_REQUIRED, AS_DETECT_SUGGESTED): New
        macros.
        (AS_SHELL_SANITIZE): Remove loop to find better shell
        and documentation for the parameter.
        (_AS_DETECT_BETTER_SHELL): Move it here.
        (_AS_SHELL_FN_WORK): Remove shell invocation, reformat.
        (_AS_RUN): Move it here, support testing with eval.
        (AS_REQUIRE_SHELL_FN): Require shell functions when
        it is used.
        (_AS_LINENO_WORKS): Put around braces, we do not
        trigger the bash bug anymore.
        * lib/autotest/general.m4: Document M4SH-SANITIZE, do not
        use AS_INIT_WITH_SHELL_FN.


Index: lib/autotest/general.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/autotest/general.m4,v
retrieving revision 1.155
diff -u -d -b -w -r1.155 general.m4
--- lib/autotest/general.m4     5 Jan 2004 07:05:22 -0000
+++ lib/autotest/general.m4     13 Jan 2004 22:49:45 -0000
@@ -60,7 +60,8 @@
 #    1. HEADER-REVISION
 #    2. HEADER-COMMENT
 #    3. HEADER-COPYRIGHT
-#    4. M4SH-INIT
+#    4. M4SH-SANITIZE
+#    5. M4SH-INIT
 # 1000. BODY
 #
 # Defined below:
@@ -142,7 +143,7 @@
          m4_defn([AT_PACKAGE_STRING])[ test suite]m4_ifval([$1], [: $1]))
 m4_define([AT_ordinal], 0)
 m4_define([AT_banner_ordinal], 0)
-AS_INIT_WITH_SHELL_FN
+AS_INIT
 AS_PREPARE
 m4_divert_push([DEFAULTS])dnl

Index: lib/m4sugar/m4sh.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/m4sugar/m4sh.m4,v
retrieving revision 1.111
diff -u -d -b -w -r1.111 m4sh.m4
--- lib/m4sugar/m4sh.m4 5 Jan 2004 07:43:48 -0000
+++ lib/m4sugar/m4sh.m4 13 Jan 2004 22:49:49 -0000
@@ -64,9 +64,10 @@
 #   Purpose of the script etc.
 # - HEADER-COPYRIGHT
 #   Copyright notice(s)
+# - M4SH-SANITIZE
+#   M4sh's shell setup
 # - M4SH-INIT
-#   M4sh's initializations
-#
+#   M4sh initialization
 # - BODY
 #   The body of the script.

@@ -81,7 +82,8 @@
 m4_define([_m4_divert(HEADER-REVISION)],   1)
 m4_define([_m4_divert(HEADER-COMMENT)],    2)
 m4_define([_m4_divert(HEADER-COPYRIGHT)],  3)
-m4_define([_m4_divert(M4SH-INIT)],         4)
+m4_define([_m4_divert(M4SH-SANITIZE)],     4)
+m4_define([_m4_divert(M4SH-INIT)],         5)
 m4_define([_m4_divert(BODY)],           1000)

 # Aaarg.  Yet it starts with compatibility issues...  Libtool wants to
@@ -141,8 +143,7 @@
 # xx_REQUIRE macros, BODY-TO-EXPAND is mandatory.
 #
 m4_define([AS_REQUIRE_SHELL_FN],
-[m4_provide_if([AS_INIT_WITH_SHELL_FN],
-              [m4_warn([syntax], [AS_INIT_WITH_SHELL_FN not called.])])dnl
+[AS_DETECT_REQUIRED([_AS_SHELL_FN_WORK])dnl
 m4_provide_if([AS_SHELL_FN_$1], [],
                [m4_provide([AS_SHELL_FN_$1])m4_divert_text([M4SH-INIT], [$1() {
 $2
@@ -167,58 +168,126 @@
 ])


-# _AS_SHELL_FN_WORK(TESTED-SHELL)
-# --------------------------------------------------
+# _AS_RUN(TEST, [SHELL])
+# ----------------------
+# Run TEST under the current shell (if one parameter is used)
+# or under the given SHELL, protecting it from syntax errors.
+m4_define([_AS_RUN],
+[m4_ifval([$2],
+[{ $2 <<\_ASEOF
+_AS_BOURNE_COMPATIBLE
+$1
+_ASEOF
+}],
+[(eval "AS_ESCAPE(m4_quote($1))")])])
+
+# AS_DETECT_REQUIRED(TEST)
+# ------------------------
+# Refuse to execute under a shell that does not pass
+# the given TEST.
+m4_define([_AS_DETECT_REQUIRED_BODY], [:])
+m4_defun([AS_DETECT_REQUIRED],
+[m4_require([_AS_DETECT_BETTER_SHELL])dnl
+m4_expand_once([m4_append([_AS_DETECT_REQUIRED_BODY], [
+($1) || AS_EXIT(1)
+])], [AS_DETECT_REQUIRED_provide($1)])])
+
+# AS_DETECT_SUGGESTED(TEST)
+# -------------------------
+# Prefer to execute under a shell that passes the given TEST.
+m4_define([_AS_DETECT_SUGGESTED_BODY], [:])
+m4_defun([AS_DETECT_SUGGESTED],
+[m4_require([_AS_DETECT_BETTER_SHELL])dnl
+m4_expand_once([m4_append([_AS_DETECT_SUGGESTED_BODY], [
+($1) || AS_EXIT(1)
+])], [AS_DETECT_SUGGESTED_provide($1)])])
+
+# _AS_DETECT_BETTER_SHELL
+# -----------------------
+# The real workhorse for detecting a shell with the correct
+# features.
+m4_defun_once([_AS_DETECT_BETTER_SHELL],
+[m4_wrap([m4_divert_text([M4SH-SANITIZE], [
+if test "x$CONFIG_SHELL" = x; then
+  AS_IF([_AS_RUN([_AS_DETECT_REQUIRED_BODY]) 2>/dev/null],
+        [as_have_required=yes],
+       [as_have_required=no])
+  AS_IF([test $as_have_required = yes && dnl
+        _AS_RUN([_AS_DETECT_SUGGESTED_BODY]) 2> /dev/null],
+    [],
+    [as_candidate_shells="$SHELL"
+    _AS_PATH_WALK([/bin$PATH_SEPARATOR/usr/bin$PATH_SEPARATOR$PATH],
+      [case $as_dir in
+        /*)
+          for as_base in sh bash ksh sh5; do
+            as_candidate_shells="$as_candidate_shells $as_dir/$as_base"
+          done
+       esac])
+
+      for as_shell in $as_candidate_shells; do
+        AS_IF([_AS_RUN([_AS_DETECT_REQUIRED_BODY], [$as_shell 2> /dev/null])],
+              [CONFIG_SHELL=$as_shell
+              as_have_required=yes
+              AS_IF([_AS_RUN([_AS_DETECT_SUGGESTED_BODY], [$as_shell 2>  
/dev/null])],
+                    [break])])
+      done
+
+      AS_IF([test "x$CONFIG_SHELL" != x],
+        [AS_UNSET([ENV])
+        AS_UNSET([BASH_ENV])
+        export CONFIG_SHELL
+        exec "$CONFIG_SHELL" "$as_myself" ${1+"address@hidden"}])
+
+    AS_IF([test $as_have_required = no],
+      [echo This script requires a shell more modern than all the
+      echo shells that I found on your system.  Please install a
+      echo modern shell, or manually run the script under such a
+      echo shell if you do have one.
+      AS_EXIT(1)])
+    ])
+fi
+])])])
+
+
+# _AS_SHELL_FN_WORK
+# -----------------
 # This is a spy to detect "in the wild" shells that do not support shell
 # functions correctly.  It is based on the m4sh.at Autotest testcases.
 m4_define([_AS_SHELL_FN_WORK],
-[{ $1 2>/dev/null <<\_ASEOF
-_AS_BOURNE_COMPATIBLE
-func_return () {
+[func_return () {
   (exit [$]1)
 }
-
 func_success () {
   func_return 0
 }
-
 func_failure () {
   func_return 1
 }
-
 func_ret_success () {
   return 0
 }
-
 func_ret_failure () {
   return 1
 }

 exitcode=0
-AS_IF([func_success], [], [
-  exitcode=1
-  echo func_failure succeeded.
-])
-AS_IF([func_failure], [
-  exitcode=1
-  echo func_success failed.
-])
-AS_IF([func_ret_success], [], [
-  exitcode=1
-  echo func_ret_success failed.
-])
-AS_IF([func_ret_failure], [
-  exitcode=1
-  echo func_ret_failure succeeded.
-])
-AS_EXIT($exitcode)
-_ASEOF
-}])
+AS_IF([func_success], [],
+  [exitcode=1
+  echo func_failure succeeded.])
+AS_IF([func_failure],
+  [exitcode=1
+  echo func_success failed.])
+AS_IF([func_ret_success], [],
+  [exitcode=1
+  echo func_ret_success failed.])
+AS_IF([func_ret_failure],
+  [exitcode=1
+  echo func_ret_failure succeeded.])
+test $exitcode = 0])

-# AS_SHELL_SANITIZE(WHAT-IF-SHELL-FUNCTIONS-DO-NOT-WORK)
-# ------------------------------------------------------
-# The parameter is temporary; it will go away and you
-# should not rely on it.
+
+# AS_SHELL_SANITIZE
+# -----------------
 m4_defun([AS_SHELL_SANITIZE],
 [## --------------------- ##
 ## M4sh Initialization.  ##
@@ -248,28 +317,6 @@
   AS_ERROR([cannot find myself; rerun with an absolute path])
 fi

-dnl In the future, the `else' branch will be that in AS_INIT_WITH_SHELL_FN.
-AS_IF([_AS_SHELL_FN_WORK([$SHELL])], [], [
-  case $CONFIG_SHELL in
-  '')
-    _AS_PATH_WALK([/bin$PATH_SEPARATOR/usr/bin$PATH_SEPARATOR$PATH],
-      [for as_base in sh bash ksh sh5; do
-        case $as_dir in
-        /*)
-          AS_IF([_AS_SHELL_FN_WORK([$as_dir/$as_base])], [
-            AS_UNSET(BASH_ENV)
-            AS_UNSET(ENV)
-            CONFIG_SHELL=$as_dir/$as_base
-            export CONFIG_SHELL
-            exec "$CONFIG_SHELL" "$as_myself" ${1+"address@hidden"}
-          ]);;
-        esac
-       done]);;
-  *)
-    $1;;
-  esac
-])
-
 # Work around bugs in pre-3.0 UWIN ksh.
 $as_unset ENV MAIL MAILPATH
 PS1='$ '
@@ -649,6 +696,7 @@
 # configure.
 m4_define([_AS_LINENO_PREPARE],
 [AS_REQUIRE([_AS_CR_PREPARE])dnl
+AS_DETECT_SUGGESTED([_AS_LINENO_WORKS])
 _AS_LINENO_WORKS || {

   # Create $as_me.lineno as a copy of $as_myself, but with $LINENO
@@ -1188,16 +1236,29 @@
 [m4_popdef([$1])])


-
 ## ----------------- ##
 ## Setting M4sh up.  ##
 ## ----------------- ##


-# AS_INIT(WHAT-IF-SHELL-FUNCTIONS-DO-NOT-WORK)
-# --------------------------------------------
-# The parameter is temporary; it will go away and you
-# should not rely on it.
+# _AS_SHELL_FN_SPY
+# ----------------
+# This temporary macro checks "in the wild" for shells that do
+# not support shell functions.
+m4_define([_AS_SHELL_FN_SPY],
+[AS_DETECT_SUGGESTED([_AS_SHELL_FN_WORK])
+_AS_RUN([_AS_SHELL_FN_WORK]) || {
+  echo No shell found that supports shell functions.
+  echo Please tell address@hidden about your system,
+  echo including any error possibly output before this
+  echo message
+}
+])
+
+
+# AS_INIT
+# -------
+# Initialize m4sh.
 m4_define([AS_INIT],
 [m4_init

@@ -1206,26 +1267,10 @@

 # Bangshe and minimal initialization.
 m4_divert_text([BINSH], address@hidden:@! /bin/sh])
-m4_divert_text([M4SH-INIT], [AS_SHELL_SANITIZE([m4_default([$1], [
-echo Found no shell that has working shell functions.
-echo
-echo Please tell address@hidden about your system.
-])])])
+m4_divert_text([M4SH-SANITIZE], [AS_SHELL_SANITIZE])
+AS_REQUIRE([_AS_SHELL_FN_SPY])

 # Let's go!
 m4_wrap([m4_divert_pop([BODY])[]])
 m4_divert_push([BODY])[]dnl
 ])
-
-# AS_INIT_WITH_SHELL_FN
-# ---------------------
-# Same as AS_INIT, but exit if shell functions are
-# not supported.
-m4_define([AS_INIT_WITH_SHELL_FN],
-[AS_INIT([
-echo Shell functions are not supported on any shell I could find
-echo on your system.  This script requires shell functions: please
-echo install a modern shell, or manually run the script under such
-echo a shell if you do have one.
-AS_EXIT(1)
-])])




reply via email to

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