From ab9cc78b7a2d58cc1a1ef1db09602dc0fa4dea64 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 12 Apr 2024 17:08:56 -0400 Subject: [PATCH] Deduplicate symbolic links between `purelib` and `platlib` (#3002) ## Summary This PR adds system install tests to verify the behavior described in #2798. It turns out this behavior _also_ affects Fedora and Amazon Linux, we just didn't have the right conditions enabled (specifically, you need to create the virtualenv with `python -m venv` to get these symlinks), so the test suite was expanded to capture that. The issue itself is also fixed by way of deduplicating the `site-packages` entries. Closes: https://github.com/astral-sh/uv/issues/2798 --- .github/workflows/ci.yml | 25 +++++++++ .../uv-interpreter/src/python_environment.rs | 12 +++-- scripts/check_system_python.py | 53 ++++++++++++++++++- 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5e8732312..5dd050844 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -371,6 +371,31 @@ jobs: - name: "Validate global Python install" run: python scripts/check_system_python.py --uv ./uv + system-test-opensuse: + needs: build-binary-linux + name: "check system | python on opensuse" + runs-on: ubuntu-latest + container: opensuse/tumbleweed + steps: + - uses: actions/checkout@v4 + + - name: "Install Python" + run: zypper install -y python310 which && python3.10 -m ensurepip && mv /usr/bin/python3.10 /usr/bin/python3 + + - name: "Download binary" + uses: actions/download-artifact@v4 + with: + name: uv-linux-${{ github.sha }} + + - name: "Prepare binary" + run: chmod +x ./uv + + - name: "Print Python path" + run: echo $(which python3) + + - name: "Validate global Python install" + run: python3 scripts/check_system_python.py --uv ./uv + # Note: rockylinux:8 is a 1-1 code compatible distro to rhel-8 # rockylinux:8 mimics centos-8 but with added maintenance stability # and avoids issues with centos stream uptime concerns diff --git a/crates/uv-interpreter/src/python_environment.rs b/crates/uv-interpreter/src/python_environment.rs index c60a97d93..d79004ffe 100644 --- a/crates/uv-interpreter/src/python_environment.rs +++ b/crates/uv-interpreter/src/python_environment.rs @@ -1,6 +1,7 @@ use std::env; use std::path::{Path, PathBuf}; +use same_file::is_same_file; use tracing::{debug, info}; use uv_cache::Cache; @@ -92,12 +93,17 @@ impl PythonEnvironment { /// /// In most cases, `purelib` and `platlib` will be the same, and so the iterator will contain /// a single element; however, in some distributions, they may be different. + /// + /// Some distributions also create symbolic links from `purelib` to `platlib`; in such cases, we + /// still deduplicate the entries, returning a single path. pub fn site_packages(&self) -> impl Iterator { - std::iter::once(self.interpreter.purelib()).chain( - if self.interpreter.purelib() == self.interpreter.platlib() { + let purelib = self.interpreter.purelib(); + let platlib = self.interpreter.platlib(); + std::iter::once(purelib).chain( + if purelib == platlib || is_same_file(purelib, platlib).unwrap_or(false) { None } else { - Some(self.interpreter.platlib()) + Some(platlib) }, ) } diff --git a/scripts/check_system_python.py b/scripts/check_system_python.py index 52a5c750b..fa5603dbc 100755 --- a/scripts/check_system_python.py +++ b/scripts/check_system_python.py @@ -8,6 +8,7 @@ To run locally, create a venv with seed packages. import argparse import logging import os +import shutil import subprocess import sys import tempfile @@ -107,7 +108,7 @@ if __name__ == "__main__": raise Exception("The package `pylint` is installed (but shouldn't be).") # Create a virtual environment with `uv`. - logging.info("Creating virtual environment...") + logging.info("Creating virtual environment with `uv`...") subprocess.run( [uv, "venv", ".venv", "--seed", "--python", sys.executable], cwd=temp_dir, @@ -126,7 +127,7 @@ if __name__ == "__main__": check=True, ) - logging.info("Installing into virtual environment...") + logging.info("Installing into `uv` virtual environment...") # Disable the `CONDA_PREFIX` and `VIRTUAL_ENV` environment variables, so that # we only rely on virtual environment discovery via the `.venv` directory. @@ -163,6 +164,26 @@ if __name__ == "__main__": "The package `pylint` isn't installed in the virtual environment." ) + # Uninstall the package (`pylint`). + logging.info("Uninstalling the package `pylint`.") + subprocess.run( + [uv, "pip", "uninstall", "pylint", "--verbose"], + cwd=temp_dir, + check=True, + env=env, + ) + + # Ensure that the package (`pylint`) isn't installed in the virtual environment. + logging.info("Checking that `pylint` isn't installed.") + code = subprocess.run( + [executable, "-m", "pip", "show", "pylint"], + cwd=temp_dir, + ) + if code.returncode == 0: + raise Exception( + "The package `pylint` is installed in the virtual environment (but shouldn't be)." + ) + # Attempt to install NumPy. # This ensures that we can successfully install a package with native libraries. # @@ -178,3 +199,31 @@ if __name__ == "__main__": # for Python 3.13 (at time of writing). if sys.version_info < (3, 13) and sys.implementation.name == "cpython": install_package(uv=uv, package="pydantic_core") + + # Next, create a virtual environment with `venv`, to ensure that `uv` can + # interoperate with `venv` virtual environments. + shutil.rmtree(os.path.join(temp_dir, ".venv")) + logging.info("Creating virtual environment with `venv`...") + subprocess.run( + [sys.executable, "-m", "venv", ".venv"], + cwd=temp_dir, + check=True, + ) + + # Install the package (`pylint`) into the virtual environment. + logging.info("Installing into `venv` virtual environment...") + subprocess.run( + [uv, "pip", "install", "pylint", "--verbose"], + cwd=temp_dir, + check=True, + env=env, + ) + + # Uninstall the package (`pylint`). + logging.info("Uninstalling the package `pylint`.") + subprocess.run( + [uv, "pip", "uninstall", "pylint", "--verbose"], + cwd=temp_dir, + check=True, + env=env, + )