From e342028ff2704564f3b4f475212d8902add05b46 Mon Sep 17 00:00:00 2001
From: Andy Chu <andy@oilshell.org>
Date: Wed, 15 Apr 2020 02:11:27 -0700
Subject: [PATCH] [errexit] Make note of 2 problems with Oil's errexit fixes.

- singleton command sub
- command sub is in another process.  (Is there a way around this?)
  - I think we could detect it statically, not dynamically?

Related to issue #476.  And #709.

Unrelated: Restore assertions in glob code
---
 osh/cmd_eval.py          |  7 +++++-
 osh/glob_.py             |  7 +++---
 spec/errexit-oil.test.sh | 50 +++++++++++++++++++++++++++++++++++++---
 test/spec.sh             |  4 ++--
 4 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/osh/cmd_eval.py b/osh/cmd_eval.py
index f03db28948..9aa46413bc 100755
--- a/osh/cmd_eval.py
+++ b/osh/cmd_eval.py
@@ -125,7 +125,12 @@
     command_e.BraceGroup, command_e.Subshell,
     command_e.WhileUntil, command_e.If, command_e.Case,
     command_e.TimeBlock,
-    command_e.CommandList,  # Happens in $(command sub)
+    # BUG: This happens in 'if echo $(echo hi; false)'
+    #           but not in 'if echo $(false)'
+    # Because the CommandSub has no CommandList!
+    # This also doesn't work bceause the CommandList is inside the
+    # SubProgramThunk, and doesn't abort the rest of the program!
+    command_e.CommandList,
 ]
 
 def _DisallowErrExit(node):
diff --git a/osh/glob_.py b/osh/glob_.py
index 3ce8e37d6b..42c87d9e3d 100644
--- a/osh/glob_.py
+++ b/osh/glob_.py
@@ -129,17 +129,18 @@ def GlobUnescape(s):  # used by cmd_eval
     c = s[i]
     if c == '\\' and i != n - 1:
       # Suppressed this to fix bug #698, #628 is still there.
-      #assert i != n - 1, 'Trailing backslash: %r' % s
+      assert i != n - 1, 'Trailing backslash: %r' % s
       i += 1
       c2 = s[i]
       if c2 in GLOB_META_CHARS:
         unescaped.append(c2)
       else:
-        #raise AssertionError("Unexpected escaped character %r" % c2)
+        # 'test/spec.sh glob -r 25' triggers this
+        raise AssertionError("Unexpected escaped character %r" % c2)
         # Hack to prevent crash for now.  Need to rewrite this.
         # Fell out of the fix to issue #695 to use _DQ_BACKSLASH in VS_ArgDQ.
         #unescaped.append(c)
-        unescaped.append(c2)
+        #unescaped.append(c2)
     else:
       unescaped.append(c)
     i += 1
diff --git a/spec/errexit-oil.test.sh b/spec/errexit-oil.test.sh
index 68e0b346c6..2eb99912e8 100644
--- a/spec/errexit-oil.test.sh
+++ b/spec/errexit-oil.test.sh
@@ -1,6 +1,4 @@
-#!/bin/bash
-#
-# Cases relevant to Oil's:
+# Cases relevant to Oil:
 #
 # - shopt -s more_errexit
 # - and maybe inherit_errexit and strict_errexit (OSH)
@@ -51,6 +49,51 @@ one two
 parent status=0
 ## END
 
+#### strict_errexit with command sub stops program
+set -o errexit
+shopt -s inherit_errexit || true
+shopt -s strict_errexit || true
+if echo $( echo 1; false; echo 2); then
+  echo A
+fi
+echo done
+
+## status: 1
+## stdout-json: ""
+
+## N-I bash/ash status: 0
+## N-I bash/ash STDOUT:
+1 2
+A
+done
+## END
+
+## N-I dash/mksh status: 0
+## N-I dash/mksh STDOUT:
+1
+A
+done
+## END
+
+#### {inherit,strict}_errexit: command sub with a single command
+set -o errexit
+shopt -s inherit_errexit || true
+shopt -s strict_errexit || true
+if echo $(false); then
+  echo A
+fi
+echo done
+## status: 1
+## stdout-json: ""
+
+## N-I dash/bash/mksh/ash status: 0
+## N-I dash/bash/mksh/ash STDOUT:
+
+A
+done
+## END
+
+
 #### command sub with more_errexit only
 set -o errexit
 shopt -s more_errexit || true
@@ -402,3 +445,4 @@ done
 ## END
 ## N-I dash status: 2
 ## N-I dash stdout-json: ""
+
diff --git a/test/spec.sh b/test/spec.sh
index 7a8ccecc22..0b669f716d 100755
--- a/test/spec.sh
+++ b/test/spec.sh
@@ -570,7 +570,7 @@ var-op-bash() {
 }
 
 var-op-strip() {
-  sh-spec spec/var-op-strip.test.sh --osh-failures-allowed 1 \
+  sh-spec spec/var-op-strip.test.sh --osh-failures-allowed 0 \
     ${REF_SHELLS[@]} $ZSH $BUSYBOX_ASH $OSH_LIST "$@"
 }
 
@@ -622,7 +622,7 @@ errexit() {
 }
 
 errexit-oil() {
-  sh-spec spec/errexit-oil.test.sh --no-cd-tmp \
+  sh-spec spec/errexit-oil.test.sh --no-cd-tmp --osh-failures-allowed 2\
     ${REF_SHELLS[@]} $BUSYBOX_ASH $OSH_LIST "$@"
 }