Support creating lock files on ExFAT on MacOS (#17115)

## Summary

Fix #16859 by falling back to simply creating the lock file and then
attempting to apply permissions in cases where the temporary lockfile
cannot be renamed without overwriting (persist_noclobber) due to lack of
underlying support from the filesystem.

I've also improved the error handling.

## Test Plan

Manually on MacOS with an ExFAT partition.

~~~ bash session
$ hdiutil create -size 1g -fs ExFAT -volname EXFATDISK exfat.dmg
$ hdiutil attach exfat.dmg
$ cd /Volumes/EXFATDISK
$ uv init --bare --cache-dir build/uv/cache -v 
~~~
This commit is contained in:
Tomasz Kramkowski 2025-12-15 14:05:05 +00:00 committed by GitHub
parent a2d64aa224
commit d20948bec2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 110 additions and 37 deletions

2
Cargo.lock generated
View File

@ -5613,7 +5613,6 @@ dependencies = [
"uv-client",
"uv-distribution-filename",
"uv-extract",
"uv-fs",
"uv-pep440",
"uv-platform",
"uv-redacted",
@ -5721,6 +5720,7 @@ dependencies = [
"same-file",
"serde",
"tempfile",
"thiserror 2.0.17",
"tracing",
"uv-cache-info",
"uv-cache-key",

View File

@ -20,7 +20,6 @@ uv-cache = { workspace = true }
uv-client = { workspace = true }
uv-distribution-filename = { workspace = true }
uv-extract = { workspace = true }
uv-fs = { workspace = true }
uv-pep440 = { workspace = true }
uv-platform = { workspace = true }
uv-redacted = { workspace = true }

View File

@ -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, Error as CacheError};
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),
Cache(#[from] CacheError),
#[error("Failed to detect platform")]
Platform(#[from] uv_platform::Error),

View File

@ -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 }

View File

@ -35,6 +35,17 @@ 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, thiserror::Error)]
pub enum Error {
#[error(transparent)]
Io(#[from] io::Error),
#[error("Could not make the path absolute")]
Absolute(#[source] io::Error),
#[error("Could not acquire lock")]
Acquire(#[from] LockedFileError),
}
/// A [`CacheEntry`] which may or may not exist yet.
#[derive(Debug, Clone)]
pub struct CacheEntry(PathBuf);
@ -80,14 +91,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, Error> {
fs_err::create_dir_all(self.dir())?;
LockedFile::acquire(
Ok(LockedFile::acquire(
self.path(),
LockedFileMode::Exclusive,
self.path().display(),
)
.await
.await?)
}
}
@ -114,14 +125,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, Error> {
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`].
@ -391,7 +402,7 @@ impl Cache {
}
/// Populate the cache scaffold.
fn create_base_files(root: &PathBuf) -> Result<(), io::Error> {
fn create_base_files(root: &PathBuf) -> io::Result<()> {
// Create the cache directory, if it doesn't exist.
fs_err::create_dir_all(root)?;
@ -441,7 +452,7 @@ impl Cache {
}
/// Initialize the [`Cache`].
pub async fn init(self) -> Result<Self, LockedFileError> {
pub async fn init(self) -> Result<Self, Error> {
let root = &self.root;
Self::create_base_files(root)?;
@ -466,18 +477,18 @@ impl Cache {
);
None
}
Err(err) => return Err(err),
Err(err) => return Err(err.into()),
};
Ok(Self {
root: std::path::absolute(root)?,
root: std::path::absolute(root).map_err(Error::Absolute)?,
lock_file,
..self
})
}
/// Initialize the [`Cache`], assuming that there are no other uv processes running.
pub fn init_no_wait(self) -> Result<Option<Self>, io::Error> {
pub fn init_no_wait(self) -> Result<Option<Self>, Error> {
let root = &self.root;
Self::create_base_files(root)?;
@ -491,7 +502,7 @@ impl Cache {
return Ok(None);
};
Ok(Some(Self {
root: std::path::absolute(root)?,
root: std::path::absolute(root).map_err(Error::Absolute)?,
lock_file: Some(Arc::new(lock_file)),
..self
}))
@ -531,7 +542,7 @@ impl Cache {
/// Remove a package from the cache.
///
/// Returns the number of entries removed from the cache.
pub fn remove(&self, name: &PackageName) -> Result<Removal, io::Error> {
pub fn remove(&self, name: &PackageName) -> io::Result<Removal> {
// Collect the set of referenced archives.
let references = self.find_archive_references()?;

View File

@ -5,8 +5,8 @@ use std::fmt::{Display, Formatter};
use std::ops::Deref;
use std::path::PathBuf;
use uv_cache::Error as CacheError;
use uv_distribution_filename::{WheelFilename, WheelFilenameError};
use uv_fs::LockedFileError;
use uv_normalize::PackageName;
use uv_redacted::DisplaySafeUrl;
@ -339,7 +339,7 @@ pub enum ErrorKind {
CacheWrite(#[source] std::io::Error),
#[error("Failed to acquire lock on the client cache")]
CacheLock(#[source] LockedFileError),
CacheLock(#[source] CacheError),
#[error(transparent)]
Io(std::io::Error),

View File

@ -5,10 +5,11 @@ use tokio::task::JoinError;
use zip::result::ZipError;
use crate::metadata::MetadataError;
use uv_cache::Error as CacheError;
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] CacheError),
#[error("Failed to deserialize cache entry")]
CacheDecode(#[from] rmp_serde::decode::Error),
#[error("Failed to serialize cache entry")]

View File

@ -1,3 +1,4 @@
use std::convert::Into;
use std::fmt::Display;
use std::path::{Path, PathBuf};
use std::sync::LazyLock;
@ -59,10 +60,18 @@ pub enum LockedFileError {
source: io::Error,
},
#[error(transparent)]
Io(#[from] io::Error),
#[error(transparent)]
#[cfg(feature = "tokio")]
JoinError(#[from] tokio::task::JoinError),
#[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(transparent)]
Io(#[from] io::Error),
}
impl LockedFileError {
@ -72,6 +81,8 @@ impl LockedFileError {
#[cfg(feature = "tokio")]
Self::JoinError(_) => None,
Self::Lock { source, .. } => Some(source),
Self::CreateTemporary(err) => Some(err),
Self::PersistTemporary { source, .. } => Some(source),
Self::Io(err) => Some(err),
}
}
@ -201,7 +212,7 @@ impl LockedFile {
mode: LockedFileMode,
resource: impl Display,
) -> Result<Self, LockedFileError> {
let file = Self::create(path)?;
let file = Self::create(&path)?;
let resource = resource.to_string();
Self::lock_file(file, mode, &resource).await
}
@ -222,10 +233,25 @@ 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, LockedFileError> {
use rustix::io::Errno;
#[allow(clippy::disallowed_types)]
use std::{fs::File, os::unix::fs::PermissionsExt};
use tempfile::NamedTempFile;
/// The permissions the lockfile should end up with
const DESIRED_MODE: u32 = 0o666;
#[allow(clippy::disallowed_types)]
fn try_set_permissions(file: &File, path: &Path) {
if let Err(err) = file.set_permissions(std::fs::Permissions::from_mode(DESIRED_MODE)) {
warn!(
"Failed to set permissions on temporary file `{path}`: {err}",
path = path.user_display()
);
}
}
// If path already exists, return it.
if let Ok(file) = fs_err::OpenOptions::new()
.read(true)
@ -238,16 +264,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(LockedFileError::CreateTemporary)?;
try_set_permissions(file.as_file(), file.path());
// Try to move the file to path, but if path exists now, just open path
match file.persist_noclobber(path.as_ref()) {
@ -258,20 +280,60 @@ impl LockedFile {
.read(true)
.write(true)
.open(path.as_ref())
.map_err(Into::into)
} 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`. For these reasons and many others,
// there isn't an ErrorKind we can use here, and in fact on MacOS `ENOTSUP` gets
// mapped to `ErrorKind::Other`
// 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())?;
// 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() != DESIRED_MODE)
{
try_set_permissions(file.file(), path.as_ref());
}
Ok(file)
} else {
Err(err.error)
let temp_path = err.file.into_temp_path();
Err(LockedFileError::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, LockedFileError> {
fs_err::OpenOptions::new()
.read(true)
.write(true)
.create(true)
.open(path.as_ref())
.map_err(Into::into)
}
}