Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid removing seed packages for uv venv --seed environments #7410

Merged
merged 1 commit into from
Sep 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/uv-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ impl SourceBuild {
false,
false,
false,
false,
)?
};

Expand Down
6 changes: 3 additions & 3 deletions crates/uv-installer/src/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
17 changes: 15 additions & 2 deletions crates/uv-python/src/virtualenv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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
Expand All @@ -159,6 +163,9 @@ impl PyVenvConfiguration {
"relocatable" => {
relocatable = value.trim().to_lowercase() == "true";
}
"seed" => {
seed = value.trim().to_lowercase() == "true";
}
_ => {}
}
}
Expand All @@ -167,6 +174,7 @@ impl PyVenvConfiguration {
virtualenv,
uv,
relocatable,
seed,
})
}

Expand All @@ -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
}
}
1 change: 1 addition & 0 deletions crates/uv-tool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ impl InstalledTools {
false,
false,
false,
false,
)?;

Ok(venv)
Expand Down
3 changes: 3 additions & 0 deletions crates/uv-virtualenv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ impl Prompt {
}

/// Create a virtualenv.
#[allow(clippy::fn_params_excessive_bools)]
pub fn create_venv(
location: &Path,
interpreter: Interpreter,
prompt: Prompt,
system_site_packages: bool,
allow_existing: bool,
relocatable: bool,
seed: bool,
) -> Result<PythonEnvironment, Error> {
// Create the virtualenv at the given location.
let virtualenv = virtualenv::create(
Expand All @@ -62,6 +64,7 @@ pub fn create_venv(
system_site_packages,
allow_existing,
relocatable,
seed,
)?;

// Create the corresponding `PythonEnvironment`.
Expand Down
18 changes: 10 additions & 8 deletions crates/uv-virtualenv/src/virtualenv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,15 @@ 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,
prompt: Prompt,
system_site_packages: bool,
allow_existing: bool,
relocatable: bool,
seed: bool,
) -> Result<VirtualEnvironment, Error> {
// 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
Expand Down Expand Up @@ -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));
}
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/project/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ impl CachedEnvironment {
false,
false,
true,
false,
)?;

sync_environment(
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ pub(crate) async fn get_or_init_environment(
false,
false,
false,
false,
)?)
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/uv/src/commands/project/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ pub(crate) async fn run(
false,
false,
false,
false,
)?;

Some(environment.into_interpreter())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -554,6 +556,7 @@ pub(crate) async fn run(
false,
false,
false,
false,
)?;
venv.into_interpreter()
} else {
Expand Down Expand Up @@ -602,6 +605,7 @@ pub(crate) async fn run(
false,
false,
false,
false,
)?
}
Some(spec) => {
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/venv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ async fn venv_impl(
system_site_packages,
allow_existing,
relocatable,
seed,
)
.map_err(VenvError::Creation)?;

Expand Down
73 changes: 73 additions & 0 deletions crates/uv/tests/pip_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
3 changes: 2 additions & 1 deletion crates/uv/tests/venv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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]
Expand Down
Loading