Skip to content

Commit 02c6e57

Browse files
committed
Use untracked env var to pass -Zon-broken-pipe=kill for tools
- Don't touch rustc's `-Zon-broken-pipe=kill` env var in `compile.rs`. - Use an untracked env var to pass `-Zon-broken-pipe=kill` for tools but skip cargo still, because cargo wants `-Zon-broken-pipe=kill` unset.
1 parent 2305aad commit 02c6e57

File tree

3 files changed

+37
-6
lines changed

3 files changed

+37
-6
lines changed

src/bootstrap/src/bin/rustc.rs

+4
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ fn main() {
137137
cmd.args(lint_flags.split_whitespace());
138138
}
139139

140+
if let Ok(flags) = env::var("UNTRACKED_BROKEN_PIPE_FLAG") {
141+
cmd.args(flags.split_whitespace());
142+
}
143+
140144
if target.is_some() {
141145
// The stage0 compiler has a special sysroot distinct from what we
142146
// actually downloaded, so we just always pass the `--sysroot` option,

src/bootstrap/src/core/build_steps/compile.rs

+13-2
Original file line numberDiff line numberDiff line change
@@ -1053,8 +1053,19 @@ pub fn rustc_cargo(
10531053

10541054
cargo.rustdocflag("-Zcrate-attr=warn(rust_2018_idioms)");
10551055

1056-
// If the rustc output is piped to e.g. `head -n1` we want the process to be
1057-
// killed, rather than having an error bubble up and cause a panic.
1056+
// If the rustc output is piped to e.g. `head -n1` we want the process to be killed, rather than
1057+
// having an error bubble up and cause a panic.
1058+
//
1059+
// FIXME(jieyouxu): this flag is load-bearing for rustc to not ICE on broken pipes, because
1060+
// rustc internally sometimes uses std `println!` -- but std `println!` by default will panic on
1061+
// broken pipes, and uncaught panics will manifest as an ICE. The compiler *should* handle this
1062+
// properly, but this flag is set in the meantime to paper over the I/O errors.
1063+
//
1064+
// See <https://github.com/rust-lang/rust/issues/131059> for details.
1065+
//
1066+
// Also see the discussion for properly handling I/O errors related to broken pipes, i.e. safe
1067+
// variants of `println!` in
1068+
// <https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Internal.20lint.20for.20raw.20.60print!.60.20and.20.60println!.60.3F>.
10581069
cargo.rustflag("-Zon-broken-pipe=kill");
10591070

10601071
if builder.config.llvm_enzyme {

src/bootstrap/src/core/build_steps/tool.rs

+20-4
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,27 @@ pub fn prepare_tool_cargo(
209209
// See https://github.com/rust-lang/rust/issues/116538
210210
cargo.rustflag("-Zunstable-options");
211211

212-
// `-Zon-broken-pipe=kill` breaks cargo tests
212+
// NOTE: The root cause of needing `-Zon-broken-pipe=kill` in the first place is because `rustc`
213+
// and `rustdoc` doesn't gracefully handle I/O errors due to usages of raw std `println!` macros
214+
// which panics upon encountering broken pipes. `-Zon-broken-pipe=kill` just papers over that
215+
// and stops rustc/rustdoc ICEing on e.g. `rustc --print=sysroot | false`.
216+
//
217+
// cargo explicitly does not want the `-Zon-broken-pipe=kill` paper because it does actually use
218+
// variants of `println!` that handles I/O errors gracefully. It's also a breaking change for a
219+
// spawn process not written in Rust, especially if the language default handler is not
220+
// `SIG_IGN`. Thankfully cargo tests will break if we do set the flag. We use a special env var
221+
// here that is not part of `RUSTFLAGS` tracked by cargo as that will cause unnecessary tool
222+
// rebuilds due to tool build cache invalidation from building e.g. cargo *without*
223+
// `-Zon-broken-pipe=kill` but then rustdoc *with* `-Zon-broken-pipe=kill`.
224+
//
225+
// For the cargo discussion, see
226+
// <https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Applying.20.60-Zon-broken-pipe.3Dkill.60.20flags.20in.20bootstrap.3F>.
227+
//
228+
// For the rustc discussion, see
229+
// <https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Internal.20lint.20for.20raw.20.60print!.60.20and.20.60println!.60.3F>
230+
// for proper solutions.
213231
if !path.ends_with("cargo") {
214-
// If the output is piped to e.g. `head -n1` we want the process to be killed,
215-
// rather than having an error bubble up and cause a panic.
216-
cargo.rustflag("-Zon-broken-pipe=kill");
232+
cargo.env("UNTRACKED_BROKEN_PIPE_FLAG", "-Zon-broken-pipe=kill");
217233
}
218234

219235
cargo

0 commit comments

Comments
 (0)