From a69edc68aa224891a7e54d8ced094ffc28c4be3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 9 Oct 2023 23:04:11 +0200 Subject: [PATCH 1/7] Add BootstrapCommand and `run_cmd` --- src/bootstrap/src/lib.rs | 34 ++++++++++++++++++++++++--------- src/bootstrap/src/utils/exec.rs | 21 ++++++++++++++++++++ src/bootstrap/src/utils/mod.rs | 1 + 3 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 src/bootstrap/src/utils/exec.rs diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 97c743074af43..b20c20a360b9a 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -39,6 +39,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::BootstrapCommand; use crate::utils::helpers::{ self, dir_is_empty, exe, libdir, mtime, output, run, run_suppressed, symlink_dir, try_run_suppressed, @@ -959,17 +960,32 @@ impl Build { /// 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; + let cmd: BootstrapCommand<'_> = cmd.into(); + self.run_cmd(cmd.delay_failure()) + } + + /// A centralized function for running commands that do not return output. + pub(crate) fn run_cmd<'a, C: Into>>(&self, cmd: C) -> bool { + 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); + + match result { + Ok(_) => true, + Err(_) => { + if command.delay_failure { + let mut failures = self.delayed_failures.borrow_mut(); + failures.push(format!("{command:?}")); + return false; + } + if self.fail_fast { + exit!(1); + } + 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..a590171984e32 --- /dev/null +++ b/src/bootstrap/src/utils/exec.rs @@ -0,0 +1,21 @@ +use std::process::Command; + +/// 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, +} + +impl<'a> BootstrapCommand<'a> { + pub fn delay_failure(self) -> Self { + Self { delay_failure: true, ..self } + } +} + +impl<'a> From<&'a mut Command> for BootstrapCommand<'a> { + fn from(command: &'a mut Command) -> Self { + Self { command, delay_failure: false } + } +} 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")] From 0e090e740c9cf4743a57ec61dd91d90357571ed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 10 Oct 2023 20:19:22 +0200 Subject: [PATCH 2/7] Add behavior on failure to BootstrapCommand --- src/bootstrap/src/lib.rs | 55 +++++++++++++-------------------- src/bootstrap/src/utils/exec.rs | 19 +++++++++--- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index b20c20a360b9a..7c052f262d866 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -39,11 +39,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::exec::BootstrapCommand; -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}; +use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, symlink_dir}; mod core; mod utils; @@ -922,44 +919,30 @@ 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()) + // FIXME: output mode -> status + err if self.is_verbose() + let cmd: BootstrapCommand<'_> = cmd.into(); + self.run_cmd(cmd.fail_fast()); } /// 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) + // FIXME: output mode -> output + err + let cmd: BootstrapCommand<'_> = cmd.into(); + self.run_cmd(cmd.fail_fast()); } /// 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 + // 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()) } @@ -975,10 +958,16 @@ impl Build { match result { Ok(_) => true, Err(_) => { - if command.delay_failure { - let mut failures = self.delayed_failures.borrow_mut(); - failures.push(format!("{command:?}")); - return false; + if let Some(failure_behavior) = command.failure_behavior { + match failure_behavior { + BehaviorOnFailure::DelayFail => { + let mut failures = self.delayed_failures.borrow_mut(); + failures.push(format!("{command:?}")); + } + BehaviorOnFailure::Exit => { + exit!(1); + } + } } if self.fail_fast { exit!(1); diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index a590171984e32..a2a0dbad240ed 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -1,21 +1,32 @@ 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, +} + /// 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 failure_behavior: Option, } impl<'a> BootstrapCommand<'a> { pub fn delay_failure(self) -> Self { - Self { delay_failure: true, ..self } + Self { failure_behavior: Some(BehaviorOnFailure::DelayFail), ..self } + } + pub fn fail_fast(self) -> Self { + Self { failure_behavior: Some(BehaviorOnFailure::Exit), ..self } } } impl<'a> From<&'a mut Command> for BootstrapCommand<'a> { fn from(command: &'a mut Command) -> Self { - Self { command, delay_failure: false } + Self { command, failure_behavior: None } } } From 7efe8fa8493d23ef0b99f31c7074957e7b298aec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 10 Oct 2023 20:54:25 +0200 Subject: [PATCH 3/7] Add output mode to BootstrapCommand --- src/bootstrap/src/lib.rs | 78 ++++++++++++++++++++++-------- src/bootstrap/src/utils/exec.rs | 18 ++++++- src/bootstrap/src/utils/helpers.rs | 34 +------------ 3 files changed, 76 insertions(+), 54 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 7c052f262d866..35d3c78aee2f9 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,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>>(&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, diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index a2a0dbad240ed..a3520ad5f983a 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/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, + 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 } } } 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") From 17011d02ead7dad95a26f9599f3ddac170da0db6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 9 Oct 2023 23:42:13 +0200 Subject: [PATCH 4/7] Remove usages of `Config::try_run` Commands should be run on Builder, if possible. --- src/bootstrap/src/core/build_steps/test.rs | 9 +++++---- src/bootstrap/src/core/build_steps/tool.rs | 4 ++-- src/bootstrap/src/lib.rs | 15 ++++++--------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 831a86940fb48..ee9effa468e1b 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(&mut cargo) { // 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..7c35e2a0e9f95 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -108,8 +108,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(&mut cargo); builder.save_toolstate( tool, diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 35d3c78aee2f9..27d5faa3289ad 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -579,15 +579,12 @@ 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( - Command::new("git") - .args(&["diff-index", "--quiet", "HEAD"]) - .current_dir(&absolute_path), - ) - .is_err(); + // diff-index reports the modifications through the exit status + let has_local_modifications = !self.run_cmd( + Command::new("git") + .args(&["diff-index", "--quiet", "HEAD"]) + .current_dir(&absolute_path), + ); if has_local_modifications { self.run(Command::new("git").args(&["stash", "push"]).current_dir(&absolute_path)); } From 03d48dd735b18593f9e361f2156efae7bb551236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 11 Oct 2023 19:00:35 +0200 Subject: [PATCH 5/7] Rename Supress variant --- src/bootstrap/src/lib.rs | 10 +++++++--- src/bootstrap/src/utils/exec.rs | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 27d5faa3289ad..6f9a9beb33165 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -937,14 +937,18 @@ impl Build { /// Runs a command, printing out nice contextual information if it fails. fn run_quiet(&self, cmd: &mut Command) { - self.run_cmd(BootstrapCommand::from(cmd).fail_fast().output_mode(OutputMode::Suppress)); + 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::Suppress)) + self.run_cmd( + BootstrapCommand::from(cmd).delay_failure().output_mode(OutputMode::SuppressOnSuccess), + ) } /// A centralized function for running commands that do not return output. @@ -965,7 +969,7 @@ impl Build { }), matches!(mode, OutputMode::PrintAll), ), - OutputMode::Suppress => (command.command.output(), true), + OutputMode::SuppressOnSuccess => (command.command.output(), true), }; let output = match output { diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index a3520ad5f983a..f244f5152beb6 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -18,7 +18,7 @@ pub enum OutputMode { /// Print the output (by inheriting stdout/stderr). PrintOutput, /// Suppress the output if the command succeeds, otherwise print the output. - Suppress, + SuppressOnSuccess, } /// Wrapper around `std::process::Command`. @@ -43,6 +43,6 @@ impl<'a> BootstrapCommand<'a> { impl<'a> From<&'a mut Command> for BootstrapCommand<'a> { fn from(command: &'a mut Command) -> Self { - Self { command, failure_behavior: None, output_mode: OutputMode::Suppress } + Self { command, failure_behavior: None, output_mode: OutputMode::SuppressOnSuccess } } } From b153a0182875b420bdecb19c81be0731e2e2f6b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 11 Oct 2023 19:02:00 +0200 Subject: [PATCH 6/7] Simplify BehaviorOnFailure --- src/bootstrap/src/lib.rs | 19 +++++++++---------- src/bootstrap/src/utils/exec.rs | 12 ++++++++---- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 6f9a9beb33165..701c8df11a723 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -997,19 +997,18 @@ impl Build { match result { Ok(_) => true, Err(_) => { - if let Some(failure_behavior) = command.failure_behavior { - match failure_behavior { - BehaviorOnFailure::DelayFail => { - let mut failures = self.delayed_failures.borrow_mut(); - failures.push(format!("{command:?}")); - } - BehaviorOnFailure::Exit => { + 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); } - } - if self.fail_fast { - exit!(1); } false } diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index f244f5152beb6..1b9e5cb174a0c 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -25,16 +25,16 @@ pub enum OutputMode { #[derive(Debug)] pub struct BootstrapCommand<'a> { pub command: &'a mut Command, - pub failure_behavior: Option, + pub failure_behavior: BehaviorOnFailure, pub output_mode: OutputMode, } impl<'a> BootstrapCommand<'a> { pub fn delay_failure(self) -> Self { - Self { failure_behavior: Some(BehaviorOnFailure::DelayFail), ..self } + Self { failure_behavior: BehaviorOnFailure::DelayFail, ..self } } pub fn fail_fast(self) -> Self { - Self { failure_behavior: Some(BehaviorOnFailure::Exit), ..self } + Self { failure_behavior: BehaviorOnFailure::Exit, ..self } } pub fn output_mode(self, output_mode: OutputMode) -> Self { Self { output_mode, ..self } @@ -43,6 +43,10 @@ impl<'a> BootstrapCommand<'a> { impl<'a> From<&'a mut Command> for BootstrapCommand<'a> { fn from(command: &'a mut Command) -> Self { - Self { command, failure_behavior: None, output_mode: OutputMode::SuppressOnSuccess } + Self { + command, + failure_behavior: BehaviorOnFailure::Exit, + output_mode: OutputMode::SuppressOnSuccess, + } } } From 5d7fca15d3c7d991733abd0d187e49e403b0b200 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 26 Oct 2023 10:54:21 +0200 Subject: [PATCH 7/7] Allow ignoring the failure of command execution --- src/bootstrap/src/core/build_steps/test.rs | 2 +- src/bootstrap/src/core/build_steps/tool.rs | 3 ++- src/bootstrap/src/lib.rs | 10 +++++++--- src/bootstrap/src/utils/exec.rs | 8 ++++++++ 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index ee9effa468e1b..246d742a99faa 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -810,7 +810,7 @@ impl Step for Clippy { let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host); // Clippy reports errors if it blessed the outputs - if builder.run_cmd(&mut cargo) { + if builder.run_cmd(BootstrapCommand::from(&mut cargo).allow_failure()) { // The tests succeeded; nothing to do. return; } diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 7c35e2a0e9f95..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; @@ -109,7 +110,7 @@ impl Step for ToolBuild { let mut cargo = Command::from(cargo); // we check this in `is_optional_tool` in a second - let is_expected = builder.run_cmd(&mut cargo); + 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 701c8df11a723..1c62d5c41e926 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -581,9 +581,12 @@ impl Build { // Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error). // diff-index reports the modifications through the exit status let has_local_modifications = !self.run_cmd( - Command::new("git") - .args(&["diff-index", "--quiet", "HEAD"]) - .current_dir(&absolute_path), + BootstrapCommand::from( + Command::new("git") + .args(&["diff-index", "--quiet", "HEAD"]) + .current_dir(&absolute_path), + ) + .allow_failure(), ); if has_local_modifications { self.run(Command::new("git").args(&["stash", "push"]).current_dir(&absolute_path)); @@ -1009,6 +1012,7 @@ impl Build { BehaviorOnFailure::Exit => { exit!(1); } + BehaviorOnFailure::Ignore => {} } false } diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 1b9e5cb174a0c..9c1c21cd95894 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -7,6 +7,8 @@ pub enum BehaviorOnFailure { 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. @@ -33,9 +35,15 @@ 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 } }