diff --git a/crates/uv-fs/src/link.rs b/crates/uv-fs/src/link.rs index 217e60459cc7f..8b73c3b1a8b01 100644 --- a/crates/uv-fs/src/link.rs +++ b/crates/uv-fs/src/link.rs @@ -415,6 +415,47 @@ where } } +/// Reflink a file from `from` to `to`, preserving file permissions. +/// +/// On macOS, `clonefile` preserves all file metadata including permissions. On Linux, +/// `ioctl_ficlone` only clones data blocks, so we implement our own reflink that copies +/// permissions via `fchmod` on the open file descriptor, avoiding TOCTOU races. +/// +/// See: +#[cfg(target_os = "linux")] +fn reflink_with_permissions(from: &Path, to: &Path) -> io::Result<()> { + use fs_err::os::unix::fs::OpenOptionsExt; + use std::os::unix::fs::PermissionsExt; + + // Open source and read permissions from the file descriptor. + let src = fs_err::File::open(from)?; + let mode = src.metadata()?.permissions().mode(); + + // Create destination exclusively with the source's permissions. + let dest = fs_err::OpenOptions::new() + .write(true) + .create_new(true) + .mode(mode) + .open(to)?; + + // Clone data blocks from source to destination. + if let Err(err) = rustix::fs::ioctl_ficlone(&dest, &src) { + // Clean up the destination file on failure. + let _ = fs_err::remove_file(to); + return Err(err.into()); + } + + Ok(()) +} + +/// Reflink a file from `from` to `to`, preserving file permissions. +/// +/// On macOS, `clonefile` preserves all file metadata including permissions natively. +#[cfg(not(target_os = "linux"))] +fn reflink_with_permissions(from: &Path, to: &Path) -> io::Result<()> { + reflink_copy::reflink(from, to) +} + /// Attempt to reflink a single file, falling back via [`link_file`] on failure. fn reflink_file_with_fallback( path: &Path, @@ -426,7 +467,7 @@ where F: Fn(&Path) -> bool, { match state.attempt { - LinkAttempt::Initial => match reflink_copy::reflink(path, target) { + LinkAttempt::Initial => match reflink_with_permissions(path, target) { Ok(()) => Ok(state.mode_working()), Err(err) if err.kind() == io::ErrorKind::AlreadyExists => { if options.on_existing_directory == OnExistingDirectory::Merge { @@ -434,7 +475,7 @@ where let parent = target.parent().unwrap(); let tempdir = tempfile::tempdir_in(parent)?; let tempfile = tempdir.path().join(target.file_name().unwrap()); - if reflink_copy::reflink(path, &tempfile).is_ok() { + if reflink_with_permissions(path, &tempfile).is_ok() { fs_err::rename(&tempfile, target)?; Ok(state.mode_working()) } else { @@ -462,17 +503,19 @@ where link_file(path, target, state.next_mode(), options) } }, - LinkAttempt::Subsequent => match reflink_copy::reflink(path, target) { + LinkAttempt::Subsequent => match reflink_with_permissions(path, target) { Ok(()) => Ok(state), Err(err) if err.kind() == io::ErrorKind::AlreadyExists => { if options.on_existing_directory == OnExistingDirectory::Merge { let parent = target.parent().unwrap(); let tempdir = tempfile::tempdir_in(parent)?; let tempfile = tempdir.path().join(target.file_name().unwrap()); - reflink_copy::reflink(path, &tempfile).map_err(|err| LinkError::Reflink { - from: path.to_path_buf(), - to: tempfile.clone(), - err, + reflink_with_permissions(path, &tempfile).map_err(|err| { + LinkError::Reflink { + from: path.to_path_buf(), + to: tempfile.clone(), + err, + } })?; fs_err::rename(&tempfile, target)?; Ok(state) diff --git a/crates/uv-test/src/lib.rs b/crates/uv-test/src/lib.rs index 123ea3c109e51..3bee921b9a48a 100755 --- a/crates/uv-test/src/lib.rs +++ b/crates/uv-test/src/lib.rs @@ -781,7 +781,10 @@ impl TestContext { let tmp = tempfile::TempDir::new_in(dir)?; self.temp_dir = ChildPath::new(tmp.path()).child("temp"); fs_err::create_dir_all(&self.temp_dir)?; - self.venv = ChildPath::new(tmp.path().canonicalize()?.join(".venv")); + // Place the venv inside temp_dir (matching the default TestContext layout) + // so that `context.venv()` creates it at the same path that `VIRTUAL_ENV` points to. + let canonical_temp_dir = self.temp_dir.canonicalize()?; + self.venv = ChildPath::new(canonical_temp_dir.join(".venv")); let temp_replacement = format!("[{name}]/[TEMP_DIR]/"); self.filters.extend( Self::path_patterns(&self.temp_dir) diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 6560d20a0a1c2..cce9961c9bc6b 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -3347,6 +3347,62 @@ fn install_executable_hardlink() { Command::new(executable).arg("--version").assert().success(); } +/// Install a package into a virtual environment using clone semantics, and ensure that the +/// executable permissions are retained. +/// +/// Requires `UV_INTERNAL__TEST_COW_FS`. +/// +/// See: +#[test] +fn install_executable_clone() -> anyhow::Result<()> { + let Some(context) = uv_test::test_context!("3.12").with_cache_on_cow_fs()? else { + return Ok(()); + }; + let Some(context) = context.with_working_dir_on_cow_fs()? else { + return Ok(()); + }; + context.venv().assert().success(); + + uv_snapshot!(context.filters(), context + .pip_install() + .arg(context.workspace_root.join("test/packages/executable_file")) + .arg("--link-mode") + .arg("clone"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + executable-file==1.0.0 (from file://[WORKSPACE]/test/packages/executable_file) + " + ); + + // Verify that the executable file inside the package retained its execute + // permission after being cloned. On Linux, `ioctl_ficlone` only clones data + // blocks without preserving metadata, so permissions must be copied separately. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let script = context + .site_packages() + .join("executable_file") + .join("bin") + .join("run.sh"); + let mode = fs_err::metadata(&script)?.permissions().mode(); + assert!( + mode & 0o111 != 0, + "Expected executable permissions on {}, got {:o}", + script.display(), + mode + ); + } + + Ok(()) +} + /// Install a package from the command line into a virtual environment, ignoring its dependencies. #[test] fn no_deps() { diff --git a/test/packages/executable_file/pyproject.toml b/test/packages/executable_file/pyproject.toml new file mode 100644 index 0000000000000..8b5ac68e214f0 --- /dev/null +++ b/test/packages/executable_file/pyproject.toml @@ -0,0 +1,9 @@ +[project] +name = "executable-file" +version = "1.0.0" +description = "A test package containing a file with executable permissions" +requires-python = ">=3.12" + +[build-system] +requires = ["uv_build>=0.8.0,<0.11.0"] +build-backend = "uv_build" diff --git a/test/packages/executable_file/src/executable_file/__init__.py b/test/packages/executable_file/src/executable_file/__init__.py new file mode 100644 index 0000000000000..48bba3f087e81 --- /dev/null +++ b/test/packages/executable_file/src/executable_file/__init__.py @@ -0,0 +1 @@ +# A test package with an executable file. diff --git a/test/packages/executable_file/src/executable_file/bin/run.sh b/test/packages/executable_file/src/executable_file/bin/run.sh new file mode 100755 index 0000000000000..ecbdf49f58782 --- /dev/null +++ b/test/packages/executable_file/src/executable_file/bin/run.sh @@ -0,0 +1,2 @@ +#!/bin/sh +echo "hello from executable_file"