Lock entire virtualenv during modifying commands (#695)

These commands all assume that the `site-packages` are constant
throughout.

Closes #691.

Cargo.lock

@@ -2471,7 +2471,6 @@ dependencies = [
  "distribution-filename",
  "distribution-types",
  "fs-err",
- "fs2",
  "futures",
  "install-wheel-rs",
  "pep440_rs 0.3.12",
@@ -2516,6 +2515,8 @@ name = "puffin-fs"
 version = "0.0.1"
 dependencies = [
  "fs-err",
+ "fs2",
+ "puffin-warnings",
  "tempfile",
  "tracing",
 ]

crates/puffin-cli/src/commands/pip_install.rs

@@ -96,6 +96,7 @@ pub(crate) async fn pip_install(
         "Using Python interpreter: {}",
         venv.python_executable().display()
     );
+    let _lock = venv.lock()?;
 
     // Determine the set of installed packages.
     let site_packages =

crates/puffin-cli/src/commands/pip_sync.rs

@@ -51,6 +51,7 @@ pub(crate) async fn pip_sync(
         "Using Python interpreter: {}",
         venv.python_executable().display()
     );
+    let _lock = venv.lock()?;
 
     // Determine the current environment markers.
     let tags = Tags::from_interpreter(venv.interpreter())?;

crates/puffin-cli/src/commands/pip_uninstall.rs

@@ -31,6 +31,7 @@ pub(crate) async fn pip_uninstall(
         "Using Python interpreter: {}",
         venv.python_executable().display()
     );
+    let _lock = venv.lock()?;
 
     // Index the current `site-packages` directory.
     let site_packages = puffin_installer::SitePackages::from_executable(&venv)?;

crates/puffin-distribution/Cargo.toml

@@ -31,7 +31,6 @@ pypi-types = { path = "../pypi-types" }
 anyhow = { workspace = true }
 bytesize = { workspace = true }
 fs-err = { workspace = true }
-fs2 = { workspace = true }
 futures = { workspace = true }
 reqwest = { workspace = true }
 rmp-serde = { workspace = true }

crates/puffin-distribution/src/locks.rs

@@ -1,12 +1,7 @@
-use std::io;
-use std::path::PathBuf;
 use std::sync::Arc;
 
-use fs2::FileExt;
-use fs_err::File;
 use rustc_hash::FxHashMap;
 use tokio::sync::Mutex;
-use tracing::error;
 
 use distribution_types::{Identifier, ResourceId};
 
@@ -23,29 +18,3 @@ impl Locks {
             .clone()
     }
 }
-
-/// A file lock that is automatically released when dropped.
-#[derive(Debug)]
-pub(crate) struct LockedFile(File);
-
-impl LockedFile {
-    pub(crate) fn new(path: impl Into<PathBuf>) -> Result<Self, io::Error> {
-        let file = File::create(path)?;
-        // TODO(konstin): Notify the user when the lock isn't free so they know why nothing is
-        // happening
-        file.file().lock_exclusive()?;
-        Ok(Self(file))
-    }
-}
-
-impl Drop for LockedFile {
-    fn drop(&mut self) {
-        if let Err(err) = self.0.file().unlock() {
-            error!(
-                "Failed to unlock {}; program may be stuck: {}",
-                self.0.path().display(),
-                err
-            );
-        }
-    }
-}

crates/puffin-distribution/src/source_dist.rs

@@ -30,13 +30,12 @@ use puffin_cache::{
     digest, CacheBucket, CacheEntry, CacheShard, CachedByTimestamp, CanonicalUrl, WheelCache,
 };
 use puffin_client::{CachedClient, CachedClientError, DataWithCachePolicy};
-use puffin_fs::write_atomic;
+use puffin_fs::{write_atomic, LockedFile};
 use puffin_git::{Fetch, GitSource};
 use puffin_normalize::PackageName;
 use puffin_traits::{BuildContext, BuildKind, SourceBuildTrait};
 use pypi_types::Metadata21;
 
-use crate::locks::LockedFile;
 use crate::Reporter;
 
 /// The caller is responsible for adding the source dist information to the error chain
@@ -652,9 +651,9 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> {
         let git_dir = self.build_context.cache().bucket(CacheBucket::Git);
 
         // Avoid races between different processes, too.
-        let locks_dir = git_dir.join("locks");
-        fs::create_dir_all(&locks_dir).await?;
-        let _lockfile = LockedFile::new(locks_dir.join(digest(&CanonicalUrl::new(url))))?;
+        let lock_dir = git_dir.join("locks");
+        fs::create_dir_all(&lock_dir).await?;
+        let _lock = LockedFile::acquire(lock_dir.join(digest(&CanonicalUrl::new(url))))?;
 
         let DirectGitUrl { url, subdirectory } =
             DirectGitUrl::try_from(url).map_err(SourceDistError::Git)?;

crates/puffin-fs/Cargo.toml

@@ -13,6 +13,9 @@ license = { workspace = true }
 workspace = true
 
 [dependencies]
+puffin-warnings = { path = "../puffin-warnings" }
+
 fs-err = { workspace = true, features = ["tokio"] }
+fs2 = { workspace = true }
 tempfile = { workspace = true }
 tracing = { workspace = true }

crates/puffin-fs/src/lib.rs

@@ -1,8 +1,11 @@
 use std::path::{Path, PathBuf};
 
+use fs2::FileExt;
 use fs_err as fs;
 use tempfile::NamedTempFile;
-use tracing::warn;
+use tracing::{error, warn};
+
+use puffin_warnings::warn_user;
 
 /// Write `data` to `path` atomically using a temporary file and atomic rename.
 pub async fn write_atomic(path: impl AsRef<Path>, data: impl AsRef<[u8]>) -> std::io::Result<()> {
@@ -90,3 +93,34 @@ pub fn directories(path: impl AsRef<Path>) -> impl Iterator<Item = PathBuf> {
         })
         .map(|entry| entry.path())
 }
+
+/// A file lock that is automatically released when dropped.
+#[derive(Debug)]
+pub struct LockedFile(fs_err::File);
+
+impl LockedFile {
+    pub fn acquire(path: impl AsRef<Path>) -> Result<Self, std::io::Error> {
+        let file = fs_err::File::create(path.as_ref())?;
+        match file.file().try_lock_exclusive() {
+            Ok(()) => Ok(Self(file)),
+            Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => {
+                warn_user!("Waiting to acquire lock on directory");
+                file.file().lock_exclusive()?;
+                Ok(Self(file))
+            }
+            Err(err) => Err(err),
+        }
+    }
+}
+
+impl Drop for LockedFile {
+    fn drop(&mut self) {
+        if let Err(err) = self.0.file().unlock() {
+            error!(
+                "Failed to unlock {}; program may be stuck: {}",
+                self.0.path().display(),
+                err
+            );
+        }
+    }
+}

crates/puffin-interpreter/src/virtual_env.rs

@@ -5,6 +5,7 @@ use tracing::debug;
 
 use platform_host::Platform;
 use puffin_cache::Cache;
+use puffin_fs::LockedFile;
 
 use crate::cfg::Configuration;
 use crate::python_platform::PythonPlatform;
@@ -100,6 +101,11 @@ impl Virtualenv {
             compile_error!("only unix (like mac and linux) and windows are supported")
         }
     }
+
+    /// Lock the virtual environment to prevent concurrent writes.
+    pub fn lock(&self) -> Result<LockedFile, std::io::Error> {
+        LockedFile::acquire(self.root.join(".lock"))
+    }
 }
 
 /// Locate the current virtual environment.