Implement weak dependency features.

Author: ehuss

src/bin/cargo/cli.rs

@@ -43,6 +43,7 @@ Available unstable (nightly-only) flags:
     -Z doctest-xcompile    -- Compile and run doctests for non-host target using runner config
     -Z terminal-width      -- Provide a terminal width to rustc for error truncation
     -Z namespaced-features -- Allow features with `dep:` prefix
+    -Z weak-dep-features    -- Allow `dep_name?/feature` feature syntax
 
 Run with 'cargo -Z [FLAG] [SUBCOMMAND]'"
         );

src/cargo/core/features.rs

@@ -358,6 +358,7 @@ pub struct CliUnstable {
     pub rustdoc_map: bool,
     pub terminal_width: Option<Option<usize>>,
     pub namespaced_features: bool,
+    pub weak_dep_features: bool,
 }
 
 fn deserialize_build_std<'de, D>(deserializer: D) -> Result<Option<Vec<String>>, D::Error>
@@ -464,6 +465,7 @@ impl CliUnstable {
             "rustdoc-map" => self.rustdoc_map = parse_empty(k, v)?,
             "terminal-width" => self.terminal_width = Some(parse_usize_opt(v)?),
             "namespaced-features" => self.namespaced_features = parse_empty(k, v)?,
+            "weak-dep-features" => self.weak_dep_features = parse_empty(k, v)?,
             _ => bail!("unknown `-Z` flag specified: {}", k),
         }
 

src/cargo/core/resolver/dep_cache.rs

@@ -494,6 +494,10 @@ impl Requirements<'_> {
                 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)?,
         };
         Ok(())

src/cargo/core/resolver/features.rs

@@ -177,6 +177,10 @@ impl FeatureOpts {
         if let ForceAllTargets::Yes = force_all_targets {
             opts.ignore_inactive_targets = false;
         }
+        if unstable_flags.weak_dep_features {
+            // Force this ON because it only works with the new resolver.
+            opts.new_resolver = true;
+        }
         Ok(opts)
     }
 }
@@ -311,6 +315,15 @@ pub struct FeatureResolver<'a, 'cfg> {
     /// This has to be separate from `FeatureOpts.decouple_host_deps` because
     /// `for_host` tracking is also needed for `itarget` to work properly.
     track_for_host: bool,
+    /// `dep_name?/feat_name` features that will be activated if `dep_name` is
+    /// ever activated.
+    ///
+    /// 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)>>,
 }
 
 impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
@@ -353,6 +366,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
             activated_dependencies: HashMap::new(),
             processed_deps: HashSet::new(),
             track_for_host,
+            deferred_weak_dependencies: HashMap::new(),
         };
         r.do_resolve(specs, requested_features)?;
         log::debug!("features={:#?}", r.activated_features);
@@ -399,6 +413,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
         for_host: bool,
         fvs: &[FeatureValue],
     ) -> CargoResult<()> {
+        log::trace!("activate_pkg {} {}", pkg_id.name(), for_host);
         // Add an empty entry to ensure everything is covered. This is intended for
         // finding bugs where the resolver missed something it should have visited.
         // Remove this in the future if `activated_features` uses an empty default.
@@ -446,56 +461,28 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
         for_host: bool,
         fv: &FeatureValue,
     ) -> CargoResult<()> {
+        log::trace!("activate_fv {} {} {}", pkg_id.name(), for_host, fv);
         match fv {
             FeatureValue::Feature(f) => {
                 self.activate_rec(pkg_id, for_host, *f)?;
             }
             FeatureValue::Dep { dep_name } => {
-                // Mark this dependency as activated.
-                self.activated_dependencies
-                    .entry((pkg_id, self.opts.decouple_host_deps && for_host))
-                    .or_default()
-                    .insert(*dep_name);
-                // Activate the optional dep.
-                for (dep_pkg_id, deps) in self.deps(pkg_id, for_host) {
-                    for (dep, dep_for_host) in deps {
-                        if dep.name_in_toml() != *dep_name {
-                            continue;
-                        }
-                        let fvs = self.fvs_from_dependency(dep_pkg_id, dep);
-                        self.activate_pkg(dep_pkg_id, dep_for_host, &fvs)?;
-                    }
-                }
+                self.activate_dependency(pkg_id, for_host, *dep_name)?;
             }
             FeatureValue::DepFeature {
                 dep_name,
                 dep_feature,
                 dep_prefix,
+                weak,
             } => {
-                // Activate a feature within a dependency.
-                for (dep_pkg_id, deps) in self.deps(pkg_id, for_host) {
-                    for (dep, dep_for_host) in deps {
-                        if dep.name_in_toml() != *dep_name {
-                            continue;
-                        }
-                        if dep.is_optional() {
-                            // Activate the dependency on self.
-                            let fv = FeatureValue::Dep {
-                                dep_name: *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
-                                // name.
-                                self.activate_rec(pkg_id, for_host, *dep_name)?;
-                            }
-                        }
-                        // Activate the feature on the dependency.
-                        let fv = FeatureValue::new(*dep_feature);
-                        self.activate_fv(dep_pkg_id, dep_for_host, &fv)?;
-                    }
-                }
+                self.activate_dep_feature(
+                    pkg_id,
+                    for_host,
+                    *dep_name,
+                    *dep_feature,
+                    *dep_prefix,
+                    *weak,
+                )?;
             }
         }
         Ok(())
@@ -509,6 +496,12 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
         for_host: bool,
         feature_to_enable: InternedString,
     ) -> CargoResult<()> {
+        log::trace!(
+            "activate_rec {} {} feat={}",
+            pkg_id.name(),
+            for_host,
+            feature_to_enable
+        );
         let enabled = self
             .activated_features
             .entry((pkg_id, self.opts.decouple_host_deps && for_host))
@@ -539,6 +532,110 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
         Ok(())
     }
 
+    /// Activate a dependency (`dep:dep_name` syntax).
+    fn activate_dependency(
+        &mut self,
+        pkg_id: PackageId,
+        for_host: bool,
+        dep_name: InternedString,
+    ) -> CargoResult<()> {
+        // Mark this dependency as activated.
+        let save_for_host = self.opts.decouple_host_deps && for_host;
+        self.activated_dependencies
+            .entry((pkg_id, save_for_host))
+            .or_default()
+            .insert(dep_name);
+        // Check for any deferred features.
+        let to_enable = self
+            .deferred_weak_dependencies
+            .remove(&(pkg_id, for_host, dep_name));
+        // Activate the optional dep.
+        for (dep_pkg_id, deps) in self.deps(pkg_id, for_host) {
+            for (dep, dep_for_host) in deps {
+                if dep.name_in_toml() != dep_name {
+                    continue;
+                }
+                if let Some(to_enable) = &to_enable {
+                    for (dep_feature, dep_prefix) in to_enable {
+                        log::trace!(
+                            "activate deferred {} {} -> {}/{}",
+                            pkg_id.name(),
+                            for_host,
+                            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)?;
+                    }
+                }
+                let fvs = self.fvs_from_dependency(dep_pkg_id, dep);
+                self.activate_pkg(dep_pkg_id, dep_for_host, &fvs)?;
+            }
+        }
+        Ok(())
+    }
+
+    /// Activate a feature within a dependency (`dep_name/feat_name` syntax).
+    fn activate_dep_feature(
+        &mut self,
+        pkg_id: PackageId,
+        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) {
+            for (dep, dep_for_host) in deps {
+                if dep.name_in_toml() != dep_name {
+                    continue;
+                }
+                if dep.is_optional() {
+                    let save_for_host = self.opts.decouple_host_deps && for_host;
+                    if weak
+                        && !self
+                            .activated_dependencies
+                            .get(&(pkg_id, save_for_host))
+                            .map(|deps| deps.contains(&dep_name))
+                            .unwrap_or(false)
+                    {
+                        // This is weak, but not yet activated. Defer in case
+                        // something comes along later and enables it.
+                        log::trace!(
+                            "deferring feature {} {} -> {}/{}",
+                            pkg_id.name(),
+                            for_host,
+                            dep_name,
+                            dep_feature
+                        );
+                        self.deferred_weak_dependencies
+                            .entry((pkg_id, for_host, dep_name))
+                            .or_default()
+                            .insert((dep_feature, dep_prefix));
+                        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
+                        // name.
+                        self.activate_rec(pkg_id, for_host, dep_name)?;
+                    }
+                }
+                // Activate the feature on the dependency.
+                let fv = FeatureValue::new(dep_feature);
+                self.activate_fv(dep_pkg_id, dep_for_host, &fv)?;
+            }
+        }
+        Ok(())
+    }
+
     /// Returns Vec of FeatureValues from a Dependency definition.
     fn fvs_from_dependency(&self, dep_id: PackageId, dep: &Dependency) -> Vec<FeatureValue> {
         let summary = self.resolve.summary(dep_id);

src/cargo/core/summary.rs

@@ -86,7 +86,11 @@ impl Summary {
 
     /// Returns an error if this Summary is using an unstable feature that is
     /// not enabled.
-    pub fn unstable_gate(&self, namespaced_features: bool) -> CargoResult<()> {
+    pub fn unstable_gate(
+        &self,
+        namespaced_features: bool,
+        weak_dep_features: bool,
+    ) -> CargoResult<()> {
         if !namespaced_features {
             if self.inner.has_namespaced_features {
                 bail!(
@@ -101,6 +105,22 @@ impl Summary {
                 )
             }
         }
+        if !weak_dep_features {
+            for (feat_name, features) in self.features() {
+                for fv in features {
+                    if matches!(fv, FeatureValue::DepFeature{weak: true, ..}) {
+                        bail!(
+                            "optional dependency features with `?` syntax are only \
+                             allowed on the nightly channel and requires the \
+                             `-Z weak-dep-features` flag on the command line\n\
+                             Feature `{}` had feature value `{}`.",
+                            feat_name,
+                            fv
+                        );
+                    }
+                }
+            }
+        }
         Ok(())
     }
 
@@ -293,7 +313,7 @@ fn build_feature_map(
                         );
                     }
                 }
-                DepFeature { dep_name, .. } => {
+                DepFeature { dep_name, weak, .. } => {
                     // Validation of the feature name will be performed in the resolver.
                     if !is_any_dep {
                         bail!(
@@ -303,6 +323,12 @@ fn build_feature_map(
                             dep_name
                         );
                     }
+                    if *weak && !is_optional_dep {
+                        bail!("feature `{}` includes `{}` with a `?`, but `{}` is not an optional dependency\n\
+                            A non-optional dependency of the same name is defined; \
+                            consider removing the `?` or changing the dependency to be optional",
+                            feature, fv, dep_name);
+                    }
                 }
             }
         }
@@ -346,6 +372,10 @@ pub enum FeatureValue {
         /// 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.
+        weak: bool,
     },
 }
 
@@ -360,10 +390,16 @@ impl FeatureValue {
                 } else {
                     (dep, false)
                 };
+                let (dep, weak) = if let Some(dep) = dep.strip_suffix('?') {
+                    (dep, true)
+                } else {
+                    (dep, false)
+                };
                 FeatureValue::DepFeature {
                     dep_name: InternedString::new(dep),
                     dep_feature: InternedString::new(dep_feat),
                     dep_prefix,
+                    weak,
                 }
             }
             None => {
@@ -393,13 +429,13 @@ impl fmt::Display for FeatureValue {
             DepFeature {
                 dep_name,
                 dep_feature,
-                dep_prefix: true,
-            } => write!(f, "dep:{}/{}", dep_name, dep_feature),
-            DepFeature {
-                dep_name,
-                dep_feature,
-                dep_prefix: false,
-            } => write!(f, "{}/{}", 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)
+            }
         }
     }
 }

src/cargo/ops/cargo_compile.rs

@@ -1082,11 +1082,20 @@ fn validate_required_features(
                     target_name
                 );
             }
+            FeatureValue::DepFeature { weak: true, .. } => {
+                anyhow::bail!(
+                    "invalid feature `{}` in required-features of target `{}`: \
+                    optional dependency with `?` is not allowed in required-features",
+                    fv,
+                    target_name
+                );
+            }
             // Handling of dependent_crate/dependent_crate_feature syntax
             FeatureValue::DepFeature {
                 dep_name,
                 dep_feature,
                 dep_prefix: false,
+                weak: false,
             } => {
                 match resolve
                     .deps(summary.package_id())