diff --git a/crates/uv-fs/src/lib.rs b/crates/uv-fs/src/lib.rs index aecac9eae..2b7f3f6fd 100644 --- a/crates/uv-fs/src/lib.rs +++ b/crates/uv-fs/src/lib.rs @@ -246,10 +246,14 @@ pub async fn rename_with_retry( } } -/// Rename a file, retrying (on Windows) if it fails due to transient operating system errors, in a synchronous context. -pub fn rename_with_retry_sync( +/// Rename or copy a file, retrying (on Windows) if it fails due to transient operating system +/// errors, in a synchronous context. +#[cfg_attr(not(windows), allow(unused_variables))] +pub fn with_retry_sync( from: impl AsRef, to: impl AsRef, + operation_name: &str, + operation: impl Fn() -> Result<(), std::io::Error>, ) -> Result<(), std::io::Error> { #[cfg(windows)] { @@ -261,15 +265,15 @@ pub fn rename_with_retry_sync( // See: & let from = from.as_ref(); let to = to.as_ref(); - let rename = || fs_err::rename(from, to); - rename + operation .retry(backoff_file_move()) .sleep(std::thread::sleep) .when(|err| err.kind() == std::io::ErrorKind::PermissionDenied) .notify(|err, _dur| { warn!( - "Retrying rename from {} to {} due to transient error: {}", + "Retrying {} from {} to {} due to transient error: {}", + operation_name, from.display(), to.display(), err @@ -280,7 +284,8 @@ pub fn rename_with_retry_sync( std::io::Error::new( std::io::ErrorKind::Other, format!( - "Failed to rename {} to {}: {}", + "Failed {} {} to {}: {}", + operation_name, from.display(), to.display(), err @@ -290,7 +295,7 @@ pub fn rename_with_retry_sync( } #[cfg(not(windows))] { - fs_err::rename(from, to) + operation() } } diff --git a/crates/uv-install-wheel/src/install.rs b/crates/uv-install-wheel/src/install.rs index 38e9f35fa..b1b8387e6 100644 --- a/crates/uv-install-wheel/src/install.rs +++ b/crates/uv-install-wheel/src/install.rs @@ -111,7 +111,6 @@ pub fn install_wheel( // 2.b Move each subtree of distribution-1.0.data/ onto its destination path. Each subdirectory of distribution-1.0.data/ is a key into a dict of destination directories, such as distribution-1.0.data/(purelib|platlib|headers|scripts|data). The initially supported paths are taken from distutils.command.install. let data_dir = site_packages.join(format!("{dist_info_prefix}.data")); if data_dir.is_dir() { - trace!(?name, "Installing data"); install_data( layout, relocatable, diff --git a/crates/uv-install-wheel/src/wheel.rs b/crates/uv-install-wheel/src/wheel.rs index 3e9a281b3..ebc8888b6 100644 --- a/crates/uv-install-wheel/src/wheel.rs +++ b/crates/uv-install-wheel/src/wheel.rs @@ -9,11 +9,11 @@ use fs_err::{DirEntry, File}; use mailparse::parse_headers; use rustc_hash::FxHashMap; use sha2::{Digest, Sha256}; -use tracing::{instrument, warn}; +use tracing::{debug, instrument, trace, warn}; use walkdir::WalkDir; use uv_cache_info::CacheInfo; -use uv_fs::{persist_with_retry_sync, relative_to, rename_with_retry_sync, Simplified}; +use uv_fs::{persist_with_retry_sync, relative_to, Simplified}; use uv_normalize::PackageName; use uv_pypi_types::DirectUrl; use uv_shell::escape_posix_for_single_quotes; @@ -312,6 +312,7 @@ pub(crate) fn move_folder_recorded( site_packages: &Path, record: &mut [RecordEntry], ) -> Result<(), Error> { + let mut rename_or_copy = RenameOrCopy::default(); fs::create_dir_all(dest_dir)?; for entry in WalkDir::new(src_dir) { let entry = entry?; @@ -330,7 +331,7 @@ pub(crate) fn move_folder_recorded( if entry.file_type().is_dir() { fs::create_dir_all(&target)?; } else { - fs::rename(src, &target)?; + rename_or_copy.rename_or_copy(src, &target)?; let entry = record .iter_mut() .find(|entry| Path::new(&entry.path) == relative_to_site_packages) @@ -349,13 +350,14 @@ pub(crate) fn move_folder_recorded( /// Installs a single script (not an entrypoint). /// -/// Has to deal with both binaries files (just move) and scripts (rewrite the shebang if applicable). +/// Binary files are moved with a copy fallback, while we rewrite scripts' shebangs if applicable. fn install_script( layout: &Layout, relocatable: bool, site_packages: &Path, record: &mut [RecordEntry], file: &DirEntry, + #[allow(unused)] rename_or_copy: &mut RenameOrCopy, ) -> Result<(), Error> { let file_type = file.file_type()?; @@ -460,12 +462,13 @@ fn install_script( use std::os::unix::fs::PermissionsExt; let permissions = fs::metadata(&path)?.permissions(); - if permissions.mode() & 0o111 == 0o111 { // If the permissions are already executable, we don't need to change them. - rename_with_retry_sync(&path, &script_absolute)?; + // We fall back to copy when the file is on another drive. + rename_or_copy.rename_or_copy(&path, &script_absolute)?; } else { - // If we have to modify the permissions, copy the file, since we might not own it. + // If we have to modify the permissions, copy the file, since we might not own it, + // and we may not be allowed to change permissions on an unowned moved file. warn!( "Copying script from {} to {} (permissions: {:o})", path.simplified_display(), @@ -484,7 +487,21 @@ fn install_script( #[cfg(not(unix))] { - rename_with_retry_sync(&path, &script_absolute)?; + // Here, two wrappers over rename are clashing: We want to retry for security software + // blocking the file, but we also need the copy fallback is the problem was trying to + // move a file cross-drive. + match uv_fs::with_retry_sync(&path, &script_absolute, "renaming", || { + fs_err::rename(&path, &script_absolute) + }) { + Ok(()) => (), + Err(err) => { + debug!("Failed to rename, falling back to copy: {err}"); + uv_fs::with_retry_sync(&path, &script_absolute, "copying", || { + fs_err::copy(&path, &script_absolute)?; + Ok(()) + })?; + } + } } None @@ -533,10 +550,13 @@ pub(crate) fn install_data( match path.file_name().and_then(|name| name.to_str()) { Some("data") => { + trace!(?dist_name, "Installing data/data"); // Move the content of the folder to the root of the venv move_folder_recorded(&path, &layout.scheme.data, site_packages, record)?; } Some("scripts") => { + trace!(?dist_name, "Installing data/scripts"); + let mut rename_or_copy = RenameOrCopy::default(); let mut initialized = false; for file in fs::read_dir(path)? { let file = file?; @@ -563,17 +583,27 @@ pub(crate) fn install_data( initialized = true; } - install_script(layout, relocatable, site_packages, record, &file)?; + install_script( + layout, + relocatable, + site_packages, + record, + &file, + &mut rename_or_copy, + )?; } } Some("headers") => { + trace!(?dist_name, "Installing data/headers"); let target_path = layout.scheme.include.join(dist_name.as_str()); move_folder_recorded(&path, &target_path, site_packages, record)?; } Some("purelib") => { + trace!(?dist_name, "Installing data/purelib"); move_folder_recorded(&path, &layout.scheme.purelib, site_packages, record)?; } Some("platlib") => { + trace!(?dist_name, "Installing data/platlib"); move_folder_recorded(&path, &layout.scheme.platlib, site_packages, record)?; } _ => { @@ -801,6 +831,37 @@ pub(crate) fn parse_scripts( scripts_from_ini(extras, python_minor, ini) } +/// Rename a file with a fallback to copy that switches over on the first failure. +#[derive(Default, Copy, Clone)] +enum RenameOrCopy { + #[default] + Rename, + Copy, +} + +impl RenameOrCopy { + /// Try to rename, and on failure, copy. + /// + /// Usually, source and target are on the same device, so we can rename, but if that fails, we + /// have to copy. If renaming failed once, we switch to copy permanently. + fn rename_or_copy(&mut self, from: impl AsRef, to: impl AsRef) -> io::Result<()> { + match self { + Self::Rename => match fs_err::rename(from.as_ref(), to.as_ref()) { + Ok(()) => {} + Err(err) => { + *self = RenameOrCopy::Copy; + debug!("Failed to rename, falling back to copy: {err}"); + fs_err::copy(from.as_ref(), to.as_ref())?; + } + }, + Self::Copy => { + fs_err::copy(from.as_ref(), to.as_ref())?; + } + } + Ok(()) + } +} + #[cfg(test)] mod test { use std::io::Cursor;