Skip to content

Commit

Permalink
[osh-language] Evaluate redirect words that are paths differently
Browse files Browse the repository at this point in the history
They now use the EvalWordSequence() algorithm, to match bash, and zsh to
a degree.

- globs and extended globs are respected
- word splitting is done
- brace expansion is disallowed, unlike bash
- expanding to zero files is an error
- expanding to two or more files is an error

For example

    echo hi > two-*

Note that other shells are inconsistent about:

    star='*'
    echo hi > two-$star

bash and OSH do globbing there too.

Add more spec tests.

This is issue #1521.
  • Loading branch information
Andy C committed Sep 1, 2023
1 parent d13435e commit 9fd4445
Show file tree
Hide file tree
Showing 10 changed files with 375 additions and 218 deletions.
8 changes: 4 additions & 4 deletions osh/braces.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
Possible optimization flags for Compound:
- has Lit_LBrace, LitRBrace -- set during word_parse phase
- it if has both, then do _BraceDetect
- has BracedTuple -- set during _BraceDetect
- it if has both, then do BraceDetect
- has BracedTuple -- set during BraceDetect
- if it does, then do the expansion
- has Lit_Star, ?, [ ] -- globbing?
- but after expansion do you still have those flags?
Expand Down Expand Up @@ -194,7 +194,7 @@ def __init__(self, cur_parts):
self.saw_comma = False


def _BraceDetect(w):
def BraceDetect(w):
# type: (CompoundWord) -> Optional[word.BracedTree]
"""Return a new word if the input word looks like a brace expansion.
Expand Down Expand Up @@ -315,7 +315,7 @@ def BraceDetectAll(words):
# a lot of garbage from being created, since otherwise nearly every word
# would be checked. We could be even more precise but this is cheap.
if len(w.parts) >= 3:
brace_tree = _BraceDetect(w)
brace_tree = BraceDetect(w)
if brace_tree:
out.append(brace_tree)
continue
Expand Down
22 changes: 11 additions & 11 deletions osh/braces_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,36 +66,36 @@ def testRangePartDetect(self):

def testBraceDetect(self):
w = _assertReadWord(self, '}')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
self.assertEqual(None, tree)

w = _assertReadWord(self, ',')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
self.assertEqual(None, tree)

w = _assertReadWord(self, 'B-{a,b}-E')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
self.assertEqual(3, len(tree.parts))
_PrettyPrint(tree)
print('--')

# Multiple parts for each alternative
w = _assertReadWord(self, 'B-{a"a",b"b",c"c"}-E')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
self.assertEqual(3, len(tree.parts))
_PrettyPrint(tree)
print('--')

# Multiple expansion
w = _assertReadWord(self, 'B-{a,b}--{c,d}-E')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
self.assertEqual(5, len(tree.parts))
_PrettyPrint(tree)
print('--')

# Nested expansion
w = _assertReadWord(self, 'B-{a,b,c,={d,e}}-E')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
_PrettyPrint(tree)
self.assertEqual(3, len(tree.parts)) # B- {} -E

Expand All @@ -112,7 +112,7 @@ def testBraceDetect(self):

# Another nested expansion
w = _assertReadWord(self, 'B-{a,={b,c}=,d}-E')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
_PrettyPrint(tree)
self.assertEqual(3, len(tree.parts)) # B- {} -E

Expand All @@ -134,7 +134,7 @@ def testBraceDetect(self):

# Third alternative is a Compound with zero parts
w = _assertReadWord(self, '{a,b,}')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
_PrettyPrint(tree)
self.assertEqual(1, len(tree.parts))
self.assertEqual(3, len(tree.parts[0].words))
Expand All @@ -148,7 +148,7 @@ def testBraceExpand(self):
print('')

w = _assertReadWord(self, 'B-{a,b}-E')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
self.assertEqual(3, len(tree.parts))
_PrettyPrint(tree)

Expand All @@ -159,7 +159,7 @@ def testBraceExpand(self):
print('')

w = _assertReadWord(self, 'B-{a,={b,c,d}=,e}-E')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
self.assertEqual(3, len(tree.parts))
_PrettyPrint(tree)

Expand All @@ -170,7 +170,7 @@ def testBraceExpand(self):
print('')

w = _assertReadWord(self, 'B-{a,b}-{c,d}-E')
tree = braces._BraceDetect(w)
tree = braces.BraceDetect(w)
self.assertEqual(5, len(tree.parts))
_PrettyPrint(tree)

Expand Down
59 changes: 44 additions & 15 deletions osh/cmd_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
for_iter_e,
pat,
pat_e,
word,
)
from _devbuild.gen.runtime_asdl import (
lvalue,
Expand Down Expand Up @@ -414,27 +415,49 @@ def _EvalRedirect(self, r):
if case(redir_param_e.Word):
arg_word = cast(CompoundWord, UP_arg)

# note: needed for redirect like 'echo foo > x$LINENO'
# Note: needed for redirect like 'echo foo > x$LINENO'
self.mem.SetTokenForLine(r.op)

redir_type = consts.RedirArgType(
r.op.id) # could be static in the LST?
# Could be computed at parse time?
redir_type = consts.RedirArgType(r.op.id)

if redir_type == redir_arg_type_e.Path:
# Note: Only bash and zsh allow globbing a path. If there
# are multiple files, zsh opens BOTH, but bash exits with
# error 1.
# Redirects with path arguments are evaluated in a special
# way. bash and zsh allow globbing a path, but
# dash/ash/mksh don't.
#
# If there are multiple files, zsh opens BOTH, but bash
# makes the command fail with status 1. We mostly follow
# bash behavior.

# These don't match bash/zsh behavior
# val = self.word_ev.EvalWordToString(arg_word)
# val, has_extglob = self.word_ev.EvalWordToPattern(arg_word)
# Short-circuit with word_.StaticEval() also doesn't work
# with globs

# mycpp needs this explicit declaration
b = braces.BraceDetect(arg_word) # type: Optional[word.BracedTree]
if b is not None:
raise error.RedirectEval(
'Brace expansion not allowed (try adding quotes)',
arg_word)

# - no globbing. You can write to a file called '*.py'.
# - set -o strict-array prevents joining by spaces
val = self.word_ev.EvalWordToString(arg_word)
filename = val.s
if len(filename) == 0:
# Whether this is fatal depends on errexit.
# Needed for globbing behavior
files = self.word_ev.EvalWordSequence([arg_word])

n = len(files)
if n == 0:
# happens in OSH on empty elision
# in YSH because simple_word_eval globs to zero
raise error.RedirectEval(
"Redirect filename can't be empty", arg_word)
"Can't redirect to zero files", arg_word)
if n > 1:
raise error.RedirectEval(
"Can't redirect to more than one file",
arg_word)

result.arg = redirect_arg.Path(filename)
result.arg = redirect_arg.Path(files[0])
return result

elif redir_type == redir_arg_type_e.Desc: # e.g. 1>&2, 1>&-, 1>&2-
Expand Down Expand Up @@ -1638,6 +1661,12 @@ def _Execute(self, node):
except error.RedirectEval as e:
self.errfmt.PrettyPrintError(e)
redirects = None
except error.FailGlob as e: # e.g. echo hi > foo-*
if not e.HasLocation():
e.location = self.mem.GetFallbackLocation()
self.errfmt.PrettyPrintError(e,
prefix='failglob: ')
redirects = None

if redirects is None: # Error evaluating redirect words
status = 1
Expand All @@ -1656,7 +1685,7 @@ def _Execute(self, node):
self.errfmt.PrettyPrintError(e,
prefix='failglob: ')
status = 1
check_errexit = True
check_errexit = True # probably not necessary?

# Compute status from @PIPESTATUS
pipe_status = cmd_st.pipe_status
Expand Down
14 changes: 0 additions & 14 deletions spec/extglob-files.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -164,20 +164,6 @@ argv.py @('foo'|'__|'|bar)
['__|', 'foo']
## END

#### Extended glob syntax in bad redirect context
shopt -s extglob
rm bad_*

# They actually write this literal file! This is what EvalWordToString() does,
# as opposed to _EvalWordToParts.
echo foo > bad_@(*.cc|*.h)
echo bad_*
## STDOUT:
bad_@(*.cc|*.h)
## END
## OK osh status: 1
## OK osh stdout-json: ""

#### Extended glob as argument to ${undef:-} (dynamic globbing)

# This case popped into my mind after inspecting osh/word_eval.py for calls to
Expand Down
11 changes: 0 additions & 11 deletions spec/glob.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,6 @@ echo _tmp/*.[[:punct:]] _tmp/*.[[:punct\:]]
## BUG mksh stdout: _tmp/*.[[:punct:]] _tmp/*.[[:punct:]]
## BUG ash stdout: _tmp/foo.- _tmp/foo.-

#### Redirect to glob, not evaluated
# This writes to *.F, not foo.F
rm _tmp/*.F
touch _tmp/f.F
echo foo > _tmp/*.F
cat '_tmp/*.F'
## status: 0
## stdout: foo
## BUG bash status: 1
## BUG bash stdout-json: ""

#### Glob after var manipulation
touch _tmp/foo.zzz _tmp/bar.zzz
g='_tmp/*.zzzZ'
Expand Down
137 changes: 0 additions & 137 deletions spec/redirect-glob.test.sh

This file was deleted.

0 comments on commit 9fd4445

Please sign in to comment.