Add output on failure to BootstrapCommand

Author: Kobzol

src/bootstrap/exec.rs

@@ -1,21 +1,34 @@
 use std::process::Command;
 
+/// What should be reported when the command fails.
+#[derive(Debug, Copy, Clone)]
+pub enum OutputOnFailure {
+    /// Status code
+    Succint,
+    /// Status code together with stdout and stderr
+    Verbose,
+}
+
 /// Wrapper around `std::process::Command`.
 #[derive(Debug)]
 pub struct BootstrapCommand<'a> {
     pub command: &'a mut Command,
     /// Report failure later instead of immediately.
     pub delay_failure: bool,
+    pub output_on_failure: Option<OutputOnFailure>,
 }
 
 impl<'a> BootstrapCommand<'a> {
     pub fn delay_failure(self) -> Self {
         Self { delay_failure: true, ..self }
     }
+    pub fn output_on_failure(self, output_on_failure: OutputOnFailure) -> Self {
+        Self { output_on_failure: Some(output_on_failure), ..self }
+    }
 }
 
 impl<'a> From<&'a mut Command> for BootstrapCommand<'a> {
     fn from(command: &'a mut Command) -> Self {
-        Self { command, delay_failure: false }
+        Self { command, delay_failure: false, output_on_failure: None }
     }
 }

src/bootstrap/lib.rs

@@ -28,16 +28,15 @@ use std::str;
 
 use build_helper::ci::{gha, CiEnv};
 use build_helper::exit;
+use build_helper::util::fail;
 use channel::GitInfo;
 use config::{DryRun, Target};
 use filetime::FileTime;
 use once_cell::sync::OnceCell;
 
 use crate::builder::Kind;
 use crate::config::{LlvmLibunwind, TargetSelection};
-use crate::util::{
-    dir_is_empty, exe, libdir, mtime, output, run, run_suppressed, symlink_dir, try_run_suppressed,
-};
+use crate::util::{dir_is_empty, exe, libdir, mtime, output, run, symlink_dir};
 
 mod builder;
 mod cache;
@@ -91,7 +90,7 @@ mod job {
 pub use crate::builder::PathSet;
 use crate::cache::{Interned, INTERNER};
 pub use crate::config::Config;
-use crate::exec::BootstrapCommand;
+use crate::exec::{BootstrapCommand, OutputOnFailure};
 pub use crate::flags::Subcommand;
 use termcolor::{ColorChoice, StandardStream, WriteColor};
 
@@ -976,31 +975,17 @@ impl Build {
 
     /// Runs a command, printing out nice contextual information if it fails.
     fn run_quiet(&self, cmd: &mut Command) {
-        if self.config.dry_run() {
-            return;
-        }
-        self.verbose(&format!("running: {cmd:?}"));
-        run_suppressed(cmd)
+        let cmd: BootstrapCommand<'_> = cmd.into();
+        // TODO: fail bootstrap immediately if this command fails
+        self.run_cmd(cmd.output_on_failure(OutputOnFailure::Verbose));
     }
 
     /// Runs a command, printing out nice contextual information if it fails.
     /// Exits if the command failed to execute at all, otherwise returns its
     /// `status.success()`.
     fn run_quiet_delaying_failure(&self, cmd: &mut Command) -> bool {
-        if self.config.dry_run() {
-            return true;
-        }
-        if !self.fail_fast {
-            self.verbose(&format!("running: {cmd:?}"));
-            if !try_run_suppressed(cmd) {
-                let mut failures = self.delayed_failures.borrow_mut();
-                failures.push(format!("{cmd:?}"));
-                return false;
-            }
-        } else {
-            self.run_quiet(cmd);
-        }
-        true
+        let cmd: BootstrapCommand<'_> = cmd.into();
+        self.run_cmd(cmd.output_on_failure(OutputOnFailure::Verbose).delay_failure())
     }
 
     /// Runs a command, printing out contextual info if it fails, and delaying errors until the build finishes.
@@ -1014,8 +999,43 @@ impl Build {
         let command = cmd.into();
         self.verbose(&format!("running: {command:?}"));
 
-        #[allow(deprecated)] // can't use Build::try_run, that's us
-        let result = self.config.try_run(command.command);
+        let output = match command.command.output() {
+            Ok(output) => output,
+            Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}", command, e)),
+        };
+        let result = if !output.status.success() {
+            let output_on_failure = match command.output_on_failure {
+                Some(output) => Some(output),
+                None if self.is_verbose() => Some(OutputOnFailure::Succint),
+                None => None,
+            };
+
+            if let Some(output_on_failure) = output_on_failure {
+                use std::fmt::Write;
+
+                let mut message = String::new();
+                writeln!(
+                    message,
+                    "\n\ncommand did not execute successfully: {:?}\n\
+                 expected success, got: {}\n\n",
+                    command.command, output.status
+                )
+                .unwrap();
+                if let OutputOnFailure::Verbose = output_on_failure {
+                    writeln!(
+                        message,
+                        "stdout ----\n{}\n\
+             stderr ----\n{}\n\n",
+                        String::from_utf8_lossy(&output.stdout),
+                        String::from_utf8_lossy(&output.stderr)
+                    )
+                    .unwrap();
+                }
+            }
+            Err(())
+        } else {
+            Ok(())
+        };
 
         match result {
             Ok(_) => true,

src/bootstrap/util.rs

@@ -239,32 +239,6 @@ pub fn check_run(cmd: &mut Command, print_cmd_on_fail: bool) -> bool {
     status.success()
 }
 
-pub fn run_suppressed(cmd: &mut Command) {
-    if !try_run_suppressed(cmd) {
-        crate::exit!(1);
-    }
-}
-
-pub fn try_run_suppressed(cmd: &mut Command) -> bool {
-    let output = match cmd.output() {
-        Ok(status) => status,
-        Err(e) => fail(&format!("failed to execute command: {cmd:?}\nerror: {e}")),
-    };
-    if !output.status.success() {
-        println!(
-            "\n\ncommand did not execute successfully: {:?}\n\
-             expected success, got: {}\n\n\
-             stdout ----\n{}\n\
-             stderr ----\n{}\n\n",
-            cmd,
-            output.status,
-            String::from_utf8_lossy(&output.stdout),
-            String::from_utf8_lossy(&output.stderr)
-        );
-    }
-    output.status.success()
-}
-
 pub fn make(host: &str) -> PathBuf {
     if host.contains("dragonfly")
         || host.contains("freebsd")