From 41af1eb8a98f0400a826a15d7386335af3c49c23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 29 Jun 2024 16:00:23 +0200 Subject: [PATCH] Refactor command outcome handling To handle the case of failing to start a `BootstrapCommand`. --- src/bootstrap/src/core/sanity.rs | 15 ++++--- src/bootstrap/src/lib.rs | 77 +++++++++++++++++++------------- src/bootstrap/src/utils/exec.rs | 52 +++++++++++++++------ 3 files changed, 93 insertions(+), 51 deletions(-) diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index d64923e69cbd0..78862ccb8cd68 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -13,7 +13,6 @@ use std::env; use std::ffi::{OsStr, OsString}; use std::fs; use std::path::PathBuf; -use std::process::Command; #[cfg(not(feature = "bootstrap-self-test"))] use crate::builder::Builder; @@ -25,7 +24,6 @@ use std::collections::HashSet; use crate::builder::Kind; use crate::core::config::Target; use crate::utils::exec::BootstrapCommand; -use crate::utils::helpers::output; use crate::Build; pub struct Finder { @@ -210,11 +208,14 @@ than building it. .or_else(|| cmd_finder.maybe_have("reuse")); #[cfg(not(feature = "bootstrap-self-test"))] - let stage0_supported_target_list: HashSet = - output(Command::new(&build.config.initial_rustc).args(["--print", "target-list"])) - .lines() - .map(|s| s.to_string()) - .collect(); + let stage0_supported_target_list: HashSet = crate::utils::helpers::output( + &mut BootstrapCommand::new(&build.config.initial_rustc) + .args(["--print", "target-list"]) + .command, + ) + .lines() + .map(|s| s.to_string()) + .collect(); // We're gonna build some custom C code here and there, host triples // also build some C++ shims for LLVM so we need a C++ compiler. diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 667001e1a0149..7f096b8fc3bf7 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -23,13 +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 std::sync::OnceLock; use build_helper::ci::{gha, CiEnv}; use build_helper::exit; -use build_helper::util::fail; use filetime::FileTime; use sha2::digest::Digest; use termcolor::{ColorChoice, StandardStream, WriteColor}; @@ -973,43 +972,61 @@ impl Build { self.verbose(|| println!("running: {command:?}")); - let output: io::Result = match command.output_mode { - OutputMode::Print => command.command.status().map(|status| status.into()), - OutputMode::CaptureAll => command.command.output().map(|o| o.into()), + let output: io::Result = match command.output_mode { + OutputMode::Print => command.command.status().map(|status| Output { + status, + stdout: vec![], + stderr: vec![], + }), + OutputMode::CaptureAll => command.command.output(), OutputMode::CaptureStdout => { command.command.stderr(Stdio::inherit()); - command.command.output().map(|o| o.into()) + command.command.output() } }; - let output = match output { - Ok(output) => output, - Err(e) => fail(&format!("failed to execute command: {command:?}\nerror: {e}")), - }; - if !output.is_success() { - use std::fmt::Write; - - // Here we build an error message, and below we decide if it should be printed or not. - let mut message = String::new(); - writeln!( - message, - "\n\nCommand {command:?} did not execute successfully.\ + use std::fmt::Write; + + let mut message = String::new(); + let output: CommandOutput = match output { + // Command has succeeded + Ok(output) if output.status.success() => output.into(), + // Command has started, but then it failed + Ok(output) => { + writeln!( + message, + "\n\nCommand {command:?} did not execute successfully.\ \nExpected success, got: {}", - output.status(), - ) - .unwrap(); - - // If the output mode is OutputMode::Print, the output has already been printed to - // stdout/stderr, and we thus don't have anything captured to print anyway. - if matches!(command.output_mode, OutputMode::CaptureAll | OutputMode::CaptureStdout) { - writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap(); + output.status, + ) + .unwrap(); + + let output: CommandOutput = output.into(); + // If the output mode is OutputMode::Print, the output has already been printed to + // stdout/stderr, and we thus don't have anything captured to print anyway. + if matches!(command.output_mode, OutputMode::CaptureAll | OutputMode::CaptureStdout) + { + writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap(); - // Stderr is added to the message only if it was captured - if matches!(command.output_mode, OutputMode::CaptureAll) { - writeln!(message, "\nSTDERR ----\n{}", output.stderr().trim()).unwrap(); + // Stderr is added to the message only if it was captured + if matches!(command.output_mode, OutputMode::CaptureAll) { + writeln!(message, "\nSTDERR ----\n{}", output.stderr().trim()).unwrap(); + } } + output } - + // The command did not even start + Err(e) => { + writeln!( + message, + "\n\nCommand {command:?} did not execute successfully.\ + \nIt was not possible to execute the command: {e:?}" + ) + .unwrap(); + CommandOutput::did_not_start() + } + }; + if !output.is_success() { match command.failure_behavior { BehaviorOnFailure::DelayFail => { if self.fail_fast { diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index bda6e0bfbac02..78a3b917e459a 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -132,25 +132,47 @@ impl From for BootstrapCommand { } } +/// Represents the outcome of starting a command. +enum CommandOutcome { + /// The command has started and finished with some status. + Finished(ExitStatus), + /// It was not even possible to start the command. + DidNotStart, +} + /// Represents the output of an executed process. #[allow(unused)] -pub struct CommandOutput(Output); +pub struct CommandOutput { + outcome: CommandOutcome, + stdout: Vec, + stderr: Vec, +} impl CommandOutput { + pub fn did_not_start() -> Self { + Self { outcome: CommandOutcome::DidNotStart, stdout: vec![], stderr: vec![] } + } + pub fn is_success(&self) -> bool { - self.0.status.success() + match self.outcome { + CommandOutcome::Finished(status) => status.success(), + CommandOutcome::DidNotStart => false, + } } pub fn is_failure(&self) -> bool { !self.is_success() } - pub fn status(&self) -> ExitStatus { - self.0.status + pub fn status(&self) -> Option { + match self.outcome { + CommandOutcome::Finished(status) => Some(status), + CommandOutcome::DidNotStart => None, + } } pub fn stdout(&self) -> String { - String::from_utf8(self.0.stdout.clone()).expect("Cannot parse process stdout as UTF-8") + String::from_utf8(self.stdout.clone()).expect("Cannot parse process stdout as UTF-8") } pub fn stdout_if_ok(&self) -> Option { @@ -158,24 +180,26 @@ impl CommandOutput { } pub fn stderr(&self) -> String { - String::from_utf8(self.0.stderr.clone()).expect("Cannot parse process stderr as UTF-8") + String::from_utf8(self.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![] }) + Self { + outcome: CommandOutcome::Finished(ExitStatus::default()), + stdout: vec![], + stderr: vec![], + } } } impl From for CommandOutput { fn from(output: Output) -> Self { - Self(output) - } -} - -impl From for CommandOutput { - fn from(status: ExitStatus) -> Self { - Self(Output { status, stdout: vec![], stderr: vec![] }) + Self { + outcome: CommandOutcome::Finished(output.status), + stdout: output.stdout, + stderr: output.stderr, + } } }