From 6c5d2582ea7b1eb77d546ead0e97f5844cf07d77 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 6 Dec 2024 09:16:19 -0600 Subject: [PATCH 1/8] refactor(build-scripts): Use references for creating CARGO_CFG --- src/cargo/core/compiler/custom_build.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 67e385688c0..a9928ab58c6 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -339,11 +339,11 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul for cfg in bcx.target_data.cfg(unit.kind) { match *cfg { Cfg::Name(ref n) => { - cfg_map.insert(n.clone(), Vec::new()); + cfg_map.insert(n.as_str(), Vec::new()); } Cfg::KeyPair(ref k, ref v) => { - let values = cfg_map.entry(k.clone()).or_default(); - values.push(v.clone()); + let values = cfg_map.entry(k.as_str()).or_default(); + values.push(v.as_str()); } } } @@ -355,7 +355,7 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul } // FIXME: We should handle raw-idents somehow instead of predenting they // don't exist here - let k = format!("CARGO_CFG_{}", super::envify(k.as_str())); + let k = format!("CARGO_CFG_{}", super::envify(k)); cmd.env(&k, v.join(",")); } From 19bb28e64dbab5f52e6dedda8415e5dc1a56e7ed Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 6 Dec 2024 09:57:18 -0600 Subject: [PATCH 2/8] refactor(build-rs): Clarify MSRV policy being added --- crates/build-rs/src/lib.rs | 2 +- crates/build-rs/src/output.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/build-rs/src/lib.rs b/crates/build-rs/src/lib.rs index 56a0df3e820..12d51dc1e31 100644 --- a/crates/build-rs/src/lib.rs +++ b/crates/build-rs/src/lib.rs @@ -26,7 +26,7 @@ macro_rules! unstable { }; } -macro_rules! msrv { +macro_rules! respected_msrv { ($ver:literal) => { concat!("> MSRV: Respected as of ", $ver, ".") }; diff --git a/crates/build-rs/src/output.rs b/crates/build-rs/src/output.rs index c275338c611..d50f7d349db 100644 --- a/crates/build-rs/src/output.rs +++ b/crates/build-rs/src/output.rs @@ -301,7 +301,7 @@ pub fn rustc_cfg_value(key: &str, value: &str) { /// and other mistakes. /// /// [`unexpected_cfgs`]: https://doc.rust-lang.org/rustc/lints/listing/warn-by-default.html#unexpected-cfgs -#[doc = msrv!("1.80")] +#[doc = respected_msrv!("1.80")] #[track_caller] pub fn rustc_check_cfgs(keys: &[&str]) { if keys.is_empty() { @@ -332,7 +332,7 @@ pub fn rustc_check_cfgs(keys: &[&str]) { /// and other mistakes. /// /// [`unexpected_cfgs`]: https://doc.rust-lang.org/rustc/lints/listing/warn-by-default.html#unexpected-cfgs -#[doc = msrv!("1.80")] +#[doc = respected_msrv!("1.80")] #[track_caller] pub fn rustc_check_cfg_values(key: &str, values: &[&str]) { if !is_ident(key) { From 0f4e698785ba35e0d39c37023d5bc0c9fa12c735 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 6 Dec 2024 10:01:13 -0600 Subject: [PATCH 3/8] docs(build-rs): Make MSRV stick out --- crates/build-rs/src/lib.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/build-rs/src/lib.rs b/crates/build-rs/src/lib.rs index 12d51dc1e31..f2339e7e842 100644 --- a/crates/build-rs/src/lib.rs +++ b/crates/build-rs/src/lib.rs @@ -28,7 +28,15 @@ macro_rules! unstable { macro_rules! respected_msrv { ($ver:literal) => { - concat!("> MSRV: Respected as of ", $ver, ".") + concat!( + r#"
+ +MSRV: Respected as of "#, + $ver, + r#". + +
"# + ) }; } From f9ef2c547c93d42a776a41b377bf2f28eaa7fb9f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 9 Dec 2024 09:22:40 -0600 Subject: [PATCH 4/8] docs(build-rs): Backtick literal values --- crates/build-rs/src/output.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/build-rs/src/output.rs b/crates/build-rs/src/output.rs index d50f7d349db..7a9bcc4270b 100644 --- a/crates/build-rs/src/output.rs +++ b/crates/build-rs/src/output.rs @@ -171,7 +171,7 @@ pub fn rustc_link_arg_benches(flag: &str) { /// to the symbols from the given lib, and the binary should access them through /// the library target’s public API. /// -/// The optional `KIND` may be one of dylib, static, or framework. See the +/// The optional `KIND` may be one of `dylib`, `static`, or `framework`. See the /// [rustc book][-l] for more detail. /// /// [-l]: https://doc.rust-lang.org/stable/rustc/command-line-arguments.html#option-l-link-lib From 0f621016768887957ccc67eec14e042d8fe33bac Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 9 Dec 2024 10:04:21 -0600 Subject: [PATCH 5/8] perf(build-rs): Always emit :: directives Our MSRV is much higher than 1.77. Also, as time goes on, there is less incentive to drop it below 1.77, especially with the MSRV-aware resolver in 1.84 --- crates/build-rs/src/allow_use.rs | 14 -------------- crates/build-rs/src/output.rs | 12 ++---------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/crates/build-rs/src/allow_use.rs b/crates/build-rs/src/allow_use.rs index 7818e722edb..115441be853 100644 --- a/crates/build-rs/src/allow_use.rs +++ b/crates/build-rs/src/allow_use.rs @@ -1,14 +1,5 @@ use std::{process::Command, sync::OnceLock}; -fn rust_version_minor() -> u32 { - static VERSION_MINOR: OnceLock = OnceLock::new(); - *VERSION_MINOR.get_or_init(|| { - version_minor(&crate::input::cargo_pkg_rust_version().unwrap_or_default()) - // assume build-rs's MSRV if none specified for the current package - .unwrap_or_else(|| version_minor(env!("CARGO_PKG_RUST_VERSION")).unwrap()) - }) -} - fn cargo_version_minor() -> u32 { static VERSION_MINOR: OnceLock = OnceLock::new(); *VERSION_MINOR.get_or_init(|| { @@ -33,11 +24,6 @@ fn version_minor(version: &str) -> Option { Some(minor) } -pub(crate) fn double_colon_directives() -> bool { - // cargo errors on `cargo::` directives with insufficient package.rust-version - rust_version_minor() >= 77 -} - pub(crate) fn check_cfg() -> bool { // emit check-cfg if the toolchain being used supports it cargo_version_minor() >= 80 diff --git a/crates/build-rs/src/output.rs b/crates/build-rs/src/output.rs index 7a9bcc4270b..dcdafeaef24 100644 --- a/crates/build-rs/src/output.rs +++ b/crates/build-rs/src/output.rs @@ -13,11 +13,7 @@ use crate::{ use std::{ffi::OsStr, fmt::Display, fmt::Write, path::Path, str}; fn emit(directive: &str, value: impl Display) { - if allow_use::double_colon_directives() { - println!("cargo::{}={}", directive, value); - } else { - println!("cargo:{}={}", directive, value); - } + println!("cargo::{}={}", directive, value); } /// The `rerun-if-changed` instruction tells Cargo to re-run the build script if the @@ -421,9 +417,5 @@ pub fn metadata(key: &str, val: &str) { panic!("cannot emit metadata: invalid value {val:?}"); } - if allow_use::double_colon_directives() { - emit("metadata", format_args!("{}={}", key, val)); - } else { - emit(key, val); - } + emit("metadata", format_args!("{}={}", key, val)); } From 5e833bfbbb232663232cefaa85073200e8464fd4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 9 Dec 2024 10:05:42 -0600 Subject: [PATCH 6/8] perf(build-rs): Always emit check-cfg These didn't require an MSRV bump. Worse case, some metadata will be emitted but that shouldn't impact things too negatively. --- crates/build-rs/src/allow_use.rs | 30 ----------------------------- crates/build-rs/src/lib.rs | 1 - crates/build-rs/src/output.rs | 33 +++++++++++++------------------- 3 files changed, 13 insertions(+), 51 deletions(-) delete mode 100644 crates/build-rs/src/allow_use.rs diff --git a/crates/build-rs/src/allow_use.rs b/crates/build-rs/src/allow_use.rs deleted file mode 100644 index 115441be853..00000000000 --- a/crates/build-rs/src/allow_use.rs +++ /dev/null @@ -1,30 +0,0 @@ -use std::{process::Command, sync::OnceLock}; - -fn cargo_version_minor() -> u32 { - static VERSION_MINOR: OnceLock = OnceLock::new(); - *VERSION_MINOR.get_or_init(|| { - let out = Command::new(crate::input::cargo()) - .arg("-V") - .output() - .expect("running `cargo -V` should succeed"); - assert!(out.status.success(), "running `cargo -V` should succeed"); - - // > cargo -V # example output - // cargo 1.82.0 (8f40fc59f 2024-08-21) - - let out = std::str::from_utf8(&out.stdout).expect("`cargo -V` should output valid UTF-8"); - let version = out.split(' ').nth(1).unwrap(); - version_minor(version).unwrap() - }) -} - -fn version_minor(version: &str) -> Option { - let minor = version.split('.').nth(1)?; - let minor = minor.parse().unwrap(); - Some(minor) -} - -pub(crate) fn check_cfg() -> bool { - // emit check-cfg if the toolchain being used supports it - cargo_version_minor() >= 80 -} diff --git a/crates/build-rs/src/lib.rs b/crates/build-rs/src/lib.rs index f2339e7e842..bc511f6f629 100644 --- a/crates/build-rs/src/lib.rs +++ b/crates/build-rs/src/lib.rs @@ -40,7 +40,6 @@ MSRV: Respected as of "#, }; } -mod allow_use; mod ident; pub mod input; diff --git a/crates/build-rs/src/output.rs b/crates/build-rs/src/output.rs index dcdafeaef24..573f0b2d8d8 100644 --- a/crates/build-rs/src/output.rs +++ b/crates/build-rs/src/output.rs @@ -6,10 +6,7 @@ //! //! Reference: -use crate::{ - allow_use, - ident::{is_ascii_ident, is_ident}, -}; +use crate::ident::{is_ascii_ident, is_ident}; use std::{ffi::OsStr, fmt::Display, fmt::Write, path::Path, str}; fn emit(directive: &str, value: impl Display) { @@ -309,13 +306,11 @@ pub fn rustc_check_cfgs(keys: &[&str]) { } } - if allow_use::check_cfg() { - let mut directive = keys[0].to_string(); - for key in &keys[1..] { - write!(directive, ", {key}").expect("writing to string should be infallible"); - } - emit("rustc-check-cfg", format_args!("cfg({directive})")); + let mut directive = keys[0].to_string(); + for key in &keys[1..] { + write!(directive, ", {key}").expect("writing to string should be infallible"); } + emit("rustc-check-cfg", format_args!("cfg({directive})")); } /// Add to the list of expected config names that is used when checking the @@ -339,17 +334,15 @@ pub fn rustc_check_cfg_values(key: &str, values: &[&str]) { return; } - if allow_use::check_cfg() { - let mut directive = format!("\"{}\"", values[0].escape_default()); - for value in &values[1..] { - write!(directive, ", \"{}\"", value.escape_default()) - .expect("writing to string should be infallible"); - } - emit( - "rustc-check-cfg", - format_args!("cfg({key}, values({directive}))"), - ); + let mut directive = format!("\"{}\"", values[0].escape_default()); + for value in &values[1..] { + write!(directive, ", \"{}\"", value.escape_default()) + .expect("writing to string should be infallible"); } + emit( + "rustc-check-cfg", + format_args!("cfg({key}, values({directive}))"), + ); } /// The `rustc-env` instruction tells Cargo to set the given environment variable From 260fcab3b35928cd3a5115accd2c59a5cf63f46b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 9 Dec 2024 10:10:28 -0600 Subject: [PATCH 7/8] refactor(build-rs): Clean up 'use's --- crates/build-rs/src/output.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/build-rs/src/output.rs b/crates/build-rs/src/output.rs index 573f0b2d8d8..37b32908cee 100644 --- a/crates/build-rs/src/output.rs +++ b/crates/build-rs/src/output.rs @@ -6,8 +6,11 @@ //! //! Reference: +use std::ffi::OsStr; +use std::path::Path; +use std::{fmt::Display, fmt::Write as _}; + use crate::ident::{is_ascii_ident, is_ident}; -use std::{ffi::OsStr, fmt::Display, fmt::Write, path::Path, str}; fn emit(directive: &str, value: impl Display) { println!("cargo::{}={}", directive, value); From 5fa8a686faf81e01916985901e371bde50e94b07 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 9 Dec 2024 09:30:57 -0600 Subject: [PATCH 8/8] feat(build-rs): Add the 'error' directive --- crates/build-rs/src/output.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/build-rs/src/output.rs b/crates/build-rs/src/output.rs index 37b32908cee..013bc4fc125 100644 --- a/crates/build-rs/src/output.rs +++ b/crates/build-rs/src/output.rs @@ -403,6 +403,26 @@ pub fn warning(message: &str) { emit("warning", message); } +/// The `error` instruction tells Cargo to display an error after the build script has finished +/// running, and then fail the build. +/// +///
+/// +/// Build script libraries should carefully consider if they want to use [`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 [`error`]. +/// +///
+#[doc = respected_msrv!("1.84")] +#[track_caller] +pub fn error(message: &str) { + if message.contains('\n') { + panic!("cannot emit warning: message contains newline"); + } + emit("error", message); +} + /// Metadata, used by `links` scripts. #[track_caller] pub fn metadata(key: &str, val: &str) {