Skip to content

Commit

Permalink
feat(build-rs): Add the 'error' directive (#14910)
Browse files Browse the repository at this point in the history
### What does this PR try to resolve?

My previous audit mostly focused on env variables and missed that
`cargo::error` hadn't been added yet.

This also includes other clean up along the way to #14902 and fixing of
some env variable handling that I'm working towards.

### How should we test and review this PR?

### Additional information
  • Loading branch information
weihanglo authored Dec 9, 2024
2 parents 58c2374 + 5fa8a68 commit bcc217d
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 86 deletions.
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;
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();
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");
}
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

0 comments on commit bcc217d

Please sign in to comment.