Skip to content

Commit

Permalink
Rollup merge of #129096 - Kobzol:bootstrap-cmd-verbosity, r=onur-ozkan
Browse files Browse the repository at this point in the history
Print more verbose error for commands that capture output

#128874 made bootstrap command errors less verbose without `-v`. However, in some cases it's too extreme. If a command fails, it now outputs just `Command has failed. Rerun with -v to see more details.`, without providing any context.

I think that I found a reasonable heuristic to figure out when we should print a more verbose error. When the command doesn't capture output, its stdout/stderr is printed, therefore the user sees context about the error. However, when the command captures its output, the user won't see any error message in the output, which is not great. So only in that case, bootstrap now prints a slightly more verbose output (and also prints the captured output).

r? `@onur-ozkan`
  • Loading branch information
matthiaskrgr committed Aug 15, 2024
2 parents 53bf554 + 478d42b commit 19e32bf
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 4 deletions.
26 changes: 22 additions & 4 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1056,11 +1056,29 @@ Executed at: {executed_at}"#,
}
};

let fail = |message: &str| {
let fail = |message: &str, output: CommandOutput| -> ! {
if self.is_verbose() {
println!("{message}");
} else {
println!("Command has failed. Rerun with -v to see more details.");
let (stdout, stderr) = (output.stdout_if_present(), output.stderr_if_present());
// If the command captures output, the user would not see any indication that
// it has failed. In this case, print a more verbose error, since to provide more
// context.
if stdout.is_some() || stderr.is_some() {
if let Some(stdout) =
output.stdout_if_present().take_if(|s| !s.trim().is_empty())
{
println!("STDOUT:\n{stdout}\n");
}
if let Some(stderr) =
output.stderr_if_present().take_if(|s| !s.trim().is_empty())
{
println!("STDERR:\n{stderr}\n");
}
println!("Command {command:?} has failed. Rerun with -v to see more details.");
} else {
println!("Command has failed. Rerun with -v to see more details.");
}
}
exit!(1);
};
Expand All @@ -1069,14 +1087,14 @@ Executed at: {executed_at}"#,
match command.failure_behavior {
BehaviorOnFailure::DelayFail => {
if self.fail_fast {
fail(&message);
fail(&message, output);
}

let mut failures = self.delayed_failures.borrow_mut();
failures.push(message);
}
BehaviorOnFailure::Exit => {
fail(&message);
fail(&message, output);
}
BehaviorOnFailure::Ignore => {
// If failures are allowed, either the error has been printed already
Expand Down
10 changes: 10 additions & 0 deletions src/bootstrap/src/utils/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,11 @@ impl CommandOutput {
.expect("Cannot parse process stdout as UTF-8")
}

#[must_use]
pub fn stdout_if_present(&self) -> Option<String> {
self.stdout.as_ref().and_then(|s| String::from_utf8(s.clone()).ok())
}

#[must_use]
pub fn stdout_if_ok(&self) -> Option<String> {
if self.is_success() { Some(self.stdout()) } else { None }
Expand All @@ -303,6 +308,11 @@ impl CommandOutput {
)
.expect("Cannot parse process stderr as UTF-8")
}

#[must_use]
pub fn stderr_if_present(&self) -> Option<String> {
self.stderr.as_ref().and_then(|s| String::from_utf8(s.clone()).ok())
}
}

impl Default for CommandOutput {
Expand Down

0 comments on commit 19e32bf

Please sign in to comment.