From 4689dc6620cc6eb3dbb076fdd6fbd587340b5268 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Mon, 11 Mar 2024 12:44:20 +0100 Subject: [PATCH] fix: fixes issue where pypi dependencies are not uninstalled --- src/install_pypi.rs | 1 + src/lock_file/update.rs | 52 +++++++++++++++++++--------------------- tests/common/builders.rs | 48 +++++++++++++++++++++++++++++++++++++ tests/common/mod.rs | 19 ++++++++++++++- tests/install_tests.rs | 35 +++++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 28 deletions(-) diff --git a/src/install_pypi.rs b/src/install_pypi.rs index 2000b8d47..b7fb95549 100644 --- a/src/install_pypi.rs +++ b/src/install_pypi.rs @@ -50,6 +50,7 @@ fn elapsed(duration: Duration) -> String { } /// Derived from uv [`uv_installer::Plan`] +#[derive(Debug)] struct PixiInstallPlan { /// The distributions that are not already installed in the current environment, but are /// available in the local cache. diff --git a/src/lock_file/update.rs b/src/lock_file/update.rs index df4e4dd65..ffbea3f53 100644 --- a/src/lock_file/update.rs +++ b/src/lock_file/update.rs @@ -107,35 +107,33 @@ impl<'p> LockFileDerivedData<'p> { .unwrap_or_default(); let pypi_records = self.pypi_records(environment, platform).unwrap_or_default(); - if environment.has_pypi_dependencies() { - let uv_context = match &self.uv_context { - None => { - let context = UvResolutionContext::from_project(self.project)?; - self.uv_context = Some(context.clone()); - context - } - Some(context) => context.clone(), - }; + let uv_context = match &self.uv_context { + None => { + let context = UvResolutionContext::from_project(self.project)?; + self.uv_context = Some(context.clone()); + context + } + Some(context) => context.clone(), + }; - let env_variables = environment.project().get_env_variables(environment).await?; - // Update the prefix with Pypi records - environment::update_prefix_pypi( - environment.name(), - &prefix, - platform, - &repodata_records, - &pypi_records, - &python_status, - &environment.system_requirements(), - uv_context, - env_variables, - ) - .await?; + let env_variables = environment.project().get_env_variables(environment).await?; + // Update the prefix with Pypi records + environment::update_prefix_pypi( + environment.name(), + &prefix, + platform, + &repodata_records, + &pypi_records, + &python_status, + &environment.system_requirements(), + uv_context, + env_variables, + ) + .await?; - // Store that we updated the environment, so we won't have to do it again. - self.updated_pypi_prefixes - .insert(environment.clone(), prefix.clone()); - } + // Store that we updated the environment, so we won't have to do it again. + self.updated_pypi_prefixes + .insert(environment.clone(), prefix.clone()); Ok(prefix) } diff --git a/tests/common/builders.rs b/tests/common/builders.rs index 6ae84c951..26ba0e383 100644 --- a/tests/common/builders.rs +++ b/tests/common/builders.rs @@ -24,6 +24,7 @@ //! ``` use futures::FutureExt; +use pixi::cli::remove; use pixi::task::TaskName; use pixi::{ cli::{add, init, install, project, task}, @@ -143,6 +144,53 @@ impl IntoFuture for AddBuilder { } } +/// Contains the arguments to pass to [`remove::execute()`]. Call `.await` to call the CLI execute method +/// and await the result at the same time. +pub struct RemoveBuilder { + pub args: remove::Args, +} + +impl RemoveBuilder { + pub fn with_spec(mut self, spec: &str) -> Self { + self.args.deps.push(spec.to_string()); + self + } + + /// Set as a host + pub fn set_type(mut self, t: DependencyType) -> Self { + match t { + DependencyType::CondaDependency(spec_type) => match spec_type { + SpecType::Host => { + self.args.host = true; + self.args.build = false; + } + SpecType::Build => { + self.args.host = false; + self.args.build = true; + } + SpecType::Run => { + self.args.host = false; + self.args.build = false; + } + }, + DependencyType::PypiDependency => { + self.args.host = false; + self.args.build = false; + self.args.pypi = true; + } + } + self + } +} + +impl IntoFuture for RemoveBuilder { + type Output = miette::Result<()>; + type IntoFuture = Pin + 'static>>; + + fn into_future(self) -> Self::IntoFuture { + remove::execute(self.args).boxed_local() + } +} pub struct TaskAddBuilder { pub manifest_path: Option, pub args: task::AddArgs, diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 4ca1db69b..7b711c97a 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -11,7 +11,7 @@ use pixi::{ cli::{ add, init, install::Args, - project, run, + project, remove, run, task::{self, AddArgs, AliasArgs}, }, consts, EnvironmentName, ExecutableTask, Project, RunOutput, SearchEnvironments, TaskGraph, @@ -35,6 +35,8 @@ use std::{ use tempfile::TempDir; use thiserror::Error; +use self::builders::RemoveBuilder; + /// To control the pixi process pub struct PixiControl { /// The path to the project working file @@ -229,6 +231,21 @@ impl PixiControl { } } + /// Remove dependencies from the project. Returns a [`RemoveBuilder`]. + pub fn remove(&self, spec: &str) -> RemoveBuilder { + RemoveBuilder { + args: remove::Args { + deps: vec![spec.to_string()], + manifest_path: Some(self.manifest_path()), + host: false, + build: false, + pypi: false, + platform: Default::default(), + feature: None, + }, + } + } + /// Add a new channel to the project. pub fn project_channel_add(&self) -> ProjectChannelAddBuilder { ProjectChannelAddBuilder { diff --git a/tests/install_tests.rs b/tests/install_tests.rs index 6cfc88eb7..3b5f0ff64 100644 --- a/tests/install_tests.rs +++ b/tests/install_tests.rs @@ -250,6 +250,41 @@ async fn pypi_reinstall_python() { } } +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +#[serial] +#[cfg_attr(not(feature = "slow_integration_tests"), ignore)] +// Check if we add and remove a pypi package that the site-packages is cleared +async fn pypi_add_remove() { + let pixi = PixiControl::new().unwrap(); + pixi.init().await.unwrap(); + // Add and update lockfile with this version of python + pixi.add("python==3.11").with_install(true).await.unwrap(); + + // Add flask from pypi + pixi.add("flask") + .with_install(true) + .set_type(pixi::DependencyType::PypiDependency) + .await + .unwrap(); + + let prefix = pixi.project().unwrap().root().join(".pixi/envs/default"); + + let cache = uv_cache::Cache::temp().unwrap(); + + // Check if site-packages has entries + let env = create_uv_environment(&prefix, &cache); + let installed_311 = uv_installer::SitePackages::from_executable(&env).unwrap(); + assert!(installed_311.iter().count() > 0); + + pixi.remove("flask") + .set_type(pixi::DependencyType::PypiDependency) + .await + .unwrap(); + + let installed_311 = uv_installer::SitePackages::from_executable(&env).unwrap(); + assert!(installed_311.iter().count() == 0); +} + #[tokio::test] async fn test_channels_changed() { // Write a channel with a package `bar` with only one version