Do not parse aliases in POSIX mode when passing to shell
Review Request #10149 — Created Sept. 19, 2018 and submitted
Information | |
---|---|
brennie | |
RBTools | |
release-0.7.x | |
07a2ccb... | |
Reviewers | |
rbtools | |
We are using
shlex.split
to parse aliases, which defaults to a POSIX mode,
which strips quotes and evaluates escape sequences. This lead to issues
where an alias of the form!find . -iname '*.pyc' -delete
would be
evaluated intofind . -iname *.pyc -delete
run by the shell, which
would result in the*.pyc
being evaluated by the shell instead of by
find
.Now we operate in POSIX mode only when we are not passing the command
to the shell so shell-specific sequences will be untouched. This results
in aliases not needing double escaping when being passed to the shell.When not passing to the shell, we still parse in POSIX mode to remove
quotes, etc. because otherwisesubprocess.call()
will pass the quoted
strings as arguments (e.g.,['cmd', '"arg with spaces"']
instead of
['cmd', 'arg with spaces']
).While I was here, I also modernized the docstrings of all the alias
methods to our doc standards.replace_arguments
also now returns a
list instead of a generator because we were always immediately wrapping
it in a call tolist
anyway.
- Ran unit tests.
- Ran
rbt clear-pycs
with.pyc
files in the current directory. All
.pyc
files were recursively removed.
Description | From | Last Updated |
---|---|---|
E303 too many blank lines (2) |
![]() |
|
E303 too many blank lines (2) |
![]() |
|
Missing a period. |
|
|
Too many spaces after # |
|
|
typo: decote |
|
|
Extra ` at the end |
|
|
I feel like this would read a little bit more clearly if you swapped them (unicode or list of unicode). |
|
|
Remove this blank line. |
|
|
Typo: veresion -> version |
|
|
This needs a period at the end. |
|
|
Should this be "Quoted tokens"? |
|
Summary: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commit: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+187 -69) |
Checks run (1 failed, 1 succeeded)
flake8
-
rbtools/utils/tests.py (Diff revision 2) Show all issues -
Change Summary:
Fix flake8 errors.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+185 -69) |
Checks run (2 succeeded)
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+189 -69) |
Checks run (2 succeeded)
-
-
-
-
-
-
rbtools/utils/aliases.py (Diff revision 4) I feel like this would read a little bit more clearly if you swapped them (unicode or list of unicode).
-
Change Summary:
Addressed David's feedback.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+189 -69) |
Checks run (2 succeeded)
-
-
-
-
rbtools/utils/aliases.py (Diff revision 5) What does this error output look like? Are the exception values good to show to a user?
Should we list the name or content of the alias that failed to parse?
Change Summary:
Addressed David's feedback
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 6 (+195 -70) |
Checks run (2 succeeded)
Change Summary:
Addressed feedback.
Branch: |
|
||||
---|---|---|---|---|---|
Commit: |
|
||||
Diff: |
Revision 7 (+76 -2) |
Checks run (2 failed with error)
Change Summary:
I apparently pushed up a patch from reviewboard to this O_O
Branch: |
|
||||
---|---|---|---|---|---|
Commit: |
|
||||
Diff: |
Revision 8 (+195 -70) |
Checks run (2 succeeded)
Change Summary:
Addressed feedback.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 9 (+195 -70) |