Skip to content

Commit

Permalink
Auto merge of #14435 - tweag:build-script-errors, r=epage
Browse files Browse the repository at this point in the history 
Allow build scripts to report error messages through `cargo::error`

Related to #10159.

Adds the `cargo::error` build script directive. It is similar to `cargo::warning`, but it emits an error message and it also fails the build.
  • Loading branch information
@bors
bors committed Sep 27, 2024
2 parents b396f2c + f94b442 commit 3395029
Show file tree
Hide file tree
Showing 5 changed files with 374 additions and 69 deletions.
90 changes: 64 additions & 26 deletions src/cargo/core/compiler/custom_build.rs
View file
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
View file
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
13 changes: 11 additions & 2 deletions src/doc/src/reference/build-scripts.md
View file
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,21 @@ 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.

> **MSRV:** Respected as of 1.82
### `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

0 comments on commit 3395029

Please sign in to comment.