diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 831a86940fb48..246d742a99faa 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -27,6 +27,7 @@ use crate::core::config::flags::Subcommand; use crate::core::config::TargetSelection; use crate::utils; use crate::utils::cache::{Interned, INTERNER}; +use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::{ self, add_link_lib_path, dylib_path, dylib_path_var, output, t, up_to_date, }; @@ -808,8 +809,8 @@ impl Step for Clippy { let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host); - #[allow(deprecated)] // Clippy reports errors if it blessed the outputs - if builder.config.try_run(&mut cargo).is_ok() { + // Clippy reports errors if it blessed the outputs + if builder.run_cmd(BootstrapCommand::from(&mut cargo).allow_failure()) { // The tests succeeded; nothing to do. return; } @@ -3094,7 +3095,7 @@ impl Step for CodegenCranelift { .arg("testsuite.extended_sysroot"); cargo.args(builder.config.test_args()); - #[allow(deprecated)] - builder.config.try_run(&mut cargo.into()).unwrap(); + let mut cmd: Command = cargo.into(); + builder.run_cmd(BootstrapCommand::from(&mut cmd).fail_fast()); } } diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 5702fa62d7ccc..d5f759ea159aa 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -8,6 +8,7 @@ use crate::core::build_steps::toolstate::ToolState; use crate::core::builder::{Builder, Cargo as CargoCommand, RunConfig, ShouldRun, Step}; use crate::core::config::TargetSelection; use crate::utils::channel::GitInfo; +use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::{add_dylib_path, exe, t}; use crate::Compiler; use crate::Mode; @@ -108,8 +109,8 @@ impl Step for ToolBuild { ); let mut cargo = Command::from(cargo); - #[allow(deprecated)] // we check this in `is_optional_tool` in a second - let is_expected = builder.config.try_run(&mut cargo).is_ok(); + // we check this in `is_optional_tool` in a second + let is_expected = builder.run_cmd(BootstrapCommand::from(&mut cargo).allow_failure()); builder.save_toolstate( tool, diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 705c27bb92233..8dd1a698dfa8e 100644 --- a/src/bootstrap/src/lib.rs +++ b/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,10 +40,8 @@ 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::helpers::{ - self, dir_is_empty, exe, libdir, mtime, output, run, run_suppressed, symlink_dir, - try_run_suppressed, -}; +use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, OutputMode}; +use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, symlink_dir}; mod core; mod utils; @@ -580,15 +579,15 @@ impl Build { } // Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error). - #[allow(deprecated)] // diff-index reports the modifications through the exit status - let has_local_modifications = self - .config - .try_run( + // diff-index reports the modifications through the exit status + let has_local_modifications = !self.run_cmd( + BootstrapCommand::from( Command::new("git") .args(&["diff-index", "--quiet", "HEAD"]) .current_dir(&absolute_path), ) - .is_err(); + .allow_failure(), + ); if has_local_modifications { self.run(Command::new("git").args(&["stash", "push"]).current_dir(&absolute_path)); } @@ -921,55 +920,103 @@ impl Build { /// Runs a command, printing out nice contextual information if it fails. fn run(&self, cmd: &mut Command) { - if self.config.dry_run() { - return; - } - self.verbose(&format!("running: {cmd:?}")); - run(cmd, self.is_verbose()) + 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) { - if self.config.dry_run() { - return; - } - self.verbose(&format!("running: {cmd:?}")); - run_suppressed(cmd) + self.run_cmd( + BootstrapCommand::from(cmd).fail_fast().output_mode(OutputMode::SuppressOnSuccess), + ); } /// 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 { + self.run_cmd( + BootstrapCommand::from(cmd).delay_failure().output_mode(OutputMode::SuppressOnSuccess), + ) + } + + /// A centralized function for running commands that do not return output. + pub(crate) fn run_cmd<'a, C: Into>>(&self, cmd: C) -> 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; + + let command = cmd.into(); + self.verbose(&format!("running: {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::SuppressOnSuccess => (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 { - self.run_quiet(cmd); - } - true - } + Ok(()) + }; - /// 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 { - if !self.fail_fast { - #[allow(deprecated)] // can't use Build::try_run, that's us - if self.config.try_run(cmd).is_err() { - let mut failures = self.delayed_failures.borrow_mut(); - failures.push(format!("{cmd:?}")); - return false; + match result { + Ok(_) => true, + Err(_) => { + match command.failure_behavior { + BehaviorOnFailure::DelayFail => { + if self.fail_fast { + exit!(1); + } + + let mut failures = self.delayed_failures.borrow_mut(); + failures.push(format!("{command:?}")); + } + BehaviorOnFailure::Exit => { + exit!(1); + } + BehaviorOnFailure::Ignore => {} + } + false } - } else { - self.run(cmd); } - true } pub fn is_verbose_than(&self, level: usize) -> bool { diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs new file mode 100644 index 0000000000000..9c1c21cd95894 --- /dev/null +++ b/src/bootstrap/src/utils/exec.rs @@ -0,0 +1,60 @@ +use std::process::Command; + +/// What should be done when the command fails. +#[derive(Debug, Copy, Clone)] +pub enum BehaviorOnFailure { + /// Immediately stop bootstrap. + Exit, + /// Delay failure until the end of bootstrap invocation. + DelayFail, + /// Ignore the failure, the command can fail in an expected way. + Ignore, +} + +/// 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. + SuppressOnSuccess, +} + +/// Wrapper around `std::process::Command`. +#[derive(Debug)] +pub struct BootstrapCommand<'a> { + pub command: &'a mut Command, + pub failure_behavior: BehaviorOnFailure, + pub output_mode: OutputMode, +} + +impl<'a> BootstrapCommand<'a> { + pub fn delay_failure(self) -> Self { + Self { failure_behavior: BehaviorOnFailure::DelayFail, ..self } + } + + pub fn fail_fast(self) -> Self { + Self { failure_behavior: BehaviorOnFailure::Exit, ..self } + } + + pub fn allow_failure(self) -> Self { + Self { failure_behavior: BehaviorOnFailure::Ignore, ..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: BehaviorOnFailure::Exit, + output_mode: OutputMode::SuppressOnSuccess, + } + } +} diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index bb84b70d987f9..b58a1c2584259 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/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>( } } -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") diff --git a/src/bootstrap/src/utils/mod.rs b/src/bootstrap/src/utils/mod.rs index 7dcb6a8286202..8ca22d00865b6 100644 --- a/src/bootstrap/src/utils/mod.rs +++ b/src/bootstrap/src/utils/mod.rs @@ -6,6 +6,7 @@ pub(crate) mod cache; pub(crate) mod cc_detect; pub(crate) mod channel; pub(crate) mod dylib; +pub(crate) mod exec; pub(crate) mod helpers; pub(crate) mod job; #[cfg(feature = "build-metrics")]