Skip to content

Trim noise from build script errors #11525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 87 additions & 23 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ use crate::core::{profiles::ProfileRoot, PackageId, Target};
use crate::util::errors::CargoResult;
use crate::util::machine_message::{self, Message};
use crate::util::{internal, profile};
use anyhow::{bail, Context as _};
use crate::VerboseError;
use anyhow::{bail, Context as _, Error};
use cargo_platform::Cfg;
use cargo_util::paths;
use cargo_util::{paths, ProcessError};
use std::collections::hash_map::{Entry, HashMap};
use std::collections::{BTreeSet, HashSet};
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -386,26 +387,24 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
let timestamp = paths::set_invocation_time(&script_run_dir)?;
let prefix = format!("[{} {}] ", id.name(), id.version());
let mut warnings_in_case_of_panic = Vec::new();
let output = cmd
.exec_with_streaming(
&mut |stdout| {
if let Some(warning) = stdout.strip_prefix(CARGO_WARNING) {
warnings_in_case_of_panic.push(warning.to_owned());
}
if extra_verbose {
state.stdout(format!("{}{}", prefix, stdout))?;
}
Ok(())
},
&mut |stderr| {
if extra_verbose {
state.stderr(format!("{}{}", prefix, stderr))?;
}
Ok(())
},
true,
)
.with_context(|| format!("failed to run custom build command for `{}`", pkg_descr));
let output = cmd.exec_with_streaming(
&mut |stdout| {
if let Some(warning) = stdout.strip_prefix(CARGO_WARNING) {
warnings_in_case_of_panic.push(warning.to_owned());
}
if extra_verbose {
state.stdout(format!("{}{}", prefix, stdout))?;
}
Ok(())
},
&mut |stderr| {
if extra_verbose {
state.stderr(format!("{}{}", prefix, stderr))?;
}
Ok(())
},
true,
);

if let Err(error) = output {
insert_warnings_in_build_outputs(
Expand All @@ -414,7 +413,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
metadata_hash,
warnings_in_case_of_panic,
);
return Err(error);
return Err(build_error_with_context(error, &pkg_descr));
}

let output = output.unwrap();
Expand Down Expand Up @@ -496,6 +495,71 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
Ok(job)
}

fn build_error_with_context(error: Error, pkg_descr: &str) -> Error {
let Some(perr) = error.downcast_ref::<ProcessError>() else {
return error.context(format!("failed to run custom build command for `{}`", pkg_descr));
};

let mut msg = format!("custom build command for `{}` has failed", pkg_descr);
let may_be_panic = perr.code == Some(101);

let mut parsing_stdout = false;
let output = perr
.stderr
.as_ref()
.and_then(|stderr| std::str::from_utf8(stderr).ok())
.filter(|stderr| !stderr.trim_start().is_empty())
.or_else(|| {
parsing_stdout = true;
perr.stdout
.as_ref()
.and_then(|stdout| std::str::from_utf8(stdout).ok())
});

if let Some(out) = output {
let mut skipping_backtrace = false;
for mut line in out.lines() {
// Cargo directives aren't part of the error message text (except cargo:warning, which is reported separately).
// Some scripts print a lot of cargo:rerun-if-env-changed that obscure the root cause of the failure.
if parsing_stdout && line.starts_with("cargo:") {
continue;
}

if !parsing_stdout && may_be_panic {
// Panics print a backtrace, but the location within the build script is not interesting to downstream users
if line == "stack backtrace:" {
skipping_backtrace = true;
continue;
}
if line == "note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace." || line == "note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace" {
skipping_backtrace = false;
continue;
}
if skipping_backtrace {
if line.starts_with(" ") {
continue;
}
skipping_backtrace = false;
}
// Build scripts tend to unwrap() errors. This skips the most common panic message boilerplate.
if let Some(rest) = line.strip_prefix("thread 'main' panicked at ") {
line = rest;
if let Some(rest) =
line.strip_prefix("'called `Result::unwrap()` on an `Err` value: ")
{
msg.push('\''); // re-add stripped quote start
line = rest;
}
}
}

msg.push_str("\n ");
msg.push_str(line);
}
}
Error::from(VerboseError::new(error)).context(msg)
}

fn insert_warnings_in_build_outputs(
build_script_outputs: Arc<Mutex<BuildScriptOutputs>>,
id: PackageId,
Expand Down
3 changes: 1 addition & 2 deletions src/cargo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,8 @@ fn _display_error(err: &Error, shell: &mut Shell, as_err: bool) -> bool {
for (i, err) in err.chain().enumerate() {
// If we're not in verbose mode then only print cause chain until one
// marked as `VerboseError` appears.
//
// Generally the top error shouldn't be verbose, but check it anyways.
if shell.verbosity() != Verbose && err.is::<VerboseError>() {
drop(shell.note("for more details, run again with '--verbose'"));
return true;
}
if err.is::<AlreadyPrintedError>() {
Expand Down
36 changes: 34 additions & 2 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::io;
use std::thread;

#[cargo_test]
fn custom_build_script_failed() {
fn custom_build_script_failed_verbose() {
let p = project()
.file(
"Cargo.toml",
Expand All @@ -38,14 +38,46 @@ fn custom_build_script_failed() {
[COMPILING] foo v0.5.0 ([CWD])
[RUNNING] `rustc --crate-name build_script_build build.rs [..]--crate-type bin [..]`
[RUNNING] `[..]/build-script-build`
[ERROR] failed to run custom build command for `foo v0.5.0 ([CWD])`
[ERROR] custom build command for `foo v0.5.0 ([CWD])` has failed

Caused by:
process didn't exit successfully: `[..]/build-script-build` (exit [..]: 101)",
)
.run();
}

#[cargo_test]
fn custom_build_script_failed_terse() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]

name = "foo"
version = "0.5.0"
authors = ["[email protected]"]
build = "build.rs"
"#,
)
.file("src/main.rs", "fn main() {}")
.file(
"build.rs",
"fn main() { println!(\"cargo:noise=1\netc\"); panic!(\"oops\"); }",
)
.build();
p.cargo("build")
.with_status(101)
.with_stderr(
"\
[COMPILING] foo v0.5.0 ([CWD])
[ERROR] custom build command for `foo v0.5.0 ([CWD])` has failed
'oops', build.rs:2:8
[NOTE] for more details, run again with '--verbose'",
)
.run();
}

#[cargo_test]
fn custom_build_env_vars() {
let p = project()
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1281,6 +1281,7 @@ fn check_fixable_error_no_fix() {
{}\
[WARNING] `foo` (lib) generated 1 warning
[ERROR] could not compile `foo` due to previous error; 1 warning emitted
[NOTE] for more details, run again with '--verbose'
",
rustc_message
);
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,7 @@ fn compile_failure() {
.with_stderr_contains(
"\
[ERROR] could not compile `foo` due to previous error
[NOTE] for more details, run again with '--verbose'
[ERROR] failed to compile `foo v0.0.1 ([..])`, intermediate artifacts can be \
found at `[..]target`
",
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ fn deduplicate_errors() {
"\
[COMPILING] foo v0.0.1 [..]
{}error: could not compile `foo` due to previous error
[NOTE] for more details, run again with '--verbose'
",
rustc_message
))
Expand Down