From 538ebe6fcf7a39221e6dca45fefc450971e13bd5 Mon Sep 17 00:00:00 2001 From: Chisato <67509746+yumeminami@users.noreply.github.com> Date: Thu, 31 Jul 2025 19:59:23 +0800 Subject: [PATCH] Fix symlink preservation in virtual environment creation (#14933) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes inconsistent symlink handling in `uv venv` command (#14670). ## Problem https://github.com/astral-sh/uv/blob/00efde06b61756f0f305fcf67b12db71a29063d3/crates/uv-virtualenv/src/virtualenv.rs#L81 The original code used `Path::metadata()` which automatically follows symlinks, causing the system to treat symlinked virtual environment paths as regular directories. When a user runs uv venv on an existing symlinked virtual environment `(.venv -> foo)`, the code incorrectly treats the symlink as a regular directory because `location.metadata()` automatically follows the symlink and returns metadata for the target directory `foo/`. This causes the removal logic to delete the symlink itself and permanently breaking the symlink relationship and replacing it with a standard directory structure. ## Solution - Use canonicalize() to resolve symlinks only when removing and recreating virtual environments - This ensures operations target the actual directory while preserving the symlink structure - Minimal change that fixes the core issue without complex path management ## Test Plan ```bash ➜ test-env alias uv-dev='/Users/wingmunfung/workspace/uv/target/debug/uv' ➜ test-env ln -s dummy foo ➜ test-env ln -s foo .venv ➜ test-env ls -lah total 0 drwxr-xr-x 4 wingmunfung staff 128B Jul 30 10:39 . drwxr-xr-x 48 wingmunfung staff 1.5K Jul 29 17:08 .. lrwxr-xr-x 1 wingmunfung staff 3B Jul 30 10:39 .venv -> foo lrwxr-xr-x 1 wingmunfung staff 5B Jul 30 10:39 foo -> dummy ➜ test-env uv-dev venv Using CPython 3.13.2 Creating virtual environment at: .venv error: Failed to create virtual environment Caused by: failed to create directory `.venv`: File exists (os error 17) ➜ test-env mkdir dummy ➜ test-env uv-dev venv Using CPython 3.13.2 Creating virtual environment at: .venv Activate with: source .venv/bin/activate ➜ test-env ls -lah total 0 drwxr-xr-x 5 wingmunfung staff 160B Jul 30 10:39 . drwxr-xr-x 48 wingmunfung staff 1.5K Jul 29 17:08 .. lrwxr-xr-x 1 wingmunfung staff 3B Jul 30 10:39 .venv -> foo drwxr-xr-x 7 wingmunfung staff 224B Jul 30 10:39 dummy lrwxr-xr-x 1 wingmunfung staff 5B Jul 30 10:39 foo -> dummy ➜ test-env uv-dev venv Using CPython 3.13.2 Creating virtual environment at: .venv ✔ A virtual environment already exists at `.venv`. Do you want to replace it? · yes Activate with: source .venv/bin/activate ➜ test-env ls -lah total 0 drwxr-xr-x 5 wingmunfung staff 160B Jul 30 10:39 . drwxr-xr-x 48 wingmunfung staff 1.5K Jul 29 17:08 .. lrwxr-xr-x 1 wingmunfung staff 3B Jul 30 10:39 .venv -> foo drwxr-xr-x@ 7 wingmunfung staff 224B Jul 30 10:39 dummy lrwxr-xr-x 1 wingmunfung staff 5B Jul 30 10:39 foo -> dummy ### the symlink still exists ``` --------- Co-authored-by: Zanie Blue --- crates/uv-virtualenv/src/virtualenv.rs | 20 ++- crates/uv/tests/it/venv.rs | 184 +++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 4 deletions(-) diff --git a/crates/uv-virtualenv/src/virtualenv.rs b/crates/uv-virtualenv/src/virtualenv.rs index 7c65ec1bf..dceb1c5a6 100644 --- a/crates/uv-virtualenv/src/virtualenv.rs +++ b/crates/uv-virtualenv/src/virtualenv.rs @@ -109,15 +109,27 @@ pub(crate) fn create( } OnExisting::Remove => { debug!("Removing existing {name} due to `--clear`"); - remove_virtualenv(location)?; - fs::create_dir_all(location)?; + // Before removing the virtual environment, we need to canonicalize the path + // because `Path::metadata` will follow the symlink but we're still operating on + // the unresolved path and will remove the symlink itself. + let location = location + .canonicalize() + .unwrap_or_else(|_| location.to_path_buf()); + remove_virtualenv(&location)?; + fs::create_dir_all(&location)?; } OnExisting::Fail => { match confirm_clear(location, name)? { Some(true) => { debug!("Removing existing {name} due to confirmation"); - remove_virtualenv(location)?; - fs::create_dir_all(location)?; + // Before removing the virtual environment, we need to canonicalize the + // path because `Path::metadata` will follow the symlink but we're still + // operating on the unresolved path and will remove the symlink itself. + let location = location + .canonicalize() + .unwrap_or_else(|_| location.to_path_buf()); + remove_virtualenv(&location)?; + fs::create_dir_all(&location)?; } Some(false) => { let hint = format!( diff --git a/crates/uv/tests/it/venv.rs b/crates/uv/tests/it/venv.rs index 726d1731b..b97299625 100644 --- a/crates/uv/tests/it/venv.rs +++ b/crates/uv/tests/it/venv.rs @@ -6,6 +6,9 @@ use predicates::prelude::*; use uv_python::{PYTHON_VERSION_FILENAME, PYTHON_VERSIONS_FILENAME}; use uv_static::EnvVars; +#[cfg(unix)] +use fs_err::os::unix::fs::symlink; + use crate::common::{TestContext, uv_snapshot}; #[test] @@ -1388,3 +1391,184 @@ fn venv_python_preference() { Activate with: source .venv/[BIN]/activate "); } + +#[test] +#[cfg(unix)] +fn create_venv_symlink_clear_preservation() -> Result<()> { + let context = TestContext::new_with_versions(&["3.12"]); + + // Create a target directory + let target_dir = context.temp_dir.child("target"); + target_dir.create_dir_all()?; + + // Create a symlink pointing to the target directory + let symlink_path = context.temp_dir.child(".venv"); + symlink(&target_dir, &symlink_path)?; + + // Verify symlink exists + assert!(symlink_path.path().is_symlink()); + + // Create virtual environment at symlink location + uv_snapshot!(context.filters(), context.venv() + .arg(symlink_path.as_os_str()) + .arg("--python") + .arg("3.12"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Creating virtual environment at: .venv + Activate with: source .venv/[BIN]/activate + "### + ); + + // Verify symlink is still preserved after creation + assert!(symlink_path.path().is_symlink()); + + // Run uv venv with --clear to test symlink preservation during clear + uv_snapshot!(context.filters(), context.venv() + .arg(symlink_path.as_os_str()) + .arg("--clear") + .arg("--python") + .arg("3.12"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Creating virtual environment at: .venv + Activate with: source .venv/[BIN]/activate + "### + ); + + // Verify symlink is STILL preserved after --clear + assert!(symlink_path.path().is_symlink()); + + Ok(()) +} + +#[test] +#[cfg(unix)] +fn create_venv_symlink_recreate_preservation() -> Result<()> { + let context = TestContext::new_with_versions(&["3.12"]); + + // Create a target directory + let target_dir = context.temp_dir.child("target"); + target_dir.create_dir_all()?; + + // Create a symlink pointing to the target directory + let symlink_path = context.temp_dir.child(".venv"); + symlink(&target_dir, &symlink_path)?; + + // Verify symlink exists + assert!(symlink_path.path().is_symlink()); + + // Create virtual environment at symlink location + uv_snapshot!(context.filters(), context.venv() + .arg(symlink_path.as_os_str()) + .arg("--python") + .arg("3.12"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Creating virtual environment at: .venv + Activate with: source .venv/[BIN]/activate + "### + ); + + // Verify symlink is preserved after first creation + assert!(symlink_path.path().is_symlink()); + + // Run uv venv again WITHOUT --clear to test recreation behavior + uv_snapshot!(context.filters(), context.venv() + .arg(symlink_path.as_os_str()) + .arg("--python") + .arg("3.12"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Creating virtual environment at: .venv + warning: A virtual environment already exists at `.venv`. In the future, uv will require `--clear` to replace it + Activate with: source .venv/[BIN]/activate + "### + ); + + // Verify symlink is STILL preserved after recreation + assert!(symlink_path.path().is_symlink()); + + Ok(()) +} + +#[test] +#[cfg(unix)] +fn create_venv_nested_symlink_preservation() -> Result<()> { + let context = TestContext::new_with_versions(&["3.12"]); + + // Create a target directory + let target_dir = context.temp_dir.child("target"); + target_dir.create_dir_all()?; + + // Create first symlink level: intermediate -> target + let intermediate_link = context.temp_dir.child("intermediate"); + symlink(&target_dir, &intermediate_link)?; + + // Create second symlink level: .venv -> intermediate (nested symlink) + let symlink_path = context.temp_dir.child(".venv"); + symlink(&intermediate_link, &symlink_path)?; + + // Verify nested symlink exists + assert!(symlink_path.path().is_symlink()); + assert!(intermediate_link.path().is_symlink()); + + // Create virtual environment at nested symlink location + uv_snapshot!(context.filters(), context.venv() + .arg(symlink_path.as_os_str()) + .arg("--python") + .arg("3.12"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Creating virtual environment at: .venv + Activate with: source .venv/[BIN]/activate + "### + ); + + // Verify both symlinks are preserved + assert!(symlink_path.path().is_symlink()); + assert!(intermediate_link.path().is_symlink()); + + // Run uv venv again to test nested symlink preservation during recreation + uv_snapshot!(context.filters(), context.venv() + .arg(symlink_path.as_os_str()) + .arg("--python") + .arg("3.12"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] + Creating virtual environment at: .venv + warning: A virtual environment already exists at `.venv`. In the future, uv will require `--clear` to replace it + Activate with: source .venv/[BIN]/activate + "### + ); + + // Verify nested symlinks are STILL preserved + assert!(symlink_path.path().is_symlink()); + assert!(intermediate_link.path().is_symlink()); + + Ok(()) +}