Skip to content

Commit afb681e

Browse files
committed
Fix cross-drive script installation
Fallback to copy if renaming a script doesn't work, similar to the site packages installation. **Test Plan** ``` 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 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 ```
1 parent 3077552 commit afb681e

File tree

2 files changed

+75
-22
lines changed

2 files changed

+75
-22
lines changed

crates/uv-install-wheel/src/install.rs

-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ pub fn install_wheel(
111111
// 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.
112112
let data_dir = site_packages.join(format!("{dist_info_prefix}.data"));
113113
if data_dir.is_dir() {
114-
trace!(?name, "Installing data");
115114
install_data(
116115
layout,
117116
relocatable,

crates/uv-install-wheel/src/wheel.rs

+75-21
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ use fs_err::{DirEntry, File};
99
use mailparse::parse_headers;
1010
use rustc_hash::FxHashMap;
1111
use sha2::{Digest, Sha256};
12-
use tracing::{instrument, warn};
12+
use tracing::{debug, instrument, trace, warn};
1313
use walkdir::WalkDir;
1414

1515
use uv_cache_info::CacheInfo;
16-
use uv_fs::{persist_with_retry_sync, relative_to, rename_with_retry_sync, Simplified};
16+
use uv_fs::{persist_with_retry_sync, relative_to, Simplified};
1717
use uv_normalize::PackageName;
1818
use uv_pypi_types::DirectUrl;
1919
use uv_shell::escape_posix_for_single_quotes;
@@ -312,6 +312,7 @@ pub(crate) fn move_folder_recorded(
312312
site_packages: &Path,
313313
record: &mut [RecordEntry],
314314
) -> Result<(), Error> {
315+
let mut rename_or_copy = RenameOrCopy::default();
315316
fs::create_dir_all(dest_dir)?;
316317
for entry in WalkDir::new(src_dir) {
317318
let entry = entry?;
@@ -330,7 +331,7 @@ pub(crate) fn move_folder_recorded(
330331
if entry.file_type().is_dir() {
331332
fs::create_dir_all(&target)?;
332333
} else {
333-
fs::rename(src, &target)?;
334+
rename_or_copy.rename_or_copy(src, &target)?;
334335
let entry = record
335336
.iter_mut()
336337
.find(|entry| Path::new(&entry.path) == relative_to_site_packages)
@@ -349,13 +350,14 @@ pub(crate) fn move_folder_recorded(
349350

350351
/// Installs a single script (not an entrypoint).
351352
///
352-
/// Has to deal with both binaries files (just move) and scripts (rewrite the shebang if applicable).
353+
/// Binary files are moved with a copy fallback, while we rewrite scripts' shebangs if applicable.
353354
fn install_script(
354355
layout: &Layout,
355356
relocatable: bool,
356357
site_packages: &Path,
357358
record: &mut [RecordEntry],
358359
file: &DirEntry,
360+
#[allow(unused)] rename_or_copy: &mut RenameOrCopy,
359361
) -> Result<(), Error> {
360362
let file_type = file.file_type()?;
361363

@@ -459,32 +461,37 @@ fn install_script(
459461
use std::fs::Permissions;
460462
use std::os::unix::fs::PermissionsExt;
461463

462-
let permissions = fs::metadata(&path)?.permissions();
464+
// We fall back to copy when the file is on another drive and if we don't own the file.
465+
rename_or_copy.rename_or_copy(&path, &script_absolute)?;
463466

464-
if permissions.mode() & 0o111 == 0o111 {
465-
// If the permissions are already executable, we don't need to change them.
466-
rename_with_retry_sync(&path, &script_absolute)?;
467-
} else {
468-
// If we have to modify the permissions, copy the file, since we might not own it.
469-
warn!(
470-
"Copying script from {} to {} (permissions: {:o})",
471-
path.simplified_display(),
472-
script_absolute.simplified_display(),
473-
permissions.mode()
474-
);
475-
476-
uv_fs::copy_atomic_sync(&path, &script_absolute)?;
467+
let permissions = fs::metadata(&script_absolute)?.permissions();
477468

469+
if permissions.mode() & 0o111 != 0o111 {
470+
trace!(
471+
"Adjusting permissions for {} from {} to {}",
472+
script_absolute.display(),
473+
permissions.mode(),
474+
permissions.mode() | 0o111
475+
);
478476
fs::set_permissions(
479-
script_absolute,
477+
&script_absolute,
480478
Permissions::from_mode(permissions.mode() | 0o111),
481479
)?;
482480
}
483481
}
484482

485483
#[cfg(not(unix))]
486484
{
487-
rename_with_retry_sync(&path, &script_absolute)?;
485+
// Here, two wrappers over rename are clashing: We want to retry for security software
486+
// blocking the file, but we also need the copy fallback is the problem was trying to
487+
// move a file cross-drive.
488+
match uv_fs::rename_with_retry_sync(&path, &script_absolute) {
489+
Ok(()) => (),
490+
Err(err) => {
491+
debug!("Failed to rename, falling back to copy: {err}");
492+
fs_err::copy(&path, &script_absolute)?;
493+
}
494+
}
488495
}
489496

490497
None
@@ -533,10 +540,13 @@ pub(crate) fn install_data(
533540

534541
match path.file_name().and_then(|name| name.to_str()) {
535542
Some("data") => {
543+
trace!(?dist_name, "Installing data/data");
536544
// Move the content of the folder to the root of the venv
537545
move_folder_recorded(&path, &layout.scheme.data, site_packages, record)?;
538546
}
539547
Some("scripts") => {
548+
trace!(?dist_name, "Installing data/scripts");
549+
let mut rename_or_copy = RenameOrCopy::default();
540550
let mut initialized = false;
541551
for file in fs::read_dir(path)? {
542552
let file = file?;
@@ -563,17 +573,27 @@ pub(crate) fn install_data(
563573
initialized = true;
564574
}
565575

566-
install_script(layout, relocatable, site_packages, record, &file)?;
576+
install_script(
577+
layout,
578+
relocatable,
579+
site_packages,
580+
record,
581+
&file,
582+
&mut rename_or_copy,
583+
)?;
567584
}
568585
}
569586
Some("headers") => {
587+
trace!(?dist_name, "Installing data/headers");
570588
let target_path = layout.scheme.include.join(dist_name.as_str());
571589
move_folder_recorded(&path, &target_path, site_packages, record)?;
572590
}
573591
Some("purelib") => {
592+
trace!(?dist_name, "Installing data/purelib");
574593
move_folder_recorded(&path, &layout.scheme.purelib, site_packages, record)?;
575594
}
576595
Some("platlib") => {
596+
trace!(?dist_name, "Installing data/platlib");
577597
move_folder_recorded(&path, &layout.scheme.platlib, site_packages, record)?;
578598
}
579599
_ => {
@@ -801,6 +821,40 @@ pub(crate) fn parse_scripts(
801821
scripts_from_ini(extras, python_minor, ini)
802822
}
803823

824+
/// Rename a file with a fallback to copy that switches over on the first failure.
825+
#[derive(Default)]
826+
enum RenameOrCopy {
827+
#[default]
828+
Rename,
829+
Copy,
830+
}
831+
832+
impl RenameOrCopy {
833+
/// Try to rename, and on failure, copy.
834+
///
835+
/// Usually, source and target are on the same device, so we can rename, but if that fails, we
836+
/// have to copy. If renaming failed once, we switch to copy permanently.
837+
fn rename_or_copy(&mut self, from: impl AsRef<Path>, to: impl AsRef<Path>) -> io::Result<()> {
838+
match self {
839+
Self::Rename => {
840+
match fs_err::rename(from.as_ref(), to.as_ref()) {
841+
Ok(()) => return Ok(()),
842+
Err(err) => {
843+
*self = RenameOrCopy::Copy;
844+
debug!("Failed to rename, falling back to copy: {err}");
845+
}
846+
}
847+
fs_err::copy(from.as_ref(), to.as_ref())?;
848+
Ok(())
849+
}
850+
Self::Copy => {
851+
fs_err::copy(from.as_ref(), to.as_ref())?;
852+
Ok(())
853+
}
854+
}
855+
}
856+
}
857+
804858
#[cfg(test)]
805859
mod test {
806860
use std::io::Cursor;

0 commit comments

Comments
 (0)