refactor(resolve): allow multiple resolved feature sets in workspace resolve

Author: mladedav

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/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,20 @@ 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)?]
+        }
     };
+    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 +250,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 +264,49 @@ 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)?;
+
+        let narrowed_features = match feature_unification {
+            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,
     })
 }