From 3b728c16ccdeffe4f64220777a18bfc827259f1b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 11 May 2024 13:02:25 -0400 Subject: [PATCH] Make cache clearing robust to directories without read permissions (#3524) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary If you run the script included in the linked issue, then `uv cache clean`, we hit permissions errors on certain directories created by `setuptools`. The permissions on those directories look like: ``` ❯ sudo ls -l /Users/crmarsh/Library/Caches/uv/built-wheels-v3/pypi/opentracing/2.4.0/M-fYsaHAaQQvedmPMUl9D/opentracing-2.4.0.tar.gz/build/bdist.macosx-14.2-arm64/wheel/opentracing Password: total 0 drwxr-xr-x 3 crmarsh staff 96 May 11 12:51 harness ``` This PR adds logic to make those directories readable by the current user. Closes https://github.com/astral-sh/uv/issues/3515. --- crates/uv-cache/src/removal.rs | 121 +++++++++++++++++++++++++-------- 1 file changed, 92 insertions(+), 29 deletions(-) diff --git a/crates/uv-cache/src/removal.rs b/crates/uv-cache/src/removal.rs index 05a3ca177..99c35b3b8 100644 --- a/crates/uv-cache/src/removal.rs +++ b/crates/uv-cache/src/removal.rs @@ -35,36 +35,53 @@ impl Removal { Err(err) => return Err(err), }; - let mut rm_file = |path: &Path, meta: Result| { - if let Ok(meta) = meta { - self.total_bytes += meta.len(); - } - remove_file(path)?; - - Ok(()) - }; - if !metadata.is_dir() { self.num_files += 1; - return rm_file(path, Ok(metadata)); + + // Remove the file. + self.total_bytes += metadata.len(); + remove_file(path)?; + + return Ok(()); } for entry in walkdir::WalkDir::new(path).contents_first(true) { + // If we hit a directory that lacks read permissions, try to make it readable. + if let Err(ref err) = entry { + if err + .io_error() + .is_some_and(|err| err.kind() == io::ErrorKind::PermissionDenied) + { + if let Some(dir) = err.path() { + if set_readable(dir).unwrap_or(false) { + // Retry the operation; if we _just_ `self.rm_rf(dir)` and continue, + // `walkdir` may give us duplicate entries for the directory. + return self.rm_rf(path); + } + } + } + } + let entry = entry?; if cfg!(windows) && entry.file_type().is_symlink() { // In this branch, we try to handle junction removal. self.num_files += 1; - fs_err::remove_dir(entry.path())?; + remove_dir(entry.path())?; } else if entry.file_type().is_dir() { self.num_dirs += 1; // The contents should have been removed by now, but sometimes a race condition is // hit where other files have been added by the OS. Fall back to `remove_dir_all`, // which will remove the directory robustly across platforms. - fs_err::remove_dir_all(entry.path())?; + remove_dir_all(entry.path())?; } else { self.num_files += 1; - rm_file(entry.path(), entry.metadata())?; + + // Remove the file. + if let Ok(meta) = entry.metadata() { + self.total_bytes += meta.len(); + } + remove_file(entry.path())?; } } @@ -80,25 +97,41 @@ impl std::ops::AddAssign for Removal { } } +/// If the directory isn't readable by the current user, change the permissions to make it readable. +#[cfg_attr(windows, allow(unused_variables, clippy::unnecessary_wraps))] +fn set_readable(path: &Path) -> io::Result { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = path.metadata()?.permissions(); + if perms.mode() & 0o500 == 0 { + perms.set_mode(perms.mode() | 0o500); + fs_err::set_permissions(path, perms)?; + return Ok(true); + } + } + Ok(false) +} + +/// If the file is readonly, change the permissions to make it _not_ readonly. +fn set_not_readonly(path: &Path) -> io::Result { + let mut perms = path.metadata()?.permissions(); + if !perms.readonly() { + return Ok(false); + } + + // We're about to delete the file, so it's fine to set the permissions to world-writable. + #[allow(clippy::permissions_set_readonly_false)] + perms.set_readonly(false); + + fs_err::set_permissions(path, perms)?; + + Ok(true) +} + /// Like [`fs_err::remove_file`], but attempts to change the permissions to force the file to be /// deleted (if it is readonly). fn remove_file(path: &Path) -> io::Result<()> { - /// If the file is readonly, change the permissions to make it _not_ readonly. - fn set_not_readonly(path: &Path) -> io::Result { - let mut perms = path.metadata()?.permissions(); - if !perms.readonly() { - return Ok(false); - } - - // We're about to delete the file, so it's fine to set the permissions to world-writable. - #[allow(clippy::permissions_set_readonly_false)] - perms.set_readonly(false); - - fs_err::set_permissions(path, perms)?; - - Ok(true) - } - match fs_err::remove_file(path) { Ok(()) => Ok(()), Err(err) @@ -110,3 +143,33 @@ fn remove_file(path: &Path) -> io::Result<()> { Err(err) => Err(err), } } + +/// Like [`fs_err::remove_dir`], but attempts to change the permissions to force the directory to +/// be deleted (if it is readonly). +fn remove_dir(path: &Path) -> io::Result<()> { + match fs_err::remove_dir(path) { + Ok(()) => Ok(()), + Err(err) + if err.kind() == io::ErrorKind::PermissionDenied + && set_readable(path).unwrap_or(false) => + { + fs_err::remove_dir(path) + } + Err(err) => Err(err), + } +} + +/// Like [`fs_err::remove_dir_all`], but attempts to change the permissions to force the directory +/// to be deleted (if it is readonly). +fn remove_dir_all(path: &Path) -> io::Result<()> { + match fs_err::remove_dir_all(path) { + Ok(()) => Ok(()), + Err(err) + if err.kind() == io::ErrorKind::PermissionDenied + && set_readable(path).unwrap_or(false) => + { + fs_err::remove_dir_all(path) + } + Err(err) => Err(err), + } +}