From 6cf06f707aa6e9407fafff3f0054834940a11fb3 Mon Sep 17 00:00:00 2001
From: Avery Pennarun <apenwarr@gmail.com>
Date: Mon, 17 Dec 2018 12:33:17 +0000
Subject: [PATCH] shelltest.od: we accidentally treated some fails as mere
 warnings.

We were setting a global variable FAIL on failure, but if we failed
inside a subshell (which a very small number of tests might do), this
setting would be lost.  The script output (a series of failed/warning
lines) was still valid, but not the return code, so the shell might be
selected even if one of these tests failed.

To avoid the problem, put the fail/warning state in the filesystem
instead, which is shared across subshells.
---
 redo/sh.do         |  2 +-
 t/.gitignore       |  2 ++
 t/200-shell/all.do |  1 -
 t/clean.do         |  3 ++-
 t/shelltest.od     | 13 +++++++------
 5 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/redo/sh.do b/redo/sh.do
index c0f0c4c..0e90d2f 100644
--- a/redo/sh.do
+++ b/redo/sh.do
@@ -13,7 +13,7 @@ WARN=
 # use some non-portable feature.
 for sh in dash /usr/xpg4/bin/sh ash posh mksh ksh ksh88 ksh93 pdksh \
 		zsh bash busybox /bin/sh; do
-	printf "%-30s" "Testing $sh..."
+	printf " %-22s" "$sh..."
 	FOUND=`which $sh 2>/dev/null` || { echo "missing"; continue; }
 	
 	# It's important for the file to actually be named 'sh'.  Some
diff --git a/t/.gitignore b/t/.gitignore
index d81f3a1..6e94cf0 100644
--- a/t/.gitignore
+++ b/t/.gitignore
@@ -1,4 +1,6 @@
 /broken
 /shellfile
+/shelltest.warned
+/shelltest.failed
 /stress.log
 /flush-cache
diff --git a/t/200-shell/all.do b/t/200-shell/all.do
index 39b791d..53ccd5b 100644
--- a/t/200-shell/all.do
+++ b/t/200-shell/all.do
@@ -1,2 +1 @@
 redo nonshelltest shelltest vartest
-
diff --git a/t/clean.do b/t/clean.do
index 035f89e..effd5ed 100644
--- a/t/clean.do
+++ b/t/clean.do
@@ -2,5 +2,6 @@
 sed 's/\.do$//' |
 xargs redo
 
-rm -f broken shellfile *~ .*~ stress.log flush-cache
+rm -f broken shellfile shelltest.warned shelltest.failed \
+	*~ .*~ stress.log flush-cache
 rm -rf 'space home dir'
diff --git a/t/shelltest.od b/t/shelltest.od
index 8dbebd1..1d94639 100644
--- a/t/shelltest.od
+++ b/t/shelltest.od
@@ -2,7 +2,7 @@
 # Most of these tests were inspired by:
 # http://www.gnu.org/software/hello/manual/autoconf/Shell-Substitutions.html
 #
-# Note that this file isn't really a test for POSIX compliance.  It's a test
+# Note that this file isn't exactly a test for POSIX compliance.  It's a test
 # for usefulness-compliance, that is, interpreting POSIX in a particular way
 # for consistency, so that users of redo can depend on all the following
 # functionality.
@@ -15,18 +15,18 @@
 exec >&2
 set +e
 
-FAIL=
+rm -f shelltest.failed shelltest.warned
 
 fail()
 {
 	echo "  failed: $1"
-	FAIL=41
+	: >shelltest.failed
 }
 
 warn()
 {
 	echo "  warning: $1"
-	[ -n "$FAIL" ] || FAIL=42
+	: >shelltest.warned
 }
 
 quiet_stderr()
@@ -417,5 +417,6 @@ set x y z
 x=$(printf "a%-5sc" "b")
 [ "$x" = "ab    c" ] || warn 119
 
-[ -n "$FAIL" ] || exit 40
-exit $FAIL
+[ -e shelltest.failed ] && exit 41
+[ -e shelltest.warned ] && exit 42
+exit 40