Skip to content

Commit 908a7c1

Browse files
authored
Preserve file permissions when using reflinks on Linux (#18187)
Fixes an regression from #18117 where executable permissions were not preserved on reflink. On Linux, the `FICLONE` ioctl only clones data blocks without preserving file metadata, so permissions must be copied separately. On macOS, `clonefile` already preserves permissions. This appears to be a well known issue: - pnpm/pnpm#8546 - https://github.com/morelj/reflink/blob/53408edf3bf3c5090b1146923f72066c7f6e9200/cloneflags.go#L6-L22 - python/cpython#81338 Closes #18181
1 parent 55cfaf9 commit 908a7c1

6 files changed

Lines changed: 122 additions & 8 deletions

File tree

crates/uv-fs/src/link.rs

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,47 @@ where
415415
}
416416
}
417417

418+
/// Reflink a file from `from` to `to`, preserving file permissions.
419+
///
420+
/// On macOS, `clonefile` preserves all file metadata including permissions. On Linux,
421+
/// `ioctl_ficlone` only clones data blocks, so we implement our own reflink that copies
422+
/// permissions via `fchmod` on the open file descriptor, avoiding TOCTOU races.
423+
///
424+
/// See: <https://github.com/astral-sh/uv/issues/18181>
425+
#[cfg(target_os = "linux")]
426+
fn reflink_with_permissions(from: &Path, to: &Path) -> io::Result<()> {
427+
use fs_err::os::unix::fs::OpenOptionsExt;
428+
use std::os::unix::fs::PermissionsExt;
429+
430+
// Open source and read permissions from the file descriptor.
431+
let src = fs_err::File::open(from)?;
432+
let mode = src.metadata()?.permissions().mode();
433+
434+
// Create destination exclusively with the source's permissions.
435+
let dest = fs_err::OpenOptions::new()
436+
.write(true)
437+
.create_new(true)
438+
.mode(mode)
439+
.open(to)?;
440+
441+
// Clone data blocks from source to destination.
442+
if let Err(err) = rustix::fs::ioctl_ficlone(&dest, &src) {
443+
// Clean up the destination file on failure.
444+
let _ = fs_err::remove_file(to);
445+
return Err(err.into());
446+
}
447+
448+
Ok(())
449+
}
450+
451+
/// Reflink a file from `from` to `to`, preserving file permissions.
452+
///
453+
/// On macOS, `clonefile` preserves all file metadata including permissions natively.
454+
#[cfg(not(target_os = "linux"))]
455+
fn reflink_with_permissions(from: &Path, to: &Path) -> io::Result<()> {
456+
reflink_copy::reflink(from, to)
457+
}
458+
418459
/// Attempt to reflink a single file, falling back via [`link_file`] on failure.
419460
fn reflink_file_with_fallback<F>(
420461
path: &Path,
@@ -426,15 +467,15 @@ where
426467
F: Fn(&Path) -> bool,
427468
{
428469
match state.attempt {
429-
LinkAttempt::Initial => match reflink_copy::reflink(path, target) {
470+
LinkAttempt::Initial => match reflink_with_permissions(path, target) {
430471
Ok(()) => Ok(state.mode_working()),
431472
Err(err) if err.kind() == io::ErrorKind::AlreadyExists => {
432473
if options.on_existing_directory == OnExistingDirectory::Merge {
433474
// File exists, overwrite atomically via temp file
434475
let parent = target.parent().unwrap();
435476
let tempdir = tempfile::tempdir_in(parent)?;
436477
let tempfile = tempdir.path().join(target.file_name().unwrap());
437-
if reflink_copy::reflink(path, &tempfile).is_ok() {
478+
if reflink_with_permissions(path, &tempfile).is_ok() {
438479
fs_err::rename(&tempfile, target)?;
439480
Ok(state.mode_working())
440481
} else {
@@ -462,17 +503,19 @@ where
462503
link_file(path, target, state.next_mode(), options)
463504
}
464505
},
465-
LinkAttempt::Subsequent => match reflink_copy::reflink(path, target) {
506+
LinkAttempt::Subsequent => match reflink_with_permissions(path, target) {
466507
Ok(()) => Ok(state),
467508
Err(err) if err.kind() == io::ErrorKind::AlreadyExists => {
468509
if options.on_existing_directory == OnExistingDirectory::Merge {
469510
let parent = target.parent().unwrap();
470511
let tempdir = tempfile::tempdir_in(parent)?;
471512
let tempfile = tempdir.path().join(target.file_name().unwrap());
472-
reflink_copy::reflink(path, &tempfile).map_err(|err| LinkError::Reflink {
473-
from: path.to_path_buf(),
474-
to: tempfile.clone(),
475-
err,
513+
reflink_with_permissions(path, &tempfile).map_err(|err| {
514+
LinkError::Reflink {
515+
from: path.to_path_buf(),
516+
to: tempfile.clone(),
517+
err,
518+
}
476519
})?;
477520
fs_err::rename(&tempfile, target)?;
478521
Ok(state)

crates/uv-test/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -781,7 +781,10 @@ impl TestContext {
781781
let tmp = tempfile::TempDir::new_in(dir)?;
782782
self.temp_dir = ChildPath::new(tmp.path()).child("temp");
783783
fs_err::create_dir_all(&self.temp_dir)?;
784-
self.venv = ChildPath::new(tmp.path().canonicalize()?.join(".venv"));
784+
// Place the venv inside temp_dir (matching the default TestContext layout)
785+
// so that `context.venv()` creates it at the same path that `VIRTUAL_ENV` points to.
786+
let canonical_temp_dir = self.temp_dir.canonicalize()?;
787+
self.venv = ChildPath::new(canonical_temp_dir.join(".venv"));
785788
let temp_replacement = format!("[{name}]/[TEMP_DIR]/");
786789
self.filters.extend(
787790
Self::path_patterns(&self.temp_dir)

crates/uv/tests/it/pip_install.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3347,6 +3347,62 @@ fn install_executable_hardlink() {
33473347
Command::new(executable).arg("--version").assert().success();
33483348
}
33493349

3350+
/// Install a package into a virtual environment using clone semantics, and ensure that the
3351+
/// executable permissions are retained.
3352+
///
3353+
/// Requires `UV_INTERNAL__TEST_COW_FS`.
3354+
///
3355+
/// See: <https://github.com/astral-sh/uv/issues/18181>
3356+
#[test]
3357+
fn install_executable_clone() -> anyhow::Result<()> {
3358+
let Some(context) = uv_test::test_context!("3.12").with_cache_on_cow_fs()? else {
3359+
return Ok(());
3360+
};
3361+
let Some(context) = context.with_working_dir_on_cow_fs()? else {
3362+
return Ok(());
3363+
};
3364+
context.venv().assert().success();
3365+
3366+
uv_snapshot!(context.filters(), context
3367+
.pip_install()
3368+
.arg(context.workspace_root.join("test/packages/executable_file"))
3369+
.arg("--link-mode")
3370+
.arg("clone"), @r"
3371+
success: true
3372+
exit_code: 0
3373+
----- stdout -----
3374+
3375+
----- stderr -----
3376+
Resolved 1 package in [TIME]
3377+
Prepared 1 package in [TIME]
3378+
Installed 1 package in [TIME]
3379+
+ executable-file==1.0.0 (from file://[WORKSPACE]/test/packages/executable_file)
3380+
"
3381+
);
3382+
3383+
// Verify that the executable file inside the package retained its execute
3384+
// permission after being cloned. On Linux, `ioctl_ficlone` only clones data
3385+
// blocks without preserving metadata, so permissions must be copied separately.
3386+
#[cfg(unix)]
3387+
{
3388+
use std::os::unix::fs::PermissionsExt;
3389+
let script = context
3390+
.site_packages()
3391+
.join("executable_file")
3392+
.join("bin")
3393+
.join("run.sh");
3394+
let mode = fs_err::metadata(&script)?.permissions().mode();
3395+
assert!(
3396+
mode & 0o111 != 0,
3397+
"Expected executable permissions on {}, got {:o}",
3398+
script.display(),
3399+
mode
3400+
);
3401+
}
3402+
3403+
Ok(())
3404+
}
3405+
33503406
/// Install a package from the command line into a virtual environment, ignoring its dependencies.
33513407
#[test]
33523408
fn no_deps() {
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[project]
2+
name = "executable-file"
3+
version = "1.0.0"
4+
description = "A test package containing a file with executable permissions"
5+
requires-python = ">=3.12"
6+
7+
[build-system]
8+
requires = ["uv_build>=0.8.0,<0.11.0"]
9+
build-backend = "uv_build"
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# A test package with an executable file.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
#!/bin/sh
2+
echo "hello from executable_file"

0 commit comments

Comments
 (0)