diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index f36e741c65b..4ee788df573 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -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. /// @@ -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 { @@ -82,11 +96,13 @@ pub struct BuildOutput { pub rerun_if_changed: Vec, /// Environment variables which, when changed, will cause a rebuild. pub rerun_if_env_changed: Vec, - /// 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, + /// 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, } /// Map of packages to build script output. @@ -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))?; @@ -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(); @@ -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>, id: PackageId, metadata_hash: Metadata, - warnings: Vec, + log_messages: Vec, ) { - 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 { @@ -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 @@ -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 \ .", - syntax_prefix, key, pkg_descr + syntax_prefix, key, pkg_descr + ), )); } linker_args.push((LinkArgTarget::Cdylib, value)) @@ -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. @@ -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" => { @@ -988,7 +1026,7 @@ impl BuildOutput { metadata, rerun_if_changed, rerun_if_env_changed, - warnings, + log_messages, }) } diff --git a/src/cargo/core/compiler/job_queue/mod.rs b/src/cargo/core/compiler/job_queue/mod.rs index 6c55697c1a0..0607a830596 100644 --- a/src/cargo/core/compiler/job_queue/mod.rs +++ b/src/cargo/core/compiler/job_queue/mod.rs @@ -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; @@ -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, @@ -962,11 +963,11 @@ 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 { @@ -974,21 +975,25 @@ impl<'gctx> DrainState<'gctx> { }; 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))?; + } + } } } } @@ -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 { diff --git a/src/doc/src/reference/build-scripts.md b/src/doc/src/reference/build-scripts.md index d8d359869a2..6c80a78e10c 100644 --- a/src/doc/src/reference/build-scripts.md +++ b/src/doc/src/reference/build-scripts.md @@ -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` @@ -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 diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index e9fda1d5be9..022d6e96fe3 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -3871,14 +3871,106 @@ fn warnings_emitted() { ) .build(); - p.cargo("build -v") + p.cargo("build") .with_stderr_data(str![[r#" [COMPILING] foo v0.5.0 ([ROOT]/foo) -[RUNNING] `rustc --crate-name build_script_build [..]` -[RUNNING] `[ROOT]/foo/target/debug/build/foo-[HASH]/build-script-build` [WARNING] foo@0.5.0: foo [WARNING] foo@0.5.0: bar -[RUNNING] `rustc --crate-name foo [..]` +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} + +#[cargo_test] +fn errors_and_warnings_emitted_and_build_failed() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.5.0" + edition = "2015" + authors = [] + build = "build.rs" + "#, + ) + .file("src/lib.rs", "") + .file( + "build.rs", + r#" + fn main() { + println!("cargo::warning=foo"); + println!("cargo::warning=bar"); + println!("cargo::error=foo err"); + println!("cargo::error=bar err"); + } + "#, + ) + .build(); + + p.cargo("build") + .with_status(101) + .with_stderr_data(str![[r#" +[COMPILING] foo v0.5.0 ([ROOT]/foo) +[WARNING] foo@0.5.0: foo +[WARNING] foo@0.5.0: bar +[ERROR] foo@0.5.0: foo err +[ERROR] foo@0.5.0: bar err +[ERROR] build script logged errors + +"#]]) + .run(); +} + +#[cargo_test] +fn warnings_emitted_from_path_dep() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.5.0" + edition = "2015" + authors = [] + + [dependencies] + a = { path = "a" } + "#, + ) + .file("src/lib.rs", "") + .file( + "a/Cargo.toml", + r#" + [package] + name = "a" + version = "0.5.0" + edition = "2015" + authors = [] + build = "build.rs" + "#, + ) + .file("a/src/lib.rs", "") + .file( + "a/build.rs", + r#" + fn main() { + println!("cargo::warning=foo"); + println!("cargo::warning=bar"); + } + "#, + ) + .build(); + + p.cargo("build") + .with_stderr_data(str![[r#" +[LOCKING] 1 package to latest compatible version +[COMPILING] a v0.5.0 ([ROOT]/foo/a) +[WARNING] a@0.5.0: foo +[WARNING] a@0.5.0: bar +[COMPILING] foo v0.5.0 ([ROOT]/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) @@ -3916,10 +4008,156 @@ fn warnings_emitted_when_build_script_panics() { .with_status(101) .with_stdout_data("") .with_stderr_data(str![[r#" -... +[COMPILING] foo v0.5.0 ([ROOT]/foo) [WARNING] foo@0.5.0: foo [WARNING] foo@0.5.0: bar -... +[ERROR] failed to run custom build command for `foo v0.5.0 ([ROOT]/foo)` + +Caused by: + process didn't exit successfully: `[ROOT]/foo/target/debug/build/foo-[HASH]/build-script-build` ([EXIT_STATUS]: 101) + --- stdout + cargo::warning=foo + cargo::warning=bar + + --- stderr + thread 'main' panicked at build.rs:5:21: + explicit panic + [NOTE] run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +"#]]) + .run(); +} + +#[cargo_test] +fn warnings_emitted_when_dependency_panics() { + Package::new("published", "0.1.0") + .file( + "build.rs", + r#" + fn main() { + println!("cargo::warning=foo"); + println!("cargo::warning=bar"); + panic!(); + } + "#, + ) + .file( + "Cargo.toml", + r#" + [package] + name = "published" + version = "0.1.0" + edition = "2015" + authors = [] + build = "build.rs" + "#, + ) + .file("src/lib.rs", "") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.5.0" + edition = "2015" + authors = [] + + [dependencies] + published = "*" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build") + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[LOCKING] 1 package to latest compatible version +[DOWNLOADING] crates ... +[DOWNLOADED] published v0.1.0 (registry `dummy-registry`) +[COMPILING] published v0.1.0 +[WARNING] published@0.1.0: foo +[WARNING] published@0.1.0: bar +[ERROR] failed to run custom build command for `published v0.1.0` + +Caused by: + process didn't exit successfully: `[ROOT]/foo/target/debug/build/published-[HASH]/build-script-build` ([EXIT_STATUS]: 101) + --- stdout + cargo::warning=foo + cargo::warning=bar + + --- stderr + thread 'main' panicked at [ROOT]/home/.cargo/registry/src/-[HASH]/published-0.1.0/build.rs:5:21: + explicit panic + [NOTE] run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +"#]]) + .run(); +} + +#[cargo_test] +fn log_messages_emitted_when_dependency_logs_errors() { + Package::new("published", "0.1.0") + .file( + "build.rs", + r#" + fn main() { + println!("cargo::warning=foo"); + println!("cargo::warning=bar"); + println!("cargo::error=foo err"); + println!("cargo::error=bar err"); + } + "#, + ) + .file( + "Cargo.toml", + r#" + [package] + name = "published" + version = "0.1.0" + edition = "2015" + authors = [] + build = "build.rs" + "#, + ) + .file("src/lib.rs", "") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.5.0" + edition = "2015" + authors = [] + + [dependencies] + published = "*" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build") + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[LOCKING] 1 package to latest compatible version +[DOWNLOADING] crates ... +[DOWNLOADED] published v0.1.0 (registry `dummy-registry`) +[COMPILING] published v0.1.0 +[WARNING] published@0.1.0: foo +[WARNING] published@0.1.0: bar +[ERROR] published@0.1.0: foo err +[ERROR] published@0.1.0: bar err +[ERROR] build script logged errors + "#]]) .run(); } diff --git a/tests/testsuite/warn_on_failure.rs b/tests/testsuite/warn_on_failure.rs index d30257e4efb..74d1706b1e1 100644 --- a/tests/testsuite/warn_on_failure.rs +++ b/tests/testsuite/warn_on_failure.rs @@ -86,13 +86,18 @@ fn no_warning_on_bin_failure() { .cargo("build") .with_status(101) .with_stdout_does_not_contain("hidden stdout") - .with_stderr_does_not_contain("hidden stderr") - .with_stderr_does_not_contain(&format!("[WARNING] {}", WARNING1)) - .with_stderr_does_not_contain(&format!("[WARNING] {}", WARNING2)) - .with_stderr_contains("[UPDATING] `[..]` index") - .with_stderr_contains("[DOWNLOADED] bar v0.0.1 ([..])") - .with_stderr_contains("[COMPILING] bar v0.0.1") - .with_stderr_contains("[COMPILING] foo v0.0.1 ([..])") + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[LOCKING] 1 package to latest compatible version +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.0.1 (registry `dummy-registry`) +[COMPILING] bar v0.0.1 +[COMPILING] foo v0.0.1 ([ROOT]/foo) +error[E0425]: cannot find function `hi` in this scope +... +[ERROR] could not compile `foo` (bin "foo") due to 1 previous error + +"#]]) .run(); } @@ -105,12 +110,18 @@ fn warning_on_lib_failure() { .cargo("build") .with_status(101) .with_stdout_does_not_contain("hidden stdout") - .with_stderr_does_not_contain("hidden stderr") - .with_stderr_does_not_contain("[COMPILING] foo v0.0.1 ([..])") - .with_stderr_contains("[UPDATING] `[..]` index") - .with_stderr_contains("[DOWNLOADED] bar v0.0.1 ([..])") - .with_stderr_contains("[COMPILING] bar v0.0.1") - .with_stderr_contains(&format!("[WARNING] bar@0.0.1: {}", WARNING1)) - .with_stderr_contains(&format!("[WARNING] bar@0.0.1: {}", WARNING2)) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[LOCKING] 1 package to latest compatible version +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.0.1 (registry `dummy-registry`) +[COMPILING] bar v0.0.1 +error[E0425]: cannot find function `err` in this scope +... +[WARNING] bar@0.0.1: Hello! I'm a warning. :) +[WARNING] bar@0.0.1: And one more! +[ERROR] could not compile `bar` (lib) due to 1 previous error + +"#]]) .run(); }