From c28181e26f64998df2d28f2050000eb65b807541 Mon Sep 17 00:00:00 2001
From: Avery Pennarun <apenwarr@gmail.com>
Date: Wed, 8 Feb 2012 23:59:46 -0500
Subject: [PATCH] minimal/do: fix a really scary bugs in "set -e" behaviour.

If you run something like

  blah_function || return 1

then everything even *inside* blah_function is *not* subject to the "set -e"
that would otherwise be in effect.  That's true even for ". subfile" inside
blah_function - which is exactly how minimal/do runs .do files.

Instead, rewrite it as

  blah_function
  [ "$?" = "0" ] || return 1

And add a bit to the unit tests to ensure that "set -e" behaviour is enabled
in .do files as we expect, and crash loudly otherwise.

(This weird behaviour may only happen in some shells and not others.)

Also, we had a "helpful" alias of redo() defined at the bottom of the file.
Combined with the way we use '.' to source the .do files, this would make it
not start a new shell just to run a recursive 'redo' command.  It almost
works, but this stupid "set -e" bug could cause a nested .do file to not
honour "set -e" if someone ran "redo foo || exit 1" from inside a .do
script.  The performance optimization is clearly not worth it here, so
rename it to _redo(); that causes it to actually re-exec the redo program
(which is a symlink to minimal/do).
---
 minimal/do                   | 10 ++++++----
 t/000-set-minus-e/.gitignore |  1 +
 t/000-set-minus-e/all.do     |  4 ++++
 t/000-set-minus-e/clean.do   |  1 +
 t/000-set-minus-e/fatal.do   |  4 ++++
 t/all.do                     |  8 ++++++++
 6 files changed, 24 insertions(+), 4 deletions(-)
 create mode 100644 t/000-set-minus-e/.gitignore
 create mode 100644 t/000-set-minus-e/all.do
 create mode 100644 t/000-set-minus-e/clean.do
 create mode 100644 t/000-set-minus-e/fatal.do

diff --git a/minimal/do b/minimal/do
index d84c442..9fe0c01 100755
--- a/minimal/do
+++ b/minimal/do
@@ -153,19 +153,21 @@ _dir_shovel()
 }
 
 
-redo()
+_redo()
 {
+	set +e
 	for i in "$@"; do
 		_dirsplit "$i"
 		_dir_shovel "$dir" "$base"
 		dir=$xdir base=$xbase basetmp=$xbasetmp
-		( cd "$dir" && _do "$dir" "$base" "$basetmp" ) || return 1
+		( cd "$dir" && _do "$dir" "$base" "$basetmp" )
+		[ "$?" = 0 ] || return 1
 	done
 }
 
 
-set -e
-redo "$@"
+_redo "$@"
+[ "$?" = 0 ] || exit 1
 
 if [ -n "$DO_TOP" ]; then
 	echo "Removing stamp files..." >&2
diff --git a/t/000-set-minus-e/.gitignore b/t/000-set-minus-e/.gitignore
new file mode 100644
index 0000000..6bfe6b1
--- /dev/null
+++ b/t/000-set-minus-e/.gitignore
@@ -0,0 +1 @@
+log
diff --git a/t/000-set-minus-e/all.do b/t/000-set-minus-e/all.do
new file mode 100644
index 0000000..03b87a0
--- /dev/null
+++ b/t/000-set-minus-e/all.do
@@ -0,0 +1,4 @@
+rm -f log
+redo fatal >&/dev/null || true
+
+[ "$(cat log)" = "ok" ] || exit 5
diff --git a/t/000-set-minus-e/clean.do b/t/000-set-minus-e/clean.do
new file mode 100644
index 0000000..4676c2d
--- /dev/null
+++ b/t/000-set-minus-e/clean.do
@@ -0,0 +1 @@
+rm -f log *~ .*~
diff --git a/t/000-set-minus-e/fatal.do b/t/000-set-minus-e/fatal.do
new file mode 100644
index 0000000..b4037c7
--- /dev/null
+++ b/t/000-set-minus-e/fatal.do
@@ -0,0 +1,4 @@
+rm -f log
+echo ok >>log
+this-should-cause-a-fatal-error
+echo fail >>log  # this line should never run
diff --git a/t/all.do b/t/all.do
index 34a6374..70c1291 100644
--- a/t/all.do
+++ b/t/all.do
@@ -1,3 +1,11 @@
+# tests that "set -e" works (.do scripts always run with -e set by default)
+rm -f 000-set-minus-e/log
+redo 000-set-minus-e/all
+if [ "$(cat 000-set-minus-e/log)" != "ok" ]; then
+    echo "FATAL! .do file not run with 'set -e' in effect!" >&2
+    exit 5
+fi
+
 # builds 1xx*/all
 /bin/ls 1[0-9][0-9]*/all.do |
 sed 's/\.do$//' |