Auto merge of #10269 - ehuss:stabilized-new-features, r=alexcrichton

Stabilize namespaced and weak dependency features.

This stabilizes the namespaced and weak dependency features.  Support is now enabled on crates.io, so this should be ready to go.

As a part of this change, the new feature resolver is now enabled all of the time. This is fairly risky, since there are likely edge cases that haven't been exercised.
NOTE: Projects using `resolver="1"` *should* continue to have the same behavior, the old resolver behavior is emulated.

Closes #8813
Closes #8832

Cargo.toml

@@ -20,7 +20,7 @@ atty = "0.2"
 bytesize = "1.0"
 cargo-platform = { path = "crates/cargo-platform", version = "0.1.2" }
 cargo-util = { path = "crates/cargo-util", version = "0.1.2" }
-crates-io = { path = "crates/crates-io", version = "0.33.1" }
+crates-io = { path = "crates/crates-io", version = "0.34.0" }
 crossbeam-utils = "0.8"
 curl = { version = "0.4.41", features = ["http2"] }
 curl-sys = "0.4.50"

crates/crates-io/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "crates-io"
-version = "0.33.1"
+version = "0.34.0"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/rust-lang/cargo"

crates/crates-io/lib.rs

@@ -55,8 +55,6 @@ pub struct NewCrate {
     pub repository: Option<String>,
     pub badges: BTreeMap<String, BTreeMap<String, String>>,
     pub links: Option<String>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub v: Option<u32>,
 }
 
 #[derive(Serialize)]

src/bin/cargo/commands/read_manifest.rs

@@ -15,8 +15,6 @@ Deprecated, use `cargo metadata --no-deps` instead.\
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
-    config
-        .shell()
-        .print_json(&ws.current()?.serialized(config))?;
+    config.shell().print_json(&ws.current()?.serialized())?;
     Ok(())
 }

src/cargo/core/features.rs

@@ -645,7 +645,6 @@ unstable_cli_options!(
     minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum"),
     mtime_on_use: bool = ("Configure Cargo to update the mtime of used files"),
     multitarget: bool = ("Allow passing multiple `--target` flags to the cargo subcommand selected"),
-    namespaced_features: bool = ("Allow features with `dep:` prefix"),
     no_index_update: bool = ("Do not update the registry index even if the cache is outdated"),
     panic_abort_tests: bool = ("Enable support to run tests with -Cpanic=abort"),
     host_config: bool = ("Enable the [host] section in the .cargo/config.toml file"),
@@ -655,7 +654,6 @@ unstable_cli_options!(
     terminal_width: Option<Option<usize>>  = ("Provide a terminal width to rustc for error truncation"),
     timings: Option<Vec<String>>  = ("Display concurrency information"),
     unstable_options: bool = ("Allow the usage of unstable options"),
-    weak_dep_features: bool = ("Allow `dep_name?/feature` feature syntax"),
     // TODO(wcrichto): move scrape example configuration into Cargo.toml before stabilization
     // See: https://github.com/rust-lang/cargo/pull/9525#discussion_r728470927
     rustdoc_scrape_examples: Option<String> = ("Allow rustdoc to scrape examples from reverse-dependencies for documentation"),
@@ -710,6 +708,10 @@ const STABILIZED_NAMED_PROFILES: &str = "The named-profiles feature is now alway
 const STABILIZED_FUTURE_INCOMPAT_REPORT: &str =
     "The future-incompat-report feature is now always enabled.";
 
+const STABILIZED_WEAK_DEP_FEATURES: &str = "Weak dependency features are now always available.";
+
+const STABILISED_NAMESPACED_FEATURES: &str = "Namespaced features are now always available.";
+
 fn deserialize_build_std<'de, D>(deserializer: D) -> Result<Option<Vec<String>>, D::Error>
 where
     D: serde::Deserializer<'de>,
@@ -876,8 +878,8 @@ impl CliUnstable {
             "multitarget" => self.multitarget = parse_empty(k, v)?,
             "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)?,
+            "namespaced-features" => stabilized_warn(k, "1.60", STABILISED_NAMESPACED_FEATURES),
+            "weak-dep-features" => stabilized_warn(k, "1.60", STABILIZED_WEAK_DEP_FEATURES),
             "credential-process" => self.credential_process = parse_empty(k, v)?,
             "rustdoc-scrape-examples" => {
                 if let Some(s) = v {

src/cargo/core/package.rs

@@ -208,7 +208,7 @@ impl Package {
         self.targets().iter().any(|t| t.is_example() || t.is_bin())
     }
 
-    pub fn serialized(&self, config: &Config) -> SerializedPackage {
+    pub fn serialized(&self) -> SerializedPackage {
         let summary = self.manifest().summary();
         let package_id = summary.package_id();
         let manmeta = self.manifest().metadata();
@@ -222,27 +222,19 @@ impl Package {
             .filter(|t| t.src_path().is_path())
             .cloned()
             .collect();
-        let features = if config.cli_unstable().namespaced_features {
-            // Convert Vec<FeatureValue> to Vec<InternedString>
-            summary
-                .features()
-                .iter()
-                .map(|(k, v)| {
-                    (
-                        *k,
-                        v.iter()
-                            .map(|fv| InternedString::new(&fv.to_string()))
-                            .collect(),
-                    )
-                })
-                .collect()
-        } else {
-            self.manifest()
-                .original()
-                .features()
-                .cloned()
-                .unwrap_or_default()
-        };
+        // Convert Vec<FeatureValue> to Vec<InternedString>
+        let features = summary
+            .features()
+            .iter()
+            .map(|(k, v)| {
+                (
+                    *k,
+                    v.iter()
+                        .map(|fv| InternedString::new(&fv.to_string()))
+                        .collect(),
+                )
+            })
+            .collect();
 
         SerializedPackage {
             name: package_id.name(),

src/cargo/core/resolver/features.rs

@@ -1,19 +1,14 @@
 //! Feature resolver.
 //!
 //! This is a new feature resolver that runs independently of the main
-//! dependency resolver. It is enabled when the user specifies `resolver =
-//! "2"` in `Cargo.toml`.
+//! dependency resolver. It has several options which can enable new feature
+//! resolution behavior.
 //!
 //! One of its key characteristics is that it can avoid unifying features for
 //! shared dependencies in some situations. See `FeatureOpts` for the
 //! different behaviors that can be enabled. If no extra options are enabled,
 //! then it should behave exactly the same as the dependency resolver's
-//! feature resolution. This can be verified by setting the
-//! `__CARGO_FORCE_NEW_FEATURES=compare` environment variable and running
-//! Cargo's test suite (or building other projects), and checking if it
-//! panics. Note: the `features2` tests will fail because they intentionally
-//! compare the old vs new behavior, so forcing the old behavior will
-//! naturally fail the tests.
+//! feature resolution.
 //!
 //! The preferred way to engage this new resolver is via
 //! `resolve_ws_with_opts`.
@@ -59,22 +54,12 @@ pub struct ResolvedFeatures {
     ///
     /// The value is the `name_in_toml` of the dependencies.
     activated_dependencies: ActivateMap,
-    /// This is only here for legacy support when the new resolver is not enabled.
-    ///
-    /// This is the set of features enabled for each package.
-    legacy_features: Option<HashMap<PackageId, Vec<InternedString>>>,
-    /// This is only here for legacy support when the new resolver is not enabled.
-    ///
-    /// This is the set of optional dependencies enabled for each package.
-    legacy_dependencies: Option<HashMap<PackageId, HashSet<InternedString>>>,
     opts: FeatureOpts,
 }
 
 /// Options for how the feature resolver works.
 #[derive(Default)]
 pub struct FeatureOpts {
-    /// Use the new resolver instead of the old one.
-    new_resolver: bool,
     /// Build deps and proc-macros will not share share features with other dep kinds.
     decouple_host_deps: bool,
     /// Dev dep features will not be activated unless needed.
@@ -132,7 +117,6 @@ impl FeatureOpts {
         let mut opts = FeatureOpts::default();
         let unstable_flags = ws.config().cli_unstable();
         let mut enable = |feat_opts: &Vec<String>| {
-            opts.new_resolver = true;
             for opt in feat_opts {
                 match opt.as_ref() {
                     "build_dep" | "host_dep" => opts.decouple_host_deps = true,
@@ -159,26 +143,13 @@ impl FeatureOpts {
                 enable(&vec!["all".to_string()]).unwrap();
             }
         }
-        // This env var is intended for testing only.
-        if let Ok(env_opts) = std::env::var("__CARGO_FORCE_NEW_FEATURES") {
-            if env_opts == "1" {
-                opts.new_resolver = true;
-            } else {
-                let env_opts = env_opts.split(',').map(|s| s.to_string()).collect();
-                enable(&env_opts)?;
-            }
-        }
         if let HasDevUnits::Yes = has_dev_units {
             // Dev deps cannot be decoupled when they are in use.
             opts.decouple_dev_deps = false;
         }
         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)
     }
 
@@ -187,7 +158,6 @@ impl FeatureOpts {
         match behavior {
             ResolveBehavior::V1 => FeatureOpts::default(),
             ResolveBehavior::V2 => FeatureOpts {
-                new_resolver: true,
                 decouple_host_deps: true,
                 decouple_dev_deps: has_dev_units == HasDevUnits::No,
                 ignore_inactive_targets: true,
@@ -306,18 +276,11 @@ impl ResolvedFeatures {
         features_for: FeaturesFor,
         dep_name: InternedString,
     ) -> bool {
-        if let Some(legacy) = &self.legacy_dependencies {
-            legacy
-                .get(&pkg_id)
-                .map(|deps| deps.contains(&dep_name))
-                .unwrap_or(false)
-        } else {
-            let is_build = self.opts.decouple_host_deps && features_for == FeaturesFor::HostDep;
-            self.activated_dependencies
-                .get(&(pkg_id, is_build))
-                .map(|deps| deps.contains(&dep_name))
-                .unwrap_or(false)
-        }
+        let is_build = self.opts.decouple_host_deps && features_for == FeaturesFor::HostDep;
+        self.activated_dependencies
+            .get(&(pkg_id, is_build))
+            .map(|deps| deps.contains(&dep_name))
+            .unwrap_or(false)
     }
 
     /// Variant of `activated_features` that returns `None` if this is
@@ -336,30 +299,28 @@ impl ResolvedFeatures {
         pkg_id: PackageId,
         features_for: FeaturesFor,
     ) -> CargoResult<Vec<InternedString>> {
-        if let Some(legacy) = &self.legacy_features {
-            Ok(legacy.get(&pkg_id).map_or_else(Vec::new, |v| v.clone()))
+        let is_build = self.opts.decouple_host_deps && features_for == FeaturesFor::HostDep;
+        if let Some(fs) = self.activated_features.get(&(pkg_id, is_build)) {
+            Ok(fs.iter().cloned().collect())
         } else {
-            let is_build = self.opts.decouple_host_deps && features_for == FeaturesFor::HostDep;
-            if let Some(fs) = self.activated_features.get(&(pkg_id, is_build)) {
-                Ok(fs.iter().cloned().collect())
-            } else {
-                bail!("features did not find {:?} {:?}", pkg_id, is_build)
-            }
+            bail!("features did not find {:?} {:?}", pkg_id, is_build)
         }
     }
 
     /// Compares the result against the original resolver behavior.
     ///
     /// Used by `cargo fix --edition` to display any differences.
     pub fn compare_legacy(&self, legacy: &ResolvedFeatures) -> DiffMap {
-        let legacy_features = legacy.legacy_features.as_ref().unwrap();
         self.activated_features
             .iter()
             .filter_map(|((pkg_id, for_host), new_features)| {
-                let old_features = match legacy_features.get(pkg_id) {
-                    Some(feats) => feats.iter().cloned().collect(),
-                    None => BTreeSet::new(),
-                };
+                let old_features = legacy
+                    .activated_features
+                    .get(&(*pkg_id, *for_host))
+                    // The new features may have for_host entries where the old one does not.
+                    .or_else(|| legacy.activated_features.get(&(*pkg_id, false)))
+                    .map(|feats| feats.iter().cloned().collect())
+                    .unwrap_or_else(|| BTreeSet::new());
                 // The new resolver should never add features.
                 assert_eq!(new_features.difference(&old_features).next(), None);
                 let removed_features: BTreeSet<_> =
@@ -427,17 +388,6 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
     ) -> CargoResult<ResolvedFeatures> {
         use crate::util::profile;
         let _p = profile::start("resolve features");
-
-        if !opts.new_resolver {
-            // Legacy mode.
-            return Ok(ResolvedFeatures {
-                activated_features: HashMap::new(),
-                activated_dependencies: HashMap::new(),
-                legacy_features: Some(resolve.features_clone()),
-                legacy_dependencies: Some(compute_legacy_deps(resolve)),
-                opts,
-            });
-        }
         let track_for_host = opts.decouple_host_deps || opts.ignore_inactive_targets;
         let mut r = FeatureResolver {
             ws,
@@ -460,8 +410,6 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
         Ok(ResolvedFeatures {
             activated_features: r.activated_features,
             activated_dependencies: r.activated_dependencies,
-            legacy_features: None,
-            legacy_dependencies: None,
             opts: r.opts,
         })
     }
@@ -826,19 +774,3 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
             .proc_macro()
     }
 }
-
-/// Computes a map of PackageId to the set of optional dependencies that are
-/// enabled for that dep (when the new resolver is not enabled).
-fn compute_legacy_deps(resolve: &Resolve) -> HashMap<PackageId, HashSet<InternedString>> {
-    let mut result: HashMap<PackageId, HashSet<InternedString>> = HashMap::new();
-    for pkg_id in resolve.iter() {
-        for (_dep_id, deps) in resolve.deps(pkg_id) {
-            for dep in deps {
-                if dep.is_optional() {
-                    result.entry(pkg_id).or_default().insert(dep.name_in_toml());
-                }
-            }
-        }
-    }
-    result
-}