Skip to content

Commit

Permalink
Auto merge of #7660 - benediktwerner:master, r=alexcrichton
Browse files Browse the repository at this point in the history
Emit error on [target.'cfg(debug_assertions)'.dependencies] and similar

Closes #7634

Cargo now emits an error if an unsupported cfg key or name is used for specifying a target.

This PR also updates the docs to clarify this behavior.
  • Loading branch information
bors committed Dec 9, 2019
2 parents 4a9b6be + aa93f61 commit 774d949
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 1 deletion.
42 changes: 42 additions & 0 deletions crates/cargo-platform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,48 @@ impl Platform {
}
Ok(())
}

pub fn check_cfg_attributes(&self, warnings: &mut Vec<String>) {
fn check_cfg_expr(expr: &CfgExpr, warnings: &mut Vec<String>) {
match *expr {
CfgExpr::Not(ref e) => check_cfg_expr(e, warnings),
CfgExpr::All(ref e) | CfgExpr::Any(ref e) => {
for e in e {
check_cfg_expr(e, warnings);
}
}
CfgExpr::Value(ref e) => match e {
Cfg::Name(name) => match name.as_str() {
"test" | "debug_assertions" | "proc_macro" =>
warnings.push(format!(
"Found `{}` in `target.'cfg(...)'.dependencies`. \
This value is not supported for selecting dependencies \
and will not work as expected. \
To learn more visit \
https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies",
name
)),
_ => (),
},
Cfg::KeyPair(name, _) => match name.as_str() {
"feature" =>
warnings.push(String::from(
"Found `feature = ...` in `target.'cfg(...)'.dependencies`. \
This key is not supported for selecting dependencies \
and will not work as expected. \
Use the [features] section instead: \
https://doc.rust-lang.org/cargo/reference/manifest.html#the-features-section"
)),
_ => (),
},
}
}
}

if let Platform::Cfg(cfg) = self {
check_cfg_expr(cfg, warnings);
}
}
}

impl serde::Serialize for Platform {
Expand Down
73 changes: 73 additions & 0 deletions crates/cargo-platform/tests/test_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,76 @@ fn round_trip_platform() {
all(target_os = \"freebsd\", target_arch = \"x86_64\")))",
);
}

#[test]
fn check_cfg_attributes() {
fn ok(s: &str) {
let p = Platform::Cfg(s.parse().unwrap());
let mut warnings = Vec::new();
p.check_cfg_attributes(&mut warnings);
assert!(
warnings.is_empty(),
"Expected no warnings but got: {:?}",
warnings,
);
}

fn warn(s: &str, names: &[&str]) {
let p = Platform::Cfg(s.parse().unwrap());
let mut warnings = Vec::new();
p.check_cfg_attributes(&mut warnings);
assert_eq!(
warnings.len(),
names.len(),
"Expecter warnings about {:?} but got {:?}",
names,
warnings,
);
for (name, warning) in names.iter().zip(warnings.iter()) {
assert!(
warning.contains(name),
"Expected warning about '{}' but got: {}",
name,
warning,
);
}
}

ok("unix");
ok("windows");
ok("any(not(unix), windows)");
ok("foo");

ok("target_arch = \"abc\"");
ok("target_feature = \"abc\"");
ok("target_os = \"abc\"");
ok("target_family = \"abc\"");
ok("target_env = \"abc\"");
ok("target_endian = \"abc\"");
ok("target_pointer_width = \"abc\"");
ok("target_vendor = \"abc\"");
ok("bar = \"def\"");

warn("test", &["test"]);
warn("debug_assertions", &["debug_assertions"]);
warn("proc_macro", &["proc_macro"]);
warn("feature = \"abc\"", &["feature"]);

warn("any(not(debug_assertions), windows)", &["debug_assertions"]);
warn(
"any(not(feature = \"def\"), target_arch = \"abc\")",
&["feature"],
);
warn(
"any(not(target_os = \"windows\"), proc_macro)",
&["proc_macro"],
);
warn(
"any(not(feature = \"windows\"), proc_macro)",
&["feature", "proc_macro"],
);
warn(
"all(not(debug_assertions), any(windows, proc_macro))",
&["debug_assertions", "proc_macro"],
);
}
6 changes: 5 additions & 1 deletion src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1085,7 +1085,11 @@ impl TomlManifest {
process_dependencies(&mut cx, build_deps, Some(Kind::Build))?;

for (name, platform) in me.target.iter().flatten() {
cx.platform = Some(name.parse()?);
cx.platform = {
let platform: Platform = name.parse()?;
platform.check_cfg_attributes(&mut cx.warnings);
Some(platform)
};
process_dependencies(&mut cx, platform.dependencies.as_ref(), None)?;
let build_deps = platform
.build_dependencies
Expand Down
5 changes: 5 additions & 0 deletions src/doc/src/reference/specifying-dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,11 @@ dependencies based on optional crate features.
Use [the `[features]` section](manifest.md#the-features-section)
instead.

The same applies to `cfg(debug_assertions)`, `cfg(test)` and `cfg(prog_macro)`.
These values will not work as expected and will always have the default value
returned by `rustc --print=cfg`.
There is currently no way to add dependencies based on these configuration values.

In addition to `#[cfg]` syntax, Cargo also supports listing out the full target
the dependencies would apply to:

Expand Down

0 comments on commit 774d949

Please sign in to comment.