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.

3 files changed, 42 insertions, 6 deletions

diff --git a/src/bootstrap/src/bin/rustc.rs b/src/bootstrap/src/bin/rustc.rs
index 780979ed981..6f7ccde5e5b 100644
--- a/src/bootstrap/src/bin/rustc.rs
+++ b/src/bootstrap/src/bin/rustc.rs
@@ -136,6 +136,14 @@ fn main() {
         cmd.args(lint_flags.split_whitespace());
     }
 
+    // Conditionally pass `-Zon-broken-pipe=kill` to rustc bin shim when this shim is *not* used to
+    // build cargo itself, i.e. set `-Zon-broken-pipe=kill` only when building non-cargo tools.
+    //
+    // See <https://github.com/rust-lang/rust/issues/131059> for more context.
+    if env::var_os("FORCE_ON_BROKEN_PIPE_KILL").is_some() {
+        cmd.arg("-Z").arg("on-broken-pipe=kill");
+    }
+
     if target.is_some() {
         // The stage0 compiler has a special sysroot distinct from what we
         // actually downloaded, so we just always pass the `--sysroot` option,
diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs
index b5be7d841dd..27bbc8bd8ff 100644
--- a/src/bootstrap/src/core/build_steps/compile.rs
+++ b/src/bootstrap/src/core/build_steps/compile.rs
@@ -1053,8 +1053,19 @@ pub fn rustc_cargo(
 
     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.
+    // 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.
+    //
+    // FIXME(jieyouxu): this flag is load-bearing for rustc to not ICE on broken pipes, because
+    // rustc internally sometimes uses std `println!` -- but std `println!` by default will panic on
+    // broken pipes, and uncaught panics will manifest as an ICE. The compiler *should* handle this
+    // properly, but this flag is set in the meantime to paper over the I/O errors.
+    //
+    // See <https://github.com/rust-lang/rust/issues/131059> for details.
+    //
+    // Also see the discussion for properly handling I/O errors related to broken pipes, i.e. safe
+    // variants of `println!` in
+    // <https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Internal.20lint.20for.20raw.20.60print!.60.20and.20.60println!.60.3F>.
     cargo.rustflag("-Zon-broken-pipe=kill");
 
     if builder.config.llvm_enzyme {
diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs
index e2fcd13efe3..a01497c2bb9 100644
--- a/src/bootstrap/src/core/build_steps/tool.rs
+++ b/src/bootstrap/src/core/build_steps/tool.rs
@@ -209,11 +209,28 @@ pub fn prepare_tool_cargo(
     // See https://github.com/rust-lang/rust/issues/116538
     cargo.rustflag("-Zunstable-options");
 
-    // `-Zon-broken-pipe=kill` breaks cargo tests
+    // NOTE: The root cause of needing `-Zon-broken-pipe=kill` in the first place is because `rustc`
+    // and `rustdoc` doesn't gracefully handle I/O errors due to usages of raw std `println!` macros
+    // which panics upon encountering broken pipes. `-Zon-broken-pipe=kill` just papers over that
+    // and stops rustc/rustdoc ICEing on e.g. `rustc --print=sysroot | false`.
+    //
+    // cargo explicitly does not want the `-Zon-broken-pipe=kill` paper because it does actually use
+    // variants of `println!` that handles I/O errors gracefully. It's also a breaking change for a
+    // spawn process not written in Rust, especially if the language default handler is not
+    // `SIG_IGN`. Thankfully cargo tests will break if we do set the flag.
+    //
+    // For the cargo discussion, see
+    // <https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Applying.20.60-Zon-broken-pipe.3Dkill.60.20flags.20in.20bootstrap.3F>.
+    //
+    // For the rustc discussion, see
+    // <https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Internal.20lint.20for.20raw.20.60print!.60.20and.20.60println!.60.3F>
+    // for proper solutions.
     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");
+        // Use an untracked env var `FORCE_ON_BROKEN_PIPE_KILL` here instead of `RUSTFLAGS`.
+        // `RUSTFLAGS` is tracked by cargo. Conditionally omitting `-Zon-broken-pipe=kill` from
+        // `RUSTFLAGS` causes unnecessary tool rebuilds due to cache invalidation from building e.g.
+        // cargo *without* `-Zon-broken-pipe=kill` but then rustdoc *with* `-Zon-broken-pipe=kill`.
+        cargo.env("FORCE_ON_BROKEN_PIPE_KILL", "-Zon-broken-pipe=kill");
     }
 
     cargo