Skip to content

Commit

Permalink
Auto merge of #12975 - epage:git, r=weihanglo
Browse files Browse the repository at this point in the history
fix(resolver): Don't do git fetches when updating workspace members

### What does this PR try to resolve?

Before, when running `cargo update <member>`, we'd not reuse the
previous resolve result and when the resolver started walking into the
dependencies, it would do a git fetch.

Now, we won't even try to resolve the workspace members and so we won't
look at those dependencies and do git fetch.

This will make `cargo update <workspace-member>`
match `cargo update --workspace`.

Fixes #12599
Fixes #8821

### How should we test and review this PR?

I considered whether there were other ways of handling this but I
figured aiming for consistency in approaches was the best way.
We can investigate improving those approaches separately.

There are other discrepancies in the different code paths (handling of
patches, adding sources) but I'm deferring looking over those.

I added a test to demonstrate the `--workspace` behavior to use as a base line to compare with.

### Additional information

Between this and #12602, this should finally resolve #12599.
  • Loading branch information
bors committed Nov 18, 2023
2 parents 9765a44 + 9ca6376 commit 4ac051e
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 6 deletions.
20 changes: 20 additions & 0 deletions src/cargo/ops/cargo_generate_lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,26 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
}
}

// Mirror `--workspace` and never avoid workspace members.
// Filtering them out here so the above processes them normally
// so their dependencies can be updated as requested
to_avoid = to_avoid
.into_iter()
.filter(|id| {
for package in ws.members() {
let member_id = package.package_id();
// Skip checking the `version` because `previous_resolve` might have a stale
// value.
// When dealing with workspace members, the other fields should be a
// sufficiently unique match.
if id.name() == member_id.name() && id.source_id() == member_id.source_id() {
return false;
}
}
true
})
.collect();

registry.add_sources(sources)?;
}

Expand Down
94 changes: 88 additions & 6 deletions tests/testsuite/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,11 +1091,8 @@ rustdns.workspace = true
p.cargo("update -p rootcrate")
.with_stderr(&format!(
"\
[UPDATING] git repository `{}`
[UPDATING] rootcrate v2.29.8 ([CWD]/rootcrate) -> v2.29.81
[UPDATING] rustdns v0.5.0 ([..]) -> [..]
[UPDATING] subcrate v2.29.8 ([CWD]/subcrate) -> v2.29.81",
git_project.url(),
))
.run();
}
Expand Down Expand Up @@ -1182,11 +1179,96 @@ rustdns.workspace = true
p.cargo("update -p crate2")
.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] rustdns v0.5.0 ([..]) -> [..]",
[UPDATING] crate2 v2.29.8 ([CWD]/crate2) -> v2.29.81",
))
.run();
}

#[cargo_test]
fn update_only_members_with_workspace() {
let git_project = git::new("rustdns", |project| {
project
.file("Cargo.toml", &basic_lib_manifest("rustdns"))
.file("src/lib.rs", "pub fn bar() {}")
});

let workspace_toml = format!(
r#"
[workspace.package]
version = "2.29.8"
edition = "2021"
publish = false
[workspace]
members = [
"crate2",
"crate1",
]
resolver = "2"
[workspace.dependencies]
# Internal crates
crate1 = {{ version = "*", path = "./crate1" }}
# External dependencies
rustdns = {{ version = "0.5.0", default-features = false, git = "{}" }}
"#,
git_project.url()
);
let p = project()
.file("Cargo.toml", &workspace_toml)
.file(
"crate2/Cargo.toml",
r#"
[package]
name = "crate2"
version.workspace = true
edition.workspace = true
publish.workspace = true
[dependencies]
crate1.workspace = true
"#,
)
.file("crate2/src/main.rs", "fn main() {}")
.file(
"crate1/Cargo.toml",
r#"
[package]
name = "crate1"
version.workspace = true
edition.workspace = true
publish.workspace = true
[dependencies]
rustdns.workspace = true
"#,
)
.file("crate1/src/lib.rs", "pub foo() {}")
.build();

// First time around we should compile both foo and bar
p.cargo("generate-lockfile")
.with_stderr(&format!(
"[UPDATING] git repository `{}`\n",
git_project.url(),
))
.run();
// Modify a file manually, shouldn't trigger a recompile
git_project.change_file("src/lib.rs", r#"pub fn bar() { println!("hello!"); }"#);
// Commit the changes and make sure we don't trigger a recompile because the
// lock file says not to change
let repo = git2::Repository::open(&git_project.root()).unwrap();
git::add(&repo);
git::commit(&repo);
p.change_file("Cargo.toml", &workspace_toml.replace("2.29.8", "2.29.81"));

p.cargo("update --workspace")
.with_stderr(
"\
[UPDATING] crate1 v2.29.8 ([CWD]/crate1) -> v2.29.81
[UPDATING] crate2 v2.29.8 ([CWD]/crate2) -> v2.29.81",
)
.run();
}

0 comments on commit 4ac051e

Please sign in to comment.