Skip to content

Commit

Permalink
Auto merge of #14530 - stormshield-guillaumed:vendor, r=weihanglo
Browse files Browse the repository at this point in the history
fix(vendor): trust crate version only when coming from registries

### What does this PR try to resolve?

Fixes #8181
Relates to #11897 and #14525

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

As mentioned in the contribution guide, I made a first commit adding a test that passes with the actual behaviour. Then, I made a second commit with a fix and modified the test with the new expected  behaviour.

### Additional information

The fix doesn't take into account switching from a git dependency to crates.io, which is not handled correctly on master either, and would probably require the vendoring to serialize the source ID to detect source changes.

I specifically limited the trust of immutable version to crates.io, but it could be extended to other registries.
  • Loading branch information
bors committed Sep 14, 2024
2 parents 643a025 + a53b81a commit d949021
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/cargo/ops/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,10 @@ fn sync(
let dst = canonical_destination.join(&dst_name);
to_remove.remove(&dst);
let cksum = dst.join(".cargo-checksum.json");
if dir_has_version_suffix && cksum.exists() {
// Always re-copy directory without version suffix in case the version changed
// Registries are the only immutable sources,
// path and git dependencies' versions cannot be trusted to mean "no change"
if dir_has_version_suffix && id.source_id().is_registry() && cksum.exists() {
// Don't re-copy directory with version suffix in case it comes from a registry
continue;
}

Expand Down
61 changes: 61 additions & 0 deletions tests/testsuite/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1568,6 +1568,67 @@ path = [..]
);
}

#[cargo_test]
fn git_update_rev() {
let (git_project, git_repo) = git::new_repo("git", |p| {
p.file("Cargo.toml", &basic_manifest("a", "0.1.0"))
.file("src/lib.rs", "")
});
let url = git_project.url();
let ref_1 = "initial";
let ref_2 = "update";

git::tag(&git_repo, ref_1);

git_project.change_file("src/lib.rs", "pub fn f() {}");
git::add(&git_repo);
git::commit(&git_repo);
git::tag(&git_repo, ref_2);

let p = project()
.file(
"Cargo.toml",
&format!(
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
a = {{ git = '{url}', rev = '{ref_1}' }}
"#
),
)
.file("src/lib.rs", "")
.build();

p.cargo("vendor --respect-source-config --versioned-dirs")
.run();

let lib = p.read_file("vendor/a-0.1.0/src/lib.rs");
assert_e2e().eq(lib, "");

p.change_file(
"Cargo.toml",
&format!(
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
a = {{ git = '{url}', rev = '{ref_2}' }}
"#
),
);

p.cargo("vendor --respect-source-config --versioned-dirs")
.run();

let lib = p.read_file("vendor/a-0.1.0/src/lib.rs");
assert_e2e().eq(lib, "pub fn f() {}");
}

#[cargo_test]
fn depend_on_vendor_dir_not_deleted() {
let p = project()
Expand Down

0 comments on commit d949021

Please sign in to comment.