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

feat(build-rs): Add the 'error' directive #14910

Merged
merged 8 commits into from
Dec 9, 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
44 changes: 0 additions & 44 deletions crates/build-rs/src/allow_use.rs

This file was deleted.

13 changes: 10 additions & 3 deletions crates/build-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,20 @@ macro_rules! unstable {
};
}

macro_rules! msrv {
macro_rules! respected_msrv {
($ver:literal) => {
concat!("> MSRV: Respected as of ", $ver, ".")
concat!(
r#"<div class="warning">

MSRV: Respected as of "#,
$ver,
r#".

</div>"#
)
};
}

mod allow_use;
mod ident;

pub mod input;
Expand Down
78 changes: 43 additions & 35 deletions crates/build-rs/src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@
//!
//! Reference: <https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script>

use crate::{
allow_use,
ident::{is_ascii_ident, is_ident},
};
use std::{ffi::OsStr, fmt::Display, fmt::Write, path::Path, str};
use std::ffi::OsStr;
use std::path::Path;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to automate this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rustfmt feature is unstable :(

use std::{fmt::Display, fmt::Write as _};

use crate::ident::{is_ascii_ident, is_ident};

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
Expand Down Expand Up @@ -171,7 +167,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
Expand Down Expand Up @@ -301,7 +297,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() {
Expand All @@ -313,13 +309,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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty much unrelated to the error directive and may have other impacts (which is extremely rare I also believe). While I am fine with each change in this PR, should they be broken down to multiple PRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MSRV for this crate is 1.81. check-cfg was added in 1.80. This would only be a concern if/when we drop the MSRV further.

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
Expand All @@ -332,7 +326,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) {
Expand All @@ -343,17 +337,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
Expand Down Expand Up @@ -411,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.
///
/// <div class="warning">
///
/// 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`].
///
/// </div>
#[doc = respected_msrv!("1.84")]
#[track_caller]
pub fn error(message: &str) {
if message.contains('\n') {
panic!("cannot emit warning: message contains newline");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
panic!("cannot emit warning: message contains newline");
panic!("cannot emit error: message contains newline");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅
Nice catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled in #14913

}
emit("error", message);
}

/// Metadata, used by `links` scripts.
#[track_caller]
pub fn metadata(key: &str, val: &str) {
Expand All @@ -421,9 +433,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));
}
8 changes: 4 additions & 4 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
}
Expand All @@ -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(","));
}

Expand Down
Loading