Skip to content

Commit

Permalink
fix(resolver): Make resolver behavior independent of package order
Browse files Browse the repository at this point in the history
This addresses the ordering issue of for one of the problems from #12599.

The intent behind the `path_pkg` check is to make sure we update
workspace members in the lockfile if their version number changed.
In this case, we don't need to recursively walk, because the change
doesn't affect dependencies.  However, we also shouldn't *prevent*
recursive walks which is what we are doing today, both for packages
marked to keep and for packages that have been "poisoned".  So we fix
this by moving that call after all recursive adds are complete so as to
not interfere with them.
  • Loading branch information
epage committed Aug 30, 2023
1 parent 2676a4a commit 0af2f65
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 15 deletions.
30 changes: 18 additions & 12 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,23 +632,11 @@ fn register_previous_locks(
// however, nothing else in the dependency graph depends on `log` and the
// newer version of `serde` requires a new version of `log` it'll get pulled
// in (as we didn't accidentally lock it to an old version).
//
// Additionally, here we process all path dependencies listed in the previous
// resolve. They can not only have their dependencies change but also
// the versions of the package change as well. If this ends up happening
// then we want to make sure we don't lock a package ID node that doesn't
// actually exist. Note that we don't do transitive visits of all the
// package's dependencies here as that'll be covered below to poison those
// if they changed.
let mut avoid_locking = HashSet::new();
registry.add_to_yanked_whitelist(resolve.iter().filter(keep));
for node in resolve.iter() {
if !keep(&node) {
add_deps(resolve, node, &mut avoid_locking);
} else if let Some(pkg) = path_pkg(node.source_id()) {
if pkg.package_id() != node {
avoid_locking.insert(node);
}
}
}

Expand Down Expand Up @@ -741,6 +729,24 @@ fn register_previous_locks(
}
}

// Additionally, here we process all path dependencies listed in the previous
// resolve. They can not only have their dependencies change but also
// the versions of the package change as well. If this ends up happening
// then we want to make sure we don't lock a package ID node that doesn't
// actually exist. Note that we don't do transitive visits of all the
// package's dependencies here as that'll be covered below to poison those
// if they changed.
//
// This must come after all other `add_deps` calls to ensure it recursively walks the tree when
// called.
for node in resolve.iter() {
if let Some(pkg) = path_pkg(node.source_id()) {
if pkg.package_id() != node {
avoid_locking.insert(node);
}
}
}

// Alright now that we've got our new, fresh, shiny, and refined `keep`
// function let's put it to action. Take a look at the previous lock file,
// filter everything by this callback, and then shove everything else into
Expand Down
9 changes: 6 additions & 3 deletions tests/testsuite/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1082,10 +1082,13 @@ rustdns.workspace = true
p.change_file("Cargo.toml", &workspace_toml.replace("2.29.8", "2.29.81"));

p.cargo("update -p crate2")
.with_stderr(
.with_stderr(&format!(
"\
[UPDATING] git repository `{}`
[UPDATING] crate1 v2.29.8 ([CWD]/crate1) -> v2.29.81
[UPDATING] crate2 v2.29.8 ([CWD]/crate2) -> v2.29.81",
)
[UPDATING] crate2 v2.29.8 ([CWD]/crate2) -> v2.29.81
[UPDATING] rustdns v0.5.0 ([..]) -> [..]",
git_project.url(),
))
.run();
}

0 comments on commit 0af2f65

Please sign in to comment.