Support creating lock files on ExFAT on MacOS

crates/uv-bin-install/src/lib.rs

@@ -19,10 +19,9 @@ use tracing::debug;
 use url::Url;
 use uv_distribution_filename::SourceDistExtension;
 
-use uv_cache::{Cache, CacheBucket, CacheEntry};
+use uv_cache::{Cache, CacheBucket, CacheEntry, LockCacheError};
 use uv_client::{BaseClient, is_transient_network_error};
 use uv_extract::{Error as ExtractError, stream};
-use uv_fs::LockedFileError;
 use uv_pep440::Version;
 use uv_platform::Platform;
 use uv_redacted::DisplaySafeUrl;
@@ -137,7 +136,7 @@ pub enum Error {
     Io(#[from] std::io::Error),
 
     #[error(transparent)]
-    LockedFile(#[from] LockedFileError),
+    LockCache(#[from] LockCacheError),
 
     #[error("Failed to detect platform")]
     Platform(#[from] uv_platform::Error),

crates/uv-cache/Cargo.toml

@@ -34,5 +34,6 @@ rustc-hash = { workspace = true }
 same-file = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 tempfile = { workspace = true }
+thiserror = { workspace = true }
 tracing = { workspace = true }
 walkdir = { workspace = true }

crates/uv-cache/src/lib.rs

@@ -7,6 +7,7 @@ use std::str::FromStr;
 use std::sync::Arc;
 
 use rustc_hash::FxHashMap;
+use thiserror::Error;
 use tracing::{debug, trace, warn};
 
 use uv_cache_info::Timestamp;
@@ -35,6 +36,24 @@ mod wheel;
 /// Must be kept in-sync with the version in [`CacheBucket::to_str`].
 pub const ARCHIVE_VERSION: u8 = 0;
 
+/// Error locking a cache entry or shard
+#[derive(Debug, Error)]
+pub enum LockCacheError {
+    #[error("Could not create path")]
+    CreateRoot(#[from] io::Error),
+    #[error("Could not acquire lock")]
+    Acquire(#[from] LockedFileError),
+}
+
+/// Error initialising the cache
+#[derive(Debug, Error)]
+pub enum InitCacheError {
+    #[error("Could not acquire lock")]
+    Acquire(#[from] LockedFileError),
+    #[error("Could not make the path absolute")]
+    Absolute(#[from] io::Error),
+}
+
 /// A [`CacheEntry`] which may or may not exist yet.
 #[derive(Debug, Clone)]
 pub struct CacheEntry(PathBuf);
@@ -80,14 +99,14 @@ impl CacheEntry {
     }
 
     /// Acquire the [`CacheEntry`] as an exclusive lock.
-    pub async fn lock(&self) -> Result<LockedFile, LockedFileError> {
+    pub async fn lock(&self) -> Result<LockedFile, LockCacheError> {
         fs_err::create_dir_all(self.dir())?;
-        LockedFile::acquire(
+        Ok(LockedFile::acquire(
             self.path(),
             LockedFileMode::Exclusive,
             self.path().display(),
         )
-        .await
+        .await?)
     }
 }
 
@@ -114,14 +133,14 @@ impl CacheShard {
     }
 
     /// Acquire the cache entry as an exclusive lock.
-    pub async fn lock(&self) -> Result<LockedFile, LockedFileError> {
+    pub async fn lock(&self) -> Result<LockedFile, LockCacheError> {
         fs_err::create_dir_all(self.as_ref())?;
-        LockedFile::acquire(
+        Ok(LockedFile::acquire(
             self.join(".lock"),
             LockedFileMode::Exclusive,
             self.display(),
         )
-        .await
+        .await?)
     }
 
     /// Return the [`CacheShard`] as a [`PathBuf`].
@@ -441,7 +460,7 @@ impl Cache {
     }
 
     /// Initialize the [`Cache`].
-    pub async fn init(self) -> Result<Self, LockedFileError> {
+    pub async fn init(self) -> Result<Self, InitCacheError> {
         let root = &self.root;
 
         Self::create_base_files(root)?;
@@ -466,7 +485,7 @@ impl Cache {
                 );
                 None
             }
-            Err(err) => return Err(err),
+            Err(err) => return Err(err.into()),
         };
 
         Ok(Self {

crates/uv-distribution/src/error.rs

@@ -2,13 +2,14 @@ use std::path::PathBuf;
 
 use owo_colors::OwoColorize;
 use tokio::task::JoinError;
+use uv_cache::LockCacheError;
 use zip::result::ZipError;
 
 use crate::metadata::MetadataError;
 use uv_client::WrappedReqwestError;
 use uv_distribution_filename::{WheelFilename, WheelFilenameError};
 use uv_distribution_types::{InstalledDist, InstalledDistError, IsBuildBackendError};
-use uv_fs::{LockedFileError, Simplified};
+use uv_fs::Simplified;
 use uv_git::GitError;
 use uv_normalize::PackageName;
 use uv_pep440::{Version, VersionSpecifiers};
@@ -42,7 +43,7 @@ pub enum Error {
     #[error("Failed to write to the distribution cache")]
     CacheWrite(#[source] std::io::Error),
     #[error("Failed to acquire lock on the distribution cache")]
-    CacheLock(#[source] LockedFileError),
+    CacheLock(#[source] LockCacheError),
     #[error("Failed to deserialize cache entry")]
     CacheDecode(#[from] rmp_serde::decode::Error),
     #[error("Failed to serialize cache entry")]

crates/uv-fs/src/locked_file.rs

@@ -33,6 +33,33 @@ static LOCK_TIMEOUT: LazyLock<Duration> = LazyLock::new(|| {
     }
 });
 
+#[derive(Debug, Error)]
+pub enum CreateLockedFileError {
+    #[error("Could not create temporary file")]
+    CreateTemporary(#[source] io::Error),
+    #[error("Could not persist temporary file `{}`", path.user_display())]
+    PersistTemporary {
+        path: PathBuf,
+        #[source]
+        source: io::Error,
+    },
+    #[error("Could not open existing file")]
+    OpenExisting(#[source] io::Error),
+    #[error("Could not create or open the file")]
+    CreateOrOpen(#[source] io::Error),
+}
+
+impl CreateLockedFileError {
+    pub fn as_io_error(&self) -> &io::Error {
+        match self {
+            Self::CreateTemporary(err) => err,
+            Self::PersistTemporary { source, .. } => source,
+            Self::OpenExisting(err) => err,
+            Self::CreateOrOpen(err) => err,
+        }
+    }
+}
+
 #[derive(Debug, Error)]
 pub enum LockedFileError {
     #[error(
@@ -58,8 +85,12 @@ pub enum LockedFileError {
         #[source]
         source: io::Error,
     },
-    #[error(transparent)]
-    Io(#[from] io::Error),
+    #[error("Could not open or create lock file at `{}`", path.user_display())]
+    Create {
+        path: PathBuf,
+        #[source]
+        source: CreateLockedFileError,
+    },
     #[error(transparent)]
     #[cfg(feature = "tokio")]
     JoinError(#[from] tokio::task::JoinError),
@@ -72,7 +103,7 @@ impl LockedFileError {
             #[cfg(feature = "tokio")]
             Self::JoinError(_) => None,
             Self::Lock { source, .. } => Some(source),
-            Self::Io(err) => Some(err),
+            Self::Create { source, .. } => Some(source.as_io_error()),
         }
     }
 }
@@ -201,7 +232,10 @@ impl LockedFile {
         mode: LockedFileMode,
         resource: impl Display,
     ) -> Result<Self, LockedFileError> {
-        let file = Self::create(path)?;
+        let file = Self::create(&path).map_err(|source| LockedFileError::Create {
+            path: path.as_ref().to_path_buf(),
+            source,
+        })?;
         let resource = resource.to_string();
         Self::lock_file(file, mode, &resource).await
     }
@@ -222,10 +256,19 @@ impl LockedFile {
     }
 
     #[cfg(unix)]
-    fn create(path: impl AsRef<Path>) -> Result<fs_err::File, std::io::Error> {
-        use std::os::unix::fs::PermissionsExt;
+    fn create(path: impl AsRef<Path>) -> Result<fs_err::File, CreateLockedFileError> {
+        use rustix::io::Errno;
+        #[allow(clippy::disallowed_types)]
+        use std::{fs::File, os::unix::fs::PermissionsExt};
         use tempfile::NamedTempFile;
 
+        #[allow(clippy::disallowed_types)]
+        fn try_set_permissions(file: &File) {
+            if let Err(err) = file.set_permissions(std::fs::Permissions::from_mode(0o666)) {
+                warn!("Failed to set permissions on temporary file: {err}");
+            }
+        }
+
         // If path already exists, return it.
         if let Ok(file) = fs_err::OpenOptions::new()
             .read(true)
@@ -238,16 +281,12 @@ impl LockedFile {
         // Otherwise, create a temporary file with 666 permissions. We must set
         // permissions _after_ creating the file, to override the `umask`.
         let file = if let Some(parent) = path.as_ref().parent() {
-            NamedTempFile::new_in(parent)?
+            NamedTempFile::new_in(parent)
         } else {
-            NamedTempFile::new()?
-        };
-        if let Err(err) = file
-            .as_file()
-            .set_permissions(std::fs::Permissions::from_mode(0o666))
-        {
-            warn!("Failed to set permissions on temporary file: {err}");
+            NamedTempFile::new()
         }
+        .map_err(CreateLockedFileError::CreateTemporary)?;
+        try_set_permissions(file.as_file());
 
         // Try to move the file to path, but if path exists now, just open path
         match file.persist_noclobber(path.as_ref()) {
@@ -258,20 +297,59 @@ impl LockedFile {
                         .read(true)
                         .write(true)
                         .open(path.as_ref())
+                        .map_err(CreateLockedFileError::OpenExisting)
+                } else if matches!(
+                    Errno::from_io_error(&err.error),
+                    Some(Errno::NOTSUP | Errno::INVAL)
+                ) {
+                    // Fallback in case `persist_noclobber`, which uses `renameat2` or
+                    // `renameatx_np` under the hood, is not supported by the FS. Linux reports this
+                    // with `EINVAL` and MacOS with `ENOTSUP`.
+
+                    // There is a race here where another process has just created the file, and we
+                    // try to open it and get permission errors because the other process hasn't set
+                    // the permission bits yet. This will lead to a transient failure, but unlike
+                    // alternative approaches it won't ever lead to a situation where two processes
+                    // are locking two different files. Also, since `persist_noclobber` is more
+                    // likely to not be supported on special filesystems which don't have permission
+                    // bits, it's less likely to ever matter.
+                    let file = fs_err::OpenOptions::new()
+                        .read(true)
+                        .write(true)
+                        .create(true)
+                        .open(path.as_ref())
+                        .map_err(CreateLockedFileError::CreateOrOpen)?;
+
+                    // We don't want to `try_set_permissions` in cases where another user's process
+                    // has already created the lockfile and changed its permissions because we might
+                    // not have permission to change the permissions which would produce a confusing
+                    // warning.
+                    if file
+                        .metadata()
+                        .is_ok_and(|metadata| metadata.permissions().mode() != 0o666)
+                    {
+                        try_set_permissions(file.file());
+                    }
+                    Ok(file)
                 } else {
-                    Err(err.error)
+                    let temp_path = err.file.into_temp_path();
+                    Err(CreateLockedFileError::PersistTemporary {
+                        path: <tempfile::TempPath as AsRef<Path>>::as_ref(&temp_path).to_path_buf(),
+                        source: err.error,
+                    })
                 }
             }
         }
     }
 
     #[cfg(not(unix))]
-    fn create(path: impl AsRef<Path>) -> std::io::Result<fs_err::File> {
+    fn create(path: impl AsRef<Path>) -> Result<fs_err::File, CreateLockedFileError> {
         fs_err::OpenOptions::new()
             .read(true)
             .write(true)
             .create(true)
             .open(path.as_ref())
+            .map_err(CreateLockedFileError::CreateOrOpen)
     }
 }