From ad4a15d0c20fb3da6afcf8125e2740e8cd4226d9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 23 Jul 2024 18:24:43 -0400 Subject: [PATCH] Allow symlinks to files in scripts directory (#5380) ## Summary Closes https://github.com/astral-sh/uv/issues/5359. ## Test Plan Unfortunately, the only packages I know of that use this are Ruff and uv, and both are too heavy to install in a recurring test, so: `uv tool install hatch==1.12.0 --with uv==0.2.27 --force --link-mode=symlink` --- crates/install-wheel-rs/src/wheel.rs | 24 ++++++++-- crates/uv/tests/pip_sync.rs | 69 ++++++++++++++++++++++++---- 2 files changed, 82 insertions(+), 11 deletions(-) diff --git a/crates/install-wheel-rs/src/wheel.rs b/crates/install-wheel-rs/src/wheel.rs index cfe1d7895..73151e6a5 100644 --- a/crates/install-wheel-rs/src/wheel.rs +++ b/crates/install-wheel-rs/src/wheel.rs @@ -441,13 +441,31 @@ fn install_script( record: &mut [RecordEntry], file: &DirEntry, ) -> Result<(), Error> { - if !file.file_type()?.is_file() { + let file_type = file.file_type()?; + + if file_type.is_dir() { return Err(Error::InvalidWheel(format!( - "Wheel contains entry in scripts directory that is not a file: {}", - file.path().display() + "Wheel contains an invalid entry (directory) in the `scripts` directory: {}", + file.path().simplified_display() ))); } + if file_type.is_symlink() { + let Ok(target) = file.path().canonicalize() else { + return Err(Error::InvalidWheel(format!( + "Wheel contains an invalid entry (broken symlink) in the `scripts` directory: {}", + file.path().simplified_display(), + ))); + }; + if target.is_dir() { + return Err(Error::InvalidWheel(format!( + "Wheel contains an invalid entry (directory symlink) in the `scripts` directory: {} ({})", + file.path().simplified_display(), + target.simplified_display() + ))); + } + } + let script_absolute = layout.scheme.scripts.join(file.file_name()); let script_relative = pathdiff::diff_paths(&script_absolute, site_packages).ok_or_else(|| { diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index a622a5079..08e9574a2 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -105,12 +105,16 @@ fn install() -> Result<()> { .join("__init__.cpython-312.pyc") .exists()); - context.assert_command("import markupsafe").success(); + context + .assert_command("from markupsafe import Markup") + .success(); // Removing the cache shouldn't invalidate the virtual environment. fs::remove_dir_all(context.cache_dir.path())?; - context.assert_command("import markupsafe").success(); + context + .assert_command("from markupsafe import Markup") + .success(); Ok(()) } @@ -140,12 +144,16 @@ fn install_copy() -> Result<()> { "### ); - context.assert_command("import markupsafe").success(); + context + .assert_command("from markupsafe import Markup") + .success(); // Removing the cache shouldn't invalidate the virtual environment. fs::remove_dir_all(context.cache_dir.path())?; - context.assert_command("import markupsafe").success(); + context + .assert_command("from markupsafe import Markup") + .success(); Ok(()) } @@ -175,12 +183,55 @@ fn install_hardlink() -> Result<()> { "### ); - context.assert_command("import markupsafe").success(); + context + .assert_command("from markupsafe import Markup") + .success(); // Removing the cache shouldn't invalidate the virtual environment. fs::remove_dir_all(context.cache_dir.path())?; - context.assert_command("import markupsafe").success(); + context + .assert_command("from markupsafe import Markup") + .success(); + + Ok(()) +} + +/// Install a package into a virtual environment using symlink semantics. +#[test] +fn install_symlink() -> Result<()> { + let context = TestContext::new("3.12"); + + let requirements_txt = context.temp_dir.child("requirements.txt"); + requirements_txt.write_str("MarkupSafe==2.1.3")?; + + uv_snapshot!(context.pip_sync() + .arg("requirements.txt") + .arg("--link-mode") + .arg("symlink") + .arg("--strict"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + markupsafe==2.1.3 + "### + ); + + context + .assert_command("from markupsafe import Markup") + .success(); + + // Removing the cache _should_ invalidate the virtual environment. + fs::remove_dir_all(context.cache_dir.path())?; + + context + .assert_command("from markupsafe import Markup") + .failure(); Ok(()) } @@ -210,7 +261,7 @@ fn install_many() -> Result<()> { ); context - .assert_command("import markupsafe; import tomli") + .assert_command("from markupsafe import Markup; import tomli") .success(); Ok(()) @@ -244,7 +295,9 @@ fn noop() -> Result<()> { "### ); - context.assert_command("import markupsafe").success(); + context + .assert_command("from markupsafe import Markup") + .success(); Ok(()) }