Unify site-packages into distribution enum (#136)

Gets rid of the custom `DistInfo` struct in the site-packages
abstraction in favor of a new kind of distribution
(`InstalledDistribution`). No change in behavior.

Cargo.lock

@@ -1310,7 +1310,6 @@ dependencies = [
  "rayon",
  "reflink-copy",
  "regex",
- "rfc2047-decoder",
  "serde",
  "serde_json",
  "sha2",

crates/install-wheel-rs/Cargo.toml

@@ -36,7 +36,6 @@ pyo3 = { version = "0.19.2", features = ["extension-module", "abi3-py37"], optio
 rayon = { version = "1.8.0", optional = true }
 reflink-copy = { workspace = true }
 regex = { workspace = true }
-rfc2047-decoder = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 serde_json = { workspace = true }
 sha2 = { workspace = true }

crates/puffin-build/src/lib.rs

@@ -13,7 +13,7 @@ use pep508_rs::Requirement;
 use platform_host::Platform;
 use platform_tags::Tags;
 use puffin_client::PypiClientBuilder;
-use puffin_installer::{Downloader, LocalDistribution, LocalIndex, RemoteDistribution, Unzipper};
+use puffin_installer::{CachedDistribution, Downloader, LocalIndex, RemoteDistribution, Unzipper};
 use puffin_interpreter::PythonExecutable;
 use puffin_package::package_name::PackageName;
 use puffin_resolver::WheelFinder;
@@ -440,7 +440,7 @@ async fn resolve_and_install(
     } else {
         LocalIndex::default()
     };
-    let (cached, uncached): (Vec<LocalDistribution>, Vec<Requirement>) =
+    let (cached, uncached): (Vec<CachedDistribution>, Vec<Requirement>) =
         requirements.iter().partition_map(|requirement| {
             let package = PackageName::normalize(&requirement.name);
             if let Some(distribution) = local_index

crates/puffin-cli/src/commands/freeze.rs

@@ -4,7 +4,8 @@ use anyhow::Result;
 use tracing::debug;
 
 use platform_host::Platform;
-use puffin_interpreter::{PythonExecutable, SitePackages};
+use puffin_installer::SitePackages;
+use puffin_interpreter::PythonExecutable;
 
 use crate::commands::ExitStatus;
 use crate::printer::Printer;

crates/puffin-cli/src/commands/pip_sync.rs

@@ -2,17 +2,19 @@ use std::fmt::Write;
 use std::path::Path;
 
 use anyhow::{bail, Context, Result};
-
 use itertools::Itertools;
 use owo_colors::OwoColorize;
-use pep508_rs::Requirement;
 use tracing::debug;
 
+use pep508_rs::Requirement;
 use platform_host::Platform;
 use platform_tags::Tags;
 use puffin_client::PypiClientBuilder;
-use puffin_installer::{LocalDistribution, LocalIndex, RemoteDistribution};
-use puffin_interpreter::{Distribution, PythonExecutable, SitePackages};
+use puffin_installer::{
+    CachedDistribution, Distribution, InstalledDistribution, LocalIndex, RemoteDistribution,
+    SitePackages,
+};
+use puffin_interpreter::PythonExecutable;
 use puffin_package::package_name::PackageName;
 use puffin_package::requirements_txt::RequirementsTxt;
 use puffin_resolver::Resolution;
@@ -230,37 +232,35 @@ pub(crate) async fn sync_requirements(
         )?;
     }
 
-    for dist in extraneous
-        .iter()
-        .map(|dist_info| PackageModification {
-            name: dist_info.name(),
-            version: dist_info.version(),
-            modification: Modification::Remove,
+    for event in extraneous
+        .into_iter()
+        .map(|distribution| ChangeEvent {
+            distribution: Distribution::from(distribution),
+            kind: ChangeEventKind::Remove,
         })
-        .chain(wheels.iter().map(|dist_info| PackageModification {
-            name: dist_info.name(),
-            version: dist_info.version(),
-            modification: Modification::Add,
+        .chain(wheels.into_iter().map(|distribution| ChangeEvent {
+            distribution: Distribution::from(distribution),
+            kind: ChangeEventKind::Add,
         }))
-        .sorted_unstable_by_key(|modification| modification.name)
+        .sorted_unstable_by_key(|event| event.distribution.name().clone())
     {
-        match dist.modification {
-            Modification::Add => {
+        match event.kind {
+            ChangeEventKind::Add => {
                 writeln!(
                     printer,
                     " {} {}{}",
                     "+".green(),
-                    dist.name.as_ref().white().bold(),
-                    format!("@{}", dist.version).dimmed()
+                    event.distribution.name().white().bold(),
+                    format!("@{}", event.distribution.version()).dimmed()
                 )?;
             }
-            Modification::Remove => {
+            ChangeEventKind::Remove => {
                 writeln!(
                     printer,
                     " {} {}{}",
                     "-".red(),
-                    dist.name.as_ref().white().bold(),
-                    format!("@{}", dist.version).dimmed()
+                    event.distribution.name().white().bold(),
+                    format!("@{}", event.distribution.version()).dimmed()
                 )?;
             }
         }
@@ -273,7 +273,7 @@ pub(crate) async fn sync_requirements(
 struct PartitionedRequirements {
     /// The distributions that are not already installed in the current environment, but are
     /// available in the local cache.
-    local: Vec<LocalDistribution>,
+    local: Vec<CachedDistribution>,
 
     /// The distributions that are not already installed in the current environment, and are
     /// not available in the local cache.
@@ -281,7 +281,7 @@ struct PartitionedRequirements {
 
     /// The distributions that are already installed in the current environment, and are
     /// _not_ necessary to satisfy the requirements.
-    extraneous: Vec<Distribution>,
+    extraneous: Vec<InstalledDistribution>,
 }
 
 impl PartitionedRequirements {
@@ -354,7 +354,7 @@ impl PartitionedRequirements {
 }
 
 #[derive(Debug)]
-enum Modification {
+enum ChangeEventKind {
     /// The package was added to the environment.
     Add,
     /// The package was removed from the environment.
@@ -362,8 +362,7 @@ enum Modification {
 }
 
 #[derive(Debug)]
-struct PackageModification<'a> {
-    name: &'a PackageName,
-    version: &'a pep440_rs::Version,
-    modification: Modification,
+struct ChangeEvent {
+    distribution: Distribution,
+    kind: ChangeEventKind,
 }

crates/puffin-cli/src/commands/pip_uninstall.rs

@@ -39,7 +39,7 @@ pub(crate) async fn pip_uninstall(
         .collect::<Result<Vec<Requirement>>>()?;
 
     // Index the current `site-packages` directory.
-    let site_packages = puffin_interpreter::SitePackages::from_executable(&python).await?;
+    let site_packages = puffin_installer::SitePackages::from_executable(&python).await?;
 
     // Sort and deduplicate the requirements.
     let packages = {

crates/puffin-installer/src/distribution.rs

@@ -13,7 +13,8 @@ use wheel_filename::WheelFilename;
 #[derive(Debug, Clone)]
 pub enum Distribution {
     Remote(RemoteDistribution),
-    Local(LocalDistribution),
+    Cached(CachedDistribution),
+    Installed(InstalledDistribution),
 }
 
 impl Distribution {
@@ -21,7 +22,8 @@ impl Distribution {
     pub fn name(&self) -> &PackageName {
         match self {
             Self::Remote(dist) => dist.name(),
-            Self::Local(dist) => dist.name(),
+            Self::Cached(dist) => dist.name(),
+            Self::Installed(dist) => dist.name(),
         }
     }
 
@@ -29,7 +31,8 @@ impl Distribution {
     pub fn version(&self) -> &Version {
         match self {
             Self::Remote(dist) => dist.version(),
-            Self::Local(dist) => dist.version(),
+            Self::Cached(dist) => dist.version(),
+            Self::Installed(dist) => dist.version(),
         }
     }
 
@@ -39,11 +42,30 @@ impl Distribution {
     pub fn id(&self) -> String {
         match self {
             Self::Remote(dist) => dist.id(),
-            Self::Local(dist) => dist.id(),
+            Self::Cached(dist) => dist.id(),
+            Self::Installed(dist) => dist.id(),
         }
     }
 }
 
+impl From<RemoteDistribution> for Distribution {
+    fn from(dist: RemoteDistribution) -> Self {
+        Self::Remote(dist)
+    }
+}
+
+impl From<CachedDistribution> for Distribution {
+    fn from(dist: CachedDistribution) -> Self {
+        Self::Cached(dist)
+    }
+}
+
+impl From<InstalledDistribution> for Distribution {
+    fn from(dist: InstalledDistribution) -> Self {
+        Self::Installed(dist)
+    }
+}
+
 /// A built distribution (wheel) that exists as a remote file (e.g., on `PyPI`).
 #[derive(Debug, Clone)]
 pub struct RemoteDistribution {
@@ -82,16 +104,16 @@ impl RemoteDistribution {
     }
 }
 
-/// A built distribution (wheel) that exists as a local file (e.g., in the wheel cache).
+/// A built distribution (wheel) that exists in a local cache.
 #[derive(Debug, Clone)]
-pub struct LocalDistribution {
+pub struct CachedDistribution {
     name: PackageName,
     version: Version,
     path: PathBuf,
 }
 
-impl LocalDistribution {
-    /// Initialize a new local distribution.
+impl CachedDistribution {
+    /// Initialize a new cached distribution.
     pub fn new(name: PackageName, version: Version, path: PathBuf) -> Self {
         Self {
             name,
@@ -100,7 +122,7 @@ impl LocalDistribution {
         }
     }
 
-    /// Try to parse a cached distribution from a directory name (like `django-5.0a1`).
+    /// Try to parse a distribution from a cached directory name (like `django-5.0a1`).
     pub(crate) fn try_from_path(path: &Path) -> Result<Option<Self>> {
         let Some(file_name) = path.file_name() else {
             return Ok(None);
@@ -116,7 +138,7 @@ impl LocalDistribution {
         let version = Version::from_str(version).map_err(|err| anyhow!(err))?;
         let path = path.to_path_buf();
 
-        Ok(Some(LocalDistribution {
+        Ok(Some(CachedDistribution {
             name,
             version,
             path,
@@ -139,3 +161,67 @@ impl LocalDistribution {
         format!("{}-{}", DistInfoName::from(self.name()), self.version())
     }
 }
+
+/// A built distribution (wheel) that exists in a virtual environment.
+#[derive(Debug, Clone)]
+pub struct InstalledDistribution {
+    name: PackageName,
+    version: Version,
+    path: PathBuf,
+}
+
+impl InstalledDistribution {
+    /// Initialize a new installed distribution.
+    pub fn new(name: PackageName, version: Version, path: PathBuf) -> Self {
+        Self {
+            name,
+            version,
+            path,
+        }
+    }
+
+    /// Try to parse a distribution from a `.dist-info` directory name (like `django-5.0a1.dist-info`).
+    ///
+    /// See: <https://packaging.python.org/en/latest/specifications/recording-installed-packages/#recording-installed-packages>
+    pub(crate) fn try_from_path(path: &Path) -> Result<Option<Self>> {
+        if path.extension().is_some_and(|ext| ext == "dist-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)) = file_stem.split_once('-') else {
+                return Ok(None);
+            };
+
+            let name = PackageName::normalize(name);
+            let version = Version::from_str(version).map_err(|err| anyhow!(err))?;
+            let path = path.to_path_buf();
+
+            return Ok(Some(Self {
+                name,
+                version,
+                path,
+            }));
+        }
+
+        Ok(None)
+    }
+
+    pub fn name(&self) -> &PackageName {
+        &self.name
+    }
+
+    pub fn version(&self) -> &Version {
+        &self.version
+    }
+
+    pub fn path(&self) -> &Path {
+        &self.path
+    }
+
+    pub fn id(&self) -> String {
+        format!("{}-{}", DistInfoName::from(self.name()), self.version())
+    }
+}

crates/puffin-installer/src/downloader.rs

@@ -100,7 +100,7 @@ async fn fetch_wheel(
     // Read from the cache, if possible.
     if let Some(cache) = cache.as_ref() {
         if let Ok(buffer) = cacache::read_hash(&cache, &sri).await {
-            debug!("Extracted wheel from cache: {:?}", remote.file().filename);
+            debug!("Extracted wheel from cache: {}", remote.file().filename);
             return Ok(InMemoryDistribution { remote, buffer });
         }
     }

crates/puffin-installer/src/index.rs

@@ -6,11 +6,11 @@ use anyhow::Result;
 use puffin_package::package_name::PackageName;
 
 use crate::cache::WheelCache;
-use crate::distribution::LocalDistribution;
+use crate::distribution::CachedDistribution;
 
 /// A local index of cached distributions.
 #[derive(Debug, Default)]
-pub struct LocalIndex(HashMap<PackageName, LocalDistribution>);
+pub struct LocalIndex(HashMap<PackageName, CachedDistribution>);
 
 impl LocalIndex {
     /// Build an index of cached distributions from a directory.
@@ -24,7 +24,7 @@ impl LocalIndex {
 
         while let Some(entry) = dir.next_entry().await? {
             if entry.file_type().await?.is_dir() {
-                if let Some(dist_info) = LocalDistribution::try_from_path(&entry.path())? {
+                if let Some(dist_info) = CachedDistribution::try_from_path(&entry.path())? {
                     index.insert(dist_info.name().clone(), dist_info);
                 }
             }
@@ -34,7 +34,7 @@ impl LocalIndex {
     }
 
     /// Returns a distribution from the index, if it exists.
-    pub fn get(&self, name: &PackageName) -> Option<&LocalDistribution> {
+    pub fn get(&self, name: &PackageName) -> Option<&CachedDistribution> {
         self.0.get(name)
     }
 }

crates/puffin-installer/src/installer.rs

@@ -5,7 +5,7 @@ use pep440_rs::Version;
 use puffin_interpreter::PythonExecutable;
 use puffin_package::package_name::PackageName;
 
-use crate::LocalDistribution;
+use crate::CachedDistribution;
 
 pub struct Installer<'a> {
     python: &'a PythonExecutable,
@@ -31,7 +31,7 @@ impl<'a> Installer<'a> {
     }
 
     /// Install a set of wheels into a Python virtual environment.
-    pub fn install(self, wheels: &[LocalDistribution]) -> Result<()> {
+    pub fn install(self, wheels: &[CachedDistribution]) -> Result<()> {
         tokio::task::block_in_place(|| {
             wheels.par_iter().try_for_each(|wheel| {
                 let location = install_wheel_rs::InstallLocation::new(

crates/puffin-installer/src/lib.rs

@@ -1,7 +1,10 @@
-pub use distribution::{Distribution, LocalDistribution, RemoteDistribution};
+pub use distribution::{
+    CachedDistribution, Distribution, InstalledDistribution, RemoteDistribution,
+};
 pub use downloader::{Downloader, Reporter as DownloadReporter};
 pub use index::LocalIndex;
 pub use installer::{Installer, Reporter as InstallReporter};
+pub use site_packages::SitePackages;
 pub use uninstall::uninstall;
 pub use unzipper::{Reporter as UnzipReporter, Unzipper};
 
@@ -10,6 +13,7 @@ mod distribution;
 mod downloader;
 mod index;
 mod installer;
+mod site_packages;
 mod uninstall;
 mod unzipper;
 mod vendor;

crates/puffin-installer/src/site_packages.rs

@@ -0,0 +1,54 @@
+use std::collections::BTreeMap;
+
+use anyhow::Result;
+use fs_err::tokio as fs;
+
+use puffin_interpreter::PythonExecutable;
+use puffin_package::package_name::PackageName;
+
+use crate::InstalledDistribution;
+
+#[derive(Debug, Default)]
+pub struct SitePackages(BTreeMap<PackageName, InstalledDistribution>);
+
+impl SitePackages {
+    /// Build an index of installed packages from the given Python executable.
+    pub async fn from_executable(python: &PythonExecutable) -> Result<Self> {
+        let mut index = BTreeMap::new();
+
+        let mut dir = fs::read_dir(python.site_packages()).await?;
+        while let Some(entry) = dir.next_entry().await? {
+            if entry.file_type().await?.is_dir() {
+                if let Some(dist_info) = InstalledDistribution::try_from_path(&entry.path())? {
+                    index.insert(dist_info.name().clone(), dist_info);
+                }
+            }
+        }
+
+        Ok(Self(index))
+    }
+
+    /// Returns an iterator over the installed packages.
+    pub fn iter(&self) -> impl Iterator<Item = (&PackageName, &InstalledDistribution)> {
+        self.0.iter()
+    }
+
+    /// Returns the version of the given package, if it is installed.
+    pub fn get(&self, name: &PackageName) -> Option<&InstalledDistribution> {
+        self.0.get(name)
+    }
+
+    /// Remove the given package from the index, returning its version if it was installed.
+    pub fn remove(&mut self, name: &PackageName) -> Option<InstalledDistribution> {
+        self.0.remove(name)
+    }
+}
+
+impl IntoIterator for SitePackages {
+    type Item = (PackageName, InstalledDistribution);
+    type IntoIter = std::collections::btree_map::IntoIter<PackageName, InstalledDistribution>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.0.into_iter()
+    }
+}

crates/puffin-installer/src/uninstall.rs

@@ -1,9 +1,11 @@
 use anyhow::Result;
 
-use puffin_interpreter::Distribution;
+use crate::InstalledDistribution;
 
 /// Uninstall a package from the specified Python environment.
-pub async fn uninstall(distribution: &Distribution) -> Result<install_wheel_rs::Uninstall> {
+pub async fn uninstall(
+    distribution: &InstalledDistribution,
+) -> Result<install_wheel_rs::Uninstall> {
     let uninstall = tokio::task::spawn_blocking({
         let path = distribution.path().to_owned();
         move || install_wheel_rs::uninstall_wheel(&path)

crates/puffin-installer/src/unzipper.rs

@@ -14,7 +14,7 @@ use puffin_package::package_name::PackageName;
 use crate::cache::WheelCache;
 use crate::downloader::InMemoryDistribution;
 use crate::vendor::CloneableSeekableReader;
-use crate::LocalDistribution;
+use crate::CachedDistribution;
 
 #[derive(Default)]
 pub struct Unzipper {
@@ -35,7 +35,7 @@ impl Unzipper {
         &self,
         downloads: Vec<InMemoryDistribution>,
         target: &Path,
-    ) -> Result<Vec<LocalDistribution>> {
+    ) -> Result<Vec<CachedDistribution>> {
         // Create the wheel cache subdirectory, if necessary.
         let wheel_cache = WheelCache::new(target);
         wheel_cache.init().await?;
@@ -63,7 +63,7 @@ impl Unzipper {
             )
             .await?;
 
-            wheels.push(LocalDistribution::new(
+            wheels.push(CachedDistribution::new(
                 remote.name().clone(),
                 remote.version().clone(),
                 wheel_cache.entry(&remote.id()),

crates/puffin-interpreter/src/lib.rs

@@ -7,11 +7,9 @@ use pep508_rs::MarkerEnvironment;
 use platform_host::Platform;
 
 use crate::python_platform::PythonPlatform;
-pub use crate::site_packages::{Distribution, SitePackages};
 
 mod markers;
 mod python_platform;
-mod site_packages;
 mod virtual_env;
 
 /// A Python executable and its associated platform markers.

crates/puffin-interpreter/src/site_packages.rs

@@ -1,106 +0,0 @@
-use std::collections::BTreeMap;
-use std::path::{Path, PathBuf};
-use std::str::FromStr;
-
-use anyhow::{anyhow, Result};
-use fs_err::tokio as fs;
-
-use pep440_rs::Version;
-use puffin_package::package_name::PackageName;
-
-use crate::PythonExecutable;
-
-#[derive(Debug, Default)]
-pub struct SitePackages(BTreeMap<PackageName, Distribution>);
-
-impl SitePackages {
-    /// Build an index of installed packages from the given Python executable.
-    pub async fn from_executable(python: &PythonExecutable) -> Result<Self> {
-        let mut index = BTreeMap::new();
-
-        let mut dir = fs::read_dir(python.site_packages()).await?;
-        while let Some(entry) = dir.next_entry().await? {
-            if entry.file_type().await?.is_dir() {
-                if let Some(dist_info) = Distribution::try_from_path(&entry.path())? {
-                    index.insert(dist_info.name().clone(), dist_info);
-                }
-            }
-        }
-
-        Ok(Self(index))
-    }
-
-    /// Returns an iterator over the installed packages.
-    pub fn iter(&self) -> impl Iterator<Item = (&PackageName, &Distribution)> {
-        self.0.iter()
-    }
-
-    /// Returns the version of the given package, if it is installed.
-    pub fn get(&self, name: &PackageName) -> Option<&Distribution> {
-        self.0.get(name)
-    }
-
-    /// Remove the given package from the index, returning its version if it was installed.
-    pub fn remove(&mut self, name: &PackageName) -> Option<Distribution> {
-        self.0.remove(name)
-    }
-}
-
-impl IntoIterator for SitePackages {
-    type Item = (PackageName, Distribution);
-    type IntoIter = std::collections::btree_map::IntoIter<PackageName, Distribution>;
-
-    fn into_iter(self) -> Self::IntoIter {
-        self.0.into_iter()
-    }
-}
-
-#[derive(Debug, Clone)]
-pub struct Distribution {
-    name: PackageName,
-    version: Version,
-    path: PathBuf,
-}
-
-impl Distribution {
-    /// Try to parse a (potential) `dist-info` directory into a package name and version.
-    ///
-    /// See: <https://packaging.python.org/en/latest/specifications/recording-installed-packages/#recording-installed-packages>
-    fn try_from_path(path: &Path) -> Result<Option<Self>> {
-        if path.extension().is_some_and(|ext| ext == "dist-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)) = file_stem.split_once('-') else {
-                return Ok(None);
-            };
-
-            let name = PackageName::normalize(name);
-            let version = Version::from_str(version).map_err(|err| anyhow!(err))?;
-            let path = path.to_path_buf();
-
-            return Ok(Some(Distribution {
-                name,
-                version,
-                path,
-            }));
-        }
-
-        Ok(None)
-    }
-
-    pub fn name(&self) -> &PackageName {
-        &self.name
-    }
-
-    pub fn version(&self) -> &Version {
-        &self.version
-    }
-
-    pub fn path(&self) -> &Path {
-        &self.path
-    }
-}