From 7c728a2e77cacdf87dc901fdb222d68e26d94376 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 21 Mar 2024 00:02:06 -0400 Subject: [PATCH] Support unnamed requirements directly in pip uninstall (#2577) ## Summary In `pip uninstall`, we shouldn't need to resolve unnamed requirements, since we already index packages in `site-packages` by their URL. This also changes `uninstall` to ignore editables, which matches pip's behavior. Part of: https://github.com/astral-sh/uv/issues/313. ## Test Plan Run `cargo run pip install ./scripts/editable-installs/black_editable`, followed by each of the following: - `cargo run pip uninstall ./scripts/editable-installs/black_editable` - `cargo run pip uninstall black` - `cargo run pip uninstall ./scripts/editable-installs/black_editable black` --- crates/uv/src/commands/pip_uninstall.rs | 58 ++++++++++++------------- crates/uv/src/main.rs | 5 --- crates/uv/tests/pip_uninstall.rs | 14 +++--- 3 files changed, 34 insertions(+), 43 deletions(-) diff --git a/crates/uv/src/commands/pip_uninstall.rs b/crates/uv/src/commands/pip_uninstall.rs index 071bfb8e1..0dfe5726b 100644 --- a/crates/uv/src/commands/pip_uninstall.rs +++ b/crates/uv/src/commands/pip_uninstall.rs @@ -1,10 +1,12 @@ use std::fmt::Write; use anyhow::Result; +use itertools::{Either, Itertools}; use owo_colors::OwoColorize; use tracing::debug; use distribution_types::{InstalledMetadata, Name}; +use pep508_rs::{Requirement, RequirementsTxtRequirement, UnnamedRequirement}; use uv_cache::Cache; use uv_client::Connectivity; use uv_fs::Simplified; @@ -12,7 +14,7 @@ use uv_interpreter::PythonEnvironment; use crate::commands::{elapsed, ExitStatus}; use crate::printer::Printer; -use crate::requirements::{NamedRequirements, RequirementsSource, RequirementsSpecification}; +use crate::requirements::{RequirementsSource, RequirementsSpecification}; /// Uninstall packages from the current environment. pub(crate) async fn pip_uninstall( @@ -63,28 +65,24 @@ pub(crate) async fn pip_uninstall( } } - // Convert from unnamed to named requirements. - let NamedRequirements { - project: _, - requirements, - constraints: _, - overrides: _, - editables, - index_url: _, - extra_index_urls: _, - no_index: _, - find_links: _, - } = NamedRequirements::from_spec(spec)?; - let _lock = venv.lock()?; // Index the current `site-packages` directory. let site_packages = uv_installer::SitePackages::from_executable(&venv)?; + // Partition the requirements into named and unnamed requirements. + let (named, unnamed): (Vec, Vec) = spec + .requirements + .into_iter() + .partition_map(|requirement| match requirement { + RequirementsTxtRequirement::Pep508(requirement) => Either::Left(requirement), + RequirementsTxtRequirement::Unnamed(requirement) => Either::Right(requirement), + }); + // Sort and deduplicate the packages, which are keyed by name. Like `pip`, we ignore the // dependency specifier (even if it's a URL). - let packages = { - let mut packages = requirements + let names = { + let mut packages = named .into_iter() .map(|requirement| requirement.name) .collect::>(); @@ -93,23 +91,23 @@ pub(crate) async fn pip_uninstall( packages }; - // Sort and deduplicate the editable packages, which are keyed by URL rather than package name. - let editables = { - let mut editables = editables - .iter() - .map(requirements_txt::EditableRequirement::raw) + // Sort and deduplicate the unnamed requirements, which are keyed by URL rather than package name. + let urls = { + let mut urls = unnamed + .into_iter() + .map(|requirement| requirement.url.to_url()) .collect::>(); - editables.sort_unstable(); - editables.dedup(); - editables + urls.sort_unstable(); + urls.dedup(); + urls }; // Map to the local distributions. let distributions = { - let mut distributions = Vec::with_capacity(packages.len() + editables.len()); + let mut distributions = Vec::with_capacity(names.len() + urls.len()); // Identify all packages that are installed. - for package in &packages { + for package in &names { let installed = site_packages.get_packages(package); if installed.is_empty() { writeln!( @@ -124,16 +122,16 @@ pub(crate) async fn pip_uninstall( } } - // Identify all editables that are installed. - for editable in &editables { - let installed = site_packages.get_editables(editable); + // Identify all unnamed distributions that are installed. + for url in &urls { + let installed = site_packages.get_urls(url); if installed.is_empty() { writeln!( printer.stderr(), "{}{} Skipping {} as it is not installed.", "warning".yellow().bold(), ":".bold(), - editable.as_ref().bold() + url.as_ref().bold() )?; } else { distributions.extend(installed); diff --git a/crates/uv/src/main.rs b/crates/uv/src/main.rs index c3edc4b89..513362162 100644 --- a/crates/uv/src/main.rs +++ b/crates/uv/src/main.rs @@ -960,10 +960,6 @@ struct PipUninstallArgs { #[clap(long, short, group = "sources")] requirement: Vec, - /// Uninstall the editable package based on the provided local file path. - #[clap(long, short, group = "sources")] - editable: Vec, - /// The Python interpreter from which packages should be uninstalled. /// /// By default, `uv` uninstalls from the virtual environment in the current working directory or @@ -1706,7 +1702,6 @@ async fn run() -> Result { .package .into_iter() .map(RequirementsSource::from_package) - .chain(args.editable.into_iter().map(RequirementsSource::Editable)) .chain( args.requirement .into_iter() diff --git a/crates/uv/tests/pip_uninstall.rs b/crates/uv/tests/pip_uninstall.rs index 9f6c00362..e374fac28 100644 --- a/crates/uv/tests/pip_uninstall.rs +++ b/crates/uv/tests/pip_uninstall.rs @@ -65,9 +65,9 @@ fn no_arguments() -> Result<()> { ----- stderr ----- error: the following required arguments were not provided: - |--editable > + > - Usage: uv pip uninstall |--editable > + Usage: uv pip uninstall > For more information, try '--help'. "### @@ -428,7 +428,7 @@ fn uninstall_editable_by_name() -> Result<()> { } #[test] -fn uninstall_editable_by_path() -> Result<()> { +fn uninstall_by_path() -> Result<()> { let context = TestContext::new("3.12"); let current_dir = std::env::current_dir()?; @@ -445,7 +445,7 @@ fn uninstall_editable_by_path() -> Result<()> { let requirements_txt = context.temp_dir.child("requirements.txt"); requirements_txt.touch()?; - requirements_txt.write_str("-e ../../scripts/editable-installs/poetry_editable")?; + requirements_txt.write_str("../../scripts/editable-installs/poetry_editable")?; sync_command(&context) .arg(requirements_txt.path()) @@ -461,7 +461,6 @@ fn uninstall_editable_by_path() -> Result<()> { // Uninstall the editable by path. uv_snapshot!(filters, uninstall_command(&context) - .arg("-e") .arg("../../scripts/editable-installs/poetry_editable") .current_dir(¤t_dir), @r###" success: true @@ -484,7 +483,7 @@ fn uninstall_editable_by_path() -> Result<()> { } #[test] -fn uninstall_duplicate_editable() -> Result<()> { +fn uninstall_duplicate_by_path() -> Result<()> { let context = TestContext::new("3.12"); let current_dir = std::env::current_dir()?; @@ -501,7 +500,7 @@ fn uninstall_duplicate_editable() -> Result<()> { let requirements_txt = context.temp_dir.child("requirements.txt"); requirements_txt.touch()?; - requirements_txt.write_str("-e ../../scripts/editable-installs/poetry_editable")?; + requirements_txt.write_str("../../scripts/editable-installs/poetry_editable")?; sync_command(&context) .arg(requirements_txt.path()) @@ -518,7 +517,6 @@ fn uninstall_duplicate_editable() -> Result<()> { // Uninstall the editable by both path and name. uv_snapshot!(filters, uninstall_command(&context) .arg("poetry-editable") - .arg("-e") .arg("../../scripts/editable-installs/poetry_editable") .current_dir(¤t_dir), @r###" success: true