-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
unix_sigpipe: Add test for SIGPIPE disposition in child processes #121573
Conversation
rustbot has assigned @Mark-Simulacrum. Use r? to explicitly pick a reviewer |
9dfde0f
to
527be25
Compare
Is there a reason this is a run-make test? It seems like it should be just a run-pass test like this one https://github.com/rust-lang/rust/blob/master/tests/ui/panics/abort-on-panic.rs#L1-L3 (or plenty of others with run-pass and revisions:) |
@Mark-Simulacrum I don't think run-pass tests supports building the separate program that this test needs? The example you linked to runs itself as a child process, but my test can't do that, since the Rust runtime would mess with |
Ah. I think it might be possible with auxiliary test files, but it's probably not worth the extra work then. @bors r+ |
…ark-Simulacrum unix_sigpipe: Add test for SIGPIPE disposition in child processes To make it clearer what the impact would be to stop using `SIG_IGN` and instead use a noop handler, like suggested [here](rust-lang#62569 (comment)) and implemented [here](rust-lang#121578). Part of rust-lang#97889
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#121573 (unix_sigpipe: Add test for SIGPIPE disposition in child processes) - rust-lang#121754 ([bootstrap] Move the `split-debuginfo` setting to the per-target section) - rust-lang#122205 (ensure that sysroot is properly set for cross-targets) - rust-lang#122257 (mir-opt tests: don't run a different set on --bless; enable --keep-stage-std) - rust-lang#122272 (Subtree update of `rust-analyzer`) Failed merges: - rust-lang#122108 (Add `target.*.runner` configuration for targets) r? `@ghost` `@rustbot` modify labels: rollup
…ark-Simulacrum unix_sigpipe: Add test for SIGPIPE disposition in child processes To make it clearer what the impact would be to stop using `SIG_IGN` and instead use a noop handler, like suggested [here](rust-lang#62569 (comment)) and implemented [here](rust-lang#121578). Part of rust-lang#97889
Rollup of 8 pull requests Successful merges: - rust-lang#121148 (Add slice::try_range) - rust-lang#121573 (unix_sigpipe: Add test for SIGPIPE disposition in child processes) - rust-lang#121633 (Win10: Use `GetSystemTimePreciseAsFileTime` directly) - rust-lang#121840 (Expose the Freeze trait again (unstably) and forbid implementing it manually) - rust-lang#121907 (skip sanity check for non-host targets in `check` builds) - rust-lang#122002 (std::threads: revisit stack address calculation on netbsd.) - rust-lang#122108 (Add `target.*.runner` configuration for targets) - rust-lang#122298 (RawVec::into_box: avoid unnecessary intermediate reference) r? `@ghost` `@rustbot` modify labels: rollup
Failed in #122327: #122327 (comment) @bors r- |
527be25
to
65eb51c
Compare
compiletest: Add support for `//@ aux-bin: foo.rs` Which enables ui tests to use auxiliary binaries. See the added self-test for an example. This is an enabler for the test in rust-lang#121573.
Rollup merge of rust-lang#122634 - Enselic:aux-bin, r=oli-obk compiletest: Add support for `//@ aux-bin: foo.rs` Which enables ui tests to use auxiliary binaries. See the added self-test for an example. This is an enabler for the test in rust-lang#121573.
☔ The latest upstream changes (presumably #122735) made this pull request unmergeable. Please resolve the merge conflicts. |
0dc26de
to
3095794
Compare
@Mark-Simulacrum After rebasing on #122634 the only commit left in this PR is the test itself (which is now a regular run-pass ui test) which you more or less already approved, so would be great if you could take another look. |
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.
r=me if we really need the extra ignore
@@ -0,0 +1,56 @@ | |||
//@ revisions: default sig_dfl sig_ign inherit | |||
//@ ignore-cross-compile because we run the compiled code |
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.
This isn't present in most run-pass tests, so what makes this one special?
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.
Turns out it is not run-pass
that is the problem here, because the following run-pass
and aux-build
test passes in cross-compilation context:
src/ci/docker/run.sh --dev armhf-gnu
../x test --target arm-unknown-linux-gnueabihf tests/ui/cross-crate/reexported-static-methods-cross-crate.rs
Instead, it is aux-bin
that does not support cross compilation yet. For example, remote-test-client
does not yet know how to upload auxiliary binaries to the target device. That it something that would be nice to solve, but it does not block this PR. I updated the motivation for the ignore.
I would be happy to @bors r=Mark-Simulacrum
the latest commit but you'll need to @bors delegate=Enselic
first.
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.
Sounds good, thanks.
For robustness, also test the disposition in our own process even if other tests in `tests/ui/attributes/unix_sigpipe` already covers it.
3095794
to
71eb763
Compare
@bors r+ rollup |
…ark-Simulacrum unix_sigpipe: Add test for SIGPIPE disposition in child processes To make it clearer what the impact would be to stop using `SIG_IGN` and instead use a noop handler, like suggested [here](rust-lang#62569 (comment)) and implemented [here](rust-lang#121578). Part of rust-lang#97889
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#121573 (unix_sigpipe: Add test for SIGPIPE disposition in child processes) - rust-lang#123170 (Replace regions in const canonical vars' types with `'static` in next-solver canonicalizer) - rust-lang#123200 (KCFI: Require -C panic=abort) - rust-lang#123201 (Improve wording in std::any explanation) - rust-lang#123224 (compiletest: print reason for failing to read tests) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#121573 - Enselic:sigpipe-child-process, r=Mark-Simulacrum unix_sigpipe: Add test for SIGPIPE disposition in child processes To make it clearer what the impact would be to stop using `SIG_IGN` and instead use a noop handler, like suggested [here](rust-lang#62569 (comment)) and implemented [here](rust-lang#121578). Part of rust-lang#97889
To make it clearer what the impact would be to stop using
SIG_IGN
and instead use a noop handler, like suggested here and implemented here.Part of #97889