]> git.proxmox.com Git - cargo.git/commitdiff
Unify weak and namespaced features.
authorEric Huss <eric@huss.org>
Fri, 11 Jun 2021 14:27:33 +0000 (07:27 -0700)
committerEric Huss <eric@huss.org>
Fri, 11 Jun 2021 14:27:33 +0000 (07:27 -0700)
src/cargo/core/resolver/dep_cache.rs
src/cargo/core/resolver/features.rs
src/cargo/core/summary.rs
src/cargo/core/workspace.rs
src/cargo/ops/cargo_compile.rs
src/cargo/ops/tree/graph.rs
tests/testsuite/features_namespaced.rs
tests/testsuite/weak_dep_features.rs

index 91afa13f7092e7f2bf7ca570c2d6b08e1497adbf..cf7b9fcfa5120cab9dae157b3fc219730c38c656 100644 (file)
@@ -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(())
     }
index eb58c391a2b8181bb16ae5ff1542f6425be394d2..f49e9fd5b50def8c5d0dfeca9d67d5dbe00b4b9c 100644 (file)
@@ -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<InternedString>>,
 }
 
 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)?;
                     }
index b0f69ae5065bbb88e3244024ca11229481f10ffe..4f48fafa6f282947eeeb5faff0d6482967968f51 100644 (file)
@@ -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)
             }
         }
     }
index a748e0890c5c3252c004bee3a6cd78d42c9d58e1..789a69dd1441221d038725a113c4156b48d9097e 100644 (file)
@@ -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::<Vec<_>>()
                 }
-                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.
index 3efc4494858273b838e6ddc2722e9ad9b242b097..cfc117ee0b2fb46f756b76c293bb5de994e23222 100644 (file)
@@ -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
index 37442a4dafd8a1e26983a293408595ceecde217f..bc0fb73dd860010b4229bb1410751f89f2217ae8 100644 (file)
@@ -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,
index 0c3022c8dd3caf04eb08dc7e4d09f5483ba8ad28..487d84bb805468cb8ba06e2d2babf456c7b6b180 100644 (file)
@@ -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(
index d51c407727f89f3b34a5c42f582784cfe3bd00c0..5db2585e9c02220bc136b5f7f6a9d8a7849f86bf 100644 (file)
@@ -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\"