Skip to content

implement package feature unification #15684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
))
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
145 changes: 82 additions & 63 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 9 additions & 1 deletion src/cargo/ops/fix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
117 changes: 88 additions & 29 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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,
})
}

Expand Down
Loading