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

Fix bug when with resolver = "1" non-virtual package was allowing unknown features #9437

Merged
merged 12 commits into from
May 24, 2021
87 changes: 49 additions & 38 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1224,7 +1224,10 @@ impl<'cfg> Workspace<'cfg> {
// really mean anything (either the member is built or it isn't).
// * With `--features nonmember?/feat`, cwd_features will
// handle processing it correctly.
let is_member = self.members().any(|member| member.name() == *dep_name);
let is_member = self.members().any(|member| {
// Check if `dep_name` is member of the workspace, but isn't associated with current package.
self.current_opt() != Some(member) && member.name() == *dep_name
});
if is_member && specs.iter().any(|spec| spec.name() == *dep_name) {
member_specific_features
.entry(*dep_name)
Expand All @@ -1237,49 +1240,57 @@ impl<'cfg> Workspace<'cfg> {
}
}

let ms = self.members().filter_map(|member| {
let member_id = member.package_id();
match self.current_opt() {
// The features passed on the command-line only apply to
// the "current" package (determined by the cwd).
Some(current) if member_id == current.package_id() => {
let feats = CliFeatures {
features: Rc::new(cwd_features.clone()),
all_features: cli_features.all_features,
uses_default_features: cli_features.uses_default_features,
};
Some((member, feats))
}
_ => {
// Ignore members that are not enabled on the command-line.
if specs.iter().any(|spec| spec.matches(member_id)) {
// -p for a workspace member that is not the "current"
// one.
//
// The odd behavior here is due to backwards
// compatibility. `--features` and
// `--no-default-features` used to only apply to the
// "current" package. As an extension, this allows
// member-name/feature-name to set member-specific
// features, which should be backwards-compatible.
let ms: Vec<_> = self
.members()
.filter_map(|member| {
let member_id = member.package_id();
match self.current_opt() {
// The features passed on the command-line only apply to
// the "current" package (determined by the cwd).
Some(current) if member_id == current.package_id() => {
let feats = CliFeatures {
features: Rc::new(
member_specific_features
.remove(member.name().as_str())
.unwrap_or_default(),
),
uses_default_features: true,
features: Rc::new(cwd_features.clone()),
all_features: cli_features.all_features,
uses_default_features: cli_features.uses_default_features,
};
Some((member, feats))
} else {
// This member was not requested on the command-line, skip.
None
}
_ => {
// Ignore members that are not enabled on the command-line.
if specs.iter().any(|spec| spec.matches(member_id)) {
// -p for a workspace member that is not the "current"
// one.
//
// The odd behavior here is due to backwards
// compatibility. `--features` and
// `--no-default-features` used to only apply to the
// "current" package. As an extension, this allows
// member-name/feature-name to set member-specific
// features, which should be backwards-compatible.
let feats = CliFeatures {
features: Rc::new(
member_specific_features
.remove(member.name().as_str())
.unwrap_or_default(),
),
uses_default_features: true,
all_features: cli_features.all_features,
};
Some((member, feats))
} else {
// This member was not requested on the command-line, skip.
None
}
}
}
}
});
ms.collect()
})
.collect();

// If any member specific features were not removed while iterating over members
// some features will be ignored.
assert!(member_specific_features.is_empty());

ms
}
}

Expand Down
50 changes: 48 additions & 2 deletions tests/testsuite/package_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,11 @@ fn virtual_no_default_features() {
.run();

p.cargo("check --features foo")
.masquerade_as_nightly_cargo()
.with_status(101)
.with_stderr("[ERROR] none of the selected packages contains these features: foo")
.run();

p.cargo("check --features a/dep1,b/f1,b/f2,f2")
.masquerade_as_nightly_cargo()
.with_status(101)
.with_stderr("[ERROR] none of the selected packages contains these features: b/f2, f2")
.run();
Expand Down Expand Up @@ -272,6 +270,48 @@ fn other_member_from_current() {
.run();
}

#[cargo_test]
fn feature_default_resolver() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "a"
version = "0.1.0"

[features]
test = []
"#,
)
.file(
"src/main.rs",
r#"
fn main() {
if cfg!(feature = "test") {
println!("feature set");
}
}
"#,
)
.build();

p.cargo("check --features testt")
.with_status(101)
.with_stderr("[ERROR] Package `a[..]` does not have the feature `testt`")
.run();

p.cargo("run --features test")
.with_status(0)
.with_stdout("feature set")
.run();

p.cargo("run --features a/test")
.with_status(101)
.with_stderr("[ERROR] package `a[..]` does not have a dependency named `a`")
.run();
}

#[cargo_test]
fn virtual_member_slash() {
// member slash feature syntax
Expand Down Expand Up @@ -458,6 +498,12 @@ fn resolver1_member_features() {
.cwd("member2")
.with_stdout("m1-feature set")
.run();

p.cargo("check -p member1 --features member1/m2-feature")
.cwd("member2")
.with_status(101)
.with_stderr("[ERROR] Package `member1[..]` does not have the feature `m2-feature`")
.run();
}

#[cargo_test]
Expand Down