Skip to content
Merged
Show file tree
Hide file tree
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
Handle activation conflicts for [patch] sources
This commit updates the resolver to ensure that it recognizes conflicts
when `[patch]` is used to augment an older version of what's already in
a source, for example. Previously the deduplication based on
semver-compatible versions didn't actually work when `[patch]` was used.
This meant that when you used `[patch]` it might not transitively affect
the entire crate graph, instead just giving you a version of a
dependency and everyone else. This violates the intention of `[patch]`!

The fix here is to catch this use case happening, when a `Dependency`
source specification mismatches an activated package we need to list a
second activation in the resolver to prevent major versions from being
selected from both the original source as well as the source of the id.

Closes #7117
  • Loading branch information
alexcrichton committed Jul 10, 2019
commit 83bb30cedfe88466976c8a2a416d414b00e3c909
35 changes: 34 additions & 1 deletion src/cargo/core/resolver/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,17 @@ impl Context {

/// Activate this summary by inserting it into our list of known activations.
///
/// The `parent` passed in here is the parent summary/dependency edge which
/// cased `summary` to get activated. This may not be present for the root
/// crate, for example.
///
/// Returns `true` if this summary with the given method is already activated.
pub fn flag_activated(&mut self, summary: &Summary, method: &Method) -> CargoResult<bool> {
pub fn flag_activated(
&mut self,
summary: &Summary,
method: &Method,
parent: Option<(&Summary, &Dependency)>,
) -> CargoResult<bool> {
let id = summary.package_id();
let age: ContextAge = self.age();
match self.activations.entry(id.as_activations_key()) {
Expand All @@ -121,6 +130,30 @@ impl Context {
);
}
v.insert((summary.clone(), age));

// If we've got a parent dependency which activated us, *and*
// the dependency has a different source id listed than the
// `summary` itself, then things get interesting. This basically
// means that a `[patch]` was used to augment `dep.source_id()`
// with `summary`.
//
// In this scenario we want to consider the activation key, as
// viewed from the perspective of `dep.source_id()`, as being
// fulfilled. This means that we need to add a second entry in
// the activations map for the source that was patched, in
// addition to the source of the actual `summary` itself.
//
// Without this it would be possible to have both 1.0.0 and
// 1.1.0 "from crates.io" in a dependency graph if one of those
// versions came from a `[patch]` source.
if let Some((_, dep)) = parent {
if dep.source_id() != id.source_id() {
let key = (id.name(), dep.source_id(), id.version().into());
let prev = self.activations.insert(key, (summary.clone(), age));
assert!(prev.is_none());
}
}

return Ok(false);
}
}
Expand Down
9 changes: 7 additions & 2 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,11 +651,16 @@ fn activate(
}
}

let activated = cx.flag_activated(&candidate, &method)?;
let activated = cx.flag_activated(&candidate, &method, parent)?;

let candidate = match registry.replacement_summary(candidate_pid) {
Some(replace) => {
if cx.flag_activated(replace, &method)? && activated {
// Note the `None` for parent here since `[replace]` is a bit wonky
// and doesn't activate the same things that `[patch]` typically
// does. TBH it basically cause panics in the test suite if
// `parent` is passed through here and `[replace]` is otherwise
// on life support so it's not critical to fix bugs anyway per se.
if cx.flag_activated(replace, &method, None)? && activated {
return Ok(None);
}
trace!(
Expand Down
59 changes: 59 additions & 0 deletions tests/testsuite/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1020,3 +1020,62 @@ fn replace_prerelease() {

p.cargo("build").run();
}

#[cargo_test]
fn patch_older() {
Package::new("baz", "1.0.2").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
bar = { path = 'bar' }
baz = "=1.0.1"
[patch.crates-io]
baz = { path = "./baz" }
"#,
)
.file("src/lib.rs", "")
.file(
"bar/Cargo.toml",
r#"
[project]
name = "bar"
version = "0.5.0"
authors = []
[dependencies]
baz = "1.0.0"
"#,
)
.file("bar/src/lib.rs", "")
.file(
"baz/Cargo.toml",
r#"
[project]
name = "baz"
version = "1.0.1"
authors = []
"#,
)
.file("baz/src/lib.rs", "")
.build();

p.cargo("build")
.with_stderr(
"\
[UPDATING] [..]
[COMPILING] baz v1.0.1 [..]
[COMPILING] bar v0.5.0 [..]
[COMPILING] foo v0.1.0 [..]
[FINISHED] [..]
",
)
.run();
}