Rollup merge of #129096 - Kobzol:bootstrap-cmd-verbosity, r=onur-ozkan

Print more verbose error for commands that capture output

https://github.com/rust-lang/rust/pull/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`
This commit is contained in:
Matthias Krüger 2024-08-15 19:32:36 +02:00 committed by GitHub
commit 19e32bfc81
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 32 additions and 4 deletions

View File

@ -1056,11 +1056,29 @@ fn run(
}
};
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);
};
@ -1069,14 +1087,14 @@ fn run(
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

View File

@ -291,6 +291,11 @@ pub fn stdout(&self) -> String {
.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 }
@ -303,6 +308,11 @@ pub fn stderr(&self) -> String {
)
.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 {