mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-29 02:03:53 +00:00
Rollup merge of #131108 - jieyouxu:revert-broken-pipe, r=onur-ozkan
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 5a7058c5a5
(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 <https://github.com/rust-lang/rust/issues/131059#issuecomment-2385822033> 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]: https://github.com/rust-lang/rust/issues/34376
[#49606]: https://github.com/rust-lang/rust/pull/49606
[#97889]: https://github.com/rust-lang/rust/issues/97889
[#102587]: https://github.com/rust-lang/rust/pull/102587
[#103495]: https://github.com/rust-lang/rust/pull/103495
[#124480]: https://github.com/rust-lang/rust/pull/124480
[#130634]: https://github.com/rust-lang/rust/issues/130634
[#130642]: https://github.com/rust-lang/rust/pull/130642
[#130739]: https://github.com/rust-lang/rust/pull/130739
[#130980]: https://github.com/rust-lang/rust/issues/130980
[#131059]: https://github.com/rust-lang/rust/issues/131059
[^1]: https://github.com/rust-lang/rust/issues/131059#issuecomment-2385822033
r? ``@onur-ozkan`` (or bootstrap)
This commit is contained in:
commit
42e3ff281a
@ -1053,6 +1053,10 @@ pub fn rustc_cargo(
|
|||||||
|
|
||||||
cargo.rustdocflag("-Zcrate-attr=warn(rust_2018_idioms)");
|
cargo.rustdocflag("-Zcrate-attr=warn(rust_2018_idioms)");
|
||||||
|
|
||||||
|
// If the rustc output is piped to e.g. `head -n1` we want the process to be
|
||||||
|
// killed, rather than having an error bubble up and cause a panic.
|
||||||
|
cargo.rustflag("-Zon-broken-pipe=kill");
|
||||||
|
|
||||||
if builder.config.llvm_enzyme {
|
if builder.config.llvm_enzyme {
|
||||||
cargo.rustflag("-l").rustflag("Enzyme-19");
|
cargo.rustflag("-l").rustflag("Enzyme-19");
|
||||||
}
|
}
|
||||||
|
@ -200,10 +200,6 @@ pub fn prepare_tool_cargo(
|
|||||||
cargo.arg("--features").arg(features.join(", "));
|
cargo.arg("--features").arg(features.join(", "));
|
||||||
}
|
}
|
||||||
|
|
||||||
// Warning: be very careful with RUSTFLAGS or command-line options, as conditionally applied
|
|
||||||
// RUSTFLAGS or cli flags can lead to hard-to-diagnose rebuilds due to flag differences, causing
|
|
||||||
// previous tool build artifacts to get invalidated.
|
|
||||||
|
|
||||||
// Enable internal lints for clippy and rustdoc
|
// Enable internal lints for clippy and rustdoc
|
||||||
// NOTE: this doesn't enable lints for any other tools unless they explicitly add `#![warn(rustc::internal)]`
|
// NOTE: this doesn't enable lints for any other tools unless they explicitly add `#![warn(rustc::internal)]`
|
||||||
// See https://github.com/rust-lang/rust/pull/80573#issuecomment-754010776
|
// See https://github.com/rust-lang/rust/pull/80573#issuecomment-754010776
|
||||||
@ -213,6 +209,13 @@ pub fn prepare_tool_cargo(
|
|||||||
// See https://github.com/rust-lang/rust/issues/116538
|
// See https://github.com/rust-lang/rust/issues/116538
|
||||||
cargo.rustflag("-Zunstable-options");
|
cargo.rustflag("-Zunstable-options");
|
||||||
|
|
||||||
|
// `-Zon-broken-pipe=kill` breaks cargo tests
|
||||||
|
if !path.ends_with("cargo") {
|
||||||
|
// If the output is piped to e.g. `head -n1` we want the process to be killed,
|
||||||
|
// rather than having an error bubble up and cause a panic.
|
||||||
|
cargo.rustflag("-Zon-broken-pipe=kill");
|
||||||
|
}
|
||||||
|
|
||||||
cargo
|
cargo
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user