diff --git a/src/cargo/core/resolver/conflict_cache.rs b/src/cargo/core/resolver/conflict_cache.rs index ea9f544f454..fcca2fc1637 100644 --- a/src/cargo/core/resolver/conflict_cache.rs +++ b/src/cargo/core/resolver/conflict_cache.rs @@ -3,7 +3,7 @@ use std::collections::{BTreeMap, HashMap}; use rustc_hash::{FxHashMap, FxHashSet}; use tracing::trace; -use super::types::ConflictMap; +use super::types::{ActivationsKey, ConflictMap}; use crate::core::resolver::ResolverContext; use crate::core::{Dependency, PackageId}; @@ -14,7 +14,12 @@ enum ConflictStoreTrie { Leaf(ConflictMap), /// A map from an element to a subtrie where /// all the sets in the subtrie contains that element. - Node(BTreeMap), + Node( + BTreeMap< + ActivationsKey, + HashMap<&'static semver::Version, ConflictStoreTrie, rustc_hash::FxBuildHasher>, + >, + ), } impl ConflictStoreTrie { @@ -24,7 +29,7 @@ impl ConflictStoreTrie { /// one that will allow for the most jump-back. fn find( &self, - is_active: &impl Fn(PackageId) -> Option, + in_activation_slot: &impl Fn(&ActivationsKey) -> Option<(PackageId, usize)>, must_contain: Option, mut max_age: usize, ) -> Option<(&ConflictMap, usize)> { @@ -39,36 +44,45 @@ impl ConflictStoreTrie { } ConflictStoreTrie::Node(m) => { let mut out = None; - for (&pid, store) in must_contain - .map(|f| m.range(..=f)) + for (activations_key, map) in must_contain + .map(|f| m.range(..=(f.as_activations_key()))) .unwrap_or_else(|| m.range(..)) { - // If the key is active, then we need to check all of the corresponding subtrie. - if let Some(age_this) = is_active(pid) { - if age_this >= max_age && must_contain != Some(pid) { - // not worth looking at, it is to old. - continue; - } - if let Some((o, age_o)) = - store.find(is_active, must_contain.filter(|&f| f != pid), max_age) - { - let age = if must_contain == Some(pid) { - // all the results will include `must_contain` - // so the age of must_contain is not relevant to find the best result. - age_o - } else { - std::cmp::max(age_this, age_o) - }; - if max_age > age { - // we found one that can jump-back further so replace the out. - out = Some((o, age)); - // and don't look at anything older - max_age = age - } - } + // Find the package that is active for this activation key. + let Some((pid, age_this)) = in_activation_slot(&activations_key) else { + // Else, if it is not active then there is no way any of the corresponding + // subtries will be conflicting. + continue; + }; + if age_this >= max_age && must_contain != Some(pid) { + // not worth looking at, it is to old. + continue; + } + // If the active package has a stored conflict ... + let Some(store) = map.get(pid.version()) else { + continue; + }; + // then we need to check the corresponding subtrie. + let Some((o, age_o)) = store.find( + in_activation_slot, + must_contain.filter(|&f| f != pid), + max_age, + ) else { + continue; + }; + let age = if must_contain == Some(pid) { + // all the results will include `must_contain` + // so the age of must_contain is not relevant to find the best result. + age_o + } else { + std::cmp::max(age_this, age_o) + }; + if max_age > age { + // we found one that can jump-back further so replace the out. + out = Some((o, age)); + // and don't look at anything older + max_age = age } - // Else, if it is not active then there is no way any of the corresponding - // subtrie will be conflicting. } out } @@ -78,7 +92,9 @@ impl ConflictStoreTrie { fn insert(&mut self, mut iter: impl Iterator, con: ConflictMap) { if let Some(pid) = iter.next() { if let ConflictStoreTrie::Node(p) = self { - p.entry(pid) + p.entry(pid.as_activations_key().clone()) + .or_default() + .entry(pid.version()) .or_insert_with(|| ConflictStoreTrie::Node(BTreeMap::new())) .insert(iter, con); } @@ -157,13 +173,13 @@ impl ConflictCache { pub fn find( &self, dep: &Dependency, - is_active: &impl Fn(PackageId) -> Option, + in_activation_slot: &impl Fn(&ActivationsKey) -> Option<(PackageId, usize)>, must_contain: Option, max_age: usize, ) -> Option<&ConflictMap> { self.con_from_dep .get(dep)? - .find(is_active, must_contain, max_age) + .find(in_activation_slot, must_contain, max_age) .map(|(c, _)| c) } /// Finds any known set of conflicts, if any, @@ -176,7 +192,12 @@ impl ConflictCache { dep: &Dependency, must_contain: Option, ) -> Option<&ConflictMap> { - let out = self.find(dep, &|id| cx.is_active(id), must_contain, usize::MAX); + let out = self.find( + dep, + &|id| cx.in_activation_slot(id), + must_contain, + usize::MAX, + ); if cfg!(debug_assertions) { if let Some(c) = &out { assert!(cx.is_conflicting(None, c).is_some()); @@ -195,6 +216,13 @@ impl ConflictCache { /// `dep` is known to be unresolvable if /// all the `PackageId` entries are activated. pub fn insert(&mut self, dep: &Dependency, con: &ConflictMap) { + if cfg!(debug_assertions) { + // Check that the iterator is sorted by activation key. + // `ConflictMap` is a `BTreeMap` so it is already sorted. + // But, if the ord on `ActivationsKey` changes to not match ord on `PackageId`, this will fail. + assert!(con.keys().is_sorted_by_key(|p| p.as_activations_key())) + } + self.con_from_dep .entry(dep.clone()) .or_insert_with(|| ConflictStoreTrie::Node(BTreeMap::new())) diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index d1bae02f8fa..0dc5568bb69 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -156,6 +156,12 @@ impl ResolverContext { .and_then(|(s, l)| if s.package_id() == id { Some(*l) } else { None }) } + /// If a package is active that has the same semver compatibility range + /// returns the `PackageId` and `ContextAge` when it was added + pub fn in_activation_slot(&self, id: &ActivationsKey) -> Option<(PackageId, ContextAge)> { + self.activations.get(id).map(|(s, l)| (s.package_id(), *l)) + } + /// Checks whether all of `parent` and the keys of `conflicting activations` /// are still active. /// If so returns the `ContextAge` when the newest one was added. diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index a35f2d4643b..0e8d57d8a8f 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -857,15 +857,16 @@ fn generalize_conflicting( .iter() .rev() // the last one to be tried is the least likely to be in the cache, so start with that. .map(|other| { + let other_activations_key = other.package_id().as_activations_key(); past_conflicting_activations .find( dep, &|id| { - if id == other.package_id() { + if id == &other_activations_key { // we are imagining that we used other instead - Some(backtrack_critical_age) + Some((other.package_id(), backtrack_critical_age)) } else { - cx.is_active(id) + cx.in_activation_slot(id) } }, Some(other.package_id()), diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 2248d79bbf3..470d420dbde 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -193,9 +193,9 @@ impl std::hash::Hash for ActivationsKey { /// same. #[derive(Clone, Copy, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)] pub enum SemverCompatibility { - Major(NonZeroU64), - Minor(NonZeroU64), Patch(u64), + Minor(NonZeroU64), + Major(NonZeroU64), } impl From<&semver::Version> for SemverCompatibility {