Add output mode to BootstrapCommand

src/bootstrap/src/lib.rs

@@ -23,11 +23,12 @@ use std::fmt::Display;
 use std::fs::{self, File};
 use std::io;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
+use std::process::{Command, Output, Stdio};
 use std::str;
 
 use build_helper::ci::{gha, CiEnv};
 use build_helper::exit;
+use build_helper::util::fail;
 use filetime::FileTime;
 use once_cell::sync::OnceCell;
 use termcolor::{ColorChoice, StandardStream, WriteColor};
@@ -39,7 +40,7 @@ use crate::core::config::flags;
 use crate::core::config::{DryRun, Target};
 use crate::core::config::{LlvmLibunwind, TargetSelection};
 use crate::utils::cache::{Interned, INTERNER};
-use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand};
+use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, OutputMode};
 use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, symlink_dir};
 
 mod core;
@@ -919,41 +920,78 @@ impl Build {
 
     /// Runs a command, printing out nice contextual information if it fails.
     fn run(&self, cmd: &mut Command) {
-        // FIXME: output mode -> status + err if self.is_verbose()
-        let cmd: BootstrapCommand<'_> = cmd.into();
-        self.run_cmd(cmd.fail_fast());
+        self.run_cmd(BootstrapCommand::from(cmd).fail_fast().output_mode(
+            match self.is_verbose() {
+                true => OutputMode::PrintAll,
+                false => OutputMode::PrintOutput,
+            },
+        ));
+    }
+
+    /// Runs a command, printing out contextual info if it fails, and delaying errors until the build finishes.
+    pub(crate) fn run_delaying_failure(&self, cmd: &mut Command) -> bool {
+        self.run_cmd(BootstrapCommand::from(cmd).delay_failure().output_mode(
+            match self.is_verbose() {
+                true => OutputMode::PrintAll,
+                false => OutputMode::PrintOutput,
+            },
+        ))
     }
 
     /// Runs a command, printing out nice contextual information if it fails.
     fn run_quiet(&self, cmd: &mut Command) {
-        // FIXME: output mode -> output + err
-        let cmd: BootstrapCommand<'_> = cmd.into();
-        self.run_cmd(cmd.fail_fast());
+        self.run_cmd(BootstrapCommand::from(cmd).fail_fast().output_mode(OutputMode::Suppress));
     }
 
     /// 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 {
-        // FIXME: output mode -> output + err
-        let cmd: BootstrapCommand<'_> = cmd.into();
-        self.run_cmd(cmd.delay_failure())
-    }
-
-    /// Runs a command, printing out contextual info if it fails, and delaying errors until the build finishes.
-    pub(crate) fn run_delaying_failure(&self, cmd: &mut Command) -> bool {
-        // FIXME: output mode -> status + err if self.is_verbose()
-        let cmd: BootstrapCommand<'_> = cmd.into();
-        self.run_cmd(cmd.delay_failure())
+        self.run_cmd(BootstrapCommand::from(cmd).delay_failure().output_mode(OutputMode::Suppress))
     }
 
     /// A centralized function for running commands that do not return output.
     pub(crate) fn run_cmd<'a, C: Into<BootstrapCommand<'a>>>(&self, cmd: C) -> bool {
+        if self.config.dry_run() {
+            return true;
+        }
+
         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, print_error) = match command.output_mode {
+            mode @ (OutputMode::PrintAll | OutputMode::PrintOutput) => (
+                command.command.status().map(|status| Output {
+                    status,
+                    stdout: Vec::new(),
+                    stderr: Vec::new(),
+                }),
+                matches!(mode, OutputMode::PrintAll),
+            ),
+            OutputMode::Suppress => (command.command.output(), true),
+        };
+
+        let output = match output {
+            Ok(output) => output,
+            Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}", command, e)),
+        };
+        let result = if !output.status.success() {
+            if print_error {
+                println!(
+                    "\n\ncommand did not execute successfully: {:?}\n\
+                    expected success, got: {}\n\n\
+                    stdout ----\n{}\n\
+                    stderr ----\n{}\n\n",
+                    command.command,
+                    output.status,
+                    String::from_utf8_lossy(&output.stdout),
+                    String::from_utf8_lossy(&output.stderr)
+                );
+            }
+            Err(())
+        } else {
+            Ok(())
+        };
 
         match result {
             Ok(_) => true,

src/bootstrap/src/utils/exec.rs

@@ -9,11 +9,24 @@ pub enum BehaviorOnFailure {
     DelayFail,
 }
 
+/// How should the output of the command be handled.
+#[derive(Debug, Copy, Clone)]
+pub enum OutputMode {
+    /// Print both the output (by inheriting stdout/stderr) and also the command itself, if it
+    /// fails.
+    PrintAll,
+    /// Print the output (by inheriting stdout/stderr).
+    PrintOutput,
+    /// Suppress the output if the command succeeds, otherwise print the output.
+    Suppress,
+}
+
 /// Wrapper around `std::process::Command`.
 #[derive(Debug)]
 pub struct BootstrapCommand<'a> {
     pub command: &'a mut Command,
     pub failure_behavior: Option<BehaviorOnFailure>,
+    pub output_mode: OutputMode,
 }
 
 impl<'a> BootstrapCommand<'a> {
@@ -23,10 +36,13 @@ impl<'a> BootstrapCommand<'a> {
     pub fn fail_fast(self) -> Self {
         Self { failure_behavior: Some(BehaviorOnFailure::Exit), ..self }
     }
+    pub fn output_mode(self, output_mode: OutputMode) -> Self {
+        Self { output_mode, ..self }
+    }
 }
 
 impl<'a> From<&'a mut Command> for BootstrapCommand<'a> {
     fn from(command: &'a mut Command) -> Self {
-        Self { command, failure_behavior: None }
+        Self { command, failure_behavior: None, output_mode: OutputMode::Suppress }
     }
 }

src/bootstrap/src/utils/helpers.rs

@@ -3,7 +3,7 @@
 //! Simple things like testing the various filesystem operations here and there,
 //! not a lot of interesting happenings here unfortunately.
 
-use build_helper::util::{fail, try_run};
+use build_helper::util::fail;
 use std::env;
 use std::fs;
 use std::io;
@@ -216,12 +216,6 @@ pub fn is_valid_test_suite_arg<'a, P: AsRef<Path>>(
     }
 }
 
-pub fn run(cmd: &mut Command, print_cmd_on_fail: bool) {
-    if try_run(cmd, print_cmd_on_fail).is_err() {
-        crate::exit!(1);
-    }
-}
-
 pub fn check_run(cmd: &mut Command, print_cmd_on_fail: bool) -> bool {
     let status = match cmd.status() {
         Ok(status) => status,
@@ -239,32 +233,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")