diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index d28931f8642..4f1f873bd8b 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -181,9 +181,14 @@ impl GitDatabase { .filter(|co| co.is_fresh()) { Some(co) => co, - None => GitCheckout::clone_into(dest, self, rev, gctx)?, + None => { + let (checkout, guard) = GitCheckout::clone_into(dest, self, rev, gctx)?; + checkout.update_submodules(gctx)?; + guard.mark_ok()?; + checkout + } }; - checkout.update_submodules(gctx)?; + Ok(checkout) } @@ -280,7 +285,7 @@ impl<'a> GitCheckout<'a> { database: &'a GitDatabase, revision: git2::Oid, gctx: &GlobalContext, - ) -> CargoResult> { + ) -> CargoResult<(GitCheckout<'a>, CheckoutGuard)> { let dirname = into.parent().unwrap(); paths::create_dir_all(&dirname)?; if into.exists() { @@ -329,8 +334,8 @@ impl<'a> GitCheckout<'a> { let repo = repo.unwrap(); let checkout = GitCheckout::new(database, revision, repo); - checkout.reset(gctx)?; - Ok(checkout) + let guard = checkout.reset(gctx)?; + Ok((checkout, guard)) } /// Checks if the `HEAD` of this checkout points to the expected revision. @@ -355,12 +360,12 @@ impl<'a> GitCheckout<'a> { /// To enable this we have a dummy file in our checkout, [`.cargo-ok`], /// which if present means that the repo has been successfully reset and is /// ready to go. Hence if we start to do a reset, we make sure this file - /// *doesn't* exist, and then once we're done we create the file. + /// *doesn't* exist. The caller of [`reset`] has an option to perform additional operations + /// (e.g. submodule update) before marking the check-out as ready. /// /// [`.cargo-ok`]: CHECKOUT_READY_LOCK - fn reset(&self, gctx: &GlobalContext) -> CargoResult<()> { - let ok_file = self.path.join(CHECKOUT_READY_LOCK); - let _ = paths::remove_file(&ok_file); + fn reset(&self, gctx: &GlobalContext) -> CargoResult { + let guard = CheckoutGuard::guard(&self.path); info!("reset {} to {}", self.repo.path().display(), self.revision); // Ensure libgit2 won't mess with newlines when we vendor. @@ -370,8 +375,8 @@ impl<'a> GitCheckout<'a> { let object = self.repo.find_object(self.revision, None)?; reset(&self.repo, &object, gctx)?; - paths::create(ok_file)?; - Ok(()) + + Ok(guard) } /// Like `git submodule update --recursive` but for this git checkout. @@ -479,6 +484,25 @@ impl<'a> GitCheckout<'a> { } } +/// See [`GitCheckout::reset`] for rationale on this type. +#[must_use] +struct CheckoutGuard { + ok_file: PathBuf, +} + +impl CheckoutGuard { + fn guard(path: &Path) -> Self { + let ok_file = path.join(CHECKOUT_READY_LOCK); + let _ = paths::remove_file(&ok_file); + Self { ok_file } + } + + fn mark_ok(self) -> CargoResult<()> { + let _ = paths::create(self.ok_file)?; + Ok(()) + } +} + /// Constructs an absolute URL for a child submodule URL with its parent base URL. /// /// Git only assumes a submodule URL is a relative path if it starts with `./` diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 6bc72423c72..a0650bc0646 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -9,7 +9,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::thread; -use cargo_test_support::git::cargo_uses_gitoxide; +use cargo_test_support::git::{add_submodule, cargo_uses_gitoxide}; use cargo_test_support::paths; use cargo_test_support::prelude::IntoData; use cargo_test_support::prelude::*; @@ -3847,11 +3847,20 @@ fn corrupted_checkout_with_cli() { } fn _corrupted_checkout(with_cli: bool) { - let git_project = git::new("dep1", |project| { + let (git_project, repository) = git::new_repo("dep1", |project| { project .file("Cargo.toml", &basic_manifest("dep1", "0.5.0")) .file("src/lib.rs", "") }); + + let project2 = git::new("dep2", |project| { + project.no_manifest().file("README.md", "") + }); + let url = project2.root().to_url().to_string(); + add_submodule(&repository, &url, Path::new("dep2")); + git::commit(&repository); + drop(repository); + let p = project() .file( "Cargo.toml", @@ -3873,17 +3882,21 @@ fn _corrupted_checkout(with_cli: bool) { p.cargo("fetch").run(); - let mut paths = t!(glob::glob( + let mut dep1_co_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"); + let dep1_co_path = dep1_co_paths.next().unwrap().unwrap(); + let dep1_ok = dep1_co_path.join(".cargo-ok"); + let dep1_manifest = dep1_co_path.join("Cargo.toml"); + let dep2_readme = dep1_co_path.join("dep2/README.md"); // Deleting this file simulates an interrupted checkout. - t!(fs::remove_file(&ok)); + t!(fs::remove_file(&dep1_ok)); + t!(fs::remove_file(&dep1_manifest)); + t!(fs::remove_file(&dep2_readme)); // This should refresh the checkout. let mut e = p.cargo("fetch"); @@ -3891,7 +3904,9 @@ fn _corrupted_checkout(with_cli: bool) { e.env("CARGO_NET_GIT_FETCH_WITH_CLI", "true"); } e.run(); - assert!(ok.exists()); + assert!(dep1_ok.exists()); + assert!(dep1_manifest.exists()); + assert!(dep2_readme.exists()); } #[cargo_test]