Skip to content

Commit 321f14e

Browse files
authored
feat: implement workspace feature unification (#15157)
### What does this PR try to resolve? Adds workspace feature unification for #14774 ### How should we test and review this PR? In a workspace that has dependencies with different activated features depending on the packages being built, add the `resolver.feature-unification = "workspace"` option to `.cargo/config.toml`. Build the entire workspace with `--workspace` and then build individual packages, ensuring no dependencies are being recompiled. ### Additional information Originally, the RFC and tracking issue mention some more complex changes required in cargo's dependency/feature resolution phases in order to support workspace feature unification. However, it seems like it also works by just modifying the list of `PackageIdSpec`s passed to the workspace resolver to include all workspace members, and then using the original list of specs when generating the build units. I'm wondering if I missed something because this change feels a bit *too* simple... I tested it on a fairly large workspace containing about 100 packages and didn't see any recompilations when building with different sets of packages. I also added an integration test that verifies the correct features are enabled. If there are any other test cases I should include, please let me know and I'll try to add it.
2 parents e3fa31e + a471adb commit 321f14e

File tree

9 files changed

+276
-32
lines changed

9 files changed

+276
-32
lines changed

src/cargo/core/features.rs

+2
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,7 @@ unstable_cli_options!(
770770
direct_minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum (direct dependencies only)"),
771771
doctest_xcompile: bool = ("Compile and run doctests for non-host target using runner config"),
772772
dual_proc_macros: bool = ("Build proc-macros for both the host and the target"),
773+
feature_unification: bool = ("Enable new feature unification modes in workspaces"),
773774
features: Option<Vec<String>>,
774775
gc: bool = ("Track cache usage and \"garbage collect\" unused files"),
775776
#[serde(deserialize_with = "deserialize_git_features")]
@@ -1271,6 +1272,7 @@ impl CliUnstable {
12711272
"direct-minimal-versions" => self.direct_minimal_versions = parse_empty(k, v)?,
12721273
"doctest-xcompile" => self.doctest_xcompile = parse_empty(k, v)?,
12731274
"dual-proc-macros" => self.dual_proc_macros = parse_empty(k, v)?,
1275+
"feature-unification" => self.feature_unification = parse_empty(k, v)?,
12741276
"gc" => self.gc = parse_empty(k, v)?,
12751277
"git" => {
12761278
self.git = v.map_or_else(

src/cargo/core/workspace.rs

+23-5
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use crate::core::{
2121
use crate::core::{EitherManifest, Package, SourceId, VirtualManifest};
2222
use crate::ops;
2323
use crate::sources::{PathSource, SourceConfigMap, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
24+
use crate::util::context::FeatureUnification;
2425
use crate::util::edit_distance;
2526
use crate::util::errors::{CargoResult, ManifestError};
2627
use crate::util::interning::InternedString;
@@ -112,7 +113,8 @@ pub struct Workspace<'gctx> {
112113
/// and other places that use rust version.
113114
/// This is set based on the resolver version, config settings, and CLI flags.
114115
resolve_honors_rust_version: bool,
115-
116+
/// The feature unification mode used when building packages.
117+
resolve_feature_unification: FeatureUnification,
116118
/// Workspace-level custom metadata
117119
custom_metadata: Option<toml::Value>,
118120

@@ -246,6 +248,7 @@ impl<'gctx> Workspace<'gctx> {
246248
requested_lockfile_path: None,
247249
resolve_behavior: ResolveBehavior::V1,
248250
resolve_honors_rust_version: false,
251+
resolve_feature_unification: FeatureUnification::Selected,
249252
custom_metadata: None,
250253
local_overlays: HashMap::new(),
251254
}
@@ -307,13 +310,20 @@ impl<'gctx> Workspace<'gctx> {
307310
}
308311
}
309312
}
310-
if let CargoResolverConfig {
311-
incompatible_rust_versions: Some(incompatible_rust_versions),
312-
} = self.gctx().get::<CargoResolverConfig>("resolver")?
313-
{
313+
let config = self.gctx().get::<CargoResolverConfig>("resolver")?;
314+
if let Some(incompatible_rust_versions) = config.incompatible_rust_versions {
314315
self.resolve_honors_rust_version =
315316
incompatible_rust_versions == IncompatibleRustVersions::Fallback;
316317
}
318+
if self.gctx().cli_unstable().feature_unification {
319+
self.resolve_feature_unification = config
320+
.feature_unification
321+
.unwrap_or(FeatureUnification::Selected);
322+
} else if config.feature_unification.is_some() {
323+
self.gctx()
324+
.shell()
325+
.warn("ignoring `resolver.feature-unification` without `-Zfeature-unification`")?;
326+
};
317327

318328
Ok(())
319329
}
@@ -663,6 +673,14 @@ impl<'gctx> Workspace<'gctx> {
663673
self.resolve_honors_rust_version
664674
}
665675

676+
pub fn set_resolve_feature_unification(&mut self, feature_unification: FeatureUnification) {
677+
self.resolve_feature_unification = feature_unification;
678+
}
679+
680+
pub fn resolve_feature_unification(&self) -> FeatureUnification {
681+
self.resolve_feature_unification
682+
}
683+
666684
pub fn custom_metadata(&self) -> Option<&toml::Value> {
667685
self.custom_metadata.as_ref()
668686
}

src/cargo/ops/cargo_install.rs

+2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::ops::{common_for_install_and_uninstall::*, FilterRule};
99
use crate::ops::{CompileFilter, Packages};
1010
use crate::sources::source::Source;
1111
use crate::sources::{GitSource, PathSource, SourceConfigMap};
12+
use crate::util::context::FeatureUnification;
1213
use crate::util::errors::CargoResult;
1314
use crate::util::{Filesystem, GlobalContext, Rustc};
1415
use crate::{drop_println, ops};
@@ -862,6 +863,7 @@ fn make_ws_rustc_target<'gctx>(
862863
ws.set_resolve_honors_rust_version(Some(false));
863864
ws
864865
};
866+
ws.set_resolve_feature_unification(FeatureUnification::Selected);
865867
ws.set_ignore_lock(gctx.lock_update_allowed());
866868
ws.set_requested_lockfile_path(lockfile_path.map(|p| p.to_path_buf()));
867869
// if --lockfile-path is set, imply --locked

src/cargo/ops/fix.rs

+1
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ pub fn fix(
122122
}
123123
let mut ws = Workspace::new(&root_manifest, gctx)?;
124124
ws.set_resolve_honors_rust_version(Some(original_ws.resolve_honors_rust_version()));
125+
ws.set_resolve_feature_unification(original_ws.resolve_feature_unification());
125126
ws.set_requested_lockfile_path(opts.requested_lockfile_path.clone());
126127

127128
// Spin up our lock server, which our subprocesses will use to synchronize fixes.

src/cargo/ops/resolve.rs

+5
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ use crate::core::Workspace;
7575
use crate::ops;
7676
use crate::sources::RecursivePathSource;
7777
use crate::util::cache_lock::CacheLockMode;
78+
use crate::util::context::FeatureUnification;
7879
use crate::util::errors::CargoResult;
7980
use crate::util::CanonicalUrl;
8081
use anyhow::Context as _;
@@ -144,6 +145,10 @@ pub fn resolve_ws_with_opts<'gctx>(
144145
force_all_targets: ForceAllTargets,
145146
dry_run: bool,
146147
) -> CargoResult<WorkspaceResolve<'gctx>> {
148+
let specs = match ws.resolve_feature_unification() {
149+
FeatureUnification::Selected => specs,
150+
FeatureUnification::Workspace => &ops::Packages::All(Vec::new()).to_package_id_specs(ws)?,
151+
};
147152
let mut registry = ws.package_registry()?;
148153
let (resolve, resolved_with_overrides) = if ws.ignore_lock() {
149154
let add_patches = true;

src/cargo/util/context/mod.rs

+8
Original file line numberDiff line numberDiff line change
@@ -2745,6 +2745,7 @@ impl BuildTargetConfig {
27452745
#[serde(rename_all = "kebab-case")]
27462746
pub struct CargoResolverConfig {
27472747
pub incompatible_rust_versions: Option<IncompatibleRustVersions>,
2748+
pub feature_unification: Option<FeatureUnification>,
27482749
}
27492750

27502751
#[derive(Debug, Deserialize, PartialEq, Eq)]
@@ -2754,6 +2755,13 @@ pub enum IncompatibleRustVersions {
27542755
Fallback,
27552756
}
27562757

2758+
#[derive(Copy, Clone, Debug, Deserialize)]
2759+
#[serde(rename_all = "kebab-case")]
2760+
pub enum FeatureUnification {
2761+
Selected,
2762+
Workspace,
2763+
}
2764+
27572765
#[derive(Deserialize, Default)]
27582766
#[serde(rename_all = "kebab-case")]
27592767
pub struct TermConfig {

tests/testsuite/cargo/z_help/stdout.term.svg

+29-27
Loading

0 commit comments

Comments
 (0)