diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 91afa13f709..cf7b9fcfa51 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -405,12 +405,12 @@ impl Requirements<'_> { &mut self, package: InternedString, feat: InternedString, - dep_prefix: bool, + weak: bool, ) -> Result<(), RequirementError> { // If `package` is indeed an optional dependency then we activate the // feature named `package`, but otherwise if `package` is a required // dependency then there's no feature associated with it. - if !dep_prefix + if !weak && self .summary .dependencies() @@ -456,12 +456,11 @@ impl Requirements<'_> { FeatureValue::DepFeature { dep_name, dep_feature, - dep_prefix, // Weak features are always activated in the dependency // resolver. They will be narrowed inside the new feature // resolver. - weak: _, - } => self.require_dep_feature(*dep_name, *dep_feature, *dep_prefix)?, + weak, + } => self.require_dep_feature(*dep_name, *dep_feature, *weak)?, }; Ok(()) } diff --git a/src/cargo/core/resolver/features.rs b/src/cargo/core/resolver/features.rs index eb58c391a2b..f49e9fd5b50 100644 --- a/src/cargo/core/resolver/features.rs +++ b/src/cargo/core/resolver/features.rs @@ -244,10 +244,7 @@ impl CliFeatures { match feature { // Maybe call validate_feature_name here once it is an error? FeatureValue::Feature(_) => {} - FeatureValue::Dep { .. } - | FeatureValue::DepFeature { - dep_prefix: true, .. - } => { + FeatureValue::Dep { .. } => { bail!( "feature `{}` is not allowed to use explicit `dep:` syntax", feature @@ -441,10 +438,8 @@ pub struct FeatureResolver<'a, 'cfg> { /// /// The key is the `(package, for_host, dep_name)` of the package whose /// dependency will trigger the addition of new features. The value is the - /// set of `(feature, dep_prefix)` features to activate (`dep_prefix` is a - /// bool that indicates if `dep:` prefix was used). - deferred_weak_dependencies: - HashMap<(PackageId, bool, InternedString), HashSet<(InternedString, bool)>>, + /// set of features to activate. + deferred_weak_dependencies: HashMap<(PackageId, bool, InternedString), HashSet>, } impl<'a, 'cfg> FeatureResolver<'a, 'cfg> { @@ -591,17 +586,9 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> { FeatureValue::DepFeature { dep_name, dep_feature, - dep_prefix, weak, } => { - self.activate_dep_feature( - pkg_id, - for_host, - *dep_name, - *dep_feature, - *dep_prefix, - *weak, - )?; + self.activate_dep_feature(pkg_id, for_host, *dep_name, *dep_feature, *weak)?; } } Ok(()) @@ -675,7 +662,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> { continue; } if let Some(to_enable) = &to_enable { - for (dep_feature, dep_prefix) in to_enable { + for dep_feature in to_enable { log::trace!( "activate deferred {} {} -> {}/{}", pkg_id.name(), @@ -683,9 +670,6 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> { dep_name, dep_feature ); - if !dep_prefix { - self.activate_rec(pkg_id, for_host, dep_name)?; - } let fv = FeatureValue::new(*dep_feature); self.activate_fv(dep_pkg_id, dep_for_host, &fv)?; } @@ -704,7 +688,6 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> { for_host: bool, dep_name: InternedString, dep_feature: InternedString, - dep_prefix: bool, weak: bool, ) -> CargoResult<()> { for (dep_pkg_id, deps) in self.deps(pkg_id, for_host) { @@ -733,16 +716,16 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> { self.deferred_weak_dependencies .entry((pkg_id, for_host, dep_name)) .or_default() - .insert((dep_feature, dep_prefix)); + .insert(dep_feature); continue; } // Activate the dependency on self. let fv = FeatureValue::Dep { dep_name }; self.activate_fv(pkg_id, for_host, &fv)?; - if !dep_prefix { - // To retain compatibility with old behavior, - // this also enables a feature of the same + if !weak { + // The old behavior before weak dependencies were + // added is to also enables a feature of the same // name. self.activate_rec(pkg_id, for_host, dep_name)?; } diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index b0f69ae5065..4f48fafa6f2 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -218,12 +218,7 @@ fn build_feature_map( .values() .flatten() .filter_map(|fv| match fv { - Dep { dep_name } - | DepFeature { - dep_name, - dep_prefix: true, - .. - } => Some(*dep_name), + Dep { dep_name } => Some(*dep_name), _ => None, }) .collect(); @@ -391,9 +386,6 @@ pub enum FeatureValue { DepFeature { dep_name: InternedString, dep_feature: InternedString, - /// If this is true, then the feature used the `dep:` prefix, which - /// prevents enabling the feature named `dep_name`. - dep_prefix: bool, /// If `true`, indicates the `?` syntax is used, which means this will /// not automatically enable the dependency unless the dependency is /// activated through some other means. @@ -407,11 +399,6 @@ impl FeatureValue { Some(pos) => { let (dep, dep_feat) = feature.split_at(pos); let dep_feat = &dep_feat[1..]; - let (dep, dep_prefix) = if let Some(dep) = dep.strip_prefix("dep:") { - (dep, true) - } else { - (dep, false) - }; let (dep, weak) = if let Some(dep) = dep.strip_suffix('?') { (dep, true) } else { @@ -420,7 +407,6 @@ impl FeatureValue { FeatureValue::DepFeature { dep_name: InternedString::new(dep), dep_feature: InternedString::new(dep_feat), - dep_prefix, weak, } } @@ -438,14 +424,7 @@ impl FeatureValue { /// Returns `true` if this feature explicitly used `dep:` syntax. pub fn has_dep_prefix(&self) -> bool { - matches!( - self, - FeatureValue::Dep { .. } - | FeatureValue::DepFeature { - dep_prefix: true, - .. - } - ) + matches!(self, FeatureValue::Dep { .. }) } } @@ -458,12 +437,10 @@ impl fmt::Display for FeatureValue { DepFeature { dep_name, dep_feature, - dep_prefix, weak, } => { - let dep_prefix = if *dep_prefix { "dep:" } else { "" }; let weak = if *weak { "?" } else { "" }; - write!(f, "{}{}{}/{}", dep_prefix, dep_name, weak, dep_feature) + write!(f, "{}{}/{}", dep_name, weak, dep_feature) } } } diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index a748e0890c5..789a69dd144 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1163,14 +1163,10 @@ impl<'cfg> Workspace<'cfg> { } } // This should be enforced by CliFeatures. - FeatureValue::Dep { .. } - | FeatureValue::DepFeature { - dep_prefix: true, .. - } => panic!("unexpected dep: syntax {}", feature), + FeatureValue::Dep { .. } => panic!("unexpected dep: syntax {}", feature), FeatureValue::DepFeature { dep_name, dep_feature, - dep_prefix: _, weak: _, } => { if dependencies.contains_key(dep_name) { @@ -1283,14 +1279,10 @@ impl<'cfg> Workspace<'cfg> { .map(|s| s.to_string()) .collect::>() } - FeatureValue::Dep { .. } - | FeatureValue::DepFeature { - dep_prefix: true, .. - } => panic!("unexpected dep: syntax {}", feature), + FeatureValue::Dep { .. } => panic!("unexpected dep: syntax {}", feature), FeatureValue::DepFeature { dep_name, dep_feature, - dep_prefix: _, weak: _, } => { // Finds set of `pkg/feat` that are very similar to current `pkg/feat`. @@ -1446,14 +1438,10 @@ impl<'cfg> Workspace<'cfg> { cwd_features.insert(feature.clone()); } // This should be enforced by CliFeatures. - FeatureValue::Dep { .. } - | FeatureValue::DepFeature { - dep_prefix: true, .. - } => panic!("unexpected dep: syntax {}", feature), + FeatureValue::Dep { .. } => panic!("unexpected dep: syntax {}", feature), FeatureValue::DepFeature { dep_name, dep_feature, - dep_prefix: _, weak: _, } => { // I think weak can be ignored here. diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 3efc4494858..cfc117ee0b2 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -1225,10 +1225,7 @@ fn validate_required_features( ))?; } } - FeatureValue::Dep { .. } - | FeatureValue::DepFeature { - dep_prefix: true, .. - } => { + FeatureValue::Dep { .. } => { anyhow::bail!( "invalid feature `{}` in required-features of target `{}`: \ `dep:` prefixed feature values are not allowed in required-features", @@ -1248,7 +1245,6 @@ fn validate_required_features( FeatureValue::DepFeature { dep_name, dep_feature, - dep_prefix: false, weak: false, } => { match resolve diff --git a/src/cargo/ops/tree/graph.rs b/src/cargo/ops/tree/graph.rs index 37442a4dafd..bc0fb73dd86 100644 --- a/src/cargo/ops/tree/graph.rs +++ b/src/cargo/ops/tree/graph.rs @@ -488,7 +488,6 @@ fn add_cli_features( FeatureValue::DepFeature { dep_name, dep_feature, - dep_prefix: _, weak, } => { let dep_connections = match graph.dep_name_map[&package_index].get(&dep_name) { @@ -596,12 +595,12 @@ fn add_feature_rec( FeatureValue::DepFeature { dep_name, dep_feature, - dep_prefix, - // `weak` is ignored, because it will be skipped if the - // corresponding dependency is not found in the map, which - // means it wasn't activated. Skipping is handled by - // `is_dep_activated` when the graph was built. - weak: _, + // Note: `weak` is mostly handled when the graph is built in + // `is_dep_activated` which is responsible for skipping + // unactivated weak dependencies. Here it is only used to + // determine if the feature of the dependency name is + // activated on self. + weak, } => { let dep_indexes = match graph.dep_name_map[&package_index].get(dep_name) { Some(indexes) => indexes.clone(), @@ -619,7 +618,7 @@ fn add_feature_rec( }; for (dep_index, is_optional) in dep_indexes { let dep_pkg_id = graph.package_id_for_index(dep_index); - if is_optional && !dep_prefix { + if is_optional && !weak { // Activate the optional dep on self. add_feature( graph, diff --git a/tests/testsuite/features_namespaced.rs b/tests/testsuite/features_namespaced.rs index 0c3022c8dd3..487d84bb805 100644 --- a/tests/testsuite/features_namespaced.rs +++ b/tests/testsuite/features_namespaced.rs @@ -516,61 +516,6 @@ syntax in the features table, so it does not have an implicit feature with that .run(); } -#[cargo_test] -fn crate_feature_explicit() { - // dep:name/feature syntax shouldn't set implicit feature. - Package::new("bar", "1.0.0") - .file( - "src/lib.rs", - r#" - #[cfg(not(feature="feat"))] - compile_error!{"feat missing"} - "#, - ) - .feature("feat", &[]) - .publish(); - let p = project() - .file( - "Cargo.toml", - r#" - [package] - name = "foo" - version = "0.1.0" - - [dependencies] - bar = {version = "1.0", optional=true} - - [features] - f1 = ["dep:bar/feat"] - "#, - ) - .file( - "src/lib.rs", - r#" - #[cfg(not(feature="f1"))] - compile_error!{"f1 missing"} - - #[cfg(feature="bar")] - compile_error!{"bar should not be set"} - "#, - ) - .build(); - - p.cargo("check -Z namespaced-features --features f1") - .masquerade_as_nightly_cargo() - .with_stderr( - "\ -[UPDATING] [..] -[DOWNLOADING] crates ... -[DOWNLOADED] bar v1.0.0 [..] -[CHECKING] bar v1.0.0 -[CHECKING] foo v0.1.0 [..] -[FINISHED] [..] -", - ) - .run(); -} - #[cargo_test] fn crate_syntax_bad_name() { // "dep:bar" = [] @@ -904,7 +849,6 @@ fn tree() { [features] a = ["bar/feat2"] - b = ["dep:bar/feat2"] bar = ["dep:bar"] "#, ) @@ -950,38 +894,6 @@ bar v1.0.0 ) .run(); - p.cargo("tree -e features --features b -Z namespaced-features") - .masquerade_as_nightly_cargo() - .with_stdout( - "\ -foo v0.1.0 ([ROOT]/foo) -├── bar feature \"default\" -│ └── bar v1.0.0 -│ └── baz feature \"default\" -│ └── baz v1.0.0 -└── bar feature \"feat1\" - └── bar v1.0.0 (*) -", - ) - .run(); - - p.cargo("tree -e features --features b -i bar -Z namespaced-features") - .masquerade_as_nightly_cargo() - .with_stdout( - "\ -bar v1.0.0 -├── bar feature \"default\" -│ └── foo v0.1.0 ([ROOT]/foo) -│ ├── foo feature \"b\" (command-line) -│ └── foo feature \"default\" (command-line) -├── bar feature \"feat1\" -│ └── foo v0.1.0 ([ROOT]/foo) (*) -└── bar feature \"feat2\" - └── foo feature \"b\" (command-line) -", - ) - .run(); - p.cargo("tree -e features --features bar -Z namespaced-features") .masquerade_as_nightly_cargo() .with_stdout( diff --git a/tests/testsuite/weak_dep_features.rs b/tests/testsuite/weak_dep_features.rs index d51c407727f..5db2585e9c0 100644 --- a/tests/testsuite/weak_dep_features.rs +++ b/tests/testsuite/weak_dep_features.rs @@ -468,29 +468,12 @@ fn weak_with_host_decouple() { } #[cargo_test] -fn deferred_with_namespaced() { - // Interaction with -Z namespaced-features using dep: syntax. - // - // `bar` is deferred with bar?/feat - // `bar2` is deferred with dep:bar2?/feat +fn weak_namespaced() { + // Behavior with a dep: dependency. Package::new("bar", "1.0.0") .feature("feat", &[]) .file("src/lib.rs", &require(&["feat"], &[])) .publish(); - Package::new("bar2", "1.0.0") - .feature("feat", &[]) - .file("src/lib.rs", &require(&["feat"], &[])) - .publish(); - Package::new("bar_includer", "1.0.0") - .add_dep(Dependency::new("bar", "1.0").optional(true)) - .add_dep(Dependency::new("bar2", "1.0").optional(true)) - .feature("feat", &["bar?/feat", "dep:bar2?/feat"]) - .feature("feat2", &["dep:bar2"]) - .file("src/lib.rs", &require(&["bar"], &["bar2"])) - .publish(); - Package::new("bar_activator", "1.0.0") - .feature_dep("bar_includer", "1.0", &["bar", "feat2"]) - .publish(); let p = project() .file( "Cargo.toml", @@ -500,27 +483,60 @@ fn deferred_with_namespaced() { version = "0.1.0" [dependencies] - bar_includer = { version = "1.0", features = ["feat"] } - bar_activator = "1.0" + bar = { version = "1.0", optional = true } + + [features] + f1 = ["bar?/feat"] + f2 = ["dep:bar"] "#, ) - .file("src/lib.rs", "") + .file("src/lib.rs", &require(&["f1"], &["f2", "bar"])) .build(); - p.cargo("check -Z weak-dep-features -Z namespaced-features") + p.cargo("check -Z weak-dep-features -Z namespaced-features --features f1") .masquerade_as_nightly_cargo() - .with_stderr_unordered( + .with_stderr( "\ [UPDATING] [..] [DOWNLOADING] crates ... -[DOWNLOADED] [..] -[DOWNLOADED] [..] -[DOWNLOADED] [..] -[DOWNLOADED] [..] +[DOWNLOADED] bar v1.0.0 [..] +[CHECKING] foo v0.1.0 [..] +[FINISHED] [..] +", + ) + .run(); + + p.cargo("tree -Z weak-dep-features -Z namespaced-features -f") + .arg("{p} feats:{f}") + .masquerade_as_nightly_cargo() + .with_stdout("foo v0.1.0 ([ROOT]/foo) feats:") + .run(); + + p.cargo("tree -Z weak-dep-features -Z namespaced-features --features f1 -f") + .arg("{p} feats:{f}") + .masquerade_as_nightly_cargo() + .with_stdout("foo v0.1.0 ([ROOT]/foo) feats:f1") + .run(); + + p.cargo("tree -Z weak-dep-features -Z namespaced-features --features f1,f2 -f") + .arg("{p} feats:{f}") + .masquerade_as_nightly_cargo() + .with_stdout( + "\ +foo v0.1.0 ([ROOT]/foo) feats:f1,f2 +└── bar v1.0.0 feats:feat +", + ) + .run(); + + // "bar" remains not-a-feature + p.change_file("src/lib.rs", &require(&["f1", "f2"], &["bar"])); + + p.cargo("check -Z weak-dep-features -Z namespaced-features --features f1,f2") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ [CHECKING] bar v1.0.0 -[CHECKING] bar2 v1.0.0 -[CHECKING] bar_includer v1.0.0 -[CHECKING] bar_activator v1.0.0 [CHECKING] foo v0.1.0 [..] [FINISHED] [..] ", @@ -586,7 +602,6 @@ bar v1.0.0 ├── bar feature \"default\" │ └── foo v0.1.0 ([ROOT]/foo) │ ├── foo feature \"bar\" (command-line) -│ │ └── foo feature \"f1\" (command-line) │ ├── foo feature \"default\" (command-line) │ └── foo feature \"f1\" (command-line) └── bar feature \"feat\"