Skip to content

Commit

Permalink
[builtin/bracket] Fix the overflow check to return status 2.
Browse files Browse the repository at this point in the history
grr, it was returning 1 which is not a boolean!  Somehow tests didn't
catch that.

- refactor: Change spec test format.
- Also document the fact that we will change the 'test' builtin exit
  status with 'shopt -s simple_test_builtin'.  This allows people to do
  more precise error handling if they want.
  • Loading branch information
Andy Chu committed Sep 5, 2019
1 parent fc65e6a commit a6daac2
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 24 deletions.
2 changes: 1 addition & 1 deletion doc/osh-quick-ref-toc.txt
Expand Up @@ -149,7 +149,7 @@ SHELL OPTIONS
simple_word_eval No splitting, static globbing
more_errexit More errexit checks -- at command sub
X longopts test -file, shopt -unset, etc.
X simple_test_builtin Only file tests (no strings), remove [
X simple_test_builtin Only file tests, remove [, status 2
[Oil Syntax] X all:parse
parse_at echo @array @arrayfunc(x, y)
X parse_brace cd ~/src { ... }
Expand Down
13 changes: 9 additions & 4 deletions osh/expr_eval.py
Expand Up @@ -693,15 +693,20 @@ def Eval(self, node):
try:
mode = posix.lstat(s).st_mode
except OSError:
# TODO: simple_test_builtin should this as status=2.
#e_die("lstat() error: %s", e, word=node.child)
return False

return stat.S_ISLNK(mode)

try:
st = posix.stat(s)
except OSError:
# TODO: Signal extra debug information?
#log("Error from stat(%r): %s" % (s, e))
except OSError as e:
# TODO: simple_test_builtin should this as status=2.
# Problem: we really need errno, because test -f / is bad argument,
# while test -f /nonexistent is a good argument but failed. Gah.
# ENOENT vs. ENAMETOOLONG.
#e_die("stat() error: %s", e, word=node.child)
return False
mode = st.st_mode

Expand Down Expand Up @@ -765,7 +770,7 @@ def Eval(self, node):
return posix.isatty(fd)
# fd is user input, and causes this exception in the binding.
except OverflowError:
return 1
e_die('File descriptor %r is too big', s, word=node.child)

# See whether 'set -o' options have been set
if op_id == Id.BoolUnary_o:
Expand Down
113 changes: 94 additions & 19 deletions spec/builtin-bracket.test.sh
Expand Up @@ -13,21 +13,35 @@ echo status=$?
echo status=$?
[ '(' ]
echo status=$?
## stdout-json: "status=0\nstatus=0\nstatus=0\nstatus=0\n"
## STDOUT:
status=0
status=0
status=0
status=0
## END

#### one arg: empty string is false. Equivalent to -n.
test 'a' && echo true
test '' || echo false
## stdout-json: "true\nfalse\n"
## STDOUT:
true
false
## END

#### -a as unary operator (alias of -e)
# NOT IMPLEMENTED FOR OSH, but could be later. See comment in core/id_kind.py.
[ -a / ]
echo status=$?
[ -a /nonexistent ]
echo status=$?
## stdout-json: "status=0\nstatus=1\n"
## N-I dash stdout-json: "status=2\nstatus=2\n"
## STDOUT:
status=0
status=1
## END
## N-I dash STDOUT:
status=2
status=2
## END

#### two args: -z with = ! ( ]
[ -z = ]
Expand All @@ -38,7 +52,12 @@ echo status=$?
echo status=$?
[ -z '(' ]
echo status=$?
## stdout-json: "status=1\nstatus=1\nstatus=1\nstatus=1\n"
## STDOUT:
status=1
status=1
status=1
status=1
## END

#### three args
[ foo = '' ]
Expand All @@ -51,14 +70,23 @@ echo status=$?
echo status=$?
[ \( foo \) ]
echo status=$?
## stdout-json: "status=1\nstatus=1\nstatus=0\nstatus=0\nstatus=0\n"
## STDOUT:
status=1
status=1
status=0
status=0
status=0
## END

#### four args
[ ! foo = foo ]
echo status=$?
[ \( -z foo \) ]
echo status=$?
## stdout-json: "status=1\nstatus=1\n"
## STDOUT:
status=1
status=1
## END

#### test with extra args is syntax error
test -n x ]
Expand Down Expand Up @@ -92,7 +120,10 @@ status=2
#### -n
test -n 'a' && echo true
test -n '' || echo false
## stdout-json: "true\nfalse\n"
## STDOUT:
true
false
## END

#### ! -a
[ -z '' -a ! -z x ]
Expand All @@ -117,25 +148,39 @@ command [ -z '' -a '(' ! -z x ')' ] && echo true
#### == is alias for =
[ a = a ] && echo true
[ a == a ] && echo true
## stdout-json: "true\ntrue\n"
## BUG dash stdout-json: "true\n"
## STDOUT:
true
true
## END
## BUG dash STDOUT:
true
## END
## BUG dash status: 2

#### == and = does not do glob
[ abc = 'a*' ]
echo status=$?
[ abc == 'a*' ]
echo status=$?
## stdout-json: "status=1\nstatus=1\n"
## N-I dash stdout-json: "status=1\nstatus=2\n"
## STDOUT:
status=1
status=1
## END
## N-I dash STDOUT:
status=1
status=2
## END

#### [ with op variable
# OK -- parsed AFTER evaluation of vars
op='='
[ a $op a ] && echo true
[ a $op b ] || echo false
## status: 0
## stdout-json: "true\nfalse\n"
## STDOUT:
true
false
## END

#### [ with unquoted empty var
empty=''
Expand All @@ -147,7 +192,10 @@ empty=''
var=-f
[ $var = -f ] && echo true
[ '-f' = $var ] && echo true
## stdout-json: "true\ntrue\n"
## STDOUT:
true
true
## END

#### [ '(' foo ] is runtime syntax error
[ '(' foo ]
Expand All @@ -158,7 +206,11 @@ echo status=$?
[ -z ] && echo true # -z is operand
[ -z '>' ] || echo false # -z is operator
[ -z '>' -- ] && echo true # -z is operand
## stdout-json: "true\nfalse\ntrue\n"
## STDOUT:
true
false
true
## END

#### operator/operand ambiguity with ]
# bash parses this as '-z' AND ']', which is true. It's a syntax error in
Expand All @@ -183,7 +235,10 @@ test -d $TMP
echo status=$?
test -d $TMP/__nonexistent_Z_Z__
echo status=$?
## stdout-json: "status=0\nstatus=1\n"
## STDOUT:
status=0
status=1
## END

#### -x
rm -f $TMP/x
Expand All @@ -192,15 +247,22 @@ test -x $TMP/x || echo 'no'
chmod +x $TMP/x
test -x $TMP/x && echo 'yes'
test -x $TMP/__nonexistent__ || echo 'bad'
## stdout-json: "no\nyes\nbad\n"
## STDOUT:
no
yes
bad
## END

#### -r
echo '1' > $TMP/testr_yes
echo '2' > $TMP/testr_no
chmod -r $TMP/testr_no # remove read permission
test -r $TMP/testr_yes && echo 'yes'
test -r $TMP/testr_no || echo 'no'
## stdout-json: "yes\nno\n"
## STDOUT:
yes
no
## END

#### -w
rm -f $TMP/testw_*
Expand All @@ -209,7 +271,10 @@ echo '2' > $TMP/testw_no
chmod -w $TMP/testw_no # remove write permission
test -w $TMP/testw_yes && echo 'yes'
test -w $TMP/testw_no || echo 'no'
## stdout-json: "yes\nno\n"
## STDOUT:
yes
no
## END

#### -h and -L test for symlink
tmp=$TMP/builtin-test-1
Expand Down Expand Up @@ -399,3 +464,13 @@ same
different
different
## END

#### Overflow error
test -t 12345678910
echo status=$?
## STDOUT:
status=2
## END
## OK dash/bash STDOUT:
status=1
## END

0 comments on commit a6daac2

Please sign in to comment.