From 82d5743e0b1ecd1995d3bee1a03ec2571b3ffdad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 7 Jul 2024 19:04:27 +0200 Subject: [PATCH] Make it easier to detect when bootstrap tries to read uncaptured stdout/stderr If e.g. only stdout is captured, but the caller tries to read stderr, previously they would get back an empty string. Now the code will explicitly panic when accessing an uncaptured output stream. --- src/bootstrap/src/lib.rs | 6 +++-- src/bootstrap/src/utils/exec.rs | 46 ++++++++++++++++++++------------- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 1d388767d7e3..899a80fa9c7d 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -968,7 +968,9 @@ impl Build { let mut message = String::new(); let output: CommandOutput = match output { // Command has succeeded - Ok(output) if output.status.success() => output.into(), + Ok(output) if output.status.success() => { + CommandOutput::from_output(output, stdout, stderr) + } // Command has started, but then it failed Ok(output) => { writeln!( @@ -982,7 +984,7 @@ Executed at: {executed_at}"#, ) .unwrap(); - let output: CommandOutput = output.into(); + let output: CommandOutput = CommandOutput::from_output(output, stdout, stderr); // If the output mode is OutputMode::Capture, we can now print the output. // If it is OutputMode::Print, then the output has already been printed to diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 22619e674f9a..d4ae8e26aaae 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -223,17 +223,31 @@ pub fn command>(program: S) -> BootstrapCommand { } /// Represents the output of an executed process. -#[allow(unused)] pub struct CommandOutput { status: CommandStatus, - stdout: Vec, - stderr: Vec, + stdout: Option>, + stderr: Option>, } impl CommandOutput { #[must_use] pub fn did_not_start() -> Self { - Self { status: CommandStatus::DidNotStart, stdout: vec![], stderr: vec![] } + Self { status: CommandStatus::DidNotStart, stdout: None, stderr: None } + } + + #[must_use] + pub fn from_output(output: Output, stdout: OutputMode, stderr: OutputMode) -> Self { + Self { + status: CommandStatus::Finished(output.status), + stdout: match stdout { + OutputMode::Print => None, + OutputMode::Capture => Some(output.stdout), + }, + stderr: match stderr { + OutputMode::Print => None, + OutputMode::Capture => Some(output.stderr), + }, + } } #[must_use] @@ -259,7 +273,10 @@ impl CommandOutput { #[must_use] pub fn stdout(&self) -> String { - String::from_utf8(self.stdout.clone()).expect("Cannot parse process stdout as UTF-8") + String::from_utf8( + self.stdout.clone().expect("Accessing stdout of a command that did not capture stdout"), + ) + .expect("Cannot parse process stdout as UTF-8") } #[must_use] @@ -269,7 +286,10 @@ impl CommandOutput { #[must_use] pub fn stderr(&self) -> String { - String::from_utf8(self.stderr.clone()).expect("Cannot parse process stderr as UTF-8") + String::from_utf8( + self.stderr.clone().expect("Accessing stderr of a command that did not capture stderr"), + ) + .expect("Cannot parse process stderr as UTF-8") } } @@ -277,18 +297,8 @@ impl Default for CommandOutput { fn default() -> Self { Self { status: CommandStatus::Finished(ExitStatus::default()), - stdout: vec![], - stderr: vec![], - } - } -} - -impl From for CommandOutput { - fn from(output: Output) -> Self { - Self { - status: CommandStatus::Finished(output.status), - stdout: output.stdout, - stderr: output.stderr, + stdout: Some(vec![]), + stderr: Some(vec![]), } } }