From cf77029dc4bfee18f0d545b6bc534413234d13eb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 21 Jul 2025 08:59:07 -0400 Subject: [PATCH] Revert "Use an ephemeral environment for `uv run --with` invocations (#14447)" This reverts commit 6df7dab2df6e5a9b3bf36183851dd9d7c0824c9f. --- crates/uv-python/src/environment.rs | 2 +- crates/uv/src/commands/project/environment.rs | 170 ++++++++++-------- crates/uv/src/commands/project/mod.rs | 3 + crates/uv/src/commands/project/run.rs | 116 ++++-------- crates/uv/src/commands/tool/run.rs | 4 + crates/uv/tests/it/lock.rs | 2 +- crates/uv/tests/it/run.rs | 13 +- 7 files changed, 149 insertions(+), 161 deletions(-) diff --git a/crates/uv-python/src/environment.rs b/crates/uv-python/src/environment.rs index 10cec16ad..170e6ceb0 100644 --- a/crates/uv-python/src/environment.rs +++ b/crates/uv-python/src/environment.rs @@ -173,7 +173,7 @@ impl PythonEnvironment { /// N.B. This function also works for system Python environments and users depend on this. pub fn from_root(root: impl AsRef, cache: &Cache) -> Result { debug!( - "Checking for Python environment at: `{}`", + "Checking for Python environment at `{}`", root.as_ref().user_display() ); match root.as_ref().try_exists() { diff --git a/crates/uv/src/commands/project/environment.rs b/crates/uv/src/commands/project/environment.rs index 4f9d936c5..5a3b7ac04 100644 --- a/crates/uv/src/commands/project/environment.rs +++ b/crates/uv/src/commands/project/environment.rs @@ -5,7 +5,7 @@ use tracing::debug; use crate::commands::pip::loggers::{InstallLogger, ResolveLogger}; use crate::commands::pip::operations::Modifications; use crate::commands::project::{ - EnvironmentSpecification, PlatformState, ProjectError, resolve_environment, sync_environment, + resolve_environment, sync_environment, EnvironmentSpecification, PlatformState, ProjectError, }; use crate::printer::Printer; use crate::settings::{NetworkSettings, ResolverInstallerSettings}; @@ -15,70 +15,7 @@ use uv_cache_key::{cache_digest, hash_digest}; use uv_configuration::{Concurrency, Constraints, PreviewMode}; use uv_distribution_types::{Name, Resolution}; use uv_fs::PythonExt; -use uv_python::{Interpreter, PythonEnvironment, canonicalize_executable}; - -/// An ephemeral [`PythonEnvironment`] for running an individual command. -#[derive(Debug)] -pub(crate) struct EphemeralEnvironment(PythonEnvironment); - -impl From for EphemeralEnvironment { - fn from(environment: PythonEnvironment) -> Self { - Self(environment) - } -} - -impl From for PythonEnvironment { - fn from(environment: EphemeralEnvironment) -> Self { - environment.0 - } -} - -impl EphemeralEnvironment { - /// Set the ephemeral overlay for a Python environment. - #[allow(clippy::result_large_err)] - pub(crate) fn set_overlay(&self, contents: impl AsRef<[u8]>) -> Result<(), ProjectError> { - let site_packages = self - .0 - .site_packages() - .next() - .ok_or(ProjectError::NoSitePackages)?; - let overlay_path = site_packages.join("_uv_ephemeral_overlay.pth"); - fs_err::write(overlay_path, contents)?; - Ok(()) - } - - /// Enable system site packages for a Python environment. - #[allow(clippy::result_large_err)] - pub(crate) fn set_system_site_packages(&self) -> Result<(), ProjectError> { - self.0 - .set_pyvenv_cfg("include-system-site-packages", "true")?; - Ok(()) - } - - /// Set the `extends-environment` key in the `pyvenv.cfg` file to the given path. - /// - /// Ephemeral environments created by `uv run --with` extend a parent (virtual or system) - /// environment by adding a `.pth` file to the ephemeral environment's `site-packages` - /// directory. The `pth` file contains Python code to dynamically add the parent - /// environment's `site-packages` directory to Python's import search paths in addition to - /// the ephemeral environment's `site-packages` directory. This works well at runtime, but - /// is too dynamic for static analysis tools like ty to understand. As such, we - /// additionally write the `sys.prefix` of the parent environment to to the - /// `extends-environment` key of the ephemeral environment's `pyvenv.cfg` file, making it - /// easier for these tools to statically and reliably understand the relationship between - /// the two environments. - #[allow(clippy::result_large_err)] - pub(crate) fn set_parent_environment( - &self, - parent_environment_sys_prefix: &Path, - ) -> Result<(), ProjectError> { - self.0.set_pyvenv_cfg( - "extends-environment", - &parent_environment_sys_prefix.escape_for_python(), - )?; - Ok(()) - } -} +use uv_python::{canonicalize_executable, Interpreter, PythonEnvironment}; /// A [`PythonEnvironment`] stored in the cache. #[derive(Debug)] @@ -107,13 +44,15 @@ impl CachedEnvironment { printer: Printer, preview: PreviewMode, ) -> Result { - let interpreter = Self::base_interpreter(interpreter, cache)?; + // Resolve the "base" interpreter, which resolves to an underlying parent interpreter if the + // given interpreter is a virtual environment. + let base_interpreter = Self::base_interpreter(interpreter, cache)?; // Resolve the requirements with the interpreter. let resolution = Resolution::from( resolve_environment( spec, - &interpreter, + &base_interpreter, build_constraints.clone(), &settings.resolver, network_settings, @@ -141,20 +80,29 @@ impl CachedEnvironment { // Use the canonicalized base interpreter path since that's the interpreter we performed the // resolution with and the interpreter the environment will be created with. // - // We cache environments independent of the environment they'd be layered on top of. The - // assumption is such that the environment will _not_ be modified by the user or uv; - // otherwise, we risk cache poisoning. For example, if we were to write a `.pth` file to - // the cached environment, it would be shared across all projects that use the same - // interpreter and the same cached dependencies. + // We also include the canonicalized `sys.prefix` of the non-base interpreter, that is, the + // virtual environment's path. Originally, we shared cached environments independent of the + // environment they'd be layered on top of. However, this causes collisions as the overlay + // `.pth` file can be overridden by another instance of uv. Including this element in the key + // avoids this problem at the cost of creating separate cached environments for identical + // `--with` invocations across projects. We use `sys.prefix` rather than `sys.executable` so + // we can canonicalize it without invalidating the purpose of the element — it'd probably be + // safe to just use the absolute `sys.executable` as well. + // + // TODO(zanieb): Since we're not sharing these environmments across projects, we should move + // [`CachedEnvironment::set_overlay`] etc. here since the values there should be constant + // now. // // TODO(zanieb): We should include the version of the base interpreter in the hash, so if // the interpreter at the canonicalized path changes versions we construct a new // environment. - let interpreter_hash = - cache_digest(&canonicalize_executable(interpreter.sys_executable())?); + let environment_hash = cache_digest(&( + &canonicalize_executable(base_interpreter.sys_executable())?, + &interpreter.sys_prefix().canonicalize()?, + )); // Search in the content-addressed cache. - let cache_entry = cache.entry(CacheBucket::Environments, interpreter_hash, resolution_hash); + let cache_entry = cache.entry(CacheBucket::Environments, environment_hash, resolution_hash); if cache.refresh().is_none() { if let Ok(root) = cache.resolve_link(cache_entry.path()) { @@ -168,7 +116,7 @@ impl CachedEnvironment { let temp_dir = cache.venv_dir()?; let venv = uv_virtualenv::create_venv( temp_dir.path(), - interpreter, + base_interpreter, uv_virtualenv::Prompt::None, false, uv_virtualenv::OnExisting::Remove, @@ -202,6 +150,76 @@ impl CachedEnvironment { Ok(Self(PythonEnvironment::from_root(root, cache)?)) } + /// Set the ephemeral overlay for a Python environment. + #[allow(clippy::result_large_err)] + pub(crate) fn set_overlay(&self, contents: impl AsRef<[u8]>) -> Result<(), ProjectError> { + let site_packages = self + .0 + .site_packages() + .next() + .ok_or(ProjectError::NoSitePackages)?; + let overlay_path = site_packages.join("_uv_ephemeral_overlay.pth"); + fs_err::write(overlay_path, contents)?; + Ok(()) + } + + /// Clear the ephemeral overlay for a Python environment, if it exists. + #[allow(clippy::result_large_err)] + pub(crate) fn clear_overlay(&self) -> Result<(), ProjectError> { + let site_packages = self + .0 + .site_packages() + .next() + .ok_or(ProjectError::NoSitePackages)?; + let overlay_path = site_packages.join("_uv_ephemeral_overlay.pth"); + match fs_err::remove_file(overlay_path) { + Ok(()) => (), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => (), + Err(err) => return Err(ProjectError::OverlayRemoval(err)), + } + Ok(()) + } + + /// Enable system site packages for a Python environment. + #[allow(clippy::result_large_err)] + pub(crate) fn set_system_site_packages(&self) -> Result<(), ProjectError> { + self.0 + .set_pyvenv_cfg("include-system-site-packages", "true")?; + Ok(()) + } + + /// Disable system site packages for a Python environment. + #[allow(clippy::result_large_err)] + pub(crate) fn clear_system_site_packages(&self) -> Result<(), ProjectError> { + self.0 + .set_pyvenv_cfg("include-system-site-packages", "false")?; + Ok(()) + } + + /// Set the `extends-environment` key in the `pyvenv.cfg` file to the given path. + /// + /// Ephemeral environments created by `uv run --with` extend a parent (virtual or system) + /// environment by adding a `.pth` file to the ephemeral environment's `site-packages` + /// directory. The `pth` file contains Python code to dynamically add the parent + /// environment's `site-packages` directory to Python's import search paths in addition to + /// the ephemeral environment's `site-packages` directory. This works well at runtime, but + /// is too dynamic for static analysis tools like ty to understand. As such, we + /// additionally write the `sys.prefix` of the parent environment to the + /// `extends-environment` key of the ephemeral environment's `pyvenv.cfg` file, making it + /// easier for these tools to statically and reliably understand the relationship between + /// the two environments. + #[allow(clippy::result_large_err)] + pub(crate) fn set_parent_environment( + &self, + parent_environment_sys_prefix: &Path, + ) -> Result<(), ProjectError> { + self.0.set_pyvenv_cfg( + "extends-environment", + &parent_environment_sys_prefix.escape_for_python(), + )?; + Ok(()) + } + /// Return the [`Interpreter`] to use for the cached environment, based on a given /// [`Interpreter`]. /// diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index becd2a26e..08ce33224 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -200,6 +200,9 @@ pub(crate) enum ProjectError { #[error("Failed to parse PEP 723 script metadata")] Pep723ScriptTomlParse(#[source] toml::de::Error), + #[error("Failed to remove ephemeral overlay")] + OverlayRemoval(#[source] std::io::Error), + #[error("Failed to find `site-packages` directory for environment")] NoSitePackages, diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index ba8935013..bd2745d2c 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -45,7 +45,7 @@ use crate::commands::pip::loggers::{ DefaultInstallLogger, DefaultResolveLogger, SummaryInstallLogger, SummaryResolveLogger, }; use crate::commands::pip::operations::Modifications; -use crate::commands::project::environment::{CachedEnvironment, EphemeralEnvironment}; +use crate::commands::project::environment::CachedEnvironment; use crate::commands::project::install_target::InstallTarget; use crate::commands::project::lock::LockMode; use crate::commands::project::lock_target::LockTarget; @@ -944,7 +944,7 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl // If necessary, create an environment for the ephemeral requirements or command. let base_site_packages = SitePackages::from_interpreter(&base_interpreter)?; - let requirements_env = match spec { + let ephemeral_env = match spec { None => None, Some(spec) if can_skip_ephemeral(&spec, &base_interpreter, &base_site_packages, &settings) => @@ -952,7 +952,7 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl None } Some(spec) => { - debug!("Syncing `--with` requirements to cached environment"); + debug!("Syncing ephemeral requirements"); // Read the build constraints from the lock file. let build_constraints = base_lock @@ -1013,92 +1013,54 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl Err(err) => return Err(err.into()), }; - Some(PythonEnvironment::from(environment)) + Some(environment) } }; - // If we're layering requirements atop the project environment, run the command in an ephemeral, - // isolated environment. Otherwise, modifications to the "active virtual environment" would - // poison the cache. - let ephemeral_dir = requirements_env - .as_ref() - .map(|_| cache.venv_dir()) - .transpose()?; - - let ephemeral_env = ephemeral_dir - .as_ref() - .map(|dir| { - debug!( - "Creating ephemeral environment at: `{}`", - dir.path().simplified_display() - ); - - uv_virtualenv::create_venv( - dir.path(), - base_interpreter.clone(), - uv_virtualenv::Prompt::None, - false, - uv_virtualenv::OnExisting::Remove, - false, - false, - false, - preview, - ) - }) - .transpose()? - .map(EphemeralEnvironment::from); - - // If we're running in an ephemeral environment, add a path file to enable loading from the - // `--with` requirements environment and the project environment site packages. + // If we're running in an ephemeral environment, add a path file to enable loading of + // the base environment's site packages. Setting `PYTHONPATH` is insufficient, as it doesn't + // resolve `.pth` files in the base environment. // - // Setting `PYTHONPATH` is insufficient, as it doesn't resolve `.pth` files in the base - // environment. Adding `sitecustomize.py` would be an alternative, but it can be shadowed by an - // existing such module in the python installation. + // `sitecustomize.py` would be an alternative, but it can be shadowed by an existing such + // module in the python installation. if let Some(ephemeral_env) = ephemeral_env.as_ref() { - if let Some(requirements_env) = requirements_env.as_ref() { - let requirements_site_packages = - requirements_env.site_packages().next().ok_or_else(|| { - anyhow!("Requirements environment has no site packages directory") - })?; - let base_site_packages = base_interpreter - .site_packages() - .next() - .ok_or_else(|| anyhow!("Base environment has no site packages directory"))?; + let site_packages = base_interpreter + .site_packages() + .next() + .ok_or_else(|| ProjectError::NoSitePackages)?; + ephemeral_env.set_overlay(format!( + "import site; site.addsitedir(\"{}\")", + site_packages.escape_for_python() + ))?; - ephemeral_env.set_overlay(format!( - "import site; site.addsitedir(\"{}\"); site.addsitedir(\"{}\");", - base_site_packages.escape_for_python(), - requirements_site_packages.escape_for_python(), - ))?; + // Write the `sys.prefix` of the parent environment to the `extends-environment` key of the `pyvenv.cfg` + // file. This helps out static-analysis tools such as ty (see docs on + // `CachedEnvironment::set_parent_environment`). + // + // Note that we do this even if the parent environment is not a virtual environment. + // For ephemeral environments created by `uv run --with`, the parent environment's + // `site-packages` directory is added to `sys.path` even if the parent environment is not + // a virtual environment and even if `--system-site-packages` was not explicitly selected. + ephemeral_env.set_parent_environment(base_interpreter.sys_prefix())?; - // Write the `sys.prefix` of the parent environment to the `extends-environment` key of the `pyvenv.cfg` - // file. This helps out static-analysis tools such as ty (see docs on - // `CachedEnvironment::set_parent_environment`). - // - // Note that we do this even if the parent environment is not a virtual environment. - // For ephemeral environments created by `uv run --with`, the parent environment's - // `site-packages` directory is added to `sys.path` even if the parent environment is not - // a virtual environment and even if `--system-site-packages` was not explicitly selected. - ephemeral_env.set_parent_environment(base_interpreter.sys_prefix())?; - - // If `--system-site-packages` is enabled, add the system site packages to the ephemeral - // environment. - if base_interpreter.is_virtualenv() - && PyVenvConfiguration::parse(base_interpreter.sys_prefix().join("pyvenv.cfg")) - .is_ok_and(|cfg| cfg.include_system_site_packages()) - { - ephemeral_env.set_system_site_packages()?; - } + // If `--system-site-packages` is enabled, add the system site packages to the ephemeral + // environment. + if base_interpreter.is_virtualenv() + && PyVenvConfiguration::parse(base_interpreter.sys_prefix().join("pyvenv.cfg")) + .is_ok_and(|cfg| cfg.include_system_site_packages()) + { + ephemeral_env.set_system_site_packages()?; + } else { + ephemeral_env.clear_system_site_packages()?; } } - // Cast to `PythonEnvironment`. + // Cast from `CachedEnvironment` to `PythonEnvironment`. let ephemeral_env = ephemeral_env.map(PythonEnvironment::from); // Determine the Python interpreter to use for the command, if necessary. let interpreter = ephemeral_env .as_ref() - .or(requirements_env.as_ref()) .map_or_else(|| &base_interpreter, |env| env.interpreter()); // Check if any run command is given. @@ -1181,12 +1143,6 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl .as_ref() .map(PythonEnvironment::scripts) .into_iter() - .chain( - requirements_env - .as_ref() - .map(PythonEnvironment::scripts) - .into_iter(), - ) .chain(std::iter::once(base_interpreter.scripts())) .chain( // On Windows, non-virtual Python distributions put `python.exe` in the top-level diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index 7c91b9fe9..1efb78c3b 100644 --- a/crates/uv/src/commands/tool/run.rs +++ b/crates/uv/src/commands/tool/run.rs @@ -1080,5 +1080,9 @@ async fn get_or_create_environment( }, }; + // Clear any existing overlay. + environment.clear_overlay()?; + environment.clear_system_site_packages()?; + Ok((from, environment.into())) } diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index ff9b711b7..f8ee65bbc 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -16332,7 +16332,7 @@ fn lock_explicit_default_index() -> Result<()> { DEBUG Adding root workspace member: `[TEMP_DIR]/` DEBUG No Python version file found in workspace: [TEMP_DIR]/ DEBUG Using Python request `>=3.12` from `requires-python` metadata - DEBUG Checking for Python environment at: `.venv` + DEBUG Checking for Python environment at `.venv` DEBUG The project environment's Python version satisfies the request: `Python >=3.12` DEBUG Using request timeout of [TIME] DEBUG Found static `pyproject.toml` for: project @ file://[TEMP_DIR]/ diff --git a/crates/uv/tests/it/run.rs b/crates/uv/tests/it/run.rs index ad8672788..8fc513648 100644 --- a/crates/uv/tests/it/run.rs +++ b/crates/uv/tests/it/run.rs @@ -1302,6 +1302,7 @@ fn run_with_pyvenv_cfg_file() -> Result<()> { uv = [UV_VERSION] version_info = 3.12.[X] include-system-site-packages = false + relocatable = true extends-environment = [PARENT_VENV] @@ -4777,6 +4778,7 @@ fn run_groups_include_requires_python() -> Result<()> { baz = ["iniconfig"] dev = ["sniffio", {include-group = "foo"}, {include-group = "baz"}] + [tool.uv.dependency-groups] foo = {requires-python="<3.13"} bar = {requires-python=">=3.13"} @@ -4921,8 +4923,8 @@ fn run_repeated() -> Result<()> { Resolved 1 package in [TIME] "###); - // Re-running as a tool doesn't require reinstalling `typing-extensions`, since the environment - // is cached. + // Re-running as a tool does require reinstalling `typing-extensions`, since the base venv is + // different. uv_snapshot!( context.filters(), context.tool_run().arg("--with").arg("typing-extensions").arg("python").arg("-c").arg("import typing_extensions; import iniconfig"), @r#" @@ -4932,6 +4934,8 @@ fn run_repeated() -> Result<()> { ----- stderr ----- Resolved 1 package in [TIME] + Installed 1 package in [TIME] + + typing-extensions==4.10.0 Traceback (most recent call last): File "", line 1, in import typing_extensions; import iniconfig @@ -4978,7 +4982,8 @@ fn run_without_overlay() -> Result<()> { + typing-extensions==4.10.0 "###); - // Import `iniconfig` in the context of a `tool run` command, which should fail. + // Import `iniconfig` in the context of a `tool run` command, which should fail. Note that + // typing-extensions gets installed again, because the venv is not shared. uv_snapshot!( context.filters(), context.tool_run().arg("--with").arg("typing-extensions").arg("python").arg("-c").arg("import typing_extensions; import iniconfig"), @r#" @@ -4988,6 +4993,8 @@ fn run_without_overlay() -> Result<()> { ----- stderr ----- Resolved 1 package in [TIME] + Installed 1 package in [TIME] + + typing-extensions==4.10.0 Traceback (most recent call last): File "", line 1, in import typing_extensions; import iniconfig