Skip to content
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

Allow build scripts to report error messages through cargo::error #14743

Merged
merged 8 commits into from
Oct 29, 2024
Merged
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
90 changes: 64 additions & 26 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ use std::path::{Path, PathBuf};
use std::str::{self, FromStr};
use std::sync::{Arc, Mutex};

/// A build script instruction that tells Cargo to display an error after the
/// build script has finished running. Read [the doc] for more.
///
/// [the doc]: https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#cargo-error
const CARGO_ERROR_SYNTAX: &str = "cargo::error=";
/// Deprecated: A build script instruction that tells Cargo to display a warning after the
/// build script has finished running. Read [the doc] for more.
///
Expand All @@ -60,6 +65,15 @@ const OLD_CARGO_WARNING_SYNTAX: &str = "cargo:warning=";
///
/// [the doc]: https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#cargo-warning
const NEW_CARGO_WARNING_SYNTAX: &str = "cargo::warning=";

#[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
pub enum Severity {
Error,
Warning,
}

pub type LogMessage = (Severity, String);

/// Contains the parsed output of a custom build script.
#[derive(Clone, Debug, Hash, Default, PartialEq, Eq, PartialOrd, Ord)]
pub struct BuildOutput {
Expand All @@ -82,11 +96,13 @@ pub struct BuildOutput {
pub rerun_if_changed: Vec<PathBuf>,
/// Environment variables which, when changed, will cause a rebuild.
pub rerun_if_env_changed: Vec<String>,
/// Warnings generated by this build.
/// Errors and warnings generated by this build.
///
/// These are only displayed if this is a "local" package, `-vv` is used,
/// or there is a build error for any target in this package.
pub warnings: Vec<String>,
/// These are only displayed if this is a "local" package, `-vv` is used, or
/// there is a build error for any target in this package. Note that any log
/// message of severity `Error` will by itself cause a build error, and will
/// cause all log messages to be displayed.
pub log_messages: Vec<LogMessage>,
}

/// Map of packages to build script output.
Expand Down Expand Up @@ -474,15 +490,18 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
state.running(&cmd);
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 mut log_messages_in_case_of_panic = Vec::new();
let output = cmd
.exec_with_streaming(
&mut |stdout| {
if let Some(error) = stdout.strip_prefix(CARGO_ERROR_SYNTAX) {
log_messages_in_case_of_panic.push((Severity::Error, error.to_owned()));
}
if let Some(warning) = stdout
.strip_prefix(OLD_CARGO_WARNING_SYNTAX)
.or(stdout.strip_prefix(NEW_CARGO_WARNING_SYNTAX))
{
warnings_in_case_of_panic.push(warning.to_owned());
log_messages_in_case_of_panic.push((Severity::Warning, warning.to_owned()));
}
if extra_verbose {
state.stdout(format!("{}{}", prefix, stdout))?;
Expand Down Expand Up @@ -522,15 +541,29 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
build_error_context
});

// If the build failed
if let Err(error) = output {
insert_warnings_in_build_outputs(
insert_log_messages_in_build_outputs(
build_script_outputs,
id,
metadata_hash,
warnings_in_case_of_panic,
log_messages_in_case_of_panic,
);
return Err(error);
}
// ... or it logged any errors
else if log_messages_in_case_of_panic
.iter()
.any(|(severity, _)| *severity == Severity::Error)
{
insert_log_messages_in_build_outputs(
build_script_outputs,
id,
metadata_hash,
log_messages_in_case_of_panic,
);
anyhow::bail!("build script logged errors");
}

let output = output.unwrap();

Expand Down Expand Up @@ -611,22 +644,23 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
Ok(job)
}

/// When a build script run fails, store only warnings and nuke other outputs,
/// as they are likely broken.
fn insert_warnings_in_build_outputs(
/// When a build script run fails, store only log messages, and nuke other
/// outputs, as they are likely broken.
fn insert_log_messages_in_build_outputs(
build_script_outputs: Arc<Mutex<BuildScriptOutputs>>,
id: PackageId,
metadata_hash: Metadata,
warnings: Vec<String>,
log_messages: Vec<LogMessage>,
) {
let build_output_with_only_warnings = BuildOutput {
warnings,
let build_output_with_only_log_messages = BuildOutput {
log_messages,
..BuildOutput::default()
};
build_script_outputs
.lock()
.unwrap()
.insert(id, metadata_hash, build_output_with_only_warnings);
build_script_outputs.lock().unwrap().insert(
id,
metadata_hash,
build_output_with_only_log_messages,
);
}

impl BuildOutput {
Expand Down Expand Up @@ -678,7 +712,7 @@ impl BuildOutput {
let mut metadata = Vec::new();
let mut rerun_if_changed = Vec::new();
let mut rerun_if_env_changed = Vec::new();
let mut warnings = Vec::new();
let mut log_messages = Vec::new();
let whence = format!("build script of `{}`", pkg_descr);
// Old syntax:
// cargo:rustc-flags=VALUE
Expand Down Expand Up @@ -850,15 +884,18 @@ impl BuildOutput {
"rustc-link-search" => library_paths.push(PathBuf::from(value)),
"rustc-link-arg-cdylib" | "rustc-cdylib-link-arg" => {
if !targets.iter().any(|target| target.is_cdylib()) {
warnings.push(format!(
"{}{} was specified in the build script of {}, \
log_messages.push((
Severity::Warning,
format!(
"{}{} was specified in the build script of {}, \
but that package does not contain a cdylib target\n\
\n\
Allowing this was an unintended change in the 1.50 \
release, and may become an error in the future. \
For more information, see \
<https://github.com/rust-lang/cargo/issues/9562>.",
syntax_prefix, key, pkg_descr
syntax_prefix, key, pkg_descr
),
));
}
linker_args.push((LinkArgTarget::Cdylib, value))
Expand Down Expand Up @@ -944,10 +981,10 @@ impl BuildOutput {
if nightly_features_allowed
|| rustc_bootstrap_allows(library_name.as_deref())
{
warnings.push(format!("Cannot set `RUSTC_BOOTSTRAP={}` from {}.\n\
log_messages.push((Severity::Warning, format!("Cannot set `RUSTC_BOOTSTRAP={}` from {}.\n\
note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.",
val, whence
));
)));
} else {
// Setting RUSTC_BOOTSTRAP would change the behavior of the crate.
// Abort with an error.
Expand All @@ -963,7 +1000,8 @@ impl BuildOutput {
env.push((key, val));
}
}
"warning" => warnings.push(value.to_string()),
"error" => log_messages.push((Severity::Error, value.to_string())),
"warning" => log_messages.push((Severity::Warning, value.to_string())),
"rerun-if-changed" => rerun_if_changed.push(PathBuf::from(value)),
"rerun-if-env-changed" => rerun_if_env_changed.push(value.to_string()),
"metadata" => {
Expand All @@ -988,7 +1026,7 @@ impl BuildOutput {
metadata,
rerun_if_changed,
rerun_if_env_changed,
warnings,
log_messages,
})
}

Expand Down
51 changes: 30 additions & 21 deletions src/cargo/core/compiler/job_queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ pub use self::job::Freshness::{self, Dirty, Fresh};
pub use self::job::{Job, Work};
pub use self::job_state::JobState;
use super::build_runner::OutputFile;
use super::custom_build::Severity;
use super::timings::Timings;
use super::{BuildContext, BuildPlan, BuildRunner, CompileMode, Unit};
use crate::core::compiler::descriptive_pkg_name;
Expand Down Expand Up @@ -684,8 +685,8 @@ impl<'gctx> DrainState<'gctx> {
self.queue.finish(&unit, &artifact);
}
Err(error) => {
let msg = "The following warnings were emitted during compilation:";
self.emit_warnings(Some(msg), &unit, build_runner)?;
let show_warnings = true;
self.emit_log_messages(&unit, build_runner, show_warnings)?;
self.back_compat_notice(build_runner, &unit)?;
return Err(ErrorToHandle {
error,
Expand Down Expand Up @@ -962,33 +963,37 @@ impl<'gctx> DrainState<'gctx> {
}
}

fn emit_warnings(
fn emit_log_messages(
&mut self,
msg: Option<&str>,
unit: &Unit,
build_runner: &mut BuildRunner<'_, '_>,
show_warnings: bool,
) -> CargoResult<()> {
let outputs = build_runner.build_script_outputs.lock().unwrap();
let Some(metadata) = build_runner.find_build_script_metadata(unit) else {
return Ok(());
};
let bcx = &mut build_runner.bcx;
if let Some(output) = outputs.get(metadata) {
if !output.warnings.is_empty() {
if let Some(msg) = msg {
writeln!(bcx.gctx.shell().err(), "{}\n", msg)?;
}

for warning in output.warnings.iter() {
let warning_with_package =
format!("{}@{}: {}", unit.pkg.name(), unit.pkg.version(), warning);

bcx.gctx.shell().warn(warning_with_package)?;
}

if msg.is_some() {
// Output an empty line.
writeln!(bcx.gctx.shell().err())?;
if !output.log_messages.is_empty()
&& (show_warnings
|| output
.log_messages
.iter()
.any(|(severity, _)| *severity == Severity::Error))
{
let msg_with_package =
|msg: &str| format!("{}@{}: {}", unit.pkg.name(), unit.pkg.version(), msg);

for (severity, message) in output.log_messages.iter() {
match severity {
Severity::Error => {
bcx.gctx.shell().error(msg_with_package(message))?;
}
Severity::Warning => {
bcx.gctx.shell().warn(msg_with_package(message))?;
}
}
}
}
}
Expand Down Expand Up @@ -1098,8 +1103,12 @@ impl<'gctx> DrainState<'gctx> {
artifact: Artifact,
build_runner: &mut BuildRunner<'_, '_>,
) -> CargoResult<()> {
if unit.mode.is_run_custom_build() && unit.show_warnings(build_runner.bcx.gctx) {
self.emit_warnings(None, unit, build_runner)?;
if unit.mode.is_run_custom_build() {
self.emit_log_messages(
unit,
build_runner,
unit.show_warnings(build_runner.bcx.gctx),
)?;
}
let unlocked = self.queue.finish(unit, &artifact);
match artifact {
Expand Down
19 changes: 17 additions & 2 deletions src/doc/src/reference/build-scripts.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ one detailed below.
* [`cargo::rustc-env=VAR=VALUE`](#rustc-env) --- Sets an environment variable.
* [`cargo::rustc-cdylib-link-arg=FLAG`](#rustc-cdylib-link-arg) --- Passes custom
flags to a linker for cdylib crates.
- [`cargo::error=MESSAGE`](#cargo-error) --- Displays an error on the terminal.
* [`cargo::warning=MESSAGE`](#cargo-warning) --- Displays a warning on the
terminal.
* [`cargo::metadata=KEY=VALUE`](#the-links-manifest-key) --- Metadata, used by `links`
Expand Down Expand Up @@ -313,13 +314,27 @@ link-arg=FLAG` option][link-arg] to the compiler, but only when building a
`cdylib` library target. Its usage is highly platform specific. It is useful
to set the shared library version or the runtime-path.

### `cargo::error=MESSAGE` {#cargo-error}

The `error` instruction tells Cargo to display an error after the build script
has finished running, and then fail the build.

> Note: Build script libraries should carefully consider if they want to
> use `cargo::error` versus returning a `Result`. It may be better to return
> a `Result`, and allow the caller to decide if the error is fatal or not.
> The caller can then decide whether or not to display the `Err` variant
> using `cargo::error`.

> **MSRV:** Respected as of 1.84

### `cargo::warning=MESSAGE` {#cargo-warning}

The `warning` instruction tells Cargo to display a warning after the build
script has finished running. Warnings are only shown for `path` dependencies
(that is, those you're working on locally), so for example warnings printed
out in [crates.io] crates are not emitted by default. The `-vv` "very verbose"
flag may be used to have Cargo display warnings for all crates.
out in [crates.io] crates are not emitted by default, unless the build fails.
The `-vv` "very verbose" flag may be used to have Cargo display warnings for
all crates.

## Build Dependencies

Expand Down
Loading