Permalink
Browse files

Fix 'unset R' where R is readonly.

OSH was too strict, and it caused the Aboriginal run to fail, because it
tries to 'unset SHELLOPTS'.

We have to rely on 'errexit' for a lot of strictness.

Also:
- add 2 shells snippets that OSH should fail on but doesn't.
- strict-options.test.sh uses the new spec test format.
  • Loading branch information...
Andy Chu
Andy Chu committed Dec 29, 2017
1 parent 9887c70 commit f9a59861f662422e4eaf8e504c7231bf46306de7
Showing with 70 additions and 29 deletions.
  1. +8 −2 core/builtin.py
  2. +8 −4 core/state.py
  3. +3 −4 spec/builtin-vars.test.sh
  4. +13 −0 spec/parse-errors.test.sh
  5. +37 −18 spec/strict-options.test.sh
  6. +1 −1 test/spec.sh
View
@@ -839,10 +839,16 @@ def Unset(argv, mem, funcs):
if name in funcs:
del funcs[name]
elif arg.v:
mem.Unset(runtime.LhsName(name), scope_e.Dynamic)
ok, _ = mem.Unset(runtime.LhsName(name), scope_e.Dynamic)
if not ok:
util.error("Can't unset readonly variable %r", name)
return 1
else:
# Try to delete var first, then func.
found = mem.Unset(runtime.LhsName(name), scope_e.Dynamic)
ok, found = mem.Unset(runtime.LhsName(name), scope_e.Dynamic)
if not ok:
util.error("Can't unset readonly variable %r", name)
return 1
#log('%s: %s', name, found)
if not found:
if name in funcs:
View
@@ -623,17 +623,21 @@ def GetVar(self, name, lookup_mode=scope_e.Dynamic):
def Unset(self, lval, lookup_mode):
"""
Returns:
Success or failure. A non-existent variable is still considered success.
ok bool, found bool.
ok is false if the cell is read-only.
found is false if the name is not there.
"""
if lval.tag == lvalue_e.LhsName: # unset x
cell, namespace = self._FindCellAndNamespace(lval.name, lookup_mode)
if cell:
found = True
if cell.readonly:
e_die("Can't unset readonly variable %r", lval.name)
return False, found
del namespace[lval.name] # it must be here
return True # found
return True, found # found
else:
return False
return True, False
elif lval.tag == lvalue_e.LhsIndexedName: # unset a[1]
raise NotImplementedError
@@ -153,11 +153,10 @@ echo status=$?
readonly R=foo
unset R
echo status=$?
# status: 1
# stdout-json: ""
# status: 0
# stdout: status=1
# OK dash status: 2
# BUG mksh/bash stdout-json: "status=1\n"
# BUG mksh/bash status: 0
# OK dash stdout-json: ""
### Unset a function without -f
f() {
View
@@ -103,3 +103,16 @@ f() {
echo a(b)
# status: 2
# OK mksh status: 1
### incomplete command sub
$(x
# status: 2
# OK mksh status: 1
### misplaced ;;
echo 1 ;; echo 2
# stdout-json: ""
# status: 2
# OK mksh status: 1
@@ -1,46 +1,65 @@
#!/usr/bin/env bash
### Sourcing a script that returns is allowed no matter what
### Sourcing a script that returns at the top level
echo one
. spec/testdata/return-helper.sh
echo $?
echo two
# stdout-json: "one\nreturn-helper.sh\n42\ntwo\n"
## STDOUT:
one
return-helper.sh
42
two
## END
### top level control flow
$SH spec/testdata/top-level-control-flow.sh
# stdout-json: "SUBSHELL\nBREAK\nCONTINUE\nRETURN\n"
# OK bash stdout-json: "SUBSHELL\nBREAK\nCONTINUE\nRETURN\nDONE\n"
# status: 0
## status: 0
## STDOUT:
SUBSHELL
BREAK
CONTINUE
RETURN
## OK bash STDOUT:
SUBSHELL
BREAK
CONTINUE
RETURN
DONE
## END
### errexit and top-level control flow
$SH -o errexit spec/testdata/top-level-control-flow.sh
# stdout-json: "SUBSHELL\n"
# status: 2
# OK bash status: 1
## status: 2
## OK bash status: 1
## STDOUT:
SUBSHELL
## END
### set -o strict-control-flow
$SH -o strict-control-flow -c 'echo break; break; echo hi'
# stdout: break
# status: 1
# N-I dash/bash status: 2
# N-I dash/bash stdout-json: ""
# N-I mksh status: 1
# N-I mksh stdout-json: ""
## stdout: break
## status: 1
## N-I dash/bash status: 2
## N-I dash/bash stdout-json: ""
## N-I mksh status: 1
## N-I mksh stdout-json: ""
### return at top level is an error
return
echo "status=$?"
# stdout-json: ""
# OK bash stdout-json: "status=1\n"
## stdout-json: ""
## OK bash STDOUT:
status=1
## END
### continue at top level is NOT an error
# NOTE: bash and mksh both print warnings, but don't exit with an error.
continue
echo status=$?
# stdout: status=0
## stdout: status=0
### break at top level is NOT an error
break
echo status=$?
# stdout: status=0
## stdout: status=0
View
@@ -338,7 +338,7 @@ explore-parsing() {
}
parse-errors() {
sh-spec spec/parse-errors.test.sh --osh-failures-allowed 3 \
sh-spec spec/parse-errors.test.sh --osh-failures-allowed 5 \
${REF_SHELLS[@]} $OSH "$@"
}

0 comments on commit f9a5986

Please sign in to comment.