From 8164be2c951c0ff7a4ee64e5f9a37498556adf51 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 28 Apr 2018 15:59:27 +0300 Subject: [PATCH] Store dependencies as edges of the graph --- src/cargo/core/resolver/context.rs | 14 ++----- src/cargo/core/resolver/encode.rs | 1 - src/cargo/core/resolver/mod.rs | 7 +--- src/cargo/core/resolver/resolve.rs | 24 +++++------- src/cargo/util/graph.rs | 62 +++++++++++++++--------------- 5 files changed, 45 insertions(+), 63 deletions(-) diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 6ba5382679a..a41f482c1ba 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -269,25 +269,19 @@ impl Context { replacements } - pub fn graph(&self) - -> (Graph, HashMap<(PackageId, PackageId), Vec>) - { - let mut graph = Graph::new(); - let mut deps = HashMap::new(); + pub fn graph(&self) -> Graph> { + let mut graph: Graph> = Graph::new(); let mut cur = &self.resolve_graph; while let Some(ref node) = cur.head { match node.0 { GraphNode::Add(ref p) => graph.add(p.clone()), GraphNode::Link(ref a, ref b, ref dep) => { - graph.link(a.clone(), b.clone()); - deps.entry((a.clone(), b.clone())) - .or_insert(Vec::new()) - .push(dep.clone()); + graph.link(a.clone(), b.clone()).push(dep.clone()); } } cur = &node.1; } - (graph, deps) + graph } } diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index 863bda81118..36880c1b80b 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -179,7 +179,6 @@ impl EncodableResolve { Ok(Resolve::new( g, - HashMap::new(), replacements, HashMap::new(), checksums, diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 9b4d6acff8b..47697d49568 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -122,16 +122,13 @@ pub fn resolve( let mut registry = RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions); let cx = activate_deps_loop(cx, &mut registry, summaries, config)?; - let (graph, deps) = cx.graph(); - let mut cksums = HashMap::new(); for summary in cx.activations.values().flat_map(|v| v.iter()) { let cksum = summary.checksum().map(|s| s.to_string()); cksums.insert(summary.package_id().clone(), cksum); } let resolve = Resolve::new( - graph, - deps, + cx.graph(), cx.resolve_replacements(), cx.resolve_features .iter() @@ -867,7 +864,7 @@ fn activation_error( candidates: &[Candidate], config: Option<&Config>, ) -> CargoError { - let (graph, _) = cx.graph(); + let graph = cx.graph(); if !candidates.is_empty() { let mut msg = format!("failed to select a version for `{}`.", dep.name()); msg.push_str("\n ... required by "); diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index 92d01e825d9..db4a5b5ee4b 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -18,8 +18,7 @@ use super::encode::Metadata; /// for each package. #[derive(PartialEq)] pub struct Resolve { - graph: Graph, - dependencies: HashMap<(PackageId, PackageId), Vec>, + graph: Graph>, replacements: HashMap, reverse_replacements: HashMap, empty_features: HashSet, @@ -31,8 +30,7 @@ pub struct Resolve { impl Resolve { pub fn new( - graph: Graph, - dependencies: HashMap<(PackageId, PackageId), Vec>, + graph: Graph>, replacements: HashMap, features: HashMap>, checksums: HashMap>, @@ -45,7 +43,6 @@ impl Resolve { .collect(); Resolve { graph, - dependencies, replacements, features, checksums, @@ -162,14 +159,13 @@ unable to verify that `{0}` is the same as when the lockfile was generated Ok(()) } - pub fn iter(&self) -> Nodes { + pub fn iter(&self) -> Nodes> { self.graph.iter() } pub fn deps(&self, pkg: &PackageId) -> Deps { Deps { edges: self.graph.edges(pkg), - id: pkg.clone(), resolve: self, } } @@ -225,11 +221,11 @@ unable to verify that `{0}` is the same as when the lockfile was generated // that's where the dependency originates from, and we only replace // targets of dependencies not the originator. if let Some(replace) = self.reverse_replacements.get(to) { - if let Some(deps) = self.dependencies.get(&(from.clone(), replace.clone())) { + if let Some(deps) = self.graph.edge(from, replace) { return deps; } } - match self.dependencies.get(&(from.clone(), to.clone())) { + match self.graph.edge(from, to) { Some(ret) => ret, None => panic!("no Dependency listed for `{}` => `{}`", from, to), } @@ -248,8 +244,7 @@ impl fmt::Debug for Resolve { } pub struct Deps<'a> { - edges: Option>, - id: PackageId, + edges: Option>>, resolve: &'a Resolve, } @@ -257,9 +252,8 @@ impl<'a> Iterator for Deps<'a> { type Item = (&'a PackageId, &'a [Dependency]); fn next(&mut self) -> Option<(&'a PackageId, &'a [Dependency])> { - let id = self.edges.as_mut()?.next()?; + let (id, deps) = self.edges.as_mut()?.next()?; let id_ret = self.resolve.replacement(id).unwrap_or(id); - let deps = &self.resolve.dependencies[&(self.id.clone(), id.clone())]; Some((id_ret, deps)) } @@ -274,14 +268,14 @@ impl<'a> Iterator for Deps<'a> { impl<'a> ExactSizeIterator for Deps<'a> {} pub struct DepsNotReplaced<'a> { - edges: Option>, + edges: Option>>, } impl<'a> Iterator for DepsNotReplaced<'a> { type Item = &'a PackageId; fn next(&mut self) -> Option<&'a PackageId> { - self.edges.as_mut().and_then(|e| e.next()) + Some(self.edges.as_mut()?.next()?.0) } fn size_hint(&self) -> (usize, Option) { diff --git a/src/cargo/util/graph.rs b/src/cargo/util/graph.rs index a179b6d237a..fcaef4e60ab 100644 --- a/src/cargo/util/graph.rs +++ b/src/cargo/util/graph.rs @@ -1,10 +1,9 @@ use std::fmt; use std::hash::Hash; -use std::collections::hash_set::{HashSet, Iter}; -use std::collections::hash_map::{HashMap, Keys}; +use std::collections::hash_map::{HashMap, Iter, Keys}; -pub struct Graph { - nodes: HashMap>, +pub struct Graph { + nodes: HashMap>, } enum Mark { @@ -12,35 +11,34 @@ enum Mark { Done, } -pub type Nodes<'a, N> = Keys<'a, N, HashSet>; -pub type Edges<'a, N> = Iter<'a, N>; +pub type Nodes<'a, N, E> = Keys<'a, N, HashMap>; +pub type Edges<'a, N, E> = Iter<'a, N, E>; -impl Graph { - pub fn new() -> Graph { +impl Graph { + pub fn new() -> Graph { Graph { nodes: HashMap::new(), } } pub fn add(&mut self, node: N) { - self.nodes - .entry(node) - .or_insert_with(HashSet::new); + self.nodes.entry(node).or_insert_with(HashMap::new); } - pub fn link(&mut self, node: N, child: N) { + pub fn link(&mut self, node: N, child: N) -> &mut E { self.nodes .entry(node) - .or_insert_with(HashSet::new) - .insert(child); + .or_insert_with(HashMap::new) + .entry(child) + .or_insert_with(Default::default) } - pub fn get_nodes(&self) -> &HashMap> { - &self.nodes + pub fn edge(&self, from: &N, to: &N) -> Option<&E> { + self.nodes.get(from)?.get(to) } - pub fn edges(&self, node: &N) -> Option> { - self.nodes.get(node).map(|set| set.iter()) + pub fn edges(&self, from: &N) -> Option> { + self.nodes.get(from).map(|set| set.iter()) } pub fn sort(&self) -> Option> { @@ -61,7 +59,7 @@ impl Graph { marks.insert(node.clone(), Mark::InProgress); - for child in &self.nodes[node] { + for child in self.nodes[node].keys() { self.visit(child, dst, marks); } @@ -69,7 +67,7 @@ impl Graph { marks.insert(node.clone(), Mark::Done); } - pub fn iter(&self) -> Nodes { + pub fn iter(&self) -> Nodes { self.nodes.keys() } @@ -81,12 +79,12 @@ impl Graph { // it's used for! let mut result = vec![pkg]; let first_pkg_depending_on = |pkg: &N, res: &[&N]| { - self.get_nodes() + self.nodes .iter() - .filter(|&(_node, adjacent)| adjacent.contains(pkg)) + .filter(|&(_node, adjacent)| adjacent.contains_key(pkg)) // Note that we can have "cycles" introduced through dev-dependency // edges, so make sure we don't loop infinitely. - .filter(|&(_node, _)| !res.contains(&_node)) + .filter(|&(node, _)| !res.contains(&node)) .next() .map(|p| p.0) }; @@ -98,20 +96,20 @@ impl Graph { } } -impl Default for Graph { - fn default() -> Graph { +impl Default for Graph { + fn default() -> Graph { Graph::new() } } -impl fmt::Debug for Graph { +impl fmt::Debug for Graph { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { writeln!(fmt, "Graph {{")?; for (n, e) in &self.nodes { writeln!(fmt, " - {}", n)?; - for n in e.iter() { + for n in e.keys() { writeln!(fmt, " - {}", n)?; } } @@ -122,15 +120,15 @@ impl fmt::Debug for Graph { } } -impl PartialEq for Graph { - fn eq(&self, other: &Graph) -> bool { +impl PartialEq for Graph { + fn eq(&self, other: &Graph) -> bool { self.nodes.eq(&other.nodes) } } -impl Eq for Graph {} +impl Eq for Graph {} -impl Clone for Graph { - fn clone(&self) -> Graph { +impl Clone for Graph { + fn clone(&self) -> Graph { Graph { nodes: self.nodes.clone(), }