From a81fb92ee67910dfffd83c7ebf91d88e34bac925 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 5 Jun 2024 21:00:00 -0400 Subject: [PATCH] Respect existing `.egg-link` files in site packages (#4082) ## Summary As with other `.egg-info` and `.egg-link` distributions, it's easy to support _existing_ `.egg-link` files. Like pip, we refuse to uninstall these, since there's no way to know which files are part of the distribution. Closes https://github.com/astral-sh/uv/issues/4059. ## Test Plan Verify that `vtk` is included here, which is installed as a `.egg-link` file: ``` > conda create -c conda-forge -n uv-test python h5py vtk pyside6 cftime psutil > cargo run pip freeze --python /opt/homebrew/Caskroom/miniforge/base/envs/uv-test/bin/python aiohttp @ file:///Users/runner/miniforge3/conda-bld/aiohttp_1713964997382/work aiosignal @ file:///home/conda/feedstock_root/build_artifacts/aiosignal_1667935791922/work attrs @ file:///home/conda/feedstock_root/build_artifacts/attrs_1704011227531/work cached-property @ file:///home/conda/feedstock_root/build_artifacts/cached_property_1615209429212/work cftime @ file:///Users/runner/miniforge3/conda-bld/cftime_1715919201099/work frozenlist @ file:///Users/runner/miniforge3/conda-bld/frozenlist_1702645558715/work h5py @ file:///Users/runner/miniforge3/conda-bld/h5py_1715968397721/work idna @ file:///home/conda/feedstock_root/build_artifacts/idna_1713279365350/work loguru @ file:///Users/runner/miniforge3/conda-bld/loguru_1695547410953/work msgpack @ file:///Users/runner/miniforge3/conda-bld/msgpack-python_1715670632250/work multidict @ file:///Users/runner/miniforge3/conda-bld/multidict_1707040780513/work numpy @ file:///Users/runner/miniforge3/conda-bld/numpy_1707225421156/work/dist/numpy-1.26.4-cp312-cp312-macosx_11_0_arm64.whl pip==24.0 psutil @ file:///Users/runner/miniforge3/conda-bld/psutil_1705722460205/work pyside6==6.7.1 setuptools==70.0.0 shiboken6==6.7.1 vtk==9.2.6 wheel==0.43.0 wslink @ file:///home/conda/feedstock_root/build_artifacts/wslink_1716591560747/work yarl @ file:///Users/runner/miniforge3/conda-bld/yarl_1705508643525/work ``` --- crates/distribution-types/src/installed.rs | 119 +++++++++++++++------ crates/distribution-types/src/traits.rs | 24 ++++- crates/uv-installer/src/site_packages.rs | 7 +- crates/uv-installer/src/uninstall.rs | 11 +- crates/uv/src/commands/pip/freeze.rs | 5 +- 5 files changed, 120 insertions(+), 46 deletions(-) diff --git a/crates/distribution-types/src/installed.rs b/crates/distribution-types/src/installed.rs index 8879ad319..63f23c2b3 100644 --- a/crates/distribution-types/src/installed.rs +++ b/crates/distribution-types/src/installed.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -20,8 +21,10 @@ pub enum InstalledDist { Registry(InstalledRegistryDist), /// The distribution was derived from an arbitrary URL. Url(InstalledDirectUrlDist), + /// The distribution was derived from pre-existing `.egg-info` file (as installed by distutils). + EggInfoFile(InstalledEggInfoFile), /// The distribution was derived from pre-existing `.egg-info` directory. - EggInfo(InstalledEggInfo), + EggInfoDirectory(InstalledEggInfoDirectory), /// The distribution was derived from an `.egg-link` pointer. LegacyEditable(InstalledLegacyEditable), } @@ -44,7 +47,14 @@ pub struct InstalledDirectUrlDist { } #[derive(Debug, Clone)] -pub struct InstalledEggInfo { +pub struct InstalledEggInfoFile { + pub name: PackageName, + pub version: Version, + pub path: PathBuf, +} + +#[derive(Debug, Clone)] +pub struct InstalledEggInfoDirectory { pub name: PackageName, pub version: Version, pub path: PathBuf, @@ -107,27 +117,49 @@ impl InstalledDist { }; } - // Ex) `zstandard-0.22.0-py3.12.egg-info` if path.extension().is_some_and(|ext| ext == "egg-info") { - let Some(file_stem) = path.file_stem() else { - return Ok(None); - }; - let Some(file_stem) = file_stem.to_str() else { - return Ok(None); - }; - let Some((name, version_python)) = file_stem.split_once('-') else { - return Ok(None); - }; - let Some((version, _)) = version_python.split_once('-') else { - return Ok(None); - }; - let name = PackageName::from_str(name)?; - let version = Version::from_str(version).map_err(|err| anyhow!(err))?; - return Ok(Some(Self::EggInfo(InstalledEggInfo { - name, - version, - path: path.to_path_buf(), - }))); + // Ex) `zstandard-0.22.0-py3.12.egg-info` + if path.is_dir() { + let Some(file_stem) = path.file_stem() else { + return Ok(None); + }; + let Some(file_stem) = file_stem.to_str() else { + return Ok(None); + }; + let Some((name, version_python)) = file_stem.split_once('-') else { + return Ok(None); + }; + let Some((version, _)) = version_python.split_once('-') else { + return Ok(None); + }; + let name = PackageName::from_str(name)?; + let version = Version::from_str(version).map_err(|err| anyhow!(err))?; + return Ok(Some(Self::EggInfoDirectory(InstalledEggInfoDirectory { + name, + version, + path: path.to_path_buf(), + }))); + } + + // Ex) `vtk-9.2.6.egg-info` + if path.is_file() { + let Some(file_stem) = path.file_stem() else { + return Ok(None); + }; + let Some(file_stem) = file_stem.to_str() else { + return Ok(None); + }; + let Some((name, version)) = file_stem.split_once('-') else { + return Ok(None); + }; + let name = PackageName::from_str(name)?; + let version = Version::from_str(version).map_err(|err| anyhow!(err))?; + return Ok(Some(Self::EggInfoFile(InstalledEggInfoFile { + name, + version, + path: path.to_path_buf(), + }))); + } } // Ex) `zstandard.egg-link` @@ -199,7 +231,8 @@ impl InstalledDist { match self { Self::Registry(dist) => &dist.path, Self::Url(dist) => &dist.path, - Self::EggInfo(dist) => &dist.path, + Self::EggInfoDirectory(dist) => &dist.path, + Self::EggInfoFile(dist) => &dist.path, Self::LegacyEditable(dist) => &dist.egg_info, } } @@ -209,7 +242,8 @@ impl InstalledDist { match self { Self::Registry(dist) => &dist.version, Self::Url(dist) => &dist.version, - Self::EggInfo(dist) => &dist.version, + Self::EggInfoDirectory(dist) => &dist.version, + Self::EggInfoFile(dist) => &dist.version, Self::LegacyEditable(dist) => &dist.version, } } @@ -238,13 +272,14 @@ impl InstalledDist { ) }) } - Self::EggInfo(_) | Self::LegacyEditable(_) => { + Self::EggInfoFile(_) | Self::EggInfoDirectory(_) | Self::LegacyEditable(_) => { let path = match self { - Self::EggInfo(dist) => dist.path.join("PKG-INFO"), - Self::LegacyEditable(dist) => dist.egg_info.join("PKG-INFO"), + Self::EggInfoFile(dist) => Cow::Borrowed(&dist.path), + Self::EggInfoDirectory(dist) => Cow::Owned(dist.path.join("PKG-INFO")), + Self::LegacyEditable(dist) => Cow::Owned(dist.egg_info.join("PKG-INFO")), _ => unreachable!(), }; - let contents = fs::read(&path)?; + let contents = fs::read(path.as_ref())?; pypi_types::Metadata23::parse_metadata(&contents).with_context(|| { format!( "Failed to parse `PKG-INFO` file at: {}", @@ -278,7 +313,8 @@ impl InstalledDist { match self { Self::Registry(_) => None, Self::Url(dist) => dist.editable.then_some(&dist.url), - Self::EggInfo(_) => None, + Self::EggInfoFile(_) => None, + Self::EggInfoDirectory(_) => None, Self::LegacyEditable(dist) => Some(&dist.target_url), } } @@ -288,7 +324,8 @@ impl InstalledDist { match self { Self::Registry(_) => false, Self::Url(dist) => matches!(&*dist.direct_url, DirectUrl::LocalDirectory { .. }), - Self::EggInfo(_) => false, + Self::EggInfoFile(_) => false, + Self::EggInfoDirectory(_) => false, Self::LegacyEditable(_) => true, } } @@ -312,7 +349,13 @@ impl Name for InstalledDirectUrlDist { } } -impl Name for InstalledEggInfo { +impl Name for InstalledEggInfoFile { + fn name(&self) -> &PackageName { + &self.name + } +} + +impl Name for InstalledEggInfoDirectory { fn name(&self) -> &PackageName { &self.name } @@ -329,7 +372,8 @@ impl Name for InstalledDist { match self { Self::Registry(dist) => dist.name(), Self::Url(dist) => dist.name(), - Self::EggInfo(dist) => dist.name(), + Self::EggInfoDirectory(dist) => dist.name(), + Self::EggInfoFile(dist) => dist.name(), Self::LegacyEditable(dist) => dist.name(), } } @@ -347,7 +391,13 @@ impl InstalledMetadata for InstalledDirectUrlDist { } } -impl InstalledMetadata for InstalledEggInfo { +impl InstalledMetadata for InstalledEggInfoFile { + fn installed_version(&self) -> InstalledVersion { + InstalledVersion::Version(&self.version) + } +} + +impl InstalledMetadata for InstalledEggInfoDirectory { fn installed_version(&self) -> InstalledVersion { InstalledVersion::Version(&self.version) } @@ -364,7 +414,8 @@ impl InstalledMetadata for InstalledDist { match self { Self::Registry(dist) => dist.installed_version(), Self::Url(dist) => dist.installed_version(), - Self::EggInfo(dist) => dist.installed_version(), + Self::EggInfoFile(dist) => dist.installed_version(), + Self::EggInfoDirectory(dist) => dist.installed_version(), Self::LegacyEditable(dist) => dist.installed_version(), } } diff --git a/crates/distribution-types/src/traits.rs b/crates/distribution-types/src/traits.rs index 871530e33..ebd9c2ada 100644 --- a/crates/distribution-types/src/traits.rs +++ b/crates/distribution-types/src/traits.rs @@ -7,9 +7,9 @@ use crate::error::Error; use crate::{ BuiltDist, CachedDirectUrlDist, CachedDist, CachedRegistryDist, DirectUrlBuiltDist, DirectUrlSourceDist, Dist, DistributionId, GitSourceDist, InstalledDirectUrlDist, - InstalledDist, InstalledRegistryDist, InstalledVersion, LocalDist, PackageId, PathBuiltDist, - PathSourceDist, RegistryBuiltWheel, RegistrySourceDist, ResourceId, SourceDist, VersionId, - VersionOrUrlRef, + InstalledDist, InstalledEggInfoDirectory, InstalledEggInfoFile, InstalledLegacyEditable, + InstalledRegistryDist, InstalledVersion, LocalDist, PackageId, PathBuiltDist, PathSourceDist, + RegistryBuiltWheel, RegistrySourceDist, ResourceId, SourceDist, VersionId, VersionOrUrlRef, }; pub trait Name { @@ -189,6 +189,24 @@ impl std::fmt::Display for InstalledRegistryDist { } } +impl std::fmt::Display for InstalledEggInfoFile { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}{}", self.name(), self.installed_version()) + } +} + +impl std::fmt::Display for InstalledEggInfoDirectory { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}{}", self.name(), self.installed_version()) + } +} + +impl std::fmt::Display for InstalledLegacyEditable { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}{}", self.name(), self.installed_version()) + } +} + impl std::fmt::Display for PathBuiltDist { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}{}", self.name(), self.version_or_url()) diff --git a/crates/uv-installer/src/site_packages.rs b/crates/uv-installer/src/site_packages.rs index 86f6d7768..47507a1e1 100644 --- a/crates/uv-installer/src/site_packages.rs +++ b/crates/uv-installer/src/site_packages.rs @@ -52,10 +52,9 @@ impl SitePackages { .filter_map(|read_dir| match read_dir { Ok(entry) => match entry.file_type() { Ok(file_type) => (file_type.is_dir() - || entry - .path() - .extension() - .map_or(false, |ext| ext == "egg-link")) + || entry.path().extension().map_or(false, |ext| { + ext == "egg-link" || ext == "egg-info" + })) .then_some(Ok(entry.path())), Err(err) => Some(Err(err)), }, diff --git a/crates/uv-installer/src/uninstall.rs b/crates/uv-installer/src/uninstall.rs index 7386ff310..257432b40 100644 --- a/crates/uv-installer/src/uninstall.rs +++ b/crates/uv-installer/src/uninstall.rs @@ -1,4 +1,4 @@ -use distribution_types::InstalledDist; +use distribution_types::{InstalledDist, InstalledEggInfoFile}; /// Uninstall a package from the specified Python environment. pub async fn uninstall( @@ -8,12 +8,13 @@ pub async fn uninstall( let dist = dist.clone(); move || match dist { InstalledDist::Registry(_) | InstalledDist::Url(_) => { - install_wheel_rs::uninstall_wheel(dist.path()) + Ok(install_wheel_rs::uninstall_wheel(dist.path())?) } - InstalledDist::EggInfo(_) => install_wheel_rs::uninstall_egg(dist.path()), + InstalledDist::EggInfoDirectory(_) => Ok(install_wheel_rs::uninstall_egg(dist.path())?), InstalledDist::LegacyEditable(dist) => { - install_wheel_rs::uninstall_legacy_editable(&dist.egg_link) + Ok(install_wheel_rs::uninstall_legacy_editable(&dist.egg_link)?) } + InstalledDist::EggInfoFile(dist) => Err(UninstallError::Distutils(dist)), } }) .await??; @@ -23,6 +24,8 @@ pub async fn uninstall( #[derive(thiserror::Error, Debug)] pub enum UninstallError { + #[error("Unable to uninstall `{0}`. distutils-installed distributions do not include the metadata required to uninstall safely.")] + Distutils(InstalledEggInfoFile), #[error(transparent)] Uninstall(#[from] install_wheel_rs::Error), #[error(transparent)] diff --git a/crates/uv/src/commands/pip/freeze.rs b/crates/uv/src/commands/pip/freeze.rs index 67aadff78..9e64b7cd2 100644 --- a/crates/uv/src/commands/pip/freeze.rs +++ b/crates/uv/src/commands/pip/freeze.rs @@ -57,7 +57,10 @@ pub(crate) fn pip_freeze( writeln!(printer.stdout(), "{} @ {}", dist.name().bold(), dist.url)?; } } - InstalledDist::EggInfo(dist) => { + InstalledDist::EggInfoFile(dist) => { + writeln!(printer.stdout(), "{}=={}", dist.name().bold(), dist.version)?; + } + InstalledDist::EggInfoDirectory(dist) => { writeln!(printer.stdout(), "{}=={}", dist.name().bold(), dist.version)?; } InstalledDist::LegacyEditable(dist) => {