diff --git a/src/cargo/core/resolver/conflict_cache.rs b/src/cargo/core/resolver/conflict_cache.rs index 29aac8fd2ed..aacab60be18 100644 --- a/src/cargo/core/resolver/conflict_cache.rs +++ b/src/cargo/core/resolver/conflict_cache.rs @@ -11,36 +11,39 @@ enum ConflictStoreTrie { Leaf(BTreeMap), /// a Node is a map from an element to a subTrie where /// all the Sets in the subTrie contains that element. - Node(HashMap), + Node(BTreeMap), } impl ConflictStoreTrie { /// Finds any known set of conflicts, if any, /// which are activated in `cx` and pass the `filter` specified? - fn find_conflicting( + fn find_conflicting( &self, cx: &Context, - filter: &F, - ) -> Option<&BTreeMap> - where - for<'r> F: Fn(&'r &BTreeMap) -> bool, - { + must_contain: Option, + ) -> Option<&BTreeMap> { match self { ConflictStoreTrie::Leaf(c) => { - if filter(&c) { + if must_contain.is_none() { // is_conflicting checks that all the elements are active, // but we have checked each one by the recursion of this function. debug_assert!(cx.is_conflicting(None, c)); Some(c) } else { + // we did not find `must_contain` so we need to keep looking. None } } ConflictStoreTrie::Node(m) => { - for (&pid, store) in m { + for (&pid, store) in must_contain + .map(|f| m.range(..=f)) + .unwrap_or_else(|| m.range(..)) + { // if the key is active then we need to check all of the corresponding subTrie. if cx.is_active(pid) { - if let Some(o) = store.find_conflicting(cx, filter) { + if let Some(o) = + store.find_conflicting(cx, must_contain.filter(|&f| f != pid)) + { return Some(o); } } // else, if it is not active then there is no way any of the corresponding @@ -59,7 +62,7 @@ impl ConflictStoreTrie { if let Some(pid) = iter.next() { if let ConflictStoreTrie::Node(p) = self { p.entry(pid) - .or_insert_with(|| ConflictStoreTrie::Node(HashMap::new())) + .or_insert_with(|| ConflictStoreTrie::Node(BTreeMap::new())) .insert(iter, con); } // else, We already have a subset of this in the ConflictStore } else { @@ -134,23 +137,31 @@ impl ConflictCache { } /// Finds any known set of conflicts, if any, /// which are activated in `cx` and pass the `filter` specified? - pub fn find_conflicting( + pub fn find_conflicting( &self, cx: &Context, dep: &Dependency, - filter: F, - ) -> Option<&BTreeMap> - where - for<'r> F: Fn(&'r &BTreeMap) -> bool, - { - self.con_from_dep.get(dep)?.find_conflicting(cx, &filter) + must_contain: Option, + ) -> Option<&BTreeMap> { + let out = self + .con_from_dep + .get(dep)? + .find_conflicting(cx, must_contain); + if cfg!(debug_assertions) { + if let Some(f) = must_contain { + if let Some(c) = &out { + assert!(c.contains_key(&f)); + } + } + } + out } pub fn conflicting( &self, cx: &Context, dep: &Dependency, ) -> Option<&BTreeMap> { - self.find_conflicting(cx, dep, |_| true) + self.find_conflicting(cx, dep, None) } /// Add to the cache a conflict of the form: @@ -159,7 +170,7 @@ impl ConflictCache { pub fn insert(&mut self, dep: &Dependency, con: &BTreeMap) { self.con_from_dep .entry(dep.clone()) - .or_insert_with(|| ConflictStoreTrie::Node(HashMap::new())) + .or_insert_with(|| ConflictStoreTrie::Node(BTreeMap::new())) .insert(con.keys().cloned(), con.clone()); trace!( diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index afe289f7d06..6df605cb1b7 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -442,9 +442,7 @@ fn activate_deps_loop( }) .filter_map(|(other_parent, other_dep)| { past_conflicting_activations - .find_conflicting(&cx, &other_dep, |con| { - con.contains_key(&pid) - }) + .find_conflicting(&cx, &other_dep, Some(pid)) .map(|con| (other_parent, con)) }) .next() diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index e9644e628d5..ce66ce06288 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -1201,3 +1201,43 @@ fn large_conflict_cache() { let reg = registry(input); let _ = resolve(&pkg_id("root"), root_deps, ®); } + +#[test] +fn conflict_store_bug() { + let input = vec![ + pkg!(("A", "0.0.3")), + pkg!(("A", "0.0.5")), + pkg!(("A", "0.0.9") => [dep("bad"),]), + pkg!(("A", "0.0.10") => [dep("bad"),]), + pkg!(("L-sys", "0.0.1") => [dep("bad"),]), + pkg!(("L-sys", "0.0.5")), + pkg!(("R", "0.0.4") => [ + dep_req("L-sys", "= 0.0.5"), + ]), + pkg!(("R", "0.0.6")), + pkg!(("a-sys", "0.0.5")), + pkg!(("a-sys", "0.0.11")), + pkg!(("c", "0.0.12") => [ + dep_req("R", ">= 0.0.3, <= 0.0.4"), + ]), + pkg!(("c", "0.0.13") => [ + dep_req("a-sys", ">= 0.0.8, <= 0.0.11"), + ]), + pkg!(("c0", "0.0.6") => [ + dep_req("L-sys", "<= 0.0.2"), + ]), + pkg!(("c0", "0.0.10") => [ + dep_req("A", ">= 0.0.9, <= 0.0.10"), + dep_req("a-sys", "= 0.0.5"), + ]), + pkg!("j" => [ + dep_req("A", ">= 0.0.3, <= 0.0.5"), + dep_req("R", ">=0.0.4, <= 0.0.6"), + dep_req("c", ">= 0.0.9"), + dep_req("c0", ">= 0.0.6"), + ]), + ]; + + let reg = registry(input.clone()); + let _ = resolve_and_validated(&pkg_id("root"), vec![dep("j")], ®); +} diff --git a/tests/testsuite/support/resolver.rs b/tests/testsuite/support/resolver.rs index 758db0ec2f6..9dca8fcfd8e 100644 --- a/tests/testsuite/support/resolver.rs +++ b/tests/testsuite/support/resolver.rs @@ -355,8 +355,13 @@ pub fn registry_strategy( ) -> impl Strategy { let name = string_regex("[A-Za-z][A-Za-z0-9_-]*(-sys)?").unwrap(); - let raw_version = [..max_versions; 3]; - let version_from_raw = |v: &[usize; 3]| format!("{}.{}.{}", v[0], v[1], v[2]); + let raw_version = ..max_versions.pow(3); + let version_from_raw = move |r: usize| { + let major = ((r / max_versions) / max_versions) % max_versions; + let minor = (r / max_versions) % max_versions; + let patch = r % max_versions; + format!("{}.{}.{}", major, minor, patch) + }; // If this is false than the crate will depend on the nonexistent "bad" // instead of the complex set we generated for it. @@ -365,7 +370,7 @@ pub fn registry_strategy( let list_of_versions = btree_map(raw_version, allow_deps, 1..=max_versions).prop_map(move |ver| { ver.into_iter() - .map(|a| (version_from_raw(&a.0), a.1)) + .map(|a| (version_from_raw(a.0), a.1)) .collect::>() });