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

Handle activation conflicts for [patch] sources #7118

Merged
merged 1 commit into from
Jul 12, 2019
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
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();
}