diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 844a4dcf58b..b9448ac1c3c 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -80,6 +80,25 @@ pub struct GitCheckout<'a> { repo: git2::Repository, } +/// See [`GitCheckout::reset`] for rationale on this type. +struct CheckoutGuard { + ok_file: PathBuf, +} + +impl CheckoutGuard { + #[must_use] + 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(()) + } +} + impl GitRemote { /// Creates an instance for a remote repository URL. pub fn new(url: &Url) -> GitRemote { @@ -181,9 +200,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 +304,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 +353,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 +379,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 +394,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. diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 0fd25bf2db0..06ff4056a10 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("bar")); + git::commit(&repository); + drop(repository); + let p = project() .file( "Cargo.toml", @@ -3962,7 +3971,7 @@ fn different_user_relative_submodules() { &format!( r#" [package] - name = "foo" + name = "foo" version = "0.5.0" edition = "2015"