Skip to content

Remove disabled optional weak dependencies from Cargo.lock #11938

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

Closed
wants to merge 5 commits into from
Closed
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
3 changes: 2 additions & 1 deletion crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::task::Poll;
use std::time::Instant;

use cargo::core::dependency::DepKind;
use cargo::core::resolver::{self, ResolveOpts, VersionPreferences};
use cargo::core::resolver::{self, ResolveOpts, ResolveVersion, VersionPreferences};
use cargo::core::source::{GitReference, QueryKind, SourceId};
use cargo::core::Resolve;
use cargo::core::{Dependency, PackageId, Registry, Summary};
Expand Down Expand Up @@ -196,6 +196,7 @@ pub fn resolve_with_config_raw(
&VersionPreferences::default(),
Some(config),
true,
ResolveVersion::default(),
);

// The largest test in our suite takes less then 30 sec.
Expand Down
59 changes: 42 additions & 17 deletions src/cargo/core/resolver/dep_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use crate::core::resolver::context::Context;
use crate::core::resolver::errors::describe_path_in_context;
use crate::core::resolver::types::{ConflictReason, DepInfo, FeaturesSet};
use crate::core::resolver::{
ActivateError, ActivateResult, CliFeatures, RequestedFeatures, ResolveOpts, VersionOrdering,
VersionPreferences,
ActivateError, ActivateResult, CliFeatures, RequestedFeatures, ResolveOpts, ResolveVersion,
VersionOrdering, VersionPreferences,
};
use crate::core::{
Dependency, FeatureValue, PackageId, PackageIdSpec, QueryKind, Registry, Summary,
Expand Down Expand Up @@ -230,6 +230,7 @@ impl<'a> RegistryQueryer<'a> {
candidate: &Summary,
opts: &ResolveOpts,
first_minimal_version: bool,
version: ResolveVersion,
) -> ActivateResult<Rc<(HashSet<InternedString>, Rc<Vec<DepInfo>>)>> {
// if we have calculated a result before, then we can just return it,
// as it is a "pure" query of its arguments.
Expand All @@ -242,7 +243,7 @@ impl<'a> RegistryQueryer<'a> {
// First, figure out our set of dependencies based on the requested set
// of features. This also calculates what features we're going to enable
// for our own dependencies.
let (used_features, deps) = resolve_features(parent, candidate, opts)?;
let (used_features, deps) = resolve_features(parent, candidate, opts, version)?;

// Next, transform all dependencies into a list of possible candidates
// which can satisfy that dependency.
Expand Down Expand Up @@ -294,22 +295,36 @@ pub fn resolve_features<'b>(
parent: Option<PackageId>,
s: &'b Summary,
opts: &'b ResolveOpts,
version: ResolveVersion,
) -> ActivateResult<(HashSet<InternedString>, Vec<(Dependency, FeaturesSet)>)> {
// First, filter by dev-dependencies.
let deps = s.dependencies();
let deps = deps.iter().filter(|d| d.is_transitive() || opts.dev_deps);

let reqs = build_requirements(parent, s, opts)?;
let mut ret = Vec::new();
let default_dep = BTreeSet::new();
let default_dep = (false, BTreeSet::new());
let mut valid_dep_names = HashSet::new();

// Next, collect all actually enabled dependencies and their features.
for dep in deps {
// Skip optional dependencies, but not those enabled through a
// feature
if dep.is_optional() && !reqs.deps.contains_key(&dep.name_in_toml()) {
continue;
if dep.is_optional() {
let disabled = reqs
.deps
.get(&dep.name_in_toml())
.map(|(weak, _)| {
// Preserve old behaviour on older resolve versions.
if version <= ResolveVersion::V3 {
return false;
}
*weak && !reqs.features.contains(&dep.name_in_toml())
})
.unwrap_or(true);
if disabled {
continue;
}
}
valid_dep_names.insert(dep.name_in_toml());
// So we want this dependency. Move the features we want from
Expand All @@ -319,7 +334,8 @@ pub fn resolve_features<'b>(
.deps
.get(&dep.name_in_toml())
.unwrap_or(&default_dep)
.clone();
.clone()
.1;
base.extend(dep.features().iter());
ret.push((dep.clone(), Rc::new(base)));
}
Expand All @@ -329,7 +345,10 @@ pub fn resolve_features<'b>(
// validation is done either in `build_requirements` or
// `build_feature_map`.
if parent.is_none() {
for dep_name in reqs.deps.keys() {
for (dep_name, (weak, _)) in reqs.deps.iter() {
if *weak && !reqs.features.contains(dep_name) {
continue;
}
if !valid_dep_names.contains(dep_name) {
let e = RequirementError::MissingDependency(*dep_name);
return Err(e.into_activate_error(parent, s));
Expand Down Expand Up @@ -400,11 +419,11 @@ fn build_requirements<'a, 'b: 'a>(
#[derive(Debug)]
struct Requirements<'a> {
summary: &'a Summary,
/// The deps map is a mapping of dependency name to list of features enabled.
/// The deps map is a mapping of dependency name to (weak, list of features enabled).
///
/// The resolver will activate all of these dependencies, with the given
/// features enabled.
deps: HashMap<InternedString, BTreeSet<InternedString>>,
/// Non-weak deps will all be activated, while for weak deps the
/// corresponding feature needs to be enabled first.
deps: HashMap<InternedString, (bool, BTreeSet<InternedString>)>,
/// The set of features enabled on this package which is later used when
/// compiling to instruct the code what features were enabled.
features: HashSet<InternedString>,
Expand Down Expand Up @@ -457,12 +476,21 @@ impl Requirements<'_> {
{
self.require_feature(package)?;
}
self.deps.entry(package).or_default().insert(feat);

let dep = self
.deps
.entry(package)
.or_insert_with(|| (weak, BTreeSet::new()));
// If the feature is non-weak, mark the entire dep as non-weak.
dep.0 &= weak;
// Add the feature as to be enabled.
dep.1.insert(feat);

Ok(())
}

fn require_dependency(&mut self, pkg: InternedString) {
self.deps.entry(pkg).or_default();
self.deps.entry(pkg).or_default().0 = false;
}

fn require_feature(&mut self, feat: InternedString) -> Result<(), RequirementError> {
Expand Down Expand Up @@ -494,9 +522,6 @@ impl Requirements<'_> {
FeatureValue::DepFeature {
dep_name,
dep_feature,
// Weak features are always activated in the dependency
// resolver. They will be narrowed inside the new feature
// resolver.
weak,
} => self.require_dep_feature(*dep_name, *dep_feature, *weak)?,
};
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/core/resolver/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ impl EncodableResolve {
let mut checksums = HashMap::new();

let mut version = match self.version {
Some(4) => ResolveVersion::V4,
Some(3) => ResolveVersion::V3,
Some(n) => bail!(
"lock file version `{}` was found, but this version of Cargo \
Expand Down Expand Up @@ -612,6 +613,7 @@ impl ser::Serialize for Resolve {
metadata,
patch,
version: match self.version() {
ResolveVersion::V4 => Some(4),
ResolveVersion::V3 => Some(3),
ResolveVersion::V2 | ResolveVersion::V1 => None,
},
Expand Down
9 changes: 8 additions & 1 deletion src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ pub fn resolve(
version_prefs: &VersionPreferences,
config: Option<&Config>,
check_public_visible_dependencies: bool,
version: ResolveVersion,
) -> CargoResult<Resolve> {
let _p = profile::start("resolving");
let minimal_versions = match config {
Expand All @@ -158,6 +159,7 @@ pub fn resolve(
summaries,
direct_minimal_versions,
config,
version,
)?;
if registry.reset_pending() {
break cx;
Expand Down Expand Up @@ -190,7 +192,7 @@ pub fn resolve(
cksums,
BTreeMap::new(),
Vec::new(),
ResolveVersion::default(),
version,
summaries,
);

Expand All @@ -212,6 +214,7 @@ fn activate_deps_loop(
summaries: &[(Summary, ResolveOpts)],
direct_minimal_versions: bool,
config: Option<&Config>,
version: ResolveVersion,
) -> CargoResult<Context> {
let mut backtrack_stack = Vec::new();
let mut remaining_deps = RemainingDeps::new();
Expand All @@ -230,6 +233,7 @@ fn activate_deps_loop(
summary.clone(),
direct_minimal_versions,
opts,
version,
);
match res {
Ok(Some((frame, _))) => remaining_deps.push(frame),
Expand Down Expand Up @@ -436,6 +440,7 @@ fn activate_deps_loop(
candidate,
direct_minimal_version,
&opts,
version,
);

let successfully_activated = match res {
Expand Down Expand Up @@ -650,6 +655,7 @@ fn activate(
candidate: Summary,
first_minimal_version: bool,
opts: &ResolveOpts,
version: ResolveVersion,
) -> ActivateResult<Option<(DepsFrame, Duration)>> {
let candidate_pid = candidate.package_id();
cx.age += 1;
Expand Down Expand Up @@ -706,6 +712,7 @@ fn activate(
&candidate,
opts,
first_minimal_version,
version,
)?;

// Record what list of features is active for this package.
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/core/resolver/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ pub enum ResolveVersion {
/// V3 by default staring in 1.53.
#[default]
V3,
/// Weak dependency features are not treated as non-weak.
V4,
}

impl Resolve {
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,8 @@ pub fn resolve_with_previous<'cfg>(
None => root_replace.to_vec(),
};

let version = previous.map(|p| p.version()).unwrap_or_default();

ws.preload(registry);
let mut resolved = resolver::resolve(
&summaries,
Expand All @@ -505,6 +507,7 @@ pub fn resolve_with_previous<'cfg>(
ws.unstable_features()
.require(Feature::public_dependency())
.is_ok(),
version,
)?;
let patches: Vec<_> = registry
.patches()
Expand Down
Loading