diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 7071d88d1aa..c24f2a3515c 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -161,6 +161,25 @@ impl Context { .and_then(|(s, l)| if s.package_id() == id { Some(*l) } else { None }) } + /// This calculates `is_active` after backtracking to `age` and activating `alternative`. + /// If the package is active returns the `ContextAge` when it was added + /// but only if it is older then `age`. + /// If it is not active but it is the `alternative` then it returns `age`. + pub fn is_older_active_or( + &self, + id: PackageId, + age: ContextAge, + alternative: PackageId, + ) -> Option { + if id == alternative { + // we are imagining that we used other instead + Some(age) + } else { + // we only care about things that are older then critical_age + self.is_active(id).filter(|&other_age| other_age < age) + } + } + /// 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 c410b7accca..4227374cf69 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -847,7 +847,7 @@ impl RemainingCandidates { } } -/// Attempts to find a new conflict that allows a `find_candidate` feather then the input one. +/// Attempts to find a new conflict that allows `find_candidate` to go further back then the input one. /// It will add the new conflict to the cache if one is found. /// /// Panics if the input conflict is not all active in `cx`. @@ -880,6 +880,49 @@ fn generalize_conflicting( // and let `backtrack_critical_id` figure this out. return None; } + + // we will need to know the range of versions we can use + let our_candidates = registry + .query(dep) + .expect("an already used dep now error!?"); + + if our_candidates.is_empty() { + return None; + } + + let our_activation_key = { + let first_activation_key = our_candidates[0].package_id().as_activations_key(); + + // If our dep only matches one semver range then we can fast path any other dep + // that also targets that semver range and has no overlap. + if our_candidates + .iter() + .all(|c| first_activation_key == c.package_id().as_activations_key()) + { + Some(first_activation_key) + } else { + None + } + }; + + let our_link = { + let first_links = our_candidates[0].links(); + + // If our dep only matches things that have a links set then we can fast path any other dep + // that also all use that links and has no overlap. + if first_links.is_some() && our_candidates.iter().all(|c| first_links == c.links()) { + // All of `our_candidates` use the same non-`None` `links`, so we can use the fast path. + first_links + } else { + // Some of `our_candidates` use a different `links` so whatever `links` get used by + // the conflicting dep we can select the other. We cant use the fast path. + None + } + }; + + let our_candidates: HashSet = + our_candidates.iter().map(|our| our.package_id()).collect(); + // What parents does that critical activation have for (critical_parent, critical_parents_deps) in cx.parents.edges(&backtrack_critical_id).filter(|(p, _)| { @@ -887,73 +930,146 @@ fn generalize_conflicting( cx.is_active(*p).expect("parent not currently active!?") < backtrack_critical_age }) { - for critical_parents_dep in critical_parents_deps.iter() { + 'dep: for critical_parents_dep in critical_parents_deps.iter() { // A dep is equivalent to one of the things it can resolve to. // Thus, if all the things it can resolve to have already ben determined // to be conflicting, then we can just say that we conflict with the parent. - if let Some(others) = registry + let mut con = conflicting_activations.clone(); + con.remove(&backtrack_critical_id); + con.insert(*critical_parent, backtrack_critical_reason.clone()); + + for other in registry .query(critical_parents_dep) .expect("an already used dep now error!?") .iter() - .rev() // the last one to be tried is the least likely to be in the cache, so start with that. - .map(|other| { - past_conflicting_activations - .find( - dep, - &|id| { - if id == other.package_id() { - // we are imagining that we used other instead - Some(backtrack_critical_age) - } else { - cx.is_active(id).filter(|&age| - // we only care about things that are older then critical_age - age < backtrack_critical_age) - } - }, - Some(other.package_id()), - ) - .map(|con| (other.package_id(), con)) - }) - .collect::>>() + .rev() + // the last one to be tried is the least likely to be in the cache, so start with that. { - let mut con = conflicting_activations.clone(); - // It is always valid to combine previously inserted conflicts. - // A, B are both known bad states each that can never be activated. - // A + B is redundant but cant be activated, as if - // A + B is active then A is active and we know that is not ok. - for (_, other) in &others { - con.extend(other.iter().map(|(&id, re)| (id, re.clone()))); + if ( + our_activation_key.map_or(false, |our| { + other.package_id().as_activations_key() == our + }) + // `other` is semver compatible with all of `our_candidates` + || our_link.map_or(false, |_| other.links() == our_link) + // or `other` have the same `links` as all of `our_candidates` + ) && !our_candidates.contains(&other.package_id()) + // and is not one of `our_candidates`. + { + // Thus `dep` can not be resolved when `critical_parents_dep` has bean resolved. + continue; } - // Now that we have this combined conflict, we can do a substitution: - // A dep is equivalent to one of the things it can resolve to. - // So we can remove all the things that it resolves to and replace with the parent. - for (other_id, _) in &others { - con.remove(other_id); + + // ok so we dont have a semver nor a links problem with `critical_parents_dep` + // getting set to `other`, maybe it still wont work do to one of its dependencies. + if let Some(conflicting) = generalize_children_conflicting( + cx, + backtrack_critical_age, + registry, + past_conflicting_activations, + dep, + &other, + *critical_parent, + ) { + con.extend(conflicting.into_iter()); + continue; } - con.insert(*critical_parent, backtrack_critical_reason); - - if cfg!(debug_assertions) { - // the entire point is to find an older conflict, so let's make sure we did - let new_age = con - .keys() - .map(|&c| cx.is_active(c).expect("not currently active!?")) - .max() - .unwrap(); - assert!( - new_age < backtrack_critical_age, - "new_age {} < backtrack_critical_age {}", - new_age, - backtrack_critical_age + + if let Some(conflicting) = past_conflicting_activations.find( + dep, + &|id| cx.is_older_active_or(id, backtrack_critical_age, other.package_id()), + Some(other.package_id()), + ) { + con.extend( + conflicting + .iter() + .filter(|(&id, _)| id != other.package_id()) + .map(|(&id, r)| (id, r.clone())), ); + continue; } - past_conflicting_activations.insert(dep, &con); - return Some(con); + + // We don't have a reason why `other` cant work. + // There for `critical_parents_dep` could have used it, and we can still be activated. + // We can not generalize over `critical_parents_dep`, but maybe the next `critical_parents_dep` + continue 'dep; + } + + if cfg!(debug_assertions) { + // the entire point is to find an older conflict, so let's make sure we did + let new_age = con + .keys() + .map(|&c| cx.is_active(c).expect("not currently active!?")) + .max() + .unwrap(); + assert!( + new_age < backtrack_critical_age, + "new_age {} < backtrack_critical_age {}", + new_age, + backtrack_critical_age + ); } + past_conflicting_activations.insert(dep, &con); + return Some(con); } } None } +/// Sees if the candidate is not usable because one of its children will have a conflict +/// +/// Panics if the input conflict is not all active in `cx`. +fn generalize_children_conflicting( + cx: &Context, + critical_age: usize, + registry: &mut RegistryQueryer<'_>, + past_conflicting_activations: &mut conflict_cache::ConflictCache, + dep: &Dependency, + candidate: &Summary, + parent: PackageId, +) -> Option { + let method = Method::Required { + dev_deps: false, + features: Rc::new(dep.features().iter().cloned().collect()), + all_features: false, + uses_default_features: dep.uses_default_features(), + }; + let mut out = ConflictMap::new(); + match registry.build_deps(Some(parent), candidate, &method) { + Err(ActivateError::Fatal(_)) => return None, + Err(ActivateError::Conflict(pid, reason)) => { + out.insert(pid, reason); + } + Ok(deps) => { + let con = deps + .1 + .iter() + .filter_map(|(ref new_dep, _, _)| { + past_conflicting_activations.find( + new_dep, + &|id| cx.is_older_active_or(id, critical_age, candidate.package_id()), + None, + ) + }) + .next()?; + out.extend( + con.iter() + .filter(|(pid, _)| **pid != candidate.package_id()) + .map(|(&pid, reason)| (pid, reason.clone())), + ); + } + }; + // If one of candidate deps is known unresolvable + // then candidate will not succeed. + // How ever if candidate is part of the reason that + // one of its deps conflicts then + // we can make a stronger statement + // because candidate will definitely be activated when + // we try its dep. + out.remove(&candidate.package_id()); + + Some(out) +} + /// Looks through the states in `backtrack_stack` for dependencies with /// remaining candidates. For each one, also checks if rolling back /// could change the outcome of the failed resolution that caused backtracking diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index 3a79477659e..2d531e23cdf 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -1352,16 +1352,21 @@ fn large_conflict_cache() { // a large number of conflicts can easily be generated by a sys crate. let sys_name = format!("{}-sys", (b'a' + name) as char); let in_len = input.len(); - input.push(pkg!(("last", format!("{}.0.0", in_len)) => [dep_req(&sys_name, "=0.0.0")])); + input.push(pkg!(("last", format!("{}.0.0", in_len)) => [dep_req(&sys_name, "<=0.0.1")])); root_deps.push(dep_req(&sys_name, ">= 0.0.1")); // a large number of conflicts can also easily be generated by a major release version. let plane_name = format!("{}", (b'a' + name) as char); let in_len = input.len(); - input.push(pkg!(("last", format!("{}.0.0", in_len)) => [dep_req(&plane_name, "=1.0.0")])); + input.push(pkg!(("last", format!("{}.0.0", in_len)) => [dep_req(&plane_name, "<=1.0.1")])); root_deps.push(dep_req(&plane_name, ">= 1.0.1")); - for i in 0..=NUM_VERSIONS { + input.push(pkg!((&sys_name, "0.0.0"))); + input.push(pkg!((&plane_name, "1.0.0"))); + // one version that can't be activated for some other reason + input.push(pkg!((&sys_name, "0.0.1") => [dep("bad")])); + input.push(pkg!((&plane_name, "1.0.1") => [dep("bad")])); + for i in 2..=NUM_VERSIONS { input.push(pkg!((&sys_name, format!("{}.0.0", i)))); input.push(pkg!((&plane_name, format!("1.0.{}", i)))); }