From c46e57b0cf7489f8d6a41a42f4eebcc08201e195 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 5 Jul 2022 21:02:08 -0700 Subject: [PATCH] Fix corrupted git checkout recovery. --- src/cargo/sources/git/utils.rs | 42 ++++++----------------- tests/testsuite/git.rs | 62 ++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 32 deletions(-) diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 2bfde7941..2763244bb 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -151,30 +151,16 @@ impl GitDatabase { dest: &Path, cargo_config: &Config, ) -> CargoResult> { - let mut checkout = None; - if let Ok(repo) = git2::Repository::open(dest) { - let mut co = GitCheckout::new(dest, self, rev, repo); - if !co.is_fresh() { - // After a successful fetch operation the subsequent reset can - // fail sometimes for corrupt repositories where the fetch - // operation succeeds but the object isn't actually there in one - // way or another. In these situations just skip the error and - // try blowing away the whole repository and trying with a - // clone. - co.fetch(cargo_config)?; - match co.reset(cargo_config) { - Ok(()) => { - assert!(co.is_fresh()); - checkout = Some(co); - } - Err(e) => debug!("failed reset after fetch {:?}", e), - } - } else { - checkout = Some(co); - } - }; - let checkout = match checkout { - Some(c) => c, + // If the existing checkout exists, and it is fresh, use it. + // A non-fresh checkout can happen if the checkout operation was + // interrupted. In that case, the checkout gets deleted and a new + // clone is created. + let checkout = match git2::Repository::open(dest) + .ok() + .map(|repo| GitCheckout::new(dest, self, rev, repo)) + .filter(|co| co.is_fresh()) + { + Some(co) => co, None => GitCheckout::clone_into(dest, self, rev, cargo_config)?, }; checkout.update_submodules(cargo_config)?; @@ -311,14 +297,6 @@ impl<'a> GitCheckout<'a> { } } - fn fetch(&mut self, cargo_config: &Config) -> CargoResult<()> { - info!("fetch {}", self.repo.path().display()); - let url = self.database.path.into_url()?; - let reference = GitReference::Rev(self.revision.to_string()); - fetch(&mut self.repo, url.as_str(), &reference, cargo_config)?; - Ok(()) - } - fn reset(&self, config: &Config) -> CargoResult<()> { // If we're interrupted while performing this reset (e.g., we die because // of a signal) Cargo needs to be sure to try to check out this repo diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 96cabeed2..1197207c8 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -3510,3 +3510,65 @@ fn git_with_force_push() { amend_commit("awesome-four"); verify("awesome-four"); } + +#[cargo_test] +fn corrupted_checkout() { + // Test what happens if the checkout is corrupted somehow. + _corrupted_checkout(false); +} + +#[cargo_test] +fn corrupted_checkout_with_cli() { + // Test what happens if the checkout is corrupted somehow with git cli. + if disable_git_cli() { + return; + } + _corrupted_checkout(true); +} + +fn _corrupted_checkout(with_cli: bool) { + let git_project = git::new("dep1", |project| { + project + .file("Cargo.toml", &basic_manifest("dep1", "0.5.0")) + .file("src/lib.rs", "") + }); + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + dep1 = {{ git = "{}" }} + "#, + git_project.url() + ), + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("fetch").run(); + + let mut paths = t!(glob::glob( + paths::home() + .join(".cargo/git/checkouts/dep1-*/*") + .to_str() + .unwrap() + )); + let path = paths.next().unwrap().unwrap(); + let ok = path.join(".cargo-ok"); + + // Deleting this file simulates an interrupted checkout. + t!(fs::remove_file(&ok)); + + // This should refresh the checkout. + let mut e = p.cargo("fetch"); + if with_cli { + e.env("CARGO_NET_GIT_FETCH_WITH_CLI", "true"); + } + e.run(); + assert!(ok.exists()); +} -- 2.39.5