Skip to content

Commit

Permalink
[errexit] Make note of 2 problems with Oil's errexit fixes.
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
Andy Chu committed Apr 15, 2020
1 parent 4c74733 commit e342028
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 9 deletions.
7 changes: 6 additions & 1 deletion osh/cmd_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
7 changes: 4 additions & 3 deletions osh/glob_.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 47 additions & 3 deletions spec/errexit-oil.test.sh
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -402,3 +445,4 @@ done
## END
## N-I dash status: 2
## N-I dash stdout-json: ""

4 changes: 2 additions & 2 deletions test/spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 "$@"
}

Expand Down Expand Up @@ -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 "$@"
}

Expand Down

0 comments on commit e342028

Please sign in to comment.