Skip to content
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

Resolver: A dep is equivalent to one of the things it can resolve to. #6776

Merged
merged 11 commits into from
Apr 2, 2019
71 changes: 61 additions & 10 deletions src/cargo/core/resolver/conflict_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,41 +18,55 @@ enum ConflictStoreTrie {

impl ConflictStoreTrie {
/// Finds any known set of conflicts, if any,
/// which are activated in `cx` and pass the `filter` specified?
/// which are activated in `cx` and contain `PackageId` specified.
/// If more then one are activated, then it will return
/// one that will allow for the most jump-back.
fn find_conflicting(
&self,
cx: &Context,
must_contain: Option<PackageId>,
) -> Option<&ConflictMap> {
) -> Option<(&ConflictMap, usize)> {
match self {
ConflictStoreTrie::Leaf(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)
debug_assert!(cx.is_conflicting(None, c).is_some());
Some((c, 0))
} else {
// We did not find `must_contain`, so we need to keep looking.
None
}
}
ConflictStoreTrie::Node(m) => {
let mut out = None;
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) =
if let Some(age_this) = cx.is_active(pid) {
if let Some((o, age_o)) =
store.find_conflicting(cx, must_contain.filter(|&f| f != pid))
{
return Some(o);
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)
};
let out_age = out.get_or_insert((o, age)).1;
if out_age > age {
// we found one that can jump-back further so replace the out.
out = Some((o, age));
}
}
}
// Else, if it is not active then there is no way any of the corresponding
// subtrie will be conflicting.
}
None
out
}
}
}
Expand Down Expand Up @@ -88,6 +102,28 @@ impl ConflictStoreTrie {
*self = ConflictStoreTrie::Leaf(con)
}
}

fn contains(&self, mut iter: impl Iterator<Item = PackageId>, con: &ConflictMap) -> bool {
match (self, iter.next()) {
(ConflictStoreTrie::Leaf(c), None) => {
if cfg!(debug_assertions) {
let a: Vec<_> = con.keys().collect();
let b: Vec<_> = c.keys().collect();
assert_eq!(a, b);
}
true
}
(ConflictStoreTrie::Leaf(_), Some(_)) => false,
(ConflictStoreTrie::Node(_), None) => false,
(ConflictStoreTrie::Node(m), Some(n)) => {
if let Some(next) = m.get(&n) {
next.contains(iter, con)
} else {
false
}
}
}
}
}

pub(super) struct ConflictCache {
Expand Down Expand Up @@ -137,7 +173,9 @@ impl ConflictCache {
}
}
/// Finds any known set of conflicts, if any,
/// which are activated in `cx` and pass the `filter` specified?
/// which are activated in `cx` and contain `PackageId` specified.
/// If more then one are activated, then it will return
/// one that will allow for the most jump-back.
pub fn find_conflicting(
&self,
cx: &Context,
Expand All @@ -147,7 +185,8 @@ impl ConflictCache {
let out = self
.con_from_dep
.get(dep)?
.find_conflicting(cx, must_contain);
.find_conflicting(cx, must_contain)
.map(|(c, _)| c);
if cfg!(debug_assertions) {
if let Some(f) = must_contain {
if let Some(c) = &out {
Expand Down Expand Up @@ -189,6 +228,18 @@ impl ConflictCache {
.insert(dep.clone());
}
}

/// Check if a conflict was previously added of the form:
/// `dep` is known to be unresolvable if
/// all the `PackageId` entries are activated.
pub fn contains(&self, dep: &Dependency, con: &ConflictMap) -> bool {
if let Some(cst) = self.con_from_dep.get(dep) {
cst.contains(con.keys().cloned(), &con)
} else {
false
}
}

pub fn dependencies_conflicting_with(&self, pid: PackageId) -> Option<&HashSet<Dependency>> {
self.dep_from_pid.get(&pid)
}
Expand Down
112 changes: 82 additions & 30 deletions src/cargo/core/resolver/context.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::{HashMap, HashSet};
use std::num::NonZeroU64;
use std::rc::Rc;

// "ensure" seems to require "bail" be in scope (macro hygiene issue?).
Expand Down Expand Up @@ -52,8 +53,47 @@ pub struct Context {
pub warnings: RcList<String>,
}

/// list all the activated versions of a particular crate name from a source
pub type Activations = im_rc::HashMap<(InternedString, SourceId), Rc<Vec<Summary>>>;
/// When backtracking it can be useful to know how far back to go.
/// The `ContextAge` of a `Context` is a monotonically increasing counter of the number
/// of decisions made to get to this state.
/// Several structures store the `ContextAge` when it was added,
/// to be used in `find_candidate` for backtracking.
pub type ContextAge = usize;

/// Find the activated version of a crate based on the name, source, and semver compatibility.
/// By storing this in a hash map we ensure that there is only one
/// semver compatible version of each crate.
/// This all so stores the `ContextAge`.
pub type Activations =
im_rc::HashMap<(InternedString, SourceId, SemverCompatibility), (Summary, ContextAge)>;

/// A type that represents when cargo treats two Versions as compatible.
/// Versions `a` and `b` are compatible if their left-most nonzero digit is the
/// same.
#[derive(Clone, Copy, Eq, PartialEq, Hash)]
pub enum SemverCompatibility {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a pretty clever way to store Activations, nice find!

Major(NonZeroU64),
Minor(NonZeroU64),
Patch(u64),
}

impl From<&semver::Version> for SemverCompatibility {
fn from(ver: &semver::Version) -> Self {
if let Some(m) = NonZeroU64::new(ver.major) {
return SemverCompatibility::Major(m);
}
if let Some(m) = NonZeroU64::new(ver.minor) {
return SemverCompatibility::Minor(m);
}
SemverCompatibility::Patch(ver.patch)
}
}

impl PackageId {
pub fn as_activations_key(&self) -> (InternedString, SourceId, SemverCompatibility) {
(self.name(), self.source_id(), self.version().into())
}
}

impl Context {
pub fn new(check_public_visible_dependencies: bool) -> Context {
Expand All @@ -78,22 +118,28 @@ impl Context {
/// Returns `true` if this summary with the given method is already activated.
pub fn flag_activated(&mut self, summary: &Summary, method: &Method<'_>) -> CargoResult<bool> {
let id = summary.package_id();
let prev = self
.activations
.entry((id.name(), id.source_id()))
.or_insert_with(|| Rc::new(Vec::new()));
if !prev.iter().any(|c| c == summary) {
self.resolve_graph.push(GraphNode::Add(id));
if let Some(link) = summary.links() {
ensure!(
self.links.insert(link, id).is_none(),
"Attempting to resolve a dependency with more then one crate with the \
links={}.\nThis will not build as is. Consider rebuilding the .lock file.",
&*link
let age: ContextAge = self.age();
match self.activations.entry(id.as_activations_key()) {
im_rc::hashmap::Entry::Occupied(o) => {
debug_assert_eq!(
&o.get().0,
summary,
"cargo does not allow two semver compatible versions"
);
}
Rc::make_mut(prev).push(summary.clone());
return Ok(false);
im_rc::hashmap::Entry::Vacant(v) => {
self.resolve_graph.push(GraphNode::Add(id));
if let Some(link) = summary.links() {
ensure!(
self.links.insert(link, id).is_none(),
"Attempting to resolve a dependency with more then one crate with the \
links={}.\nThis will not build as is. Consider rebuilding the .lock file.",
&*link
);
}
v.insert((summary.clone(), age));
return Ok(false);
}
}
debug!("checking if {} is already activated", summary.package_id());
let (features, use_default) = match *method {
Expand Down Expand Up @@ -149,31 +195,37 @@ impl Context {
Ok(deps)
}

pub fn prev_active(&self, dep: &Dependency) -> &[Summary] {
self.activations
.get(&(dep.package_name(), dep.source_id()))
.map(|v| &v[..])
.unwrap_or(&[])
/// Returns the `ContextAge` of this `Context`.
/// For now we use (len of activations) as the age.
/// See the `ContextAge` docs for more details.
pub fn age(&self) -> ContextAge {
self.activations.len()
}

pub fn is_active(&self, id: PackageId) -> bool {
/// If the package is active returns the `ContextAge` when it was added
pub fn is_active(&self, id: PackageId) -> Option<ContextAge> {
self.activations
.get(&(id.name(), id.source_id()))
.map(|v| v.iter().any(|s| s.package_id() == id))
.unwrap_or(false)
.get(&id.as_activations_key())
.and_then(|(s, l)| if s.package_id() == id { Some(*l) } else { None })
}

/// 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.
pub fn is_conflicting(
&self,
parent: Option<PackageId>,
conflicting_activations: &ConflictMap,
) -> bool {
conflicting_activations
.keys()
.chain(parent.as_ref())
.all(|&id| self.is_active(id))
) -> Option<usize> {
let mut max = 0;
for &id in conflicting_activations.keys().chain(parent.as_ref()) {
if let Some(age) = self.is_active(id) {
max = std::cmp::max(max, age);
} else {
return None;
}
}
Some(max)
}

/// Returns all dependencies and the features we want from them.
Expand Down
Loading