diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 55b068574c9..8304756d1b8 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -46,11 +46,12 @@ //! that we're implementing something that probably shouldn't be allocating all //! over the place. -use std::collections::HashSet; -use std::collections::hash_map::HashMap; +use std::cmp::Ordering; +use std::collections::{HashSet, HashMap, BinaryHeap}; use std::fmt; use std::ops::Range; use std::rc::Rc; + use semver; use core::{PackageId, Registry, SourceId, Summary, Dependency}; @@ -142,7 +143,6 @@ impl fmt::Debug for Resolve { struct Context { activations: HashMap<(String, SourceId), Vec>>, resolve: Resolve, - visited: HashSet, } /// Builds the list of all packages required to build the first argument. @@ -154,10 +154,11 @@ pub fn resolve(summary: &Summary, method: &Method, let cx = Context { resolve: Resolve::new(summary.package_id().clone()), activations: HashMap::new(), - visited: HashSet::new(), }; let _p = profile::start(format!("resolving: {}", summary.package_id())); - activate_deps_loop(cx, registry, summary, method) + let cx = try!(activate_deps_loop(cx, registry, summary, method)); + try!(check_cycles(&cx)); + Ok(cx.resolve) } /// Attempts to activate the summary `parent` in the context `cx`. @@ -174,13 +175,9 @@ fn activate(cx: &mut Context, // Dependency graphs are required to be a DAG, so we keep a set of // packages we're visiting and bail if we hit a dupe. let id = parent.package_id().clone(); - if !cx.visited.insert(id.clone()) { - bail!("cyclic package dependency: package `{}` depends on itself", id) - } // If we're already activated, then that was easy! if cx.flag_activated(&parent, method) { - cx.visited.remove(&id); return Ok(None); } trace!("activating {}", parent.package_id()); @@ -207,10 +204,16 @@ impl RcVecIter { vec: Rc::new(vec), } } + fn cur_index(&self) -> usize { self.rest.start - 1 } + + fn as_slice(&self) -> &[T] { + &self.vec[self.rest.start..self.rest.end] + } } + impl Iterator for RcVecIter where T: Clone { type Item = (usize, T); fn next(&mut self) -> Option<(usize, T)> { @@ -218,6 +221,9 @@ impl Iterator for RcVecIter where T: Clone { self.vec.get(i).map(|val| (i, val.clone())) }) } + fn size_hint(&self) -> (usize, Option) { + self.rest.size_hint() + } } #[derive(Clone)] @@ -227,9 +233,46 @@ struct DepsFrame { id: PackageId, } +impl DepsFrame { + /// Returns the least number of candidates that any of this frame's siblings + /// has. + /// + /// The `remaining_siblings` array is already sorted with the smallest + /// number of candidates at the front, so we just return the number of + /// candidates in that entry. + fn min_candidates(&self) -> usize { + self.remaining_siblings.as_slice().get(0).map(|&(_, ref candidates, _)| { + candidates.len() + }).unwrap_or(0) + } +} + +impl PartialEq for DepsFrame { + fn eq(&self, other: &DepsFrame) -> bool { + self.min_candidates() == other.min_candidates() + } +} + +impl Eq for DepsFrame {} + +impl PartialOrd for DepsFrame { + fn partial_cmp(&self, other: &DepsFrame) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for DepsFrame { + fn cmp(&self, other: &DepsFrame) -> Ordering { + // the frame with the sibling that has the least number of candidates + // needs to get the bubbled up to the top of the heap we use below, so + // reverse the order of the comparison here. + other.min_candidates().cmp(&self.min_candidates()) + } +} + struct BacktrackFrame { context_backup: Context, - deps_backup: Vec, + deps_backup: BinaryHeap, remaining_candidates: RcVecIter>, parent: Rc, dep: Dependency, @@ -243,9 +286,16 @@ struct BacktrackFrame { fn activate_deps_loop(mut cx: Context, registry: &mut Registry, top: Rc, - top_method: &Method) -> CargoResult { + top_method: &Method) -> CargoResult { + // Note that a `BinaryHeap` is used for the remaining dependencies that need + // activation. This heap is sorted such that the "largest value" is the most + // constrained dependency, or the one with the least candidates. + // + // This helps us get through super constrained portions of the dependency + // graph quickly and hopefully lock down what later larger dependencies can + // use (those with more candidates). let mut backtrack_stack = Vec::new(); - let mut remaining_deps = Vec::new(); + let mut remaining_deps = BinaryHeap::new(); remaining_deps.extend(try!(activate(&mut cx, registry, top, &top_method))); // Main resolution loop, this is the workhorse of the resolution algorithm. @@ -268,10 +318,7 @@ fn activate_deps_loop(mut cx: Context, remaining_deps.push(deps_frame); (parent, sibling) } - None => { - cx.visited.remove(&deps_frame.id); - continue - } + None => continue, }; let (mut parent, (mut cur, (mut dep, candidates, features))) = frame; assert!(!remaining_deps.is_empty()); @@ -353,32 +400,28 @@ fn activate_deps_loop(mut cx: Context, candidate.version()); cx.resolve.graph.link(parent.package_id().clone(), candidate.package_id().clone()); - - // If we hit an intransitive dependency then clear out the visitation - // list as we can't induce a cycle through transitive dependencies. - if !dep.is_transitive() { - cx.visited.clear(); - } remaining_deps.extend(try!(activate(&mut cx, registry, candidate, &method))); } trace!("resolved: {:?}", cx.resolve); - Ok(cx.resolve) + Ok(cx) } // Searches up `backtrack_stack` until it finds a dependency with remaining // candidates. Resets `cx` and `remaining_deps` to that level and returns the // next candidate. If all candidates have been exhausted, returns None. fn find_candidate(backtrack_stack: &mut Vec, - cx: &mut Context, remaining_deps: &mut Vec, - parent: &mut Rc, cur: &mut usize, + cx: &mut Context, + remaining_deps: &mut BinaryHeap, + parent: &mut Rc, + cur: &mut usize, dep: &mut Dependency) -> Option> { while let Some(mut frame) = backtrack_stack.pop() { if let Some((_, candidate)) = frame.remaining_candidates.next() { *cx = frame.context_backup.clone(); *remaining_deps = frame.deps_backup.clone(); *parent = frame.parent.clone(); - *cur = remaining_deps.last().unwrap().remaining_siblings.cur_index(); + *cur = remaining_deps.peek().unwrap().remaining_siblings.cur_index(); *dep = frame.dep.clone(); backtrack_stack.push(frame); return Some(candidate) @@ -705,3 +748,51 @@ impl Context { Ok(ret) } } + +fn check_cycles(cx: &Context) -> CargoResult<()> { + let mut summaries = HashMap::new(); + for summary in cx.activations.values().flat_map(|v| v) { + summaries.insert(summary.package_id(), &**summary); + } + return visit(&cx.resolve, + cx.resolve.root(), + &summaries, + &mut HashSet::new(), + &mut HashSet::new()); + + fn visit<'a>(resolve: &'a Resolve, + id: &'a PackageId, + summaries: &HashMap<&'a PackageId, &Summary>, + visited: &mut HashSet<&'a PackageId>, + checked: &mut HashSet<&'a PackageId>) + -> CargoResult<()> { + // See if we visited ourselves + if !visited.insert(id) { + bail!("cyclic package dependency: package `{}` depends on itself", + id); + } + + // If we've already checked this node no need to recurse again as we'll + // just conclude the same thing as last time, so we only execute the + // recursive step if we successfully insert into `checked`. + // + // Note that if we hit an intransitive dependency then we clear out the + // visitation list as we can't induce a cycle through transitive + // dependencies. + if checked.insert(id) { + let summary = summaries[id]; + for dep in resolve.deps(id).into_iter().flat_map(|a| a) { + let is_transitive = summary.dependencies().iter().any(|d| { + d.matches_id(dep) && d.is_transitive() + }); + let mut empty = HashSet::new(); + let visited = if is_transitive {&mut *visited} else {&mut empty}; + try!(visit(resolve, dep, summaries, visited, checked)); + } + } + + // Ok, we're done, no longer visiting our node any more + visited.remove(id); + Ok(()) + } +}