Use an advisory lock to co-ordinate access to git checkout repos

forc-pkg/Cargo.toml

@@ -10,6 +10,7 @@ description = "Building, locking, fetching and updating Sway projects as Forc pa
 
 [dependencies]
 anyhow = "1"
+fd-lock = "3.0"
 forc-tracing = { version = "0.32.1", path = "../forc-tracing" }
 forc-util = { version = "0.32.1", path = "../forc-util" }
 fuels-types = "0.32" 

forc-pkg/src/pkg.rs

@@ -9,7 +9,7 @@ use crate::{
 use anyhow::{anyhow, bail, Context, Error, Result};
 use forc_util::{
     default_output_directory, find_file_name, git_checkouts_directory, kebab_to_snake_case,
-    print_on_failure, print_on_success,
+    print_on_failure, print_on_success, user_forc_directory,
 };
 use petgraph::{
     self,
@@ -839,6 +839,9 @@ fn dep_path(
     match &dep.source {
         SourcePinned::Git(git) => {
             let repo_path = git_commit_path(&dep.name, &git.source.repo, &git.commit_hash);
+            // Co-ordinate access to the git checkout directory using an advisory file lock.
+            let lock = path_lock(&repo_path)?;
+            let _guard = lock.read()?;
             find_dir_within(&repo_path, &dep.name).ok_or_else(|| {
                 anyhow!(
                     "failed to find package `{}` in {}",
@@ -1750,23 +1753,32 @@ fn pin_pkg(
             let pinned = Pinned { name, source };
             let id = pinned.id();
             if let hash_map::Entry::Vacant(entry) = manifest_map.entry(id) {
+                // Co-ordinate access to the git checkout directory using an advisory file lock.
+                let mut lock = path_lock(&repo_path)?;
+
                 // TODO: Here we assume that if the local path already exists, that it contains the
                 // full and correct source for that commit and hasn't been tampered with. This is
                 // probably fine for most cases as users should never be touching these
                 // directories, however we should add some code to validate this. E.g. can we
                 // recreate the git hash by hashing the directory or something along these lines
                 // using git?
-                if !repo_path.exists() {
-                    info!("  Fetching {}", pinned_git.to_string());
-                    fetch_git(fetch_id, &pinned.name, &pinned_git)?;
+                {
+                    let _guard = lock.write()?;
+                    if !repo_path.exists() {
+                        info!("  Fetching {}", pinned_git.to_string());
+                        fetch_git(fetch_id, &pinned.name, &pinned_git)?;
+                    }
                 }
-                let path = find_dir_within(&repo_path, &pinned.name).ok_or_else(|| {
-                    anyhow!(
-                        "failed to find package `{}` in {}",
-                        pinned.name,
-                        pinned_git.to_string()
-                    )
-                })?;
+                let path = {
+                    let _guard = lock.read()?;
+                    find_dir_within(&repo_path, &pinned.name).ok_or_else(|| {
+                        anyhow!(
+                            "failed to find package `{}` in {}",
+                            pinned.name,
+                            pinned_git.to_string()
+                        )
+                    })?
+                };
                 let manifest = PackageManifestFile::from_dir(&path)?;
                 entry.insert(manifest);
             }
@@ -1784,6 +1796,48 @@ fn pin_pkg(
     Ok(pinned)
 }
 
+/// Given a path to a directory we wish to lock, produce a path for an associated lock file.
+///
+/// Note that the lock file itself is simply a placeholder for co-ordinating access. As a result,
+/// we want to create the lock file if it doesn't exist, but we can never reliably remove it
+/// without risking invalidation of an existing lock. As a result, we use a dedicated, hidden
+/// directory with a lock file named after the checkout path.
+///
+/// Note: This has nothing to do with `Forc.lock` files, rather this is about fd locks for
+/// coordinating access to particular paths (e.g. git checkout directories).
+fn fd_lock_path(path: &Path) -> PathBuf {
+    const LOCKS_DIR_NAME: &str = ".locks";
+    const LOCK_EXT: &str = "forc-lock";
+
+    // Hash the path to produce a file-system friendly lock file name.
+    // Append the file stem for improved readability.
+    let mut hasher = hash_map::DefaultHasher::default();
+    path.hash(&mut hasher);
+    let hash = hasher.finish();
+    let file_name = match path.file_stem().and_then(|s| s.to_str()) {
+        None => format!("{:X}", hash),
+        Some(stem) => format!("{:X}-{}", hash, stem),
+    };
+
+    user_forc_directory()
+        .join(LOCKS_DIR_NAME)
+        .join(file_name)
+        .with_extension(LOCK_EXT)
+}
+
+/// Create an advisory lock over the given path.
+///
+/// See [fd_lock_path] for details.
+fn path_lock(path: &Path) -> Result<fd_lock::RwLock<File>> {
+    let lock_path = fd_lock_path(path);
+    let lock_dir = lock_path
+        .parent()
+        .expect("lock path has no parent directory");
+    std::fs::create_dir_all(lock_dir).context("failed to create forc advisory lock directory")?;
+    let lock_file = File::create(&lock_path).context("failed to create advisory lock file")?;
+    Ok(fd_lock::RwLock::new(lock_file))
+}
+
 /// The path to which a git package commit should be checked out.
 ///
 /// The resulting directory is:
@@ -1803,17 +1857,23 @@ pub fn git_commit_path(name: &str, repo: &Url, commit_hash: &str) -> PathBuf {
 /// Fetch the repo at the given git package's URL and checkout the pinned commit.
 ///
 /// Returns the location of the checked out commit.
+///
+/// NOTE: This function assumes that the caller has aquired an advisory lock to co-ordinate access
+/// to the git repository checkout path.
 pub fn fetch_git(fetch_id: u64, name: &str, pinned: &SourceGitPinned) -> Result<PathBuf> {
     let path = git_commit_path(name, &pinned.source.repo, &pinned.commit_hash);
     // Checkout the pinned hash to the path.
     with_tmp_git_repo(fetch_id, name, &pinned.source, |repo| {
         // Change HEAD to point to the pinned commit.
         let id = git2::Oid::from_str(&pinned.commit_hash)?;
         repo.set_head_detached(id)?;
+
+        // If the directory exists, remove it. Note that we already check for an existing,
+        // cached checkout directory for re-use prior to reaching the `fetch_git` function.
         if path.exists() {
-            let _ = std::fs::remove_dir_all(&path);
+            let _ = fs::remove_dir_all(&path);
         }
-        std::fs::create_dir_all(&path)?;
+        fs::create_dir_all(&path)?;
 
         // Checkout HEAD to the target directory.
         let mut checkout = git2::build::CheckoutBuilder::new();
@@ -1831,6 +1891,7 @@ pub fn fetch_git(fetch_id: u64, name: &str, pinned: &SourceGitPinned) -> Result<
             pinned.source.reference.clone(),
             pinned.commit_hash.clone(),
         );
+
         // Write the index file
         fs::write(
             path.join(".forc_index"),