From ffa597e0714eb89c23111e42cc5890d51ef164ed Mon Sep 17 00:00:00 2001 From: bors Date: Mon, 18 Oct 2021 22:00:14 +0000 Subject: [PATCH] Auto merge of #9979 - ehuss:git-fetch-force, r=alexcrichton Fix fetching git repos after a force push. Users have been reporting that the index has not been updating for them. This was caused by the update to libgit2 1.3 (from 1.1) which has changed some behavior around force pushes. The index was squashed on 2021-09-24, and if a user had the index fetched from before that point, and they used nightly-2021-10-14 or newer, then the fetch would succeed, but the `refs/remotes/origin/HEAD` would not get updated. Cargo uses the `origin/HEAD` ref to know what to look at, and thus was looking at old data. The solution here is to use `+` on the refspec to force libgit2 to do a forced update (a fast-forward). I think this may have been introduced in libgit2 1.2 via https://github.com/libgit2/libgit2/pull/5854, though that is just a guess. Fixes #9976 --- src/cargo/sources/git/utils.rs | 15 +++-- tests/testsuite/git.rs | 102 +++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 6 deletions(-) diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 507ba90bb1b..8f73a1a1247 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -785,29 +785,32 @@ pub fn fetch( // which need to get fetched. Additionally record if we're fetching tags. let mut refspecs = Vec::new(); let mut tags = false; + // The `+` symbol on the refspec means to allow a forced (fast-forward) + // update which is needed if there is ever a force push that requires a + // fast-forward. match reference { // For branches and tags we can fetch simply one reference and copy it // locally, no need to fetch other branches/tags. GitReference::Branch(b) => { - refspecs.push(format!("refs/heads/{0}:refs/remotes/origin/{0}", b)); + refspecs.push(format!("+refs/heads/{0}:refs/remotes/origin/{0}", b)); } GitReference::Tag(t) => { - refspecs.push(format!("refs/tags/{0}:refs/remotes/origin/tags/{0}", t)); + refspecs.push(format!("+refs/tags/{0}:refs/remotes/origin/tags/{0}", t)); } GitReference::DefaultBranch => { - refspecs.push(String::from("HEAD:refs/remotes/origin/HEAD")); + refspecs.push(String::from("+HEAD:refs/remotes/origin/HEAD")); } GitReference::Rev(rev) => { if rev.starts_with("refs/") { - refspecs.push(format!("{0}:{0}", rev)); + refspecs.push(format!("+{0}:{0}", rev)); } else { // We don't know what the rev will point to. To handle this // situation we fetch all branches and tags, and then we pray // it's somewhere in there. - refspecs.push(String::from("refs/heads/*:refs/remotes/origin/*")); - refspecs.push(String::from("HEAD:refs/remotes/origin/HEAD")); + refspecs.push(String::from("+refs/heads/*:refs/remotes/origin/*")); + refspecs.push(String::from("+HEAD:refs/remotes/origin/HEAD")); tags = true; } } diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index d7974a3ef87..92a84e26848 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -3280,3 +3280,105 @@ fn metadata_master_consistency() { let bar_source = format!("git+{}", git_project.url()); p.cargo("metadata").with_json(&metadata(&bar_source)).run(); } + +#[cargo_test] +fn git_with_force_push() { + // Checks that cargo can handle force-pushes to git repos. + // This works by having a git dependency that is updated with an amend + // commit, and tries with various forms (default branch, branch, rev, + // tag). + let main = |text| format!(r#"pub fn f() {{ println!("{}"); }}"#, text); + let (git_project, repo) = git::new_repo("dep1", |project| { + project + .file("Cargo.toml", &basic_lib_manifest("dep1")) + .file("src/lib.rs", &main("one")) + }); + let manifest = |extra| { + format!( + r#" + [project] + name = "foo" + version = "0.0.1" + edition = "2018" + + [dependencies] + dep1 = {{ git = "{}"{} }} + "#, + git_project.url(), + extra + ) + }; + let p = project() + .file("Cargo.toml", &manifest("")) + .file("src/main.rs", "fn main() { dep1::f(); }") + .build(); + // Download the original and make sure it is OK. + p.cargo("build").run(); + p.rename_run("foo", "foo1").with_stdout("one").run(); + + let find_head = || t!(t!(repo.head()).peel_to_commit()); + + let amend_commit = |text| { + // commit --amend a change that will require a force fetch. + git_project.change_file("src/lib.rs", &main(text)); + git::add(&repo); + let commit = find_head(); + let tree_id = t!(t!(repo.index()).write_tree()); + t!(commit.amend( + Some("HEAD"), + None, + None, + None, + None, + Some(&t!(repo.find_tree(tree_id))) + )); + }; + + let mut rename_annoyance = 1; + + let mut verify = |text: &str| { + // Perform the fetch. + p.cargo("update").run(); + p.cargo("build").run(); + rename_annoyance += 1; + p.rename_run("foo", &format!("foo{}", rename_annoyance)) + .with_stdout(text) + .run(); + }; + + amend_commit("two"); + verify("two"); + + // Try with a rev. + let head1 = find_head().id().to_string(); + let extra = format!(", rev = \"{}\"", head1); + p.change_file("Cargo.toml", &manifest(&extra)); + verify("two"); + amend_commit("three"); + let head2 = find_head().id().to_string(); + assert_ne!(&head1, &head2); + let extra = format!(", rev = \"{}\"", head2); + p.change_file("Cargo.toml", &manifest(&extra)); + verify("three"); + + // Try with a tag. + git::tag(&repo, "my-tag"); + p.change_file("Cargo.toml", &manifest(", tag = \"my-tag\"")); + verify("three"); + amend_commit("tag-three"); + let head = t!(t!(repo.head()).peel(git2::ObjectType::Commit)); + t!(repo.tag("my-tag", &head, &t!(repo.signature()), "move tag", true)); + verify("tag-three"); + + // Try with a branch. + let br = t!(repo.branch("awesome-stuff", &find_head(), false)); + t!(repo.checkout_tree(&t!(br.get().peel(git2::ObjectType::Tree)), None)); + t!(repo.set_head("refs/heads/awesome-stuff")); + git_project.change_file("src/lib.rs", &main("awesome-three")); + git::add(&repo); + git::commit(&repo); + p.change_file("Cargo.toml", &manifest(", branch = \"awesome-stuff\"")); + verify("awesome-three"); + amend_commit("awesome-four"); + verify("awesome-four"); +}