Rollup merge of #135035 - Noratrieb:fixfmt, r=jieyouxu

Fix formatting command

The formatting command previously had two issues:
- if rustfmt failed, it would print the command invocation. this is unnecessarily noisy
- there was a race condition that lead to orphan rustfmts that would print their output after bootstrap exited

We fix this by
- removing the printing, it's not really useful
- threading failure through properly instead of just yoloing exit(1)

src/bootstrap/src/core/build_steps/format.rs

@@ -14,7 +14,19 @@ use crate::core::builder::Builder;
 use crate::utils::exec::command;
 use crate::utils::helpers::{self, program_out_of_date, t};
 
-fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl FnMut(bool) -> bool {
+#[must_use]
+enum RustfmtStatus {
+    InProgress,
+    Ok,
+    Failed,
+}
+
+fn rustfmt(
+    src: &Path,
+    rustfmt: &Path,
+    paths: &[PathBuf],
+    check: bool,
+) -> impl FnMut(bool) -> RustfmtStatus {
     let mut cmd = Command::new(rustfmt);
     // Avoid the submodule config paths from coming into play. We only allow a single global config
     // for the workspace for now.
@@ -26,30 +38,20 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F
         cmd.arg("--check");
     }
     cmd.args(paths);
-    let cmd_debug = format!("{cmd:?}");
     let mut cmd = cmd.spawn().expect("running rustfmt");
     // Poor man's async: return a closure that might wait for rustfmt's completion (depending on
     // the value of the `block` argument).
-    move |block: bool| -> bool {
+    move |block: bool| -> RustfmtStatus {
         let status = if !block {
             match cmd.try_wait() {
                 Ok(Some(status)) => Ok(status),
-                Ok(None) => return false,
+                Ok(None) => return RustfmtStatus::InProgress,
                 Err(err) => Err(err),
             }
         } else {
             cmd.wait()
         };
-        if !status.unwrap().success() {
+        if status.unwrap().success() { RustfmtStatus::Ok } else { RustfmtStatus::Failed }
-            eprintln!(
-                "fmt error: Running `{}` failed.\nIf you're running `tidy`, \
-                try again with `--bless`. Or, if you just want to format \
-                code, run `./x.py fmt` instead.",
-                cmd_debug,
-            );
-            crate::exit!(1);
-        }
-        true
     }
 }
 
@@ -240,6 +242,8 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
     // Spawn child processes on a separate thread so we can batch entries we have received from
     // ignore.
     let thread = std::thread::spawn(move || {
+        let mut result = Ok(());
+
         let mut children = VecDeque::new();
         while let Ok(path) = rx.recv() {
             // Try getting more paths from the channel to amortize the overhead of spawning
@@ -251,22 +255,38 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
 
             // Poll completion before waiting.
             for i in (0..children.len()).rev() {
-                if children[i](false) {
+                match children[i](false) {
+                    RustfmtStatus::InProgress => {}
+                    RustfmtStatus::Failed => {
+                        result = Err(());
                         children.swap_remove_back(i);
                         break;
                     }
+                    RustfmtStatus::Ok => {
+                        children.swap_remove_back(i);
+                        break;
+                    }
+                }
             }
 
             if children.len() >= max_processes {
                 // Await oldest child.
-                children.pop_front().unwrap()(true);
+                match children.pop_front().unwrap()(true) {
+                    RustfmtStatus::InProgress | RustfmtStatus::Ok => {}
+                    RustfmtStatus::Failed => result = Err(()),
+                }
             }
         }
 
         // Await remaining children.
         for mut child in children {
-            child(true);
+            match child(true) {
+                RustfmtStatus::InProgress | RustfmtStatus::Ok => {}
+                RustfmtStatus::Failed => result = Err(()),
             }
+        }
+
+        result
     });
 
     let formatted_paths = Mutex::new(Vec::new());
@@ -299,7 +319,12 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
 
     drop(tx);
 
-    thread.join().unwrap();
+    let result = thread.join().unwrap();
+
+    if result.is_err() {
+        crate::exit!(1);
+    }
+
     if !check {
         update_rustfmt_version(build);
     }