Skip to content

Commit

Permalink
Support unnamed requirements directly in pip uninstall (#2577)
Browse files Browse the repository at this point in the history
## 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: #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`
  • Loading branch information
charliermarsh committed Mar 21, 2024
1 parent 34bef37 commit 7c728a2
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 43 deletions.
58 changes: 28 additions & 30 deletions crates/uv/src/commands/pip_uninstall.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
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;
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(
Expand Down Expand Up @@ -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<Requirement>, Vec<UnnamedRequirement>) = 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::<Vec<_>>();
Expand All @@ -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::<Vec<_>>();
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!(
Expand All @@ -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);
Expand Down
5 changes: 0 additions & 5 deletions crates/uv/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -960,10 +960,6 @@ struct PipUninstallArgs {
#[clap(long, short, group = "sources")]
requirement: Vec<PathBuf>,

/// Uninstall the editable package based on the provided local file path.
#[clap(long, short, group = "sources")]
editable: Vec<String>,

/// The Python interpreter from which packages should be uninstalled.
///
/// By default, `uv` uninstalls from the virtual environment in the current working directory or
Expand Down Expand Up @@ -1706,7 +1702,6 @@ async fn run() -> Result<ExitStatus> {
.package
.into_iter()
.map(RequirementsSource::from_package)
.chain(args.editable.into_iter().map(RequirementsSource::Editable))
.chain(
args.requirement
.into_iter()
Expand Down
14 changes: 6 additions & 8 deletions crates/uv/tests/pip_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ fn no_arguments() -> Result<()> {
----- stderr -----
error: the following required arguments were not provided:
<PACKAGE|--requirement <REQUIREMENT>|--editable <EDITABLE>>
<PACKAGE|--requirement <REQUIREMENT>>
Usage: uv pip uninstall <PACKAGE|--requirement <REQUIREMENT>|--editable <EDITABLE>>
Usage: uv pip uninstall <PACKAGE|--requirement <REQUIREMENT>>
For more information, try '--help'.
"###
Expand Down Expand Up @@ -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()?;
Expand All @@ -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())
Expand All @@ -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(&current_dir), @r###"
success: true
Expand All @@ -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()?;
Expand All @@ -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())
Expand All @@ -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(&current_dir), @r###"
success: true
Expand Down

0 comments on commit 7c728a2

Please sign in to comment.