Re: [PATCH] init.sh: disqualify shells for which set -x corrupts stderr

[Jim Meyering]

Jim Meyering wrote:

> Ralf Wildenhues wrote:
>> * Eric Blake wrote on Wed, Sep 08, 2010 at 04:58:11PM CEST:
>>> On 09/08/2010 08:45 AM, Jim Meyering wrote:
>>> >Besides, isn't Irix 5.x approaching effective museum-only status?
>>>
>>> Probably, but I don't (yet) have access to Irix 6.5 to test a newer
>>> version of that platform.
>>
>> sh and ksh on IRIX 6.5 also produce output, but at least both now
>> prepend '+ ', and ineligible bsh also prints 'P=1'.  No bash in
>> /usr/bin here.
>
> Thanks, Ralf.
>
> Irix 6.5 is slated to have support at least until December 2013,
> so I may be stuck...
>
> Hmm... an alternative solution is simply to disable the
> VERBOSE=yes -> "set -x" code on offending systems.
> That will make debugging things a little harder (when using
> the broken shells), but that's still better than the alternative.

Here's a patch that does that.
At first I did just this:

  Subject: [PATCH] test.sh: penalize a set-x-impaired shell; don't disqualify it

  * tests/init.sh: Too many shells corrupt application stderr when
  you set -x, so we can't afford to disqualify them, since at least
  on Irix-6.5, that would disqualify all bourne shells.  Instead,
  when VERBOSE=yes is requested and set -x might cause trouble,
  simply issue a warning and refrain from enabling debug output.
  ---
   ChangeLog     |    9 +++++++++
   tests/init.sh |   17 ++++++++++++-----

But that's no good if your initial shell is zsh: VERBOSE=yes would
not do anything useful for you, whereas it could work fine if
you have bash, dash, etc. available.

Then I realized I could do even better:
[this is not yet well tested]

>From f0e462d8625bb3820a26d39920e8698edaff79a5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 8 Sep 2010 22:24:22 +0200
Subject: [PATCH] init.sh: penalize a set-x-impaired shell; don't disqualify it

* tests/init.sh: Too many shells corrupt application stderr when
you set -x, so we can't afford to disqualify them, since at least
on Irix-6.5, that would disqualify all bourne shells.
Instead, use a two-pass approach.
On the first pass, try to find a shell that meets the stricter
condition that set -x does not corrupt stderr.
If no shell meets the stricter condition, retest each candidate
shell, but without that extra condition.  Finally, when
VERBOSE=yes is requested and set -x might cause trouble, simply
issue a warning and refrain from enabling debug output.
---
 ChangeLog     |   14 ++++++++++++
 tests/init.sh |   63 ++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 57 insertions(+), 20 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 82937ac..2ac4e00 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2010-09-08  Jim Meyering  <address@hidden>
+
+       init.sh: penalize a set-x-impaired shell; don't disqualify it
+       * tests/init.sh: Too many shells corrupt application stderr when
+       you set -x, so we can't afford to disqualify them, since at least
+       on Irix-6.5, that would disqualify all bourne shells.
+       Instead, use a two-pass approach.
+       On the first pass, try to find a shell that meets the stricter
+       condition that set -x does not corrupt stderr.
+       If no shell meets the stricter condition, retest each candidate
+       shell, but without that extra condition.  Finally, when
+       VERBOSE=yes is requested and set -x might cause trouble, simply
+       issue a warning and refrain from enabling debug output.
+
 2010-09-08  Eric Blake  <address@hidden>

        unsetenv: fix OpenBSD bug
diff --git a/tests/init.sh b/tests/init.sh
index 9886a8d..690da63 100644
--- a/tests/init.sh
+++ b/tests/init.sh
@@ -109,9 +109,16 @@ fi
 # Use "9" to indicate success (rather than 0), in case some shell acts
 # like Solaris 10's /bin/sh but exits successfully instead of with status 2.

+# Whether to reject a shell for which "set -x" corrupts stderr.
+strict_=yes
+
+gl_set_x_corrupts_stderr_='$( (exec 3>&1; set -x; P=1 true 2>&3) 2> /dev/null)'
+
 gl_shell_test_script_='
 test $(echo y) = y || exit 1
-test -z "$( (exec 3>&1; set -x; P=1 true 2>&3) 2> /dev/null)" || exit 1
+if test $strict_ = yes && test -n "$gl_set_x_corrupts_stderr_"; then
+  exit 1
+fi
 test -z "$EXEEXT" && exit 9
 shopt -s expand_aliases
 alias a-b="echo zoo"
@@ -132,24 +139,29 @@ else
   if test $? = 9; then
     : # The current shell is adequate.  No re-exec required.
   else
-    # Search for a shell that meets our requirements.
-    for re_shell_ in "${CONFIG_SHELL:-no_shell}" /bin/sh bash dash zsh pdksh 
fail
-    do
-      test "$re_shell_" = no_shell && continue
-      test "$re_shell_" = fail && skip_ failed to find an adequate shell
-      "$re_shell_" -c "$gl_shell_test_script_" 2>/dev/null
-      if test $? = 9; then
-        # Found an acceptable shell.  Preserve -v and -x.
-        case $- in
-          *v*x* | *x*v*) opts_=-vx ;;
-          *v*) opts_=-v ;;
-          *x*) opts_=-x ;;
-          *) opts_= ;;
-        esac
-        exec "$re_shell_" $opts_ "$0" --no-reexec "$@"
-        echo "$ME_: exec failed" 1>&2
-        exit 127
-      fi
+    # First try to find a shell meeting the stricter condition
+    # that "set -x" does not corrupt stderr.  If that fails, retry
+    # but without that extra condition.
+    for strict_ in yes no; do
+      # Search for a shell that meets our requirements.
+      for re_shell_ in "${CONFIG_SHELL:-no_shell}" /bin/sh bash dash zsh pdksh
+      do
+        test "$re_shell_" = no_shell && continue
+        test "$re_shell_" = fail && skip_ failed to find an adequate shell
+        "$re_shell_" -c "$gl_shell_test_script_" 2>/dev/null
+        if test $? = 9; then
+          # Found an acceptable shell.  Preserve -v and -x.
+          case $- in
+            *v*x* | *x*v*) opts_=-vx ;;
+            *v*) opts_=-v ;;
+            *x*) opts_=-x ;;
+            *) opts_= ;;
+          esac
+          exec "$re_shell_" $opts_ "$0" --no-reexec "$@"
+          echo "$ME_: exec failed" 1>&2
+          exit 127
+        fi
+      done
     done
   fi
 fi
@@ -269,7 +281,18 @@ path_prepend_()

 setup_()
 {
-  test "$VERBOSE" = yes && set -x
+  if test "$VERBOSE" = yes; then
+    # Test whether set -x may cause the selected shell to corrupt an
+    # application's stderr.  Many do, including zsh-4.3.10 and the /bin/sh
+    # from SunOS 5.11, OpenBSD 4.7 and Irix 5.x and 6.5.
+    # If enabling verbose output this way would cause trouble, simply
+    # issue a warning and refrain.
+    if test -n "$gl_set_x_corrupts_stderr_"; then
+      warn_ "using SHELL=$SHELL with 'set -x' corrupts stderr"
+    else
+      set -x
+    fi
+  fi

   initial_cwd_=$PWD

--
1.7.3.rc0.174.g69763