From aa40b3f07fa4e83706f456e7fb147240baa8fbdd Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 15 Sep 2024 13:19:51 -0400 Subject: [PATCH] Avoid removing seed packages for uv venv --seed environments --- crates/uv-build/src/lib.rs | 1 + crates/uv-installer/src/plan.rs | 6 +- crates/uv-python/src/virtualenv.rs | 17 ++++- crates/uv-tool/src/lib.rs | 1 + crates/uv-virtualenv/src/lib.rs | 3 + crates/uv-virtualenv/src/virtualenv.rs | 18 +++-- crates/uv/src/commands/project/environment.rs | 1 + crates/uv/src/commands/project/mod.rs | 1 + crates/uv/src/commands/project/run.rs | 4 + crates/uv/src/commands/venv.rs | 1 + crates/uv/tests/pip_sync.rs | 73 +++++++++++++++++++ crates/uv/tests/venv.rs | 3 +- 12 files changed, 115 insertions(+), 14 deletions(-) diff --git a/crates/uv-build/src/lib.rs b/crates/uv-build/src/lib.rs index 801d2fd3f5f9..38c1134f0f1d 100644 --- a/crates/uv-build/src/lib.rs +++ b/crates/uv-build/src/lib.rs @@ -290,6 +290,7 @@ impl SourceBuild { false, false, false, + false, )? }; diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index 26bcb051d4c6..f6562d906462 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -412,9 +412,9 @@ impl<'a> Planner<'a> { // Remove any unnecessary packages. if site_packages.any() { - // If uv created the virtual environment, then remove all packages, regardless of - // whether they're considered "seed" packages. - let seed_packages = !venv.cfg().is_ok_and(|cfg| cfg.is_uv()); + // Retain seed packages unless: (1) the virtual environment was created by uv and + // (2) the `--seed` argument was not passed to `uv venv`. + let seed_packages = !venv.cfg().is_ok_and(|cfg| cfg.is_uv() && !cfg.is_seed()); for dist_info in site_packages { if seed_packages && matches!( diff --git a/crates/uv-python/src/virtualenv.rs b/crates/uv-python/src/virtualenv.rs index d18265f8ae0b..474273955bf8 100644 --- a/crates/uv-python/src/virtualenv.rs +++ b/crates/uv-python/src/virtualenv.rs @@ -23,13 +23,16 @@ pub struct VirtualEnvironment { /// A parsed `pyvenv.cfg` #[derive(Debug, Clone)] +#[allow(clippy::struct_excessive_bools)] pub struct PyVenvConfiguration { - /// If the virtualenv package was used to create the virtual environment. + /// Was the virtual environment created with the `virtualenv` package? pub(crate) virtualenv: bool, - /// If the uv package was used to create the virtual environment. + /// Was the virtual environment created with the `uv` package? pub(crate) uv: bool, /// Is the virtual environment relocatable? pub(crate) relocatable: bool, + /// Was the virtual environment populated with seed packages? + pub(crate) seed: bool, } #[derive(Debug, Error)] @@ -139,6 +142,7 @@ impl PyVenvConfiguration { let mut virtualenv = false; let mut uv = false; let mut relocatable = false; + let mut seed = false; // Per https://snarky.ca/how-virtual-environments-work/, the `pyvenv.cfg` file is not a // valid INI file, and is instead expected to be parsed by partitioning each line on the @@ -159,6 +163,9 @@ impl PyVenvConfiguration { "relocatable" => { relocatable = value.trim().to_lowercase() == "true"; } + "seed" => { + seed = value.trim().to_lowercase() == "true"; + } _ => {} } } @@ -167,6 +174,7 @@ impl PyVenvConfiguration { virtualenv, uv, relocatable, + seed, }) } @@ -184,4 +192,9 @@ impl PyVenvConfiguration { pub fn is_relocatable(&self) -> bool { self.relocatable } + + /// Returns true if the virtual environment was populated with seed packages. + pub fn is_seed(&self) -> bool { + self.seed + } } diff --git a/crates/uv-tool/src/lib.rs b/crates/uv-tool/src/lib.rs index 153eeac4bf26..e7235ec3df37 100644 --- a/crates/uv-tool/src/lib.rs +++ b/crates/uv-tool/src/lib.rs @@ -258,6 +258,7 @@ impl InstalledTools { false, false, false, + false, )?; Ok(venv) diff --git a/crates/uv-virtualenv/src/lib.rs b/crates/uv-virtualenv/src/lib.rs index 30e8ed4de9eb..5116f152c876 100644 --- a/crates/uv-virtualenv/src/lib.rs +++ b/crates/uv-virtualenv/src/lib.rs @@ -46,6 +46,7 @@ impl Prompt { } /// Create a virtualenv. +#[allow(clippy::fn_params_excessive_bools)] pub fn create_venv( location: &Path, interpreter: Interpreter, @@ -53,6 +54,7 @@ pub fn create_venv( system_site_packages: bool, allow_existing: bool, relocatable: bool, + seed: bool, ) -> Result { // Create the virtualenv at the given location. let virtualenv = virtualenv::create( @@ -62,6 +64,7 @@ pub fn create_venv( system_site_packages, allow_existing, relocatable, + seed, )?; // Create the corresponding `PythonEnvironment`. diff --git a/crates/uv-virtualenv/src/virtualenv.rs b/crates/uv-virtualenv/src/virtualenv.rs index 7637d42d5ad0..e51cccfa23e7 100644 --- a/crates/uv-virtualenv/src/virtualenv.rs +++ b/crates/uv-virtualenv/src/virtualenv.rs @@ -43,6 +43,7 @@ fn write_cfg(f: &mut impl Write, data: &[(String, String)]) -> io::Result<()> { } /// Create a [`VirtualEnvironment`] at the given location. +#[allow(clippy::fn_params_excessive_bools)] pub(crate) fn create( location: &Path, interpreter: &Interpreter, @@ -50,6 +51,7 @@ pub(crate) fn create( system_site_packages: bool, allow_existing: bool, relocatable: bool, + seed: bool, ) -> Result { // Determine the base Python executable; that is, the Python executable that should be // considered the "base" for the virtual environment. This is typically the Python executable @@ -350,16 +352,16 @@ pub(crate) fn create( "false".to_string() }, ), - ( - "relocatable".to_string(), - if relocatable { - "true".to_string() - } else { - "false".to_string() - }, - ), ]; + if relocatable { + pyvenv_cfg_data.push(("relocatable".to_string(), "true".to_string())); + } + + if seed { + pyvenv_cfg_data.push(("seed".to_string(), "true".to_string())); + } + if let Some(prompt) = prompt { pyvenv_cfg_data.push(("prompt".to_string(), prompt)); } diff --git a/crates/uv/src/commands/project/environment.rs b/crates/uv/src/commands/project/environment.rs index 68dd9a2a44a7..70a65a2b65b2 100644 --- a/crates/uv/src/commands/project/environment.rs +++ b/crates/uv/src/commands/project/environment.rs @@ -104,6 +104,7 @@ impl CachedEnvironment { false, false, true, + false, )?; sync_environment( diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index 81648b744cb7..a9df0254449d 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -531,6 +531,7 @@ pub(crate) async fn get_or_init_environment( false, false, false, + false, )?) } } diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 049d6c322260..110c2a618fac 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -247,6 +247,7 @@ pub(crate) async fn run( false, false, false, + false, )?; Some(environment.into_interpreter()) @@ -432,6 +433,7 @@ pub(crate) async fn run( false, false, false, + false, )? } else { // If we're not isolating the environment, reuse the base environment for the @@ -554,6 +556,7 @@ pub(crate) async fn run( false, false, false, + false, )?; venv.into_interpreter() } else { @@ -602,6 +605,7 @@ pub(crate) async fn run( false, false, false, + false, )? } Some(spec) => { diff --git a/crates/uv/src/commands/venv.rs b/crates/uv/src/commands/venv.rs index 2bfe36af54f6..45c64102ee67 100644 --- a/crates/uv/src/commands/venv.rs +++ b/crates/uv/src/commands/venv.rs @@ -255,6 +255,7 @@ async fn venv_impl( system_site_packages, allow_existing, relocatable, + seed, ) .map_err(VenvError::Creation)?; diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index 57ba15277fb4..96ad4344d1f3 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -5536,3 +5536,76 @@ fn compatible_build_constraint() -> Result<()> { Ok(()) } + +#[test] +fn sync_seed() -> Result<()> { + let context = TestContext::new("3.8"); + + let requirements_txt = context.temp_dir.child("requirements.txt"); + requirements_txt.write_str("requests==1.2")?; + + // Add `pip` to the environment. + uv_snapshot!(context.filters(), context.pip_install() + .arg("pip"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + pip==24.0 + "### + ); + + // Syncing should remove the seed packages. + uv_snapshot!(context.filters(), context.pip_sync() + .arg("requirements.txt"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Uninstalled 1 package in [TIME] + Installed 1 package in [TIME] + - pip==24.0 + + requests==1.2.0 + "### + ); + + // Re-create the environment with seed packages. + uv_snapshot!(context.filters(), context.venv() + .arg("--seed"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using Python 3.8.[X] interpreter at: [PYTHON-3.8] + Creating virtualenv with seed packages at: .venv + + pip==24.0 + + setuptools==69.2.0 + + wheel==0.43.0 + Activate with: source .venv/bin/activate + "### + ); + + // Syncing should retain the seed packages. + uv_snapshot!(context.filters(), context.pip_sync() + .arg("requirements.txt"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Installed 1 package in [TIME] + + requests==1.2.0 + "### + ); + + Ok(()) +} diff --git a/crates/uv/tests/venv.rs b/crates/uv/tests/venv.rs index 83698045eb31..2e3c1bfcd5c9 100644 --- a/crates/uv/tests/venv.rs +++ b/crates/uv/tests/venv.rs @@ -4,6 +4,7 @@ use anyhow::Result; use assert_cmd::prelude::*; use assert_fs::prelude::*; use indoc::indoc; +use predicates::prelude::*; use uv_python::{PYTHON_VERSIONS_FILENAME, PYTHON_VERSION_FILENAME}; use crate::common::{uv_snapshot, TestContext}; @@ -913,7 +914,7 @@ fn verify_pyvenv_cfg() { pyvenv_cfg.assert(predicates::str::contains(search_string)); // Not relocatable by default. - pyvenv_cfg.assert(predicates::str::contains("relocatable = false")); + pyvenv_cfg.assert(predicates::str::contains("relocatable").not()); } #[test]