Re: more efficient AS_EXIT

[Eric Blake]

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

According to Eric Blake on 11/18/2008 6:56 PM:
> Thanks for the additional research (I don't have personal access to a
> Tru64 machine).  I'll update the docs accordingly, unless you beat me.

Done in the first patch below.

>> Hmm.  How about 'append @samp{|| AS_EXIT}' and make AS_EXIT use
>> m4_default([$1], [$?])?  Current documented default is 1, and a couple
>> of places in Autoconf exploit that, but I wonder whether $? would be a
>> more consistent one: that's the default of `exit' (except for
>> portability warts).
> 
> Well, as AS_EXIT was previously undocumented, I am comfortable making that
> change.  It does seem like making $? instead of 1 is a better default,
> even if it might make some scripts in the wild start returning 0.

Even better: if AS_EXIT is called with arguments, exit with $? unless it
is 0, in which case, exit with status 1.  That way, existing clients of
plain AS_EXIT are still guaranteed a non-zero status; to exit with status
0, you must pass an explicit argument (as before).

I'll be committing these two patches soon.

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

iEYEARECAAYFAkkjkI0ACgkQ84KuGfSFAYCO4QCfYplLA2pGniAVd48rrv708XwE
kcgAn0V6OOZpwKHt5T0rHVOyzFqW5qfQ
=r6Qa
-----END PGP SIGNATURE-----

>From 12bb35717bf10ca68f2e40008334ab123dcbe152 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 18 Nov 2008 20:07:19 -0700
Subject: [PATCH] Update example to match actual Tru64 behavior.

* doc/autoconf.texi (Limitations of Builtins) <trap>: Correct
the example.
Reported by Ralf Wildenhues.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog         |    5 +++++
 doc/autoconf.texi |   10 +++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1e56b6e..a817ee2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2008-11-18  Eric Blake  <address@hidden>
 
+       Update example to match actual Tru64 behavior.
+       * doc/autoconf.texi (Limitations of Builtins) <trap>: Correct
+       the example.
+       Reported by Ralf Wildenhues.
+
        Add AS_SET_STATUS, make AS_EXIT more efficient.
        * lib/m4sugar/m4sh.m4 (_AS_EXIT_PREPARE, AS_SET_STATUS): New
        macros.
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 71d72f4..3cc165b 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -15435,14 +15435,14 @@ Limitations of Builtins
 
 Portable scripts should not use @samp{set -e} if @command{trap} is used
 to install an exit handler.  This is because Tru64/OSF 5.1 @command{sh}
-enters the trap handler with the exit status of the command prior to the
-one that triggered the errexit handler:
+sometimes enters the trap handler with the exit status of the command
+prior to the one that triggered the errexit handler:
 
 @example
-$ @kbd{bash -c 'trap '\''echo $?'\'' 0; set -e; false'}
-1
-$ @kbd{sh -c 'trap '\''echo $?'\'' 0; set -e; false'}
+$ @kbd{sh -ec 'trap '\''echo $?'\'' 0; false'}
 0
+$ @kbd{sh -c 'set -e; trap '\''echo $?'\'' 0; false'}
+1
 @end example
 
 @noindent
-- 
1.6.0.4


>From e84b404ff0cfc9ab0b56a538fa17283d6688284e Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 18 Nov 2008 20:58:19 -0700
Subject: [PATCH] Alter default value of AS_EXIT.

* lib/m4sugar/m4sh.m4 (_AS_EXIT_PREPARE): Let as_func_exit
parameter be optional.
(AS_EXIT): Use it to make better default.
(_AS_DETECT_BETTER_SHELL): Use new default.
* bin/autoconf.as (exit_missing_arg, getopt): Likewise.
* lib/autoconf/status.m4 (AC_OUTPUT): Likewise.
* tests/m4sh.at (AS@&address@hidden): Update test.
* doc/autoconf.texi (Common Shell Constructs) <AS_EXIT>: Mention
new default behavior.
(Limitations of Builtins) <trap>: Adjust to use new default.
* NEWS: Mention the semantic change.
Suggested by Ralf Wildenhues.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog              |   14 ++++++++++++++
 NEWS                   |    4 +++-
 bin/autoconf.as        |    8 ++++----
 doc/autoconf.texi      |   15 +++++++++------
 lib/autoconf/status.m4 |    2 +-
 lib/m4sugar/m4sh.m4    |   24 ++++++++++++++----------
 tests/m4sh.at          |   18 +++++++++++-------
 7 files changed, 56 insertions(+), 29 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a817ee2..7ce0fa5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
 2008-11-18  Eric Blake  <address@hidden>
 
+       Alter default value of AS_EXIT.
+       * lib/m4sugar/m4sh.m4 (_AS_EXIT_PREPARE): Let as_func_exit
+       parameter be optional.
+       (AS_EXIT): Use it to make better default.
+       (_AS_DETECT_BETTER_SHELL): Use new default.
+       * bin/autoconf.as (exit_missing_arg, getopt): Likewise.
+       * lib/autoconf/status.m4 (AC_OUTPUT): Likewise.
+       * tests/m4sh.at (AS@&address@hidden): Update test.
+       * doc/autoconf.texi (Common Shell Constructs) <AS_EXIT>: Mention
+       new default behavior.
+       (Limitations of Builtins) <trap>: Adjust to use new default.
+       * NEWS: Mention the semantic change.
+       Suggested by Ralf Wildenhues.
+
        Update example to match actual Tru64 behavior.
        * doc/autoconf.texi (Limitations of Builtins) <trap>: Correct
        the example.
diff --git a/NEWS b/NEWS
index b67d82c..6ef0725 100644
--- a/NEWS
+++ b/NEWS
@@ -33,7 +33,9 @@ GNU Autoconf NEWS - User visible changes.
    AS_LINENO_PREPARE  AS_ME_PREPARE  AS_SET_STATUS  AS_VAR_APPEND
    AS_VAR_ARITH  AS_VAR_COPY
 
-** The following m4sh macros are documented now:
+** The following m4sh macros are documented now, but in some cases
+   with slightly different semantics than what the previous
+   undocumented version had:
    AS_ECHO  AS_ECHO_N  AS_EXIT  AS_LITERAL_IF  AS_UNSET  AS_VAR_IF
    AS_VAR_POPDEF  AS_VAR_PUSHDEF  AS_VAR_SET  AS_VAR_SET_IF
    AS_VAR_TEST_SET  AS_VERSION_COMPARE
diff --git a/bin/autoconf.as b/bin/autoconf.as
index c678e55..c6bb0f6 100644
--- a/bin/autoconf.as
+++ b/bin/autoconf.as
@@ -81,7 +81,7 @@ Try \`$as_me --help' for more information."
 exit_missing_arg='
   AS_ECHO(["$as_me: option \`$[1]'\'' requires an argument"]) >&2
   AS_ECHO(["$help"]) >&2
-  AS_EXIT([1])
+  AS_EXIT
 ' # restore font-lock: "
 
 # Variables.
@@ -156,7 +156,7 @@ while test $# -gt 0 ; do
        exec >&2
        AS_ECHO(["$as_me: invalid option $[1]"])
        AS_ECHO(["$help"])
-       AS_EXIT([1]) ;;
+       AS_EXIT ;;
     * )
        break ;;
   esac
@@ -175,7 +175,7 @@ case $# in
       infile=configure.in
     else
       AS_ECHO(["$as_me: no input file"]) >&2
-      AS_EXIT([1])
+      AS_EXIT
     fi
     test -z "$traces" && test -z "$outfile" && outfile=configure;;
   1)
@@ -183,7 +183,7 @@ case $# in
   *) exec >&2
      AS_ECHO(["$as_me: invalid number of arguments."])
      AS_ECHO(["$help"])
-     AS_EXIT([1]) ;;
+     AS_EXIT ;;
 esac
 
 # Unless specified, the output is stdout.
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index 3cc165b..0382412 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -12010,12 +12010,15 @@ Common Shell Constructs
 Redirections can be placed outside the macro invocation.
 @end defmac
 
address@hidden AS_EXIT (@dvar{status, 1})
address@hidden AS_EXIT (@dvar{status, max($?/1)})
 @asindex{EXIT}
-Emit code to exit the shell with @var{status}.  This works around shells
-that see the exit status of the command prior to @code{exit} inside a
address@hidden 0} handler (@pxref{Limitations of Builtins, , Limitations of
-Shell Builtins}).
+Emit code to exit the shell with @var{status}.  If @var{status} is
+omitted, then @samp{$?} is used, except that a status of zero is
+converted to @samp{1}.  To exit with successful status, it is necessary
+to supply an explicit @var{status} that expands to @samp{0}.  This macro
+works around shells that see the exit status of the command prior to
address@hidden inside a @samp{trap 0} handler (@pxref{Limitations of
+Builtins, , Limitations of Shell Builtins}).
 @end defmac
 
 @defmac AS_IF (@var{test1}, @ovar{run-if-true1}, @dots{}, @ovar{run-if-false})
@@ -15447,7 +15450,7 @@ Limitations of Builtins
 
 @noindent
 Thus, when writing a script in M4sh, rather than trying to rely on
address@hidden -e}, it is better to append @samp{|| AS_EXIT([$?])} to any
address@hidden -e}, it is better to append @samp{|| AS_EXIT} to any
 statement where it is desirable to abort on failure.
 
 @item @command{shift}
diff --git a/lib/autoconf/status.m4 b/lib/autoconf/status.m4
index c134bb7..fe78358 100644
--- a/lib/autoconf/status.m4
+++ b/lib/autoconf/status.m4
@@ -1307,7 +1307,7 @@ if test "$no_create" != yes; then
   exec AS_MESSAGE_LOG_FD>>config.log
   # Use ||, not &&, to avoid exiting from the if with $? = 1, which
   # would make configure fail if this is the last instruction.
-  $ac_cs_success || AS_EXIT([1])
+  $ac_cs_success || AS_EXIT
 fi
 dnl config.status should not do recursion.
 AC_PROVIDE_IFELSE([AC_CONFIG_SUBDIRS], [_AC_OUTPUT_SUBDIRS()])dnl
diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index f056ee3..c800108 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -270,7 +270,7 @@ _m4_defn([AC_PACKAGE_BUGREPORT]), [], [and 
_m4_defn([AC_PACKAGE_BUGREPORT])])])
 Then install a modern shell, or manually run the script under such a
 shell if you do have one.], [$[]0: ], [], [62])")
   fi
-      AS_EXIT(1)])])
+  AS_EXIT])])
 fi
 SHELL=${CONFIG_SHELL-/bin/sh}
 export SHELL
@@ -539,19 +539,23 @@ m4_defun([_AS_EXIT_PREPARE],
   [AS_FUNCTION_DESCRIBE([as_func_set_status], [STATUS],
     [Set $? to STATUS, without forking.])], [  return $[]1])]dnl
 [AS_REQUIRE_SHELL_FN([as_func_exit],
-  [AS_FUNCTION_DESCRIBE([as_func_exit], [STATUS],
-    [Exit the shell with STATUS, even in a "trap 0" or "set -e" context.])],
-[  set +e
-  as_func_set_status $[]1
-  exit $[]1])])#_AS_EXIT_PREPARE
+  [AS_FUNCTION_DESCRIBE([as_func_exit], [[[STATUS]]],
+    [Exit the shell with STATUS, even in a "trap 0" or "set -e" context.
+     If STATUS is omitted, use the maximum of $? and 1.])],
+[  as_status=$?
+  set +e
+  test $as_status = 0 && as_status=1
+  as_func_set_status ${1-$as_status}
+  exit ${1-$as_status}])])#_AS_EXIT_PREPARE
 
 
-# AS_EXIT([EXIT-CODE = 1])
-# ------------------------
+# AS_EXIT([EXIT-CODE = $?/1])
+# ---------------------------
 # Exit, with status set to EXIT-CODE in the way that it's seen
-# within "trap 0", and without interference from "set -e".
+# within "trap 0", and without interference from "set -e".  If
+# EXIT-CODE is omitted, then use $?, except use 1 if $? is 0.
 m4_defun([AS_EXIT],
-[AS_REQUIRE([_AS_EXIT_PREPARE])[]as_func_exit m4_default([$1], 1)])
+[AS_REQUIRE([_AS_EXIT_PREPARE])[]as_func_exit[]m4_ifval([$1], [ $1])])
 
 
 # AS_FOR(MACRO, SHELL-VAR, [LIST = "$@"], [BODY = :])
diff --git a/tests/m4sh.at b/tests/m4sh.at
index 85ad5d8..1f39bb1 100644
--- a/tests/m4sh.at
+++ b/tests/m4sh.at
@@ -305,11 +305,12 @@ AT_DATA_M4SH([script.as],
 [[AS_INIT
 test x${1} = xa && AS_EXIT
 test x${1} = xb && AS_EXIT([${2}])
-test x${1} = xc && trap 's=$?; echo $s; AS_EXIT([$s])' 0
-test x${2} = xd && set -e
+test x${1} = xc && { AS_SET_STATUS([${2}]); AS_EXIT; }
+test x${1} = xd && trap 's=$?; echo $s; AS_EXIT([$s])' 0
+test x${2} = xe && set -e
 AS_SET_STATUS([3])
 dnl Solaris /bin/sh 'set -e' doesn't react to failed function calls
-test x${2} = xd \
+test x${2} = xe \
   && { echo 'skipping rest of test: set -e support is lousy'; exit 77; }
 AS_SET_STATUS([4])
 ]])
@@ -317,9 +318,12 @@ AS_SET_STATUS([4])
 AT_CHECK_M4SH
 AT_CHECK([./script], [4])
 AT_CHECK([./script a], [1])
-AT_CHECK([./script b], [0])
+AT_CHECK([./script b], [1])
+AT_CHECK([./script b 0], [0])
 AT_CHECK([./script b 2], [2])
-AT_CHECK([./script c], [4], [[4
+AT_CHECK([./script c 0], [1])
+AT_CHECK([./script c 2], [2])
+AT_CHECK([./script d], [4], [[4
 ]])
 dnl If we got to this point without a FAIL, then AS_EXIT at least works.
 dnl The rest of this test relies on semi-decent 'set -e' support, even
@@ -327,8 +331,8 @@ dnl though m4sh in general should not try to rely on it 
because of
 dnl portability nightmares on what constructs are considered errors across
 dnl various shells; therefore, an overall SKIP result is desirable on
 dnl broken shells like Solaris /bin/sh.
-AT_CHECK([./script '' d], [3])
-AT_CHECK([./script c d], [3], [[3
+AT_CHECK([./script '' e], [3])
+AT_CHECK([./script d e], [3], [[3
 ]])
 
 AT_CLEANUP
-- 
1.6.0.4