Skip to content

Commit

Permalink
Auto merge of #10539 - Urgau:check-cfg-build-script, r=ehuss
Browse files Browse the repository at this point in the history
Add unstable `rustc-check-cfg` build script output

This PR adds a new build script output as unstable behind `-Zcheck-cfg=output`: `rustc-check-cfg`.

### What does this PR try to resolve?

This PR add a way to add to use the unstable `--check-cfg` command line option of `rustc` and `rustdoc`.

It was discover in [Bump bootstrap compiler to 1.61.0 beta](rust-lang/rust#95678 (comment)) that `rustc_llvm` sets some custom `cfg` from a build script and because `--check-cfg=values()` is globally enable in the Rust codebase that cause the compilation to fail. For now no values are checked in stage 0 for the entire codebase which is a shame and should be fixed with the addition of this feature.

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

Commits are separated in: implementation, tests and doc.

Testing should simply be done by adding a valid `cargo:rustc-check-cfg` in a build script.
Watch the added tests or doc to have an example.

### Additional information

This PR is also the logical next step after `-Zcheck-cfg-features`.
  • Loading branch information
bors committed May 20, 2022
2 parents a4c1cd0 + f2a1e43 commit c8c6e33
Show file tree
Hide file tree
Showing 7 changed files with 289 additions and 36 deletions.
8 changes: 8 additions & 0 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,14 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
args.push(cfg.into());
}

if !output.check_cfgs.is_empty() {
args.push("-Zunstable-options".into());
for check_cfg in &output.check_cfgs {
args.push("--check-cfg".into());
args.push(check_cfg.into());
}
}

for (lt, arg) in &output.linker_args {
if lt.applies_to(&unit.target) {
args.push("-C".into());
Expand Down
24 changes: 24 additions & 0 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ pub struct BuildOutput {
pub linker_args: Vec<(LinkType, String)>,
/// Various `--cfg` flags to pass to the compiler.
pub cfgs: Vec<String>,
/// Various `--check-cfg` flags to pass to the compiler.
pub check_cfgs: Vec<String>,
/// Additional environment variables to run the compiler with.
pub env: Vec<(String, String)>,
/// Metadata to pass to the immediate dependencies.
Expand Down Expand Up @@ -322,6 +324,10 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
paths::create_dir_all(&script_out_dir)?;

let nightly_features_allowed = cx.bcx.config.nightly_features_allowed;
let extra_check_cfg = match cx.bcx.config.cli_unstable().check_cfg {
Some((_, _, _, output)) => output,
None => false,
};
let targets: Vec<Target> = unit.pkg.targets().to_vec();
// Need a separate copy for the fresh closure.
let targets_fresh = targets.clone();
Expand Down Expand Up @@ -432,6 +438,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
&pkg_descr,
&script_out_dir,
&script_out_dir,
extra_check_cfg,
nightly_features_allowed,
&targets,
)?;
Expand Down Expand Up @@ -459,6 +466,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
&pkg_descr,
&prev_script_out_dir,
&script_out_dir,
extra_check_cfg,
nightly_features_allowed,
&targets_fresh,
)?,
Expand Down Expand Up @@ -511,6 +519,7 @@ impl BuildOutput {
pkg_descr: &str,
script_out_dir_when_generated: &Path,
script_out_dir: &Path,
extra_check_cfg: bool,
nightly_features_allowed: bool,
targets: &[Target],
) -> CargoResult<BuildOutput> {
Expand All @@ -521,6 +530,7 @@ impl BuildOutput {
pkg_descr,
script_out_dir_when_generated,
script_out_dir,
extra_check_cfg,
nightly_features_allowed,
targets,
)
Expand All @@ -536,13 +546,15 @@ impl BuildOutput {
pkg_descr: &str,
script_out_dir_when_generated: &Path,
script_out_dir: &Path,
extra_check_cfg: bool,
nightly_features_allowed: bool,
targets: &[Target],
) -> CargoResult<BuildOutput> {
let mut library_paths = Vec::new();
let mut library_links = Vec::new();
let mut linker_args = Vec::new();
let mut cfgs = Vec::new();
let mut check_cfgs = Vec::new();
let mut env = Vec::new();
let mut metadata = Vec::new();
let mut rerun_if_changed = Vec::new();
Expand Down Expand Up @@ -669,6 +681,13 @@ impl BuildOutput {
linker_args.push((LinkType::All, value));
}
"rustc-cfg" => cfgs.push(value.to_string()),
"rustc-check-cfg" => {
if extra_check_cfg {
check_cfgs.push(value.to_string());
} else {
warnings.push(format!("cargo:{} requires -Zcheck-cfg=output flag", key));
}
}
"rustc-env" => {
let (key, val) = BuildOutput::parse_rustc_env(&value, &whence)?;
// Build scripts aren't allowed to set RUSTC_BOOTSTRAP.
Expand Down Expand Up @@ -728,6 +747,7 @@ impl BuildOutput {
library_links,
linker_args,
cfgs,
check_cfgs,
env,
metadata,
rerun_if_changed,
Expand Down Expand Up @@ -968,6 +988,10 @@ fn prev_build_output(cx: &mut Context<'_, '_>, unit: &Unit) -> (Option<BuildOutp
&unit.pkg.to_string(),
&prev_script_out_dir,
&script_out_dir,
match cx.bcx.config.cli_unstable().check_cfg {
Some((_, _, _, output)) => output,
None => false,
},
cx.bcx.config.nightly_features_allowed,
unit.pkg.targets(),
)
Expand Down
64 changes: 33 additions & 31 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
)?;
add_plugin_deps(&mut rustc, &script_outputs, &build_scripts, &root_output)?;
}
add_custom_env(&mut rustc, &script_outputs, script_metadata);
add_custom_flags(&mut rustc, &script_outputs, script_metadata)?;
}

for output in outputs.iter() {
Expand Down Expand Up @@ -408,9 +408,6 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
}

if key.0 == current_id {
for cfg in &output.cfgs {
rustc.arg("--cfg").arg(cfg);
}
if pass_l_flag {
for name in output.library_links.iter() {
rustc.arg("-l").arg(name);
Expand All @@ -431,22 +428,6 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
}
Ok(())
}

// Add all custom environment variables present in `state` (after they've
// been put there by one of the `build_scripts`) to the command provided.
fn add_custom_env(
rustc: &mut ProcessBuilder,
build_script_outputs: &BuildScriptOutputs,
metadata: Option<Metadata>,
) {
if let Some(metadata) = metadata {
if let Some(output) = build_script_outputs.get(metadata) {
for &(ref name, ref value) in output.env.iter() {
rustc.env(name, value);
}
}
}
}
}

/// Link the compiled target (often of form `foo-{metadata_hash}`) to the
Expand Down Expand Up @@ -713,16 +694,11 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
let mut output_options = OutputOptions::new(cx, unit);
let script_metadata = cx.find_build_script_metadata(unit);
Ok(Work::new(move |state| {
if let Some(script_metadata) = script_metadata {
if let Some(output) = build_script_outputs.lock().unwrap().get(script_metadata) {
for cfg in output.cfgs.iter() {
rustdoc.arg("--cfg").arg(cfg);
}
for &(ref name, ref value) in output.env.iter() {
rustdoc.env(name, value);
}
}
}
add_custom_flags(
&mut rustdoc,
&build_script_outputs.lock().unwrap(),
script_metadata,
)?;
let crate_dir = doc_dir.join(&crate_name);
if crate_dir.exists() {
// Remove output from a previous build. This ensures that stale
Expand Down Expand Up @@ -1055,7 +1031,7 @@ fn features_args(unit: &Unit) -> Vec<OsString> {

/// Generate the --check-cfg arguments for the unit
fn check_cfg_args(cx: &Context<'_, '_>, unit: &Unit) -> Vec<OsString> {
if let Some((features, well_known_names, well_known_values)) =
if let Some((features, well_known_names, well_known_values, _output)) =
cx.bcx.config.cli_unstable().check_cfg
{
let mut args = Vec::with_capacity(unit.pkg.summary().features().len() * 2 + 4);
Expand Down Expand Up @@ -1184,6 +1160,32 @@ fn build_deps_args(
Ok(())
}

/// Add custom flags from the output a of build-script to a `ProcessBuilder`
fn add_custom_flags(
cmd: &mut ProcessBuilder,
build_script_outputs: &BuildScriptOutputs,
metadata: Option<Metadata>,
) -> CargoResult<()> {
if let Some(metadata) = metadata {
if let Some(output) = build_script_outputs.get(metadata) {
for cfg in output.cfgs.iter() {
cmd.arg("--cfg").arg(cfg);
}
if !output.check_cfgs.is_empty() {
cmd.arg("-Zunstable-options");
for check_cfg in &output.check_cfgs {
cmd.arg("--check-cfg").arg(check_cfg);
}
}
for &(ref name, ref value) in output.env.iter() {
cmd.env(name, value);
}
}
}

Ok(())
}

/// Generates a list of `--extern` arguments.
pub fn extern_args(
cx: &Context<'_, '_>,
Expand Down
15 changes: 11 additions & 4 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ unstable_cli_options!(
build_std_features: Option<Vec<String>> = ("Configure features enabled for the standard library itself when building the standard library"),
config_include: bool = ("Enable the `include` key in config files"),
credential_process: bool = ("Add a config setting to fetch registry authentication tokens by calling an external process"),
check_cfg: Option<(/*features:*/ bool, /*well_known_names:*/ bool, /*well_known_values:*/ bool)> = ("Specify scope of compile-time checking of `cfg` names/values"),
check_cfg: Option<(/*features:*/ bool, /*well_known_names:*/ bool, /*well_known_values:*/ bool, /*output:*/ bool)> = ("Specify scope of compile-time checking of `cfg` names/values"),
doctest_in_workspace: bool = ("Compile doctests with paths relative to the workspace root"),
doctest_xcompile: bool = ("Compile and run doctests for non-host target using runner config"),
dual_proc_macros: bool = ("Build proc-macros for both the host and the target"),
Expand Down Expand Up @@ -783,22 +783,29 @@ impl CliUnstable {
}
}

fn parse_check_cfg(value: Option<&str>) -> CargoResult<Option<(bool, bool, bool)>> {
fn parse_check_cfg(value: Option<&str>) -> CargoResult<Option<(bool, bool, bool, bool)>> {
if let Some(value) = value {
let mut features = false;
let mut well_known_names = false;
let mut well_known_values = false;
let mut output = false;

for e in value.split(',') {
match e {
"features" => features = true,
"names" => well_known_names = true,
"values" => well_known_values = true,
_ => bail!("flag -Zcheck-cfg only takes `features`, `names` or `values` as valid inputs"),
"output" => output = true,
_ => bail!("flag -Zcheck-cfg only takes `features`, `names`, `values` or `output` as valid inputs"),
}
}

Ok(Some((features, well_known_names, well_known_values)))
Ok(Some((
features,
well_known_names,
well_known_values,
output,
)))
} else {
Ok(None)
}
Expand Down
19 changes: 18 additions & 1 deletion src/cargo/util/config/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ fn load_config_table(config: &Config, prefix: &str) -> CargoResult<TargetConfig>
// Links do not support environment variables.
let target_key = ConfigKey::from_str(prefix);
let links_overrides = match config.get_table(&target_key)? {
Some(links) => parse_links_overrides(&target_key, links.val)?,
Some(links) => parse_links_overrides(&target_key, links.val, config)?,
None => BTreeMap::new(),
};
Ok(TargetConfig {
Expand All @@ -134,8 +134,14 @@ fn load_config_table(config: &Config, prefix: &str) -> CargoResult<TargetConfig>
fn parse_links_overrides(
target_key: &ConfigKey,
links: HashMap<String, CV>,
config: &Config,
) -> CargoResult<BTreeMap<String, BuildOutput>> {
let mut links_overrides = BTreeMap::new();
let extra_check_cfg = match config.cli_unstable().check_cfg {
Some((_, _, _, output)) => output,
None => false,
};

for (lib_name, value) in links {
// Skip these keys, it shares the namespace with `TargetConfig`.
match lib_name.as_str() {
Expand Down Expand Up @@ -200,6 +206,17 @@ fn parse_links_overrides(
let list = value.list(key)?;
output.cfgs.extend(list.iter().map(|v| v.0.clone()));
}
"rustc-check-cfg" => {
if extra_check_cfg {
let list = value.list(key)?;
output.check_cfgs.extend(list.iter().map(|v| v.0.clone()));
} else {
config.shell().warn(format!(
"target config `{}.{}` requires -Zcheck-cfg=output flag",
target_key, key
))?;
}
}
"rustc-env" => {
for (name, val) in value.table(key)?.0 {
let val = val.string(name)?.0;
Expand Down
24 changes: 24 additions & 0 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -1201,6 +1201,7 @@ It's values are:
Note than this command line options will probably become the default when stabilizing.
- `names`: enables well known names checking via `--check-cfg=names()`.
- `values`: enables well known values checking via `--check-cfg=values()`.
- `output`: enable the use of `rustc-check-cfg` in build script.

For instance:

Expand All @@ -1211,6 +1212,29 @@ cargo check -Z unstable-options -Z check-cfg=values
cargo check -Z unstable-options -Z check-cfg=features,names,values
```

Or for `output`:

```rust,no_run
// build.rs
println!("cargo:rustc-check-cfg=names(foo, bar)");
```

```
cargo check -Z unstable-options -Z check-cfg=output
```

### `cargo:rustc-check-cfg=CHECK_CFG`

The `rustc-check-cfg` instruction tells Cargo to pass the given value to the
`--check-cfg` flag to the compiler. This may be used for compile-time
detection of unexpected conditional compilation name and/or values.

This can only be used in combination with `-Zcheck-cfg=output` otherwise it is ignored
with a warning.

If you want to integrate with Cargo features, use `-Zcheck-cfg=features` instead of
trying to do it manually with this option.

### workspace-inheritance

* RFC: [#2906](https://github.com/rust-lang/rfcs/blob/master/text/2906-cargo-workspace-deduplicate.md)
Expand Down
Loading

0 comments on commit c8c6e33

Please sign in to comment.