implement package feature unification (#15684)

### What does this PR try to resolve?

Implements another part of feature unification (#14774,
[rfc](https://github.com/rust-lang/rfcs/blob/1c590ce05d676e72e2217845ee054758d3a6df34/text/3692-feature-unification.md)).
The `workspace` option was implemented in #15157, this adds the
`package` option.

### How to test and review this PR?

The important change is changing `WorkspaceResolve` so it can contain
multiple `ResolvedFeature`s. Along with that, it also needs to know
which specs those features are resolved for. This was used in several
other places:
- `cargo fix --edition` (from 2018 to 2021) - I think it should be ok to
disallow using `cargo fix --edition` when someone already uses this
feature.
- building std - it should be safe to assume std is not using this
feature so I just unwrap there. I'm not sure if some attempt to later
feature unification would be better.
- `cargo tree` - I just use the first feature set. This is definitely
not ideal, but I'm not entirely sure what's the correct solution here.
Printing multiple trees? Disallowing this, forcing users to select only
one package?

Based on comments in #15157 I've added tests first with `selected`
feature unification and then changed that after implementation. I'm not
sure if that's how you expect the tests to be added first, if not, I can
change the history.

I've expanded the test checking that this is ignored for `cargo install`
although it should work the same way even if it is not ignored
(`selected` and `package` are the same thing when just one package is
selected).

Author: epage

src/cargo/core/compiler/standard_lib.rs

@@ -96,7 +96,7 @@ pub fn resolve_std<'gctx>(
         &features, /*all_features*/ false, /*uses_default_features*/ false,
     )?;
     let dry_run = false;
-    let resolve = ops::resolve_ws_with_opts(
+    let mut resolve = ops::resolve_ws_with_opts(
         &std_ws,
         target_data,
         &build_config.requested_kinds,
@@ -106,10 +106,15 @@ pub fn resolve_std<'gctx>(
         crate::core::resolver::features::ForceAllTargets::No,
         dry_run,
     )?;
+    debug_assert_eq!(resolve.specs_and_features.len(), 1);
     Ok((
         resolve.pkg_set,
         resolve.targeted_resolve,
-        resolve.resolved_features,
+        resolve
+            .specs_and_features
+            .pop()
+            .expect("resolve should have a single spec with resolved features")
+            .resolved_features,
     ))
 }
 

src/cargo/core/workspace.rs

@@ -1287,7 +1287,7 @@ impl<'gctx> Workspace<'gctx> {
         self.target_dir = Some(target_dir);
     }
 
-    /// Returns a Vec of `(&Package, RequestedFeatures)` tuples that
+    /// Returns a Vec of `(&Package, CliFeatures)` tuples that
     /// represent the workspace members that were requested on the command-line.
     ///
     /// `specs` may be empty, which indicates it should return all workspace

src/cargo/ops/cargo_compile/mod.rs

@@ -52,7 +52,7 @@ use crate::core::resolver::{HasDevUnits, Resolve};
 use crate::core::{PackageId, PackageSet, SourceId, TargetKind, Workspace};
 use crate::drop_println;
 use crate::ops;
-use crate::ops::resolve::WorkspaceResolve;
+use crate::ops::resolve::{SpecsAndResolvedFeatures, WorkspaceResolve};
 use crate::util::context::{GlobalContext, WarningHandling};
 use crate::util::interning::InternedString;
 use crate::util::{CargoResult, StableHasher};
@@ -284,7 +284,7 @@ pub fn create_bcx<'a, 'gctx>(
         mut pkg_set,
         workspace_resolve,
         targeted_resolve: resolve,
-        resolved_features,
+        specs_and_features,
     } = resolve;
 
     let std_resolve_features = if let Some(crates) = &gctx.cli_unstable().build_std {
@@ -363,72 +363,91 @@ pub fn create_bcx<'a, 'gctx>(
         })
         .collect();
 
-    // Passing `build_config.requested_kinds` instead of
-    // `explicit_host_kinds` here so that `generate_root_units` can do
-    // its own special handling of `CompileKind::Host`. It will
-    // internally replace the host kind by the `explicit_host_kind`
-    // before setting as a unit.
-    let generator = UnitGenerator {
-        ws,
-        packages: &to_builds,
-        spec,
-        target_data: &target_data,
-        filter,
-        requested_kinds: &build_config.requested_kinds,
-        explicit_host_kind,
-        intent: build_config.intent,
-        resolve: &resolve,
-        workspace_resolve: &workspace_resolve,
-        resolved_features: &resolved_features,
-        package_set: &pkg_set,
-        profiles: &profiles,
-        interner,
-        has_dev_units,
-    };
-    let mut units = generator.generate_root_units()?;
+    let mut units = Vec::new();
+    let mut unit_graph = HashMap::new();
+    let mut scrape_units = Vec::new();
 
-    if let Some(args) = target_rustc_crate_types {
-        override_rustc_crate_types(&mut units, args, interner)?;
-    }
+    for SpecsAndResolvedFeatures {
+        specs,
+        resolved_features,
+    } in &specs_and_features
+    {
+        // Passing `build_config.requested_kinds` instead of
+        // `explicit_host_kinds` here so that `generate_root_units` can do
+        // its own special handling of `CompileKind::Host`. It will
+        // internally replace the host kind by the `explicit_host_kind`
+        // before setting as a unit.
+        let spec_names = specs.iter().map(|spec| spec.name()).collect::<Vec<_>>();
+        let packages = to_builds
+            .iter()
+            .filter(|package| spec_names.contains(&package.name().as_str()))
+            .cloned()
+            .collect::<Vec<_>>();
+        let generator = UnitGenerator {
+            ws,
+            packages: &packages,
+            spec,
+            target_data: &target_data,
+            filter,
+            requested_kinds: &build_config.requested_kinds,
+            explicit_host_kind,
+            intent: build_config.intent,
+            resolve: &resolve,
+            workspace_resolve: &workspace_resolve,
+            resolved_features: &resolved_features,
+            package_set: &pkg_set,
+            profiles: &profiles,
+            interner,
+            has_dev_units,
+        };
+        let mut targeted_root_units = generator.generate_root_units()?;
 
-    let should_scrape = build_config.intent.is_doc() && gctx.cli_unstable().rustdoc_scrape_examples;
-    let mut scrape_units = if should_scrape {
-        generator.generate_scrape_units(&units)?
-    } else {
-        Vec::new()
-    };
+        if let Some(args) = target_rustc_crate_types {
+            override_rustc_crate_types(&mut targeted_root_units, args, interner)?;
+        }
+
+        let should_scrape =
+            build_config.intent.is_doc() && gctx.cli_unstable().rustdoc_scrape_examples;
+        let targeted_scrape_units = if should_scrape {
+            generator.generate_scrape_units(&targeted_root_units)?
+        } else {
+            Vec::new()
+        };
+
+        let std_roots = if let Some(crates) = gctx.cli_unstable().build_std.as_ref() {
+            let (std_resolve, std_features) = std_resolve_features.as_ref().unwrap();
+            standard_lib::generate_std_roots(
+                &crates,
+                &targeted_root_units,
+                std_resolve,
+                std_features,
+                &explicit_host_kinds,
+                &pkg_set,
+                interner,
+                &profiles,
+                &target_data,
+            )?
+        } else {
+            Default::default()
+        };
 
-    let std_roots = if let Some(crates) = gctx.cli_unstable().build_std.as_ref() {
-        let (std_resolve, std_features) = std_resolve_features.as_ref().unwrap();
-        standard_lib::generate_std_roots(
-            &crates,
-            &units,
-            std_resolve,
-            std_features,
-            &explicit_host_kinds,
+        unit_graph.extend(build_unit_dependencies(
+            ws,
             &pkg_set,
-            interner,
-            &profiles,
+            &resolve,
+            &resolved_features,
+            std_resolve_features.as_ref(),
+            &targeted_root_units,
+            &targeted_scrape_units,
+            &std_roots,
+            build_config.intent,
             &target_data,
-        )?
-    } else {
-        Default::default()
-    };
-
-    let mut unit_graph = build_unit_dependencies(
-        ws,
-        &pkg_set,
-        &resolve,
-        &resolved_features,
-        std_resolve_features.as_ref(),
-        &units,
-        &scrape_units,
-        &std_roots,
-        build_config.intent,
-        &target_data,
-        &profiles,
-        interner,
-    )?;
+            &profiles,
+            interner,
+        )?);
+        units.extend(targeted_root_units);
+        scrape_units.extend(targeted_scrape_units);
+    }
 
     // TODO: In theory, Cargo should also dedupe the roots, but I'm uncertain
     // what heuristics to use in that case.

src/cargo/ops/fix/mod.rs

@@ -588,7 +588,15 @@ fn check_resolver_change<'gctx>(
             feature_opts,
         )?;
 
-        let diffs = v2_features.compare_legacy(&ws_resolve.resolved_features);
+        if ws_resolve.specs_and_features.len() != 1 {
+            bail!(r#"cannot fix edition when using `feature-unification = "package"`."#);
+        }
+        let resolved_features = &ws_resolve
+            .specs_and_features
+            .first()
+            .expect("We've already checked that there is exactly one.")
+            .resolved_features;
+        let diffs = v2_features.compare_legacy(resolved_features);
         Ok((ws_resolve, diffs))
     };
     let (_, without_dev_diffs) = resolve_differences(HasDevUnits::No)?;

src/cargo/ops/resolve.rs

@@ -81,7 +81,9 @@ use crate::util::CanonicalUrl;
 use anyhow::Context as _;
 use cargo_util::paths;
 use cargo_util_schemas::core::PartialVersion;
+use std::borrow::Cow;
 use std::collections::{HashMap, HashSet};
+use std::rc::Rc;
 use tracing::{debug, trace};
 
 /// Filter for keep using Package ID from previous lockfile.
@@ -96,9 +98,18 @@ pub struct WorkspaceResolve<'gctx> {
     /// This may be `None` for things like `cargo install` and `-Zavoid-dev-deps`.
     /// This does not include `paths` overrides.
     pub workspace_resolve: Option<Resolve>,
-    /// The narrowed resolve, with the specific features enabled, and only the
-    /// given package specs requested.
+    /// The narrowed resolve, with the specific features enabled.
     pub targeted_resolve: Resolve,
+    /// Package specs requested for compilation along with specific features enabled. This usually
+    /// has the length of one but there may be more specs with different features when using the
+    /// `package` feature resolver.
+    pub specs_and_features: Vec<SpecsAndResolvedFeatures>,
+}
+
+/// Pair of package specs requested for compilation along with enabled features.
+pub struct SpecsAndResolvedFeatures {
+    /// Packages that are supposed to be built.
+    pub specs: Vec<PackageIdSpec>,
     /// The features activated per package.
     pub resolved_features: ResolvedFeatures,
 }
@@ -145,10 +156,21 @@ pub fn resolve_ws_with_opts<'gctx>(
     force_all_targets: ForceAllTargets,
     dry_run: bool,
 ) -> CargoResult<WorkspaceResolve<'gctx>> {
-    let specs = match ws.resolve_feature_unification() {
-        FeatureUnification::Selected => specs,
-        FeatureUnification::Workspace => &ops::Packages::All(Vec::new()).to_package_id_specs(ws)?,
+    let feature_unification = ws.resolve_feature_unification();
+    let individual_specs = match feature_unification {
+        FeatureUnification::Selected => vec![specs.to_owned()],
+        FeatureUnification::Workspace => {
+            vec![ops::Packages::All(Vec::new()).to_package_id_specs(ws)?]
+        }
+        FeatureUnification::Package => specs.iter().map(|spec| vec![spec.clone()]).collect(),
     };
+    let specs: Vec<_> = individual_specs
+        .iter()
+        .map(|specs| specs.iter())
+        .flatten()
+        .cloned()
+        .collect();
+    let specs = &specs[..];
     let mut registry = ws.package_registry()?;
     let (resolve, resolved_with_overrides) = if ws.ignore_lock() {
         let add_patches = true;
@@ -229,9 +251,9 @@ pub fn resolve_ws_with_opts<'gctx>(
 
     let pkg_set = get_resolved_packages(&resolved_with_overrides, registry)?;
 
-    let member_ids = ws
-        .members_with_features(specs, cli_features)?
-        .into_iter()
+    let members_with_features = ws.members_with_features(specs, cli_features)?;
+    let member_ids = members_with_features
+        .iter()
         .map(|(p, _fts)| p.package_id())
         .collect::<Vec<_>>();
     pkg_set.download_accessible(
@@ -243,33 +265,70 @@ pub fn resolve_ws_with_opts<'gctx>(
         force_all_targets,
     )?;
 
-    let feature_opts = FeatureOpts::new(ws, has_dev_units, force_all_targets)?;
-    let resolved_features = FeatureResolver::resolve(
-        ws,
-        target_data,
-        &resolved_with_overrides,
-        &pkg_set,
-        cli_features,
-        specs,
-        requested_targets,
-        feature_opts,
-    )?;
+    let mut specs_and_features = Vec::new();
 
-    pkg_set.warn_no_lib_packages_and_artifact_libs_overlapping_deps(
-        ws,
-        &resolved_with_overrides,
-        &member_ids,
-        has_dev_units,
-        requested_targets,
-        target_data,
-        force_all_targets,
-    )?;
+    for specs in individual_specs {
+        let feature_opts = FeatureOpts::new(ws, has_dev_units, force_all_targets)?;
+
+        // We want to narrow the features to the current specs so that stuff like `cargo check -p a
+        // -p b -F a/a,b/b` works and the resolver does not contain that `a` does not have feature
+        // `b` and vice-versa. However, resolver v1 needs to see even features of unselected
+        // packages turned on if it was because of working directory being inside the unselected
+        // package, because they might turn on a feature of a selected package.
+        let narrowed_features = match feature_unification {
+            FeatureUnification::Package => {
+                let mut narrowed_features = cli_features.clone();
+                let enabled_features = members_with_features
+                    .iter()
+                    .filter_map(|(package, cli_features)| {
+                        specs
+                            .iter()
+                            .any(|spec| spec.matches(package.package_id()))
+                            .then_some(cli_features.features.iter())
+                    })
+                    .flatten()
+                    .cloned()
+                    .collect();
+                narrowed_features.features = Rc::new(enabled_features);
+                Cow::Owned(narrowed_features)
+            }
+            FeatureUnification::Selected | FeatureUnification::Workspace => {
+                Cow::Borrowed(cli_features)
+            }
+        };
+
+        let resolved_features = FeatureResolver::resolve(
+            ws,
+            target_data,
+            &resolved_with_overrides,
+            &pkg_set,
+            &*narrowed_features,
+            &specs,
+            requested_targets,
+            feature_opts,
+        )?;
+
+        pkg_set.warn_no_lib_packages_and_artifact_libs_overlapping_deps(
+            ws,
+            &resolved_with_overrides,
+            &member_ids,
+            has_dev_units,
+            requested_targets,
+            target_data,
+            force_all_targets,
+        )?;
+
+        specs_and_features.push(SpecsAndResolvedFeatures {
+            specs,
+            resolved_features,
+        });
+    }
 
     Ok(WorkspaceResolve {
         pkg_set,
         workspace_resolve: resolve,
         targeted_resolve: resolved_with_overrides,
-        resolved_features,
+        specs_and_features,
     })
 }