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

Faster resolver: use a inverse-index to not activate the causes of conflict #5213

Merged
merged 9 commits into from
Mar 24, 2018
Prev Previous commit
Next Next commit
problematic extension and solution
  • Loading branch information
Eh2406 committed Mar 24, 2018
commit efefe0017707252b57e1c3fb9f98d26080a6e081
6 changes: 5 additions & 1 deletion src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1276,11 +1276,15 @@ fn activate_deps_loop(
.flat_map(|x| x)
.find(|con| cx.is_conflicting(None, con))
{
conflicting_activations.extend(conflicting.clone());
let mut conflicting = conflicting.clone();
conflicting.remove(&pid);
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment for this is removed? To me it's not immediately obvious but I've always got a hard time following the specifics here anyway!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and then you can help me rewrite it so someone else can understand it. :-P


conflicting_activations.extend(conflicting);
has_past_conflicting_dep = true;
}
}
if !has_past_conflicting_dep {
// TODO: this is ugly and slow, replace!
'deps: for debs in remaining_deps.iter() {
for (_, (other_dep, _, _)) in debs.remaining_siblings.clone() {
if let Some(conflict) = past_conflicting_activations
Expand Down
76 changes: 76 additions & 0 deletions tests/testsuite/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,55 @@ fn resolving_with_deep_traps() {
assert!(res.is_err());
}

#[test]
fn resolving_with_constrained_cousins_backtrack_s1() {
// ToDo: Remove when the longer version below passes! DON'T merge
let mut reglist = Vec::new();

const DEPTH: usize = 200;
const BRANCHING_FACTOR: usize = 100;

// Each backtrack_trap depends on the next.
// The last depends on a specific ver of constrained.
for l in 0..DEPTH {
let name = format!("backtrack_trap{}", l);
let next = format!("backtrack_trap{}", l + 1);
for i in 1..BRANCHING_FACTOR {
let vsn = format!("1.0.{}", i);
reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep_req(next.as_str(), "*")]));
}
}
{
let name = format!("backtrack_trap{}", DEPTH);
for i in 1..BRANCHING_FACTOR {
let vsn = format!("1.0.{}", i);
reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep_req("constrained", "=1.0.0")]));
}
}
{
// slightly less constrained to make sure `constrained` gets picked last.
for i in 0..(BRANCHING_FACTOR + 10) {
let vsn = format!("1.0.{}", i);
reglist.push(pkg!(("constrained", vsn.as_str())));
}
}

let reg = registry(reglist.clone());

// `backtrack_trap0 = "*"` is a lot of ways of saying `constrained = "=1.0.0"`
// Only then to try and solve `constrained= "1.0.1"` which is incompatible.
let res = resolve(
&pkg_id("root"),
vec![
dep_req("backtrack_trap0", "*"),
dep_req("constrained", "1.0.1"),
],
&reg,
);

assert!(res.is_err());
}

#[test]
fn resolving_with_constrained_cousins_backtrack() {
let mut reglist = Vec::new();
Expand Down Expand Up @@ -803,6 +852,33 @@ fn resolving_with_constrained_cousins_backtrack() {
);

assert!(res.is_err());

// Each level depends on the next but the last depends on incompatible deps.
// Let's make sure that we can cache that a dep has incompatible deps.
for l in 0..DEPTH {
let name = format!("level{}", l);
let next = format!("level{}", l + 1);
for i in 1..BRANCHING_FACTOR {
let vsn = format!("1.0.{}", i);
reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep_req(next.as_str(), "*")]));
}
}
reglist.push(pkg!((format!("level{}", DEPTH).as_str(), "1.0.0") => [dep_req("backtrack_trap0", "*"),
dep_req("constrained", "1.0.1")]));

let reg = registry(reglist.clone());

// `backtrack_trap0 = "*"` is a lot of ways of saying `constrained = "=1.0.0"`
// Only then to try and solve `constrained= "1.0.1"` which is incompatible.
let res = resolve(
&pkg_id("root"),
vec![
dep_req("level0", "*"),
],
&reg,
);

assert!(res.is_err());
}

#[test]
Expand Down