Skip to content

De-duplicate edges #7993

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

Merged
merged 3 commits into from
Mar 16, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions src/cargo/core/resolver/context.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::collections::HashMap;
use std::num::NonZeroU64;
use std::rc::Rc;

use anyhow::format_err;
use log::debug;
@@ -35,7 +34,7 @@ pub struct Context {

/// a way to look up for a package in activations what packages required it
/// and all of the exact deps that it fulfilled.
pub parents: Graph<PackageId, Rc<Vec<Dependency>>>,
pub parents: Graph<PackageId, im_rc::HashSet<Dependency>>,
}

/// When backtracking it can be useful to know how far back to go.
@@ -255,8 +254,8 @@ impl Context {
.collect()
}

pub fn graph(&self) -> Graph<PackageId, Vec<Dependency>> {
let mut graph: Graph<PackageId, Vec<Dependency>> = Graph::new();
pub fn graph(&self) -> Graph<PackageId, std::collections::HashSet<Dependency>> {
let mut graph: Graph<PackageId, std::collections::HashSet<Dependency>> = Graph::new();
self.activations
.values()
.for_each(|(r, _)| graph.add(r.package_id()));
@@ -265,14 +264,14 @@ impl Context {
for (o, e) in self.parents.edges(i) {
let old_link = graph.link(*o, *i);
assert!(old_link.is_empty());
*old_link = e.to_vec();
*old_link = e.iter().cloned().collect();
}
}
graph
}
}

impl Graph<PackageId, Rc<Vec<Dependency>>> {
impl Graph<PackageId, im_rc::HashSet<Dependency>> {
pub fn parents_of(&self, p: PackageId) -> impl Iterator<Item = (PackageId, bool)> + '_ {
self.edges(&p)
.map(|(grand, d)| (*grand, d.iter().any(|x| x.is_public())))
@@ -338,7 +337,7 @@ impl PublicDependency {
parent_pid: PackageId,
is_public: bool,
age: ContextAge,
parents: &Graph<PackageId, Rc<Vec<Dependency>>>,
parents: &Graph<PackageId, im_rc::HashSet<Dependency>>,
) {
// one tricky part is that `candidate_pid` may already be active and
// have public dependencies of its own. So we not only need to mark
@@ -383,7 +382,7 @@ impl PublicDependency {
b_id: PackageId,
parent: PackageId,
is_public: bool,
parents: &Graph<PackageId, Rc<Vec<Dependency>>>,
parents: &Graph<PackageId, im_rc::HashSet<Dependency>>,
) -> Result<
(),
(
11 changes: 5 additions & 6 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
@@ -609,12 +609,11 @@ fn activate(
cx.age += 1;
if let Some((parent, dep)) = parent {
let parent_pid = parent.package_id();
Rc::make_mut(
// add a edge from candidate to parent in the parents graph
cx.parents.link(candidate_pid, parent_pid),
)
// and associate dep with that edge
.push(dep.clone());
// add a edge from candidate to parent in the parents graph
cx.parents
.link(candidate_pid, parent_pid)
// and associate dep with that edge
.insert(dep.clone());
if let Some(public_dependency) = cx.public_dependency.as_mut() {
public_dependency.add_edge(
candidate_pid,
17 changes: 8 additions & 9 deletions src/cargo/core/resolver/resolve.rs
Original file line number Diff line number Diff line change
@@ -18,7 +18,7 @@ pub struct Resolve {
/// A graph, whose vertices are packages and edges are dependency specifications
/// from `Cargo.toml`. We need a `Vec` here because the same package
/// might be present in both `[dependencies]` and `[build-dependencies]`.
graph: Graph<PackageId, Vec<Dependency>>,
graph: Graph<PackageId, HashSet<Dependency>>,
/// Replacements from the `[replace]` table.
replacements: HashMap<PackageId, PackageId>,
/// Inverted version of `replacements`.
@@ -70,7 +70,7 @@ pub enum ResolveVersion {

impl Resolve {
pub fn new(
graph: Graph<PackageId, Vec<Dependency>>,
graph: Graph<PackageId, HashSet<Dependency>>,
replacements: HashMap<PackageId, PackageId>,
features: HashMap<PackageId, Vec<InternedString>>,
checksums: HashMap<PackageId, Option<String>>,
@@ -264,18 +264,16 @@ unable to verify that `{0}` is the same as when the lockfile was generated
self.graph.iter().cloned()
}

pub fn deps(&self, pkg: PackageId) -> impl Iterator<Item = (PackageId, &[Dependency])> {
pub fn deps(&self, pkg: PackageId) -> impl Iterator<Item = (PackageId, &HashSet<Dependency>)> {
self.deps_not_replaced(pkg)
.map(move |(id, deps)| (self.replacement(id).unwrap_or(id), deps))
}

pub fn deps_not_replaced(
&self,
pkg: PackageId,
) -> impl Iterator<Item = (PackageId, &[Dependency])> {
self.graph
.edges(&pkg)
.map(|(id, deps)| (*id, deps.as_slice()))
) -> impl Iterator<Item = (PackageId, &HashSet<Dependency>)> {
self.graph.edges(&pkg).map(|(id, deps)| (*id, deps))
}

pub fn replacement(&self, pkg: PackageId) -> Option<PackageId> {
@@ -325,8 +323,9 @@ unable to verify that `{0}` is the same as when the lockfile was generated
to: PackageId,
to_target: &Target,
) -> CargoResult<String> {
let empty_set: HashSet<Dependency> = HashSet::new();
let deps = if from == to {
&[]
&empty_set
} else {
self.dependencies_listed(from, to)
};
@@ -349,7 +348,7 @@ unable to verify that `{0}` is the same as when the lockfile was generated
Ok(name)
}

fn dependencies_listed(&self, from: PackageId, to: PackageId) -> &[Dependency] {
fn dependencies_listed(&self, from: PackageId, to: PackageId) -> &HashSet<Dependency> {
// We've got a dependency on `from` to `to`, but this dependency edge
// may be affected by [replace]. If the `to` package is listed as the
// target of a replacement (aka the key of a reverse replacement map)
6 changes: 1 addition & 5 deletions src/cargo/ops/cargo_output_metadata.rs
Original file line number Diff line number Diff line change
@@ -184,11 +184,7 @@ fn build_resolve_graph_r(
CompileKind::Host => true,
})
.filter_map(|(dep_id, deps)| {
let mut dep_kinds: Vec<_> = deps.iter().map(DepKindInfo::from).collect();
// Duplicates may appear if the same package is used by different
// members of a workspace with different features selected.
dep_kinds.sort_unstable();
dep_kinds.dedup();
let dep_kinds: Vec<_> = deps.iter().map(DepKindInfo::from).collect();
package_map
.get(&dep_id)
.and_then(|pkg| pkg.targets().iter().find(|t| t.is_lib()))