Skip to content

Commit

Permalink
Make it easier to detect when bootstrap tries to read uncaptured stdo…
Browse files Browse the repository at this point in the history
…ut/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.
  • Loading branch information
Kobzol committed Jul 19, 2024
1 parent ead6aad commit 1984a46
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 20 deletions.
6 changes: 4 additions & 2 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand All @@ -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
Expand Down
46 changes: 28 additions & 18 deletions src/bootstrap/src/utils/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,31 @@ pub fn command<S: AsRef<OsStr>>(program: S) -> BootstrapCommand {
}

/// Represents the output of an executed process.
#[allow(unused)]
pub struct CommandOutput {
status: CommandStatus,
stdout: Vec<u8>,
stderr: Vec<u8>,
stdout: Option<Vec<u8>>,
stderr: Option<Vec<u8>>,
}

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]
Expand All @@ -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]
Expand All @@ -269,26 +286,19 @@ 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")
}
}

impl Default for CommandOutput {
fn default() -> Self {
Self {
status: CommandStatus::Finished(ExitStatus::default()),
stdout: vec![],
stderr: vec![],
}
}
}

impl From<Output> 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![]),
}
}
}

0 comments on commit 1984a46

Please sign in to comment.