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

Emit error on [target.'cfg(debug_assertions)'.dependencies] and similar #7660

Merged
merged 4 commits into from
Dec 9, 2019
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
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