-
-
Notifications
You must be signed in to change notification settings - Fork 181
Add exec -a flag to set argv[0] #2634
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@andychu review. |
andychu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this!
(sorry for the delayed review, usually I aim for 24 hours -- but I'm still busy with family stuff)
spec/builtin-process.test.sh
Outdated
| ## BUG mksh stdout-json: "" | ||
|
|
||
| #### exec -a sets argv[0] | ||
| exec -a FOOPROC /bin/bash -c 'echo $0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make this bash or better yet just sh because /bin/bash may not exist on every system
builtin/process_osh.py
Outdated
| c2_argv = [arg.a] | ||
| c2_argv.extend(cmd_val.argv[i+1:]) | ||
| else: | ||
| c2_argv = cmd_val.argv[i:] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be simpler like:
argv = cmd_val.argv[i:]
if arg.a is not None:
argv[0] = arg.a
OK and actually I guess there is no way to change the location now ... but that's OK
exec -a foo python -c
^^^^^ # location is still python, not foo
Maybe write a comment about the location -- that controls this error
$ osh -c 'exec foo bar'
exec foo bar
^~~
[ -c flag ]:1: fatal: exec: 'foo' not found


The exec builtin now supports the -a flag to set argv[0], matching bash behavior.
close #2558