Skip to content

Commit

Permalink
Auto merge of #7962 - ehuss:features2-required-feature-inactive, r=al…
Browse files Browse the repository at this point in the history
…excrichton

Fix bug with new feature resolver and required-features.

If required-features are used, then the code for checking those features would crash if a dependency was not activated.  The solution here is to not be strict about only requesting activated packages.

For context, the reason this can panic is to check for any bugs in the resolver or places that make bad assumptions.  I missed this particular case, though.
  • Loading branch information
bors committed Mar 3, 2020
2 parents bda5051 + 887ee6c commit 208e499
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/cargo/core/resolver/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ impl ResolvedFeatures {
}

/// Variant of `activated_features` that returns an empty Vec if this is
/// not a valid pkg_id/is_build combination. Used by `cargo clean` which
/// doesn't know the exact set.
/// not a valid pkg_id/is_build combination. Used in places which do
/// not know which packages are activated (like `cargo clean`).
pub fn activated_features_unverified(
&self,
pkg_id: PackageId,
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -956,8 +956,8 @@ fn resolve_all_features(
} else {
FeaturesFor::NormalOrDev
};
for feature in resolved_features.activated_features(dep_id, features_for) {
features.insert(dep.name_in_toml().to_string() + "/" + &feature);
for feature in resolved_features.activated_features_unverified(dep_id, features_for) {
features.insert(format!("{}/{}", dep.name_in_toml(), feature));
}
}
}
Expand Down
40 changes: 39 additions & 1 deletion tests/testsuite/features2.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Tests for the new feature resolver.

use cargo_test_support::project;
use cargo_test_support::registry::{Dependency, Package};
use cargo_test_support::{basic_manifest, project};

#[cargo_test]
fn inactivate_targets() {
Expand Down Expand Up @@ -893,3 +893,41 @@ fn disabled_shared_build_dep() {
.with_stdout("hello from somedep")
.run();
}

#[cargo_test]
fn required_features_inactive_dep() {
// required-features with an inactivated dep.
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[target.'cfg(whatever)'.dependencies]
bar = {path="bar"}
[[bin]]
name = "foo"
required-features = ["feat1"]
[features]
feat1 = []
"#,
)
.file("src/main.rs", "fn main() {}")
.file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0"))
.file("bar/src/lib.rs", "")
.build();

p.cargo("check -Zfeatures=itarget")
.masquerade_as_nightly_cargo()
.with_stderr("[FINISHED] [..]")
.run();

p.cargo("check -Zfeatures=itarget --features=feat1")
.masquerade_as_nightly_cargo()
.with_stderr("[CHECKING] foo[..]\n[FINISHED] [..]")
.run();
}

0 comments on commit 208e499

Please sign in to comment.