Fix cross-drive script installation (#11167)

Fallback to copy if renaming a script doesn't work, similar to the site
packages installation.

Fixes #11163

**Test Plan**

Failing:
```
docker volume rm -f gh11163_usr_local_bin
docker run -v gh11163_usr_local_bin:/usr/local/bin -e UV_LINK_MODE=copy -v $(pwd):/io -it --rm ghcr.io/astral-sh/uv:0.5-python3.11-bookworm-slim uv pip install --system ruff
```

Passing:
```
cargo build --target x86_64-unknown-linux-musl
docker volume rm -f gh11163_usr_local_bin
docker run -v gh11163_usr_local_bin:/usr/local/bin -e UV_LINK_MODE=copy -v $(pwd):/io -it --rm ghcr.io/astral-sh/uv:0.5-python3.11-bookworm-slim /io/target/x86_64-unknown-linux-musl/debug/uv pip install --system ruff
```
This commit is contained in:
konsti 2025-02-12 19:00:41 +01:00 committed by GitHub
parent 62b7e16f3c
commit 070120e1c2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 82 additions and 17 deletions

View File

@ -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<Path>,
to: impl AsRef<Path>,
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: <https://github.com/astral-sh/uv/issues/1491> & <https://github.com/astral-sh/uv/issues/9531>
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()
}
}

View File

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

View File

@ -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<Path>, to: impl AsRef<Path>) -> 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;