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

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<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.
@@ -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<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 {
@@ -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 \
                              <https://github.com/rust-lang/cargo/issues/9562>.",
-                            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,
         })
     }
 

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,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))?;
+                        }
+                    }
                 }
             }
         }
@@ -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 {

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
 

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();
 }

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();
 }