rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` #102587

Enselic · 2022-10-02T15:55:34Z

This is the first (known) step towards starting to use unix_sigpipe in the wild. Eventually, rustc_driver::set_sigpipe_handler can be removed and all clients can use unix_sigpipe instead.

For now we just start using unix_sigpipe in one place: rustc itself.

It is easy to manually verify this change. If you remove #[unix_sigpipe = "sig_dfl"] and run ./x.py build you will get an ICE when you do ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false. Add back #[unix_sigpipe = "sig_dfl"] and the ICE disappears again.

PR that added set_sigpipe_handler: #49606

Tracking issue for unix_sigpipe: #97889

Not sure exactly how to label this PR. Going with T-libs for now since this is a T-libs feature.

@rustdoc labels +T-libs

This is the first (known) step towards starting to use `unix_sigpipe` in the wild. Eventually, `rustc_driver::set_sigpipe_handler` can be removed and all clients can use `unix_sigpipe` instead. For now we just start using `unix_sigpipe` in once place: `rustc` itself. It is easy to manually verify this change. If you remove `#[unix_sigpipe = "sig_dfl"]` and run `./x.py build` you will get an ICE when you do `./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false`. Add back `#[unix_sigpipe = "sig_dfl"]` and the ICE disappears again.

rust-highfive · 2022-10-02T15:55:37Z

r? @fee1-dead

(rust-highfive has picked a reviewer for you, use r? to override)

fee1-dead · 2022-10-02T15:57:15Z

I am unfamiliar with this feature, sorry.

r? rust-lang/compiler

joshtriplett · 2022-10-14T11:28:01Z

Has this been around for long enough that it works without needing a conditional for the bootstrap compiler?

If so, this seems fine.

Enselic · 2022-10-14T16:00:13Z

Yep, the current bootstrap compiler knows about this attribute:

$ cat hello-world.rs
#![feature(unix_sigpipe)]
#[unix_sigpipe = "sig_dfl"]
fn main() {
    loop {
        println!("hello world")
    }
}

$ RUSTC_BOOTSTRAP=1 ./build/aarch64-apple-darwin/stage0/bin/rustc hello-world.rs

$ ./hello-world | head -n 1
hello world

$ ./build/aarch64-apple-darwin/stage0/bin/rustc -V
rustc 1.65.0-beta.1 (2a65764f2 2022-09-19)

joshtriplett · 2022-10-14T16:27:20Z

LGTM then. (I'll leave it to a compiler reviewer to r+.)

jackh726 · 2022-10-24T00:04:17Z

@bors r+

bors · 2022-10-24T00:04:19Z

📌 Commit 093b075 has been approved by jackh726

It is now in the queue for this repository.

…h726 rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` This is the first (known) step towards starting to use `unix_sigpipe` in the wild. Eventually, `rustc_driver::set_sigpipe_handler` can be removed and all clients can use `unix_sigpipe` instead. For now we just start using `unix_sigpipe` in one place: `rustc` itself. It is easy to manually verify this change. If you remove `#[unix_sigpipe = "sig_dfl"]` and run `./x.py build` you will get an ICE when you do `./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false`. Add back `#[unix_sigpipe = "sig_dfl"]` and the ICE disappears again. PR that added `set_sigpipe_handler`: rust-lang#49606 Tracking issue for `unix_sigpipe`: rust-lang#97889 Not sure exactly how to label this PR. Going with T-libs for now since this is a T-libs feature. `@rustdoc` labels +T-libs

…triddle rustdoc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` Do what was already done for `rustc` in rust-lang#102587, namely start using `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`. After this has been merged, we can completely remove `rustc_driver::set_sigpipe_handler`. PR that added `set_sigpipe_handler`: rust-lang#49606 Tracking issue for `unix_sigpipe`: rust-lang#97889 Verification of this change --------------------------- 1. Remove `#[unix_sigpipe = "sig_dfl"]` 1. Run `./x.py build` 1. Run `./build/aarch64-apple-darwin/stage1/bin/rustdoc --help | false` 1. Observe ICE 1. Add back `#[unix_sigpipe = "sig_dfl"]` 1. Run `./x.py build` 1. Run `./build/aarch64-apple-darwin/stage1/bin/rustdoc --help | false` 1. Observe ICE fixed `@rustbot` labels +T-rustdoc

…triddle rustdoc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` Do what was already done for `rustc` in rust-lang#102587, namely start using `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`. After this has been merged, we can completely remove `rustc_driver::set_sigpipe_handler`. PR that added `set_sigpipe_handler`: rust-lang#49606 Tracking issue for `unix_sigpipe`: rust-lang#97889 Verification of this change --------------------------- 1. Remove `#[unix_sigpipe = "sig_dfl"]` 1. Run `./x.py build` 1. Run `./build/aarch64-apple-darwin/stage1/bin/rustdoc --help | false` 1. Observe ICE 1. Add back `#[unix_sigpipe = "sig_dfl"]` 1. Run `./x.py build` 1. Run `./build/aarch64-apple-darwin/stage1/bin/rustdoc --help | false` 1. Observe ICE fixed ``@rustbot`` labels +T-rustdoc

…h726 rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` This is the first (known) step towards starting to use `unix_sigpipe` in the wild. Eventually, `rustc_driver::set_sigpipe_handler` can be removed and all clients can use `unix_sigpipe` instead. For now we just start using `unix_sigpipe` in one place: `rustc` itself. It is easy to manually verify this change. If you remove `#[unix_sigpipe = "sig_dfl"]` and run `./x.py build` you will get an ICE when you do `./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false`. Add back `#[unix_sigpipe = "sig_dfl"]` and the ICE disappears again. PR that added `set_sigpipe_handler`: rust-lang#49606 Tracking issue for `unix_sigpipe`: rust-lang#97889 Not sure exactly how to label this PR. Going with T-libs for now since this is a T-libs feature. ``@rustdoc`` labels +T-libs

…h726 rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` This is the first (known) step towards starting to use `unix_sigpipe` in the wild. Eventually, `rustc_driver::set_sigpipe_handler` can be removed and all clients can use `unix_sigpipe` instead. For now we just start using `unix_sigpipe` in one place: `rustc` itself. It is easy to manually verify this change. If you remove `#[unix_sigpipe = "sig_dfl"]` and run `./x.py build` you will get an ICE when you do `./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false`. Add back `#[unix_sigpipe = "sig_dfl"]` and the ICE disappears again. PR that added `set_sigpipe_handler`: rust-lang#49606 Tracking issue for `unix_sigpipe`: rust-lang#97889 Not sure exactly how to label this PR. Going with T-libs for now since this is a T-libs feature. ```@rustdoc``` labels +T-libs

Rollup of 6 pull requests Successful merges: - rust-lang#98204 (Stabilize `Option::unzip()`) - rust-lang#102587 (rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`) - rust-lang#103122 (Remove misc_cast and validate types when casting) - rust-lang#103379 (Truncate thread names on Linux and Apple targets) - rust-lang#103482 (Clairify Vec::capacity docs) - rust-lang#103511 (Codegen tweaks) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…, r=tmiasko Remove `rustc_driver::set_sigpipe_handler()` Its usage was removed in rust-lang#102587 and rust-lang#103495, so we do not need to keep it around any longer. According to [preliminary input](https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/Find.20.60rustc_driver.60.20dependent.20projects.3F/near/304490764), we do not need to worry about any deprecation cycle for this explicitly unstable API, and can just straight up remove it. PR that added `set_sigpipe_handler`: rust-lang#49606 Tracking issue for `unix_sigpipe`: rust-lang#97889 Migration instructions for any remaining clients --- Change from ```rust #![feature(rustc_private)] extern crate rustc_driver; fn main() { rustc_driver::set_sigpipe_handler(); // ... ``` to ```rust #![feature(unix_sigpipe)] #[unix_sigpipe = "sig_dfl"] fn main() { // ... ``` `@rustbot` labels +T-compiler

…, r=tmiasko Remove `rustc_driver::set_sigpipe_handler()` Its usage was removed in rust-lang#102587 and rust-lang#103495, so we do not need to keep it around any longer. According to [preliminary input](https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/Find.20.60rustc_driver.60.20dependent.20projects.3F/near/304490764), we do not need to worry about any deprecation cycle for this explicitly unstable API, and can just straight up remove it. PR that added `set_sigpipe_handler`: rust-lang#49606 Tracking issue for `unix_sigpipe`: rust-lang#97889 Migration instructions for any remaining clients --- Change from ```rust #![feature(rustc_private)] extern crate rustc_driver; fn main() { rustc_driver::set_sigpipe_handler(); // ... ``` to ```rust #![feature(unix_sigpipe)] #[unix_sigpipe = "sig_dfl"] fn main() { // ... ``` ``@rustbot`` labels +T-compiler

…h726 rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` This is the first (known) step towards starting to use `unix_sigpipe` in the wild. Eventually, `rustc_driver::set_sigpipe_handler` can be removed and all clients can use `unix_sigpipe` instead. For now we just start using `unix_sigpipe` in one place: `rustc` itself. It is easy to manually verify this change. If you remove `#[unix_sigpipe = "sig_dfl"]` and run `./x.py build` you will get an ICE when you do `./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false`. Add back `#[unix_sigpipe = "sig_dfl"]` and the ICE disappears again. PR that added `set_sigpipe_handler`: rust-lang#49606 Tracking issue for `unix_sigpipe`: rust-lang#97889 Not sure exactly how to label this PR. Going with T-libs for now since this is a T-libs feature. ````@rustdoc```` labels +T-libs

Rollup of 6 pull requests Successful merges: - rust-lang#98204 (Stabilize `Option::unzip()`) - rust-lang#102587 (rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler`) - rust-lang#103122 (Remove misc_cast and validate types when casting) - rust-lang#103379 (Truncate thread names on Linux and Apple targets) - rust-lang#103482 (Clairify Vec::capacity docs) - rust-lang#103511 (Codegen tweaks) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…r-ozkan Revert rust-lang#131060 "Drop conditionally applied cargo `-Zon-broken-pipe=kill` flags" In [rust-lang#131059] we found out that `-Zon-broken-pipe=kill` is actually **load-bearing**[^1] for (at least) `rustc` and `rustdoc` to have the kill-process-on-broken-pipe behavior, e.g. `rustc --print=sysroot | false` will ICE and `rustdoc --print=sysroot | false` will panic on a broken pipe. This PR reverts 5a7058c (reverts PR rust-lang#131060) in favor of a future fix to *unconditionally* apply `-Zon-broken-pipe=kill` to tool builds and also not drop the `-Zon-broken-pipe=kill` flag for rustc binary builds. I could not figure out how to write a regression test for the `rustc --print=sysroot | false` behavior on Unix, so this is a plain revert for now. This revert will unfortunately reintroduce rust-lang#130980 until we fix it again with the different approach. See more details at <rust-lang#131059 (comment)> and in the timeline below. ### Timeline of kill-process-on-broken-pipe behavior changes See [`unix_sigpipe` tracking issue rust-lang#97889][rust-lang#97889] for more context around unix sigpipe handling. - From the very beginning since 2014, Rust binaries by default use `sig_ign`. This meant that if output pipe is broken yet the program tries to use `println!` and such, there will be a broken pipe panic from std. This lead to ICEs from e.g. `rustc --help | false` [rust-lang#34376]. - [rust-lang#49606] mitigated [rust-lang#34376] by adding an explicit signal handler to `rustc_driver` register a sigpipe handler with `SIG_DFL` which will cause the binary using `rustc_driver` to terminate if `rustc_driver::set_sigpipe_handler()` is called. `rustc`'s main binary wrapper uses `rustc_driver::set_sigpipe_handler()`, and so does `rustdoc`. - A more universal way to set sigpipe behavior for Unix was introduced as part of [rust-lang#97889], i.e. `# [unix_sigpipe = "sig_dfl"]` attribute. - [rust-lang#102587] migrated `rustc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. - [rust-lang#103495] migrated `rustdoc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. `rustc_driver::set_sigpipe_handler` was removed. - Following concerns about sigpipe setting UI in [rust-lang#97889], the UI for specifying sigpipe behavior was changed in [rust-lang#124480] from `#[unix_sigpipe = "sig_dfl"]` attribute to the commmand line flag `-Zon-broken-pipe=kill`. - In the same PR, `#[unix_sigpipe = "sig_dfl"]` were removed from `rustc` and `rustdoc` main binary crate entry points in favor of the command line flag. Kill-process-on-broken-pipe behavior was preserved by adding `-Zon-broken-pipe=kill` for `rustdoc` tool build step and `rustc` during compile steps. - [rust-lang#126934] added `-Zon-broken-pipe=kill` for tool builds *except* for cargo to help with some miri tests because at the time the PR was written, this would lead to a couple of cargo test failures. Conditionally setting `RUSTFLAGS` can lead to tool build invalidation, e.g. building `cargo` without `-Zon-broken-pipe=kill` but `clippy` with the flag can lead to invalidation of the tool build cache. This is not a problem at the time, because nothing (not even miri) tests built stage 1 cargo (all used initial cargo). - In [rust-lang#130634] we found out that `run-make` tests like `compiler-builtins` needed stage 1 cargo, not just beta bootstrap cargo, because there can be changes that are present in stage 1 cargo but absent in beta cargo, which was blocking a beta backport. - [rust-lang#130642] and later [rust-lang#130739] now build stage 1 cargo. And as previously mentioned, since `-Zon-broken-pipe=kill` was specifically *not* set for cargo, this caused tool build cache invalidation meaning rebuilds of stage 1 even if nothing in source was changed due to differing `RUSTFLAGS` since `run-make` also builds `rustdoc` and such [rust-lang#130980]. [rust-lang#34376]: rust-lang#34376 [rust-lang#49606]: rust-lang#49606 [rust-lang#97889]: rust-lang#97889 [rust-lang#102587]: rust-lang#102587 [rust-lang#103495]: rust-lang#103495 [rust-lang#124480]: rust-lang#124480 [rust-lang#130634]: rust-lang#130634 [rust-lang#130642]: rust-lang#130642 [rust-lang#130739]: rust-lang#130739 [rust-lang#130980]: rust-lang#130980 [rust-lang#131059]: rust-lang#131059 [^1]: rust-lang#131059 (comment) r? `@onur-ozkan` (or bootstrap)

…r-ozkan Revert rust-lang#131060 "Drop conditionally applied cargo `-Zon-broken-pipe=kill` flags" In [rust-lang#131059] we found out that `-Zon-broken-pipe=kill` is actually **load-bearing**[^1] for (at least) `rustc` and `rustdoc` to have the kill-process-on-broken-pipe behavior, e.g. `rustc --print=sysroot | false` will ICE and `rustdoc --print=sysroot | false` will panic on a broken pipe. This PR reverts 5a7058c (reverts PR rust-lang#131060) in favor of a future fix to *unconditionally* apply `-Zon-broken-pipe=kill` to tool builds and also not drop the `-Zon-broken-pipe=kill` flag for rustc binary builds. I could not figure out how to write a regression test for the `rustc --print=sysroot | false` behavior on Unix, so this is a plain revert for now. This revert will unfortunately reintroduce rust-lang#130980 until we fix it again with the different approach. See more details at <rust-lang#131059 (comment)> and in the timeline below. ### Timeline of kill-process-on-broken-pipe behavior changes See [`unix_sigpipe` tracking issue rust-lang#97889][rust-lang#97889] for more context around unix sigpipe handling. - From the very beginning since 2014, Rust binaries by default use `sig_ign`. This meant that if output pipe is broken yet the program tries to use `println!` and such, there will be a broken pipe panic from std. This lead to ICEs from e.g. `rustc --help | false` [rust-lang#34376]. - [rust-lang#49606] mitigated [rust-lang#34376] by adding an explicit signal handler to `rustc_driver` register a sigpipe handler with `SIG_DFL` which will cause the binary using `rustc_driver` to terminate if `rustc_driver::set_sigpipe_handler()` is called. `rustc`'s main binary wrapper uses `rustc_driver::set_sigpipe_handler()`, and so does `rustdoc`. - A more universal way to set sigpipe behavior for Unix was introduced as part of [rust-lang#97889], i.e. `# [unix_sigpipe = "sig_dfl"]` attribute. - [rust-lang#102587] migrated `rustc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. - [rust-lang#103495] migrated `rustdoc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. `rustc_driver::set_sigpipe_handler` was removed. - Following concerns about sigpipe setting UI in [rust-lang#97889], the UI for specifying sigpipe behavior was changed in [rust-lang#124480] from `#[unix_sigpipe = "sig_dfl"]` attribute to the commmand line flag `-Zon-broken-pipe=kill`. - In the same PR, `#[unix_sigpipe = "sig_dfl"]` were removed from `rustc` and `rustdoc` main binary crate entry points in favor of the command line flag. Kill-process-on-broken-pipe behavior was preserved by adding `-Zon-broken-pipe=kill` for `rustdoc` tool build step and `rustc` during compile steps. - [rust-lang#126934] added `-Zon-broken-pipe=kill` for tool builds *except* for cargo to help with some miri tests because at the time the PR was written, this would lead to a couple of cargo test failures. Conditionally setting `RUSTFLAGS` can lead to tool build invalidation, e.g. building `cargo` without `-Zon-broken-pipe=kill` but `clippy` with the flag can lead to invalidation of the tool build cache. This is not a problem at the time, because nothing (not even miri) tests built stage 1 cargo (all used initial cargo). - In [rust-lang#130634] we found out that `run-make` tests like `compiler-builtins` needed stage 1 cargo, not just beta bootstrap cargo, because there can be changes that are present in stage 1 cargo but absent in beta cargo, which was blocking a beta backport. - [rust-lang#130642] and later [rust-lang#130739] now build stage 1 cargo. And as previously mentioned, since `-Zon-broken-pipe=kill` was specifically *not* set for cargo, this caused tool build cache invalidation meaning rebuilds of stage 1 even if nothing in source was changed due to differing `RUSTFLAGS` since `run-make` also builds `rustdoc` and such [rust-lang#130980]. [rust-lang#34376]: rust-lang#34376 [rust-lang#49606]: rust-lang#49606 [rust-lang#97889]: rust-lang#97889 [rust-lang#102587]: rust-lang#102587 [rust-lang#103495]: rust-lang#103495 [rust-lang#124480]: rust-lang#124480 [rust-lang#130634]: rust-lang#130634 [rust-lang#130642]: rust-lang#130642 [rust-lang#130739]: rust-lang#130739 [rust-lang#130980]: rust-lang#130980 [rust-lang#131059]: rust-lang#131059 [^1]: rust-lang#131059 (comment) r? ``@onur-ozkan`` (or bootstrap)

Rollup merge of rust-lang#131108 - jieyouxu:revert-broken-pipe, r=onur-ozkan Revert rust-lang#131060 "Drop conditionally applied cargo `-Zon-broken-pipe=kill` flags" In [rust-lang#131059] we found out that `-Zon-broken-pipe=kill` is actually **load-bearing**[^1] for (at least) `rustc` and `rustdoc` to have the kill-process-on-broken-pipe behavior, e.g. `rustc --print=sysroot | false` will ICE and `rustdoc --print=sysroot | false` will panic on a broken pipe. This PR reverts 5a7058c (reverts PR rust-lang#131060) in favor of a future fix to *unconditionally* apply `-Zon-broken-pipe=kill` to tool builds and also not drop the `-Zon-broken-pipe=kill` flag for rustc binary builds. I could not figure out how to write a regression test for the `rustc --print=sysroot | false` behavior on Unix, so this is a plain revert for now. This revert will unfortunately reintroduce rust-lang#130980 until we fix it again with the different approach. See more details at <rust-lang#131059 (comment)> and in the timeline below. ### Timeline of kill-process-on-broken-pipe behavior changes See [`unix_sigpipe` tracking issue rust-lang#97889][rust-lang#97889] for more context around unix sigpipe handling. - From the very beginning since 2014, Rust binaries by default use `sig_ign`. This meant that if output pipe is broken yet the program tries to use `println!` and such, there will be a broken pipe panic from std. This lead to ICEs from e.g. `rustc --help | false` [rust-lang#34376]. - [rust-lang#49606] mitigated [rust-lang#34376] by adding an explicit signal handler to `rustc_driver` register a sigpipe handler with `SIG_DFL` which will cause the binary using `rustc_driver` to terminate if `rustc_driver::set_sigpipe_handler()` is called. `rustc`'s main binary wrapper uses `rustc_driver::set_sigpipe_handler()`, and so does `rustdoc`. - A more universal way to set sigpipe behavior for Unix was introduced as part of [rust-lang#97889], i.e. `# [unix_sigpipe = "sig_dfl"]` attribute. - [rust-lang#102587] migrated `rustc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. - [rust-lang#103495] migrated `rustdoc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. `rustc_driver::set_sigpipe_handler` was removed. - Following concerns about sigpipe setting UI in [rust-lang#97889], the UI for specifying sigpipe behavior was changed in [rust-lang#124480] from `#[unix_sigpipe = "sig_dfl"]` attribute to the commmand line flag `-Zon-broken-pipe=kill`. - In the same PR, `#[unix_sigpipe = "sig_dfl"]` were removed from `rustc` and `rustdoc` main binary crate entry points in favor of the command line flag. Kill-process-on-broken-pipe behavior was preserved by adding `-Zon-broken-pipe=kill` for `rustdoc` tool build step and `rustc` during compile steps. - [rust-lang#126934] added `-Zon-broken-pipe=kill` for tool builds *except* for cargo to help with some miri tests because at the time the PR was written, this would lead to a couple of cargo test failures. Conditionally setting `RUSTFLAGS` can lead to tool build invalidation, e.g. building `cargo` without `-Zon-broken-pipe=kill` but `clippy` with the flag can lead to invalidation of the tool build cache. This is not a problem at the time, because nothing (not even miri) tests built stage 1 cargo (all used initial cargo). - In [rust-lang#130634] we found out that `run-make` tests like `compiler-builtins` needed stage 1 cargo, not just beta bootstrap cargo, because there can be changes that are present in stage 1 cargo but absent in beta cargo, which was blocking a beta backport. - [rust-lang#130642] and later [rust-lang#130739] now build stage 1 cargo. And as previously mentioned, since `-Zon-broken-pipe=kill` was specifically *not* set for cargo, this caused tool build cache invalidation meaning rebuilds of stage 1 even if nothing in source was changed due to differing `RUSTFLAGS` since `run-make` also builds `rustdoc` and such [rust-lang#130980]. [rust-lang#34376]: rust-lang#34376 [rust-lang#49606]: rust-lang#49606 [rust-lang#97889]: rust-lang#97889 [rust-lang#102587]: rust-lang#102587 [rust-lang#103495]: rust-lang#103495 [rust-lang#124480]: rust-lang#124480 [rust-lang#130634]: rust-lang#130634 [rust-lang#130642]: rust-lang#130642 [rust-lang#130739]: rust-lang#130739 [rust-lang#130980]: rust-lang#130980 [rust-lang#131059]: rust-lang#131059 [^1]: rust-lang#131059 (comment) r? ``@onur-ozkan`` (or bootstrap)

Revert #131060 "Drop conditionally applied cargo `-Zon-broken-pipe=kill` flags" In [#131059] we found out that `-Zon-broken-pipe=kill` is actually **load-bearing**[^1] for (at least) `rustc` and `rustdoc` to have the kill-process-on-broken-pipe behavior, e.g. `rustc --print=sysroot | false` will ICE and `rustdoc --print=sysroot | false` will panic on a broken pipe. This PR reverts 5a7058c5a542ec42d1fa9b524f7b4f7d6845d1e9 (reverts PR #131060) in favor of a future fix to *unconditionally* apply `-Zon-broken-pipe=kill` to tool builds and also not drop the `-Zon-broken-pipe=kill` flag for rustc binary builds. I could not figure out how to write a regression test for the `rustc --print=sysroot | false` behavior on Unix, so this is a plain revert for now. This revert will unfortunately reintroduce #130980 until we fix it again with the different approach. See more details at <rust-lang/rust#131059 (comment)> and in the timeline below. ### Timeline of kill-process-on-broken-pipe behavior changes See [`unix_sigpipe` tracking issue #97889][#97889] for more context around unix sigpipe handling. - From the very beginning since 2014, Rust binaries by default use `sig_ign`. This meant that if output pipe is broken yet the program tries to use `println!` and such, there will be a broken pipe panic from std. This lead to ICEs from e.g. `rustc --help | false` [#34376]. - [#49606] mitigated [#34376] by adding an explicit signal handler to `rustc_driver` register a sigpipe handler with `SIG_DFL` which will cause the binary using `rustc_driver` to terminate if `rustc_driver::set_sigpipe_handler()` is called. `rustc`'s main binary wrapper uses `rustc_driver::set_sigpipe_handler()`, and so does `rustdoc`. - A more universal way to set sigpipe behavior for Unix was introduced as part of [#97889], i.e. `# [unix_sigpipe = "sig_dfl"]` attribute. - [#102587] migrated `rustc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. - [#103495] migrated `rustdoc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. `rustc_driver::set_sigpipe_handler` was removed. - Following concerns about sigpipe setting UI in [#97889], the UI for specifying sigpipe behavior was changed in [#124480] from `#[unix_sigpipe = "sig_dfl"]` attribute to the commmand line flag `-Zon-broken-pipe=kill`. - In the same PR, `#[unix_sigpipe = "sig_dfl"]` were removed from `rustc` and `rustdoc` main binary crate entry points in favor of the command line flag. Kill-process-on-broken-pipe behavior was preserved by adding `-Zon-broken-pipe=kill` for `rustdoc` tool build step and `rustc` during compile steps. - [#126934] added `-Zon-broken-pipe=kill` for tool builds *except* for cargo to help with some miri tests because at the time the PR was written, this would lead to a couple of cargo test failures. Conditionally setting `RUSTFLAGS` can lead to tool build invalidation, e.g. building `cargo` without `-Zon-broken-pipe=kill` but `clippy` with the flag can lead to invalidation of the tool build cache. This is not a problem at the time, because nothing (not even miri) tests built stage 1 cargo (all used initial cargo). - In [#130634] we found out that `run-make` tests like `compiler-builtins` needed stage 1 cargo, not just beta bootstrap cargo, because there can be changes that are present in stage 1 cargo but absent in beta cargo, which was blocking a beta backport. - [#130642] and later [#130739] now build stage 1 cargo. And as previously mentioned, since `-Zon-broken-pipe=kill` was specifically *not* set for cargo, this caused tool build cache invalidation meaning rebuilds of stage 1 even if nothing in source was changed due to differing `RUSTFLAGS` since `run-make` also builds `rustdoc` and such [#130980]. [#34376]: rust-lang/rust#34376 [#49606]: rust-lang/rust#49606 [#97889]: rust-lang/rust#97889 [#102587]: rust-lang/rust#102587 [#103495]: rust-lang/rust#103495 [#124480]: rust-lang/rust#124480 [#130634]: rust-lang/rust#130634 [#130642]: rust-lang/rust#130642 [#130739]: rust-lang/rust#130739 [#130980]: rust-lang/rust#130980 [#131059]: rust-lang/rust#131059 [^1]: rust-lang/rust#131059 (comment) r? ``@onur-ozkan`` (or bootstrap)

rust-highfive assigned fee1-dead Oct 2, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 2, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 2, 2022

rust-highfive assigned jackh726 and unassigned fee1-dead Oct 2, 2022

Enselic mentioned this pull request Oct 2, 2022

Tracking Issue for unix_sigpipe #97889

Open

23 tasks

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 24, 2022

Dylan-DPC mentioned this pull request Oct 24, 2022

Rollup of 7 pull requests #103461

Closed

Enselic mentioned this pull request Oct 24, 2022

rustdoc: Use unix_sigpipe instead of rustc_driver::set_sigpipe_handler #103495

Merged

Dylan-DPC mentioned this pull request Oct 25, 2022

Rollup of 6 pull requests #103510

Closed

Dylan-DPC mentioned this pull request Oct 25, 2022

Rollup of 6 pull requests #103512

Closed

Dylan-DPC mentioned this pull request Oct 25, 2022

Rollup of 6 pull requests #103513

Merged

bors merged commit 8a5ce1d into rust-lang:master Oct 25, 2022

rustbot added this to the 1.66.0 milestone Oct 25, 2022

Enselic mentioned this pull request Oct 25, 2022

Remove rustc_driver::set_sigpipe_handler() #103536

Merged

Enselic deleted the rustc-unix_sigpipe branch January 9, 2023 08:59

This was referenced Oct 1, 2024

Adjusting -Zon-broken-pipe=kill to only be applied to rustc/rustdoc binary wrappers break two cargo tests #131059

Closed

Revert #131060 "Drop conditionally applied cargo -Zon-broken-pipe=kill flags" #131108

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` #102587

rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` #102587

Enselic commented Oct 2, 2022 •

edited

Loading

rust-highfive commented Oct 2, 2022

fee1-dead commented Oct 2, 2022

joshtriplett commented Oct 14, 2022 •

edited

Loading

Enselic commented Oct 14, 2022 •

edited

Loading

joshtriplett commented Oct 14, 2022

jackh726 commented Oct 24, 2022

bors commented Oct 24, 2022

rustc: Use unix_sigpipe instead of rustc_driver::set_sigpipe_handler #102587

rustc: Use unix_sigpipe instead of rustc_driver::set_sigpipe_handler #102587

Conversation

Enselic commented Oct 2, 2022 • edited Loading

rust-highfive commented Oct 2, 2022

fee1-dead commented Oct 2, 2022

joshtriplett commented Oct 14, 2022 • edited Loading

Enselic commented Oct 14, 2022 • edited Loading

joshtriplett commented Oct 14, 2022

jackh726 commented Oct 24, 2022

bors commented Oct 24, 2022

rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` #102587

rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` #102587

Enselic commented Oct 2, 2022 •

edited

Loading

joshtriplett commented Oct 14, 2022 •

edited

Loading

Enselic commented Oct 14, 2022 •

edited

Loading