From a12f541a18087550d080225835066cd1ddd60ccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 20 Jun 2024 11:33:23 +0200 Subject: [PATCH 1/8] Implement new command execution logic This function both handles error printing and early/late failures, but it also always returns the actual output of the command --- src/bootstrap/src/lib.rs | 61 ++++++++++++++++++++++++++++++++- src/bootstrap/src/utils/exec.rs | 45 +++++++++++++++++++++++- 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 449d8c128ec63..b53f054c18740 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -41,7 +41,7 @@ use crate::core::builder::Kind; use crate::core::config::{flags, LldMode}; use crate::core::config::{DryRun, Target}; use crate::core::config::{LlvmLibunwind, TargetSelection}; -use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, OutputMode}; +use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, CommandOutput, OutputMode}; use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, symlink_dir}; mod core; @@ -958,6 +958,65 @@ impl Build { }) } + fn run_tracked(&self, command: BootstrapCommand) -> CommandOutput { + if self.config.dry_run() { + return CommandOutput::default(); + } + + self.verbose(|| println!("running: {command:?}")); + + let (output, print_error): (io::Result, bool) = match command.output_mode { + mode @ (OutputMode::PrintAll | OutputMode::PrintOutput) => ( + command.command.status().map(|status| status.into()), + matches!(mode, OutputMode::PrintAll), + ), + OutputMode::SuppressOnSuccess => (command.command.output().map(|o| o.into()), true), + }; + + let output = match output { + Ok(output) => output, + Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}", command, e)), + }; + if !output.is_success() { + if print_error { + println!( + "\n\nCommand did not execute successfully.\ + \nExpected success, got: {}", + output.status(), + ); + + if !self.is_verbose() { + println!("Add `-v` to see more details.\n"); + } + + self.verbose(|| { + println!( + "\nSTDOUT ----\n{}\n\ + STDERR ----\n{}\n", + output.stdout(), + output.stderr(), + ) + }); + } + + 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 => {} + } + } + output + } + /// Runs a command, printing out nice contextual information if it fails. fn run(&self, cmd: &mut Command) { self.run_cmd(BootstrapCommand::from(cmd).fail_fast().output_mode( diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 0aede2022badd..e7dedfa99f2c2 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -1,4 +1,4 @@ -use std::process::Command; +use std::process::{Command, ExitStatus, Output}; /// What should be done when the command fails. #[derive(Debug, Copy, Clone)] @@ -58,3 +58,46 @@ impl<'a> From<&'a mut Command> for BootstrapCommand<'a> { } } } + +/// Represents the output of an executed process. +#[allow(unused)] +#[derive(Default)] +pub struct CommandOutput { + status: ExitStatus, + stdout: Vec, + stderr: Vec, +} + +impl CommandOutput { + pub fn is_success(&self) -> bool { + self.status.success() + } + + pub fn is_failure(&self) -> bool { + !self.is_success() + } + + pub fn status(&self) -> ExitStatus { + self.status + } + + pub fn stdout(&self) -> String { + String::from_utf8(self.stdout.clone()).expect("Cannot parse process stdout as UTF-8") + } + + pub fn stderr(&self) -> String { + String::from_utf8(self.stderr.clone()).expect("Cannot parse process stderr as UTF-8") + } +} + +impl From for CommandOutput { + fn from(output: Output) -> Self { + Self { status: output.status, stdout: output.stdout, stderr: output.stderr } + } +} + +impl From for CommandOutput { + fn from(status: ExitStatus) -> Self { + Self { status, stdout: Default::default(), stderr: Default::default() } + } +} From 949e667d3fc97770134aaa5ea23be4972fde91ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 20 Jun 2024 11:40:15 +0200 Subject: [PATCH 2/8] Remove `run_quiet` --- src/bootstrap/src/core/build_steps/test.rs | 5 +++-- src/bootstrap/src/lib.rs | 15 ++++----------- src/bootstrap/src/utils/exec.rs | 2 +- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 445096e9786d4..834257a6e21c1 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -26,7 +26,7 @@ use crate::core::builder::{Builder, Compiler, Kind, RunConfig, ShouldRun, Step}; use crate::core::config::flags::get_completion; use crate::core::config::flags::Subcommand; use crate::core::config::TargetSelection; -use crate::utils::exec::BootstrapCommand; +use crate::utils::exec::{BootstrapCommand, OutputMode}; use crate::utils::helpers::{ self, add_link_lib_path, add_rustdoc_cargo_linker_args, dylib_path, dylib_path_var, linker_args, linker_flags, output, t, target_supports_cranelift_backend, up_to_date, @@ -2312,7 +2312,8 @@ impl Step for ErrorIndex { let guard = builder.msg(Kind::Test, compiler.stage, "error-index", compiler.host, compiler.host); let _time = helpers::timeit(builder); - builder.run_quiet(&mut tool); + builder + .run_tracked(BootstrapCommand::from(&mut tool).output_mode(OutputMode::PrintOnFailure)); drop(guard); // The tests themselves need to link to std, so make sure it is // available. diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index b53f054c18740..5b2913b1a83f6 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -958,7 +958,7 @@ impl Build { }) } - fn run_tracked(&self, command: BootstrapCommand) -> CommandOutput { + fn run_tracked(&self, command: BootstrapCommand<'_>) -> CommandOutput { if self.config.dry_run() { return CommandOutput::default(); } @@ -970,7 +970,7 @@ impl Build { command.command.status().map(|status| status.into()), matches!(mode, OutputMode::PrintAll), ), - OutputMode::SuppressOnSuccess => (command.command.output().map(|o| o.into()), true), + OutputMode::PrintOnFailure => (command.command.output().map(|o| o.into()), true), }; let output = match output { @@ -1037,19 +1037,12 @@ 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::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), + BootstrapCommand::from(cmd).delay_failure().output_mode(OutputMode::PrintOnFailure), ) } @@ -1071,7 +1064,7 @@ impl Build { }), matches!(mode, OutputMode::PrintAll), ), - OutputMode::SuppressOnSuccess => (command.command.output(), true), + OutputMode::PrintOnFailure => (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 e7dedfa99f2c2..21013345f0914 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -20,7 +20,7 @@ pub enum OutputMode { /// Print the output (by inheriting stdout/stderr). PrintOutput, /// Suppress the output if the command succeeds, otherwise print the output. - SuppressOnSuccess, + PrintOnFailure, } /// Wrapper around `std::process::Command`. From e933cfb13ce6367b8b51b5c40c5041ef16294b08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 20 Jun 2024 11:43:42 +0200 Subject: [PATCH 3/8] Remove `run_quiet_delaying_failure` --- src/bootstrap/src/core/build_steps/test.rs | 8 ++++---- src/bootstrap/src/lib.rs | 10 +--------- src/bootstrap/src/utils/exec.rs | 5 +++++ 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 834257a6e21c1..7eed3fdb9cb54 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2342,11 +2342,11 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) -> let test_args = builder.config.test_args().join(" "); cmd.arg("--test-args").arg(test_args); - if builder.config.verbose_tests { - builder.run_delaying_failure(&mut cmd) - } else { - builder.run_quiet_delaying_failure(&mut cmd) + let mut cmd = BootstrapCommand::from(&mut cmd).delay_failure(); + if !builder.config.verbose_tests { + cmd = cmd.quiet(); } + builder.run_tracked(cmd).is_success() } #[derive(Debug, Clone, PartialEq, Eq, Hash)] diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 5b2913b1a83f6..2c9cc39474f1b 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -958,6 +958,7 @@ impl Build { }) } + /// Execute a command and return its output. fn run_tracked(&self, command: BootstrapCommand<'_>) -> CommandOutput { if self.config.dry_run() { return CommandOutput::default(); @@ -1037,15 +1038,6 @@ impl Build { )) } - /// 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::PrintOnFailure), - ) - } - /// 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() { diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 21013345f0914..4b7029eebac08 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -44,6 +44,11 @@ impl<'a> BootstrapCommand<'a> { Self { failure_behavior: BehaviorOnFailure::Ignore, ..self } } + /// Do not print the output of the command, unless it fails. + pub fn quiet(self) -> Self { + self.output_mode(OutputMode::PrintOnFailure) + } + pub fn output_mode(self, output_mode: OutputMode) -> Self { Self { output_mode, ..self } } From 0de7b92cc679c0d6326a3f24dfd7affd5a5f0c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 20 Jun 2024 11:52:48 +0200 Subject: [PATCH 4/8] Remove `run_delaying_failure` --- src/bootstrap/src/core/build_steps/test.rs | 30 ++++++++++++++-------- src/bootstrap/src/lib.rs | 22 ++++++++-------- src/bootstrap/src/utils/exec.rs | 10 +++----- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 7eed3fdb9cb54..9917d5b7e5002 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -156,7 +156,10 @@ You can skip linkcheck with --skip src/tools/linkchecker" let _guard = builder.msg(Kind::Test, compiler.stage, "Linkcheck", bootstrap_host, bootstrap_host); let _time = helpers::timeit(builder); - builder.run_delaying_failure(linkchecker.arg(builder.out.join(host.triple).join("doc"))); + builder.run_tracked( + BootstrapCommand::from(linkchecker.arg(builder.out.join(host.triple).join("doc"))) + .delay_failure(), + ); } fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { @@ -213,8 +216,11 @@ impl Step for HtmlCheck { builder, )); - builder.run_delaying_failure( - builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)), + builder.run_tracked( + BootstrapCommand::from( + builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)), + ) + .delay_failure(), ); } } @@ -261,7 +267,7 @@ impl Step for Cargotest { .env("RUSTC", builder.rustc(compiler)) .env("RUSTDOC", builder.rustdoc(compiler)); add_rustdoc_cargo_linker_args(cmd, builder, compiler.host, LldThreads::No); - builder.run_delaying_failure(cmd); + builder.run_tracked(BootstrapCommand::from(cmd).delay_failure()); } } @@ -813,7 +819,7 @@ impl Step for RustdocTheme { .env("RUSTC_BOOTSTRAP", "1"); cmd.args(linker_args(builder, self.compiler.host, LldThreads::No)); - builder.run_delaying_failure(&mut cmd); + builder.run_tracked(BootstrapCommand::from(&mut cmd).delay_failure()); } } @@ -1093,7 +1099,7 @@ HELP: to skip test's attempt to check tidiness, pass `--skip src/tools/tidy` to } builder.info("tidy check"); - builder.run_delaying_failure(&mut cmd); + builder.run_tracked(BootstrapCommand::from(&mut cmd).delay_failure()); builder.info("x.py completions check"); let [bash, zsh, fish, powershell] = ["x.py.sh", "x.py.zsh", "x.py.fish", "x.py.ps1"] @@ -2179,7 +2185,8 @@ impl BookTest { compiler.host, ); let _time = helpers::timeit(builder); - let toolstate = if builder.run_delaying_failure(&mut rustbook_cmd) { + let cmd = BootstrapCommand::from(&mut rustbook_cmd).delay_failure(); + let toolstate = if builder.run_tracked(cmd).is_success() { ToolState::TestPass } else { ToolState::TestFail @@ -2371,7 +2378,8 @@ impl Step for RustcGuide { let src = builder.src.join(relative_path); let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook); - let toolstate = if builder.run_delaying_failure(rustbook_cmd.arg("linkcheck").arg(&src)) { + let cmd = BootstrapCommand::from(rustbook_cmd.arg("linkcheck").arg(&src)).delay_failure(); + let toolstate = if builder.run_tracked(cmd).is_success() { ToolState::TestPass } else { ToolState::TestFail @@ -2985,7 +2993,7 @@ impl Step for Bootstrap { .current_dir(builder.src.join("src/bootstrap/")); // NOTE: we intentionally don't pass test_args here because the args for unittest and cargo test are mutually incompatible. // Use `python -m unittest` manually if you want to pass arguments. - builder.run_delaying_failure(&mut check_bootstrap); + builder.run_tracked(BootstrapCommand::from(&mut check_bootstrap).delay_failure()); let mut cmd = Command::new(&builder.initial_cargo); cmd.arg("test") @@ -3062,7 +3070,7 @@ impl Step for TierCheck { self.compiler.host, self.compiler.host, ); - builder.run_delaying_failure(&mut cargo.into()); + builder.run_tracked(BootstrapCommand::from(&mut cargo.into()).delay_failure()); } } @@ -3148,7 +3156,7 @@ impl Step for RustInstaller { cmd.env("CARGO", &builder.initial_cargo); cmd.env("RUSTC", &builder.initial_rustc); cmd.env("TMP_DIR", &tmpdir); - builder.run_delaying_failure(&mut cmd); + builder.run_tracked(BootstrapCommand::from(&mut cmd).delay_failure()); } fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 2c9cc39474f1b..b75b17bf7559a 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -966,7 +966,11 @@ impl Build { self.verbose(|| println!("running: {command:?}")); - let (output, print_error): (io::Result, bool) = match command.output_mode { + let output_mode = command.output_mode.unwrap_or_else(|| match self.is_verbose() { + true => OutputMode::PrintAll, + false => OutputMode::PrintOutput, + }); + let (output, print_error): (io::Result, bool) = match output_mode { mode @ (OutputMode::PrintAll | OutputMode::PrintOutput) => ( command.command.status().map(|status| status.into()), matches!(mode, OutputMode::PrintAll), @@ -1028,16 +1032,6 @@ 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 { - self.run_cmd(BootstrapCommand::from(cmd).delay_failure().output_mode( - match self.is_verbose() { - true => OutputMode::PrintAll, - false => OutputMode::PrintOutput, - }, - )) - } - /// 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() { @@ -1047,7 +1041,11 @@ impl Build { let command = cmd.into(); self.verbose(|| println!("running: {command:?}")); - let (output, print_error) = match command.output_mode { + let output_mode = command.output_mode.unwrap_or_else(|| match self.is_verbose() { + true => OutputMode::PrintAll, + false => OutputMode::PrintOutput, + }); + let (output, print_error) = match output_mode { mode @ (OutputMode::PrintAll | OutputMode::PrintOutput) => ( command.command.status().map(|status| Output { status, diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 4b7029eebac08..784d46a282f36 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -28,7 +28,7 @@ pub enum OutputMode { pub struct BootstrapCommand<'a> { pub command: &'a mut Command, pub failure_behavior: BehaviorOnFailure, - pub output_mode: OutputMode, + pub output_mode: Option, } impl<'a> BootstrapCommand<'a> { @@ -50,17 +50,13 @@ impl<'a> BootstrapCommand<'a> { } pub fn output_mode(self, output_mode: OutputMode) -> Self { - Self { output_mode, ..self } + Self { output_mode: Some(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::PrintAll, - } + Self { command, failure_behavior: BehaviorOnFailure::Exit, output_mode: None } } } From 5c4318d02c74c1ee9809741be28fe39fd7eaaadb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 20 Jun 2024 11:54:06 +0200 Subject: [PATCH 5/8] Implement `run_cmd` in terms of `run_tracked` --- src/bootstrap/src/lib.rs | 73 +--------------------------------------- 1 file changed, 1 insertion(+), 72 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index b75b17bf7559a..584d00e7dbae4 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -1034,79 +1034,8 @@ impl Build { /// 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(|| println!("running: {command:?}")); - - let output_mode = command.output_mode.unwrap_or_else(|| match self.is_verbose() { - true => OutputMode::PrintAll, - false => OutputMode::PrintOutput, - }); - let (output, print_error) = match 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::PrintOnFailure => (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.\ - \nExpected success, got: {}", - output.status, - ); - - if !self.is_verbose() { - println!("Add `-v` to see more details.\n"); - } - - self.verbose(|| { - println!( - "\nSTDOUT ----\n{}\n\ - STDERR ----\n{}\n", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) - ) - }); - } - Err(()) - } else { - Ok(()) - }; - - 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 - } - } + self.run_tracked(command).is_success() } /// Check if verbosity is greater than the `level` From c15293407fe25a29880c8173a392b5f44061a714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 20 Jun 2024 12:30:41 +0200 Subject: [PATCH 6/8] Remove unused import --- src/bootstrap/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 584d00e7dbae4..c34fc9fa05462 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -23,7 +23,7 @@ use std::fmt::Display; use std::fs::{self, File}; use std::io; use std::path::{Path, PathBuf}; -use std::process::{Command, Output, Stdio}; +use std::process::{Command, Stdio}; use std::str; use std::sync::OnceLock; From 3fe4d134dd709404c3e7effb80e4272561874703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 20 Jun 2024 13:00:12 +0200 Subject: [PATCH 7/8] Appease `clippy` --- src/bootstrap/src/core/build_steps/test.rs | 2 +- src/bootstrap/src/lib.rs | 18 +++++++++--------- src/bootstrap/src/utils/exec.rs | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 9917d5b7e5002..155da24691412 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2320,7 +2320,7 @@ impl Step for ErrorIndex { builder.msg(Kind::Test, compiler.stage, "error-index", compiler.host, compiler.host); let _time = helpers::timeit(builder); builder - .run_tracked(BootstrapCommand::from(&mut tool).output_mode(OutputMode::PrintOnFailure)); + .run_tracked(BootstrapCommand::from(&mut tool).output_mode(OutputMode::OnlyOnFailure)); drop(guard); // The tests themselves need to link to std, so make sure it is // available. diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index c34fc9fa05462..9b414b24d2f3a 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -585,8 +585,8 @@ impl Build { BootstrapCommand::from(submodule_git().args(["diff-index", "--quiet", "HEAD"])) .allow_failure() .output_mode(match self.is_verbose() { - true => OutputMode::PrintAll, - false => OutputMode::PrintOutput, + true => OutputMode::All, + false => OutputMode::OnlyOutput, }), ); if has_local_modifications { @@ -967,15 +967,15 @@ impl Build { self.verbose(|| println!("running: {command:?}")); let output_mode = command.output_mode.unwrap_or_else(|| match self.is_verbose() { - true => OutputMode::PrintAll, - false => OutputMode::PrintOutput, + true => OutputMode::All, + false => OutputMode::OnlyOutput, }); let (output, print_error): (io::Result, bool) = match output_mode { - mode @ (OutputMode::PrintAll | OutputMode::PrintOutput) => ( + mode @ (OutputMode::All | OutputMode::OnlyOutput) => ( command.command.status().map(|status| status.into()), - matches!(mode, OutputMode::PrintAll), + matches!(mode, OutputMode::All), ), - OutputMode::PrintOnFailure => (command.command.output().map(|o| o.into()), true), + OutputMode::OnlyOnFailure => (command.command.output().map(|o| o.into()), true), }; let output = match output { @@ -1026,8 +1026,8 @@ impl Build { fn run(&self, cmd: &mut Command) { self.run_cmd(BootstrapCommand::from(cmd).fail_fast().output_mode( match self.is_verbose() { - true => OutputMode::PrintAll, - false => OutputMode::PrintOutput, + true => OutputMode::All, + false => OutputMode::OnlyOutput, }, )); } diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 784d46a282f36..2ac8796191500 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -16,11 +16,11 @@ pub enum BehaviorOnFailure { pub enum OutputMode { /// Print both the output (by inheriting stdout/stderr) and also the command itself, if it /// fails. - PrintAll, + All, /// Print the output (by inheriting stdout/stderr). - PrintOutput, + OnlyOutput, /// Suppress the output if the command succeeds, otherwise print the output. - PrintOnFailure, + OnlyOnFailure, } /// Wrapper around `std::process::Command`. @@ -46,7 +46,7 @@ impl<'a> BootstrapCommand<'a> { /// Do not print the output of the command, unless it fails. pub fn quiet(self) -> Self { - self.output_mode(OutputMode::PrintOnFailure) + self.output_mode(OutputMode::OnlyOnFailure) } pub fn output_mode(self, output_mode: OutputMode) -> Self { From 250586cb2e79106586172fb3fd8396907fe0644a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 22 Jun 2024 09:18:58 +0200 Subject: [PATCH 8/8] Wrap std `Output` in `CommandOutput` --- src/bootstrap/src/utils/exec.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 2ac8796191500..e8c588b75b381 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -62,16 +62,11 @@ impl<'a> From<&'a mut Command> for BootstrapCommand<'a> { /// Represents the output of an executed process. #[allow(unused)] -#[derive(Default)] -pub struct CommandOutput { - status: ExitStatus, - stdout: Vec, - stderr: Vec, -} +pub struct CommandOutput(Output); impl CommandOutput { pub fn is_success(&self) -> bool { - self.status.success() + self.0.status.success() } pub fn is_failure(&self) -> bool { @@ -79,26 +74,32 @@ impl CommandOutput { } pub fn status(&self) -> ExitStatus { - self.status + self.0.status } pub fn stdout(&self) -> String { - String::from_utf8(self.stdout.clone()).expect("Cannot parse process stdout as UTF-8") + String::from_utf8(self.0.stdout.clone()).expect("Cannot parse process stdout as UTF-8") } pub fn stderr(&self) -> String { - String::from_utf8(self.stderr.clone()).expect("Cannot parse process stderr as UTF-8") + String::from_utf8(self.0.stderr.clone()).expect("Cannot parse process stderr as UTF-8") + } +} + +impl Default for CommandOutput { + fn default() -> Self { + Self(Output { status: Default::default(), stdout: vec![], stderr: vec![] }) } } impl From for CommandOutput { fn from(output: Output) -> Self { - Self { status: output.status, stdout: output.stdout, stderr: output.stderr } + Self(output) } } impl From for CommandOutput { fn from(status: ExitStatus) -> Self { - Self { status, stdout: Default::default(), stderr: Default::default() } + Self(Output { status, stdout: vec![], stderr: vec![] }) } }