-
-
Notifications
You must be signed in to change notification settings - Fork 174
[spec/builtin-trap] Add test case for ignored traps in subshells #2468
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
base: soil-staging
Are you sure you want to change the base?
Conversation
|
I would think of it in terms of what "ignored" is an ambiguous term
|
|
Is it good now? Note: Detailed information on the commits can be found in the commit messages. |
|
Looking at the source code the fix seems simple. |
|
OK the general idea does look good ! Other shells pass and OSH fails We need assertions for the other shells, like See https://github.com/oils-for-unix/oils/wiki/Spec-Tests And we are inhibited by this is a really bad bug ... seems like it may block this spec test, gah |
|
I think we have to figure out what OSH does here first: Most shells don't respect traps in subshells: pipelines etc. I think these tests are a bit confused by this issue, especially because bash behaves differently Let's discuss on Zulip about what we're trying to accomplish |
POSIX specifies that if the action of trap is null the shell shall ignore each specified condition (with SIG_IGN). oils doesn't do it. POSIX also says: > When a subshell is entered, traps that are not being ignored shall > be set to the default actions, except in the case of a command > substitution containing only a single trap command, when the traps > need not be altered. `dash` and `bash` always reset traps (except ignored ones).
e32bcc4 to
58a6842
Compare
POSIX specifies that if the action of trap is null the shell shall ignore each specified condition. Currently oils doesn't adhere.
POSIX also says:
However
dashandbashdon't fully follow this behavior, which I am pretty sure is a bug in both shells. I haven't tested other shells yet, including oils.TODO: Add the spec tests for the exception of traps.