Move distribution abstraction in shared crate (#258)

This also allows us to get rid of `PinnedPackage` _and_ to remove some
`Result<...>` types due to needless conversions between
otherwise-identical types.

Cargo.lock

@@ -1986,6 +1986,7 @@ dependencies = [
  "pubgrub",
  "puffin-client",
  "puffin-dispatch",
+ "puffin-distribution",
  "puffin-installer",
  "puffin-interpreter",
  "puffin-package",
@@ -2063,6 +2064,7 @@ dependencies = [
  "platform-tags",
  "puffin-build",
  "puffin-client",
+ "puffin-distribution",
  "puffin-installer",
  "puffin-interpreter",
  "puffin-package",
@@ -2072,6 +2074,16 @@ dependencies = [
  "tracing",
 ]
 
+[[package]]
+name = "puffin-distribution"
+version = "0.1.0"
+dependencies = [
+ "anyhow",
+ "distribution-filename",
+ "pep440_rs 0.3.12",
+ "puffin-package",
+]
+
 [[package]]
 name = "puffin-installer"
 version = "0.0.1"
@@ -2084,6 +2096,7 @@ dependencies = [
  "pep440_rs 0.3.12",
  "pep508_rs",
  "puffin-client",
+ "puffin-distribution",
  "puffin-interpreter",
  "puffin-package",
  "rayon",
@@ -2160,6 +2173,7 @@ dependencies = [
  "platform-tags",
  "pubgrub",
  "puffin-client",
+ "puffin-distribution",
  "puffin-interpreter",
  "puffin-package",
  "puffin-traits",

crates/distribution-filename/Cargo.toml

@@ -10,8 +10,8 @@ authors = { workspace = true }
 license = { workspace = true }
 
 [dependencies]
+pep440_rs = { path = "../pep440-rs" }
 platform-tags = { path = "../platform-tags" }
 puffin-package = { path = "../puffin-package" }
-pep440_rs = { path = "../pep440-rs" }
 
 thiserror = { workspace = true }

crates/distribution-filename/src/lib.rs

@@ -1,7 +1,7 @@
 pub use source_distribution::{
     SourceDistributionExtension, SourceDistributionFilename, SourceDistributionFilenameError,
 };
-pub use wheel_filename::{WheelFilename, WheelFilenameError};
+pub use wheel::{WheelFilename, WheelFilenameError};
 
 mod source_distribution;
-mod wheel_filename;
+mod wheel;

crates/puffin-cli/Cargo.toml

@@ -1,7 +1,13 @@
 [package]
 name = "puffin-cli"
 version = "0.0.1"
-edition = "2021"
+edition = { workspace = true }
+rust-version = { workspace = true }
+homepage = { workspace = true }
+documentation = { workspace = true }
+repository = { workspace = true }
+authors = { workspace = true }
+license = { workspace = true }
 
 [[bin]]
 name = "puffin"
@@ -17,6 +23,7 @@ platform-tags = { path = "../platform-tags" }
 pubgrub = { path = "../../vendor/pubgrub" }
 puffin-client = { path = "../puffin-client" }
 puffin-dispatch = { path = "../puffin-dispatch" }
+puffin-distribution = { path = "../puffin-distribution" }
 puffin-installer = { path = "../puffin-installer" }
 puffin-interpreter = { path = "../puffin-interpreter" }
 puffin-package = { path = "../puffin-package" }

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

@@ -11,7 +11,8 @@ use pep508_rs::Requirement;
 use platform_host::Platform;
 use platform_tags::Tags;
 use puffin_client::RegistryClientBuilder;
-use puffin_installer::{Distribution, PartitionedRequirements, RemoteDistribution};
+use puffin_distribution::Distribution;
+use puffin_installer::PartitionedRequirements;
 use puffin_interpreter::Virtualenv;
 
 use crate::commands::reporters::{
@@ -132,10 +133,7 @@ pub(crate) async fn sync_requirements(
             .dimmed()
         )?;
 
-        resolution
+        resolution.into_distributions().collect::<Vec<_>>()
-            .into_files()
-            .map(RemoteDistribution::from_file)
-            .collect::<Result<Vec<_>>>()?
     };
 
     // Download any missing distributions.

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

@@ -2,6 +2,7 @@ use indicatif::{ProgressBar, ProgressStyle};
 use std::time::Duration;
 
 use pep440_rs::Version;
+use puffin_distribution::RemoteDistribution;
 use puffin_package::package_name::PackageName;
 
 use crate::printer::Printer;
@@ -31,7 +32,7 @@ impl WheelFinderReporter {
 }
 
 impl puffin_resolver::WheelFinderReporter for WheelFinderReporter {
-    fn on_progress(&self, package: &puffin_resolver::PinnedPackage) {
+    fn on_progress(&self, package: &RemoteDistribution) {
         self.progress
             .set_message(format!("{}=={}", package.name(), package.version()));
         self.progress.inc(1);

crates/puffin-dispatch/Cargo.toml

@@ -17,6 +17,7 @@ platform-host = { path = "../platform-host" }
 platform-tags = { path = "../platform-tags" }
 puffin-build = { path = "../puffin-build" }
 puffin-client = { path = "../puffin-client" }
+puffin-distribution = { path = "../puffin-distribution" }
 puffin-installer = { path = "../puffin-installer" }
 puffin-interpreter = { path = "../puffin-interpreter" }
 puffin-package = { path = "../puffin-package" }

crates/puffin-dispatch/src/lib.rs

@@ -15,9 +15,7 @@ use pep508_rs::Requirement;
 use platform_tags::Tags;
 use puffin_build::SourceDistributionBuilder;
 use puffin_client::RegistryClient;
-use puffin_installer::{
+use puffin_installer::{Downloader, Installer, PartitionedRequirements, Unzipper};
-    Downloader, Installer, PartitionedRequirements, RemoteDistribution, Unzipper,
-};
 use puffin_interpreter::{InterpreterInfo, Virtualenv};
 use puffin_resolver::{Manifest, PreReleaseMode, ResolutionMode, Resolver, WheelFinder};
 use puffin_traits::BuildContext;
@@ -131,10 +129,7 @@ impl BuildContext for BuildDispatch {
                     .resolve(&remote)
                     .await
                     .context("Failed to resolve build dependencies")?;
-                resolution
+                resolution.into_distributions().collect::<Vec<_>>()
-                    .into_files()
-                    .map(RemoteDistribution::from_file)
-                    .collect::<Result<Vec<_>>>()?
             };
 
             // Download any missing distributions.

crates/puffin-distribution/Cargo.toml

@@ -0,0 +1,17 @@
+[package]
+name = "puffin-distribution"
+version = "0.1.0"
+edition = { workspace = true }
+rust-version = { workspace = true }
+homepage = { workspace = true }
+documentation = { workspace = true }
+repository = { workspace = true }
+authors = { workspace = true }
+license = { workspace = true }
+
+[dependencies]
+distribution-filename = { path = "../distribution-filename" }
+pep440_rs = { path = "../pep440-rs" }
+puffin-package = { path = "../puffin-package" }
+
+anyhow = { workspace = true }

crates/puffin-distribution/src/lib.rs

@@ -75,6 +75,15 @@ pub struct RemoteDistribution {
 }
 
 impl RemoteDistribution {
+    /// Initialize a new [`RemoteDistribution`].
+    pub fn new(name: PackageName, version: Version, file: File) -> Self {
+        Self {
+            name,
+            version,
+            file,
+        }
+    }
+
     /// Try to parse a remote distribution from a remote file (like `django-5.0a1-py3-none-any.whl`).
     pub fn from_file(file: File) -> Result<Self> {
         let filename = WheelFilename::from_str(&file.filename)?;
@@ -128,7 +137,7 @@ impl CachedDistribution {
     }
 
     /// 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>> {
+    pub fn try_from_path(path: &Path) -> Result<Option<Self>> {
         let Some(file_name) = path.file_name() else {
             return Ok(None);
         };
@@ -194,7 +203,7 @@ impl InstalledDistribution {
     /// 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>> {
+    pub 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);

crates/puffin-installer/Cargo.toml

@@ -14,6 +14,7 @@ install-wheel-rs = { path = "../install-wheel-rs", default-features = false }
 pep440_rs = { path = "../pep440-rs" }
 pep508_rs = { path = "../pep508-rs" }
 puffin-client = { path = "../puffin-client" }
+puffin-distribution = { path = "../puffin-distribution" }
 puffin-interpreter = { path = "../puffin-interpreter" }
 puffin-package = { path = "../puffin-package" }
 distribution-filename = { path = "../distribution-filename" }

crates/puffin-installer/src/downloader.rs

@@ -10,10 +10,9 @@ use url::Url;
 
 use pep440_rs::Version;
 use puffin_client::RegistryClient;
+use puffin_distribution::RemoteDistribution;
 use puffin_package::package_name::PackageName;
 
-use crate::distribution::RemoteDistribution;
-
 pub struct Downloader<'a> {
     client: &'a RegistryClient,
     cache: Option<&'a Path>,

crates/puffin-installer/src/installer.rs

@@ -2,11 +2,10 @@ use anyhow::{Context, Error, Result};
 use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
 
 use pep440_rs::Version;
+use puffin_distribution::CachedDistribution;
 use puffin_interpreter::Virtualenv;
 use puffin_package::package_name::PackageName;
 
-use crate::CachedDistribution;
-
 pub struct Installer<'a> {
     venv: &'a Virtualenv,
     link_mode: install_wheel_rs::linker::LinkMode,

crates/puffin-installer/src/lib.rs

@@ -1,6 +1,3 @@
-pub use distribution::{
-    CachedDistribution, Distribution, InstalledDistribution, RemoteDistribution,
-};
 pub use downloader::{Downloader, Reporter as DownloadReporter};
 pub use installer::{Installer, Reporter as InstallReporter};
 pub use local_index::LocalIndex;
@@ -10,7 +7,6 @@ pub use uninstall::uninstall;
 pub use unzipper::{Reporter as UnzipReporter, Unzipper};
 
 mod cache;
-mod distribution;
 mod downloader;
 mod installer;
 mod local_index;

crates/puffin-installer/src/local_index.rs

@@ -3,10 +3,10 @@ use std::path::Path;
 
 use anyhow::Result;
 
+use puffin_distribution::CachedDistribution;
 use puffin_package::package_name::PackageName;
 
 use crate::cache::WheelCache;
-use crate::distribution::CachedDistribution;
 
 /// A local index of cached distributions.
 #[derive(Debug, Default)]

crates/puffin-installer/src/plan.rs

@@ -4,10 +4,11 @@ use anyhow::Result;
 use tracing::debug;
 
 use pep508_rs::Requirement;
+use puffin_distribution::{CachedDistribution, InstalledDistribution};
 use puffin_interpreter::Virtualenv;
 use puffin_package::package_name::PackageName;
 
-use crate::{CachedDistribution, InstalledDistribution, LocalIndex, SitePackages};
+use crate::{LocalIndex, SitePackages};
 
 #[derive(Debug, Default)]
 pub struct PartitionedRequirements {

crates/puffin-installer/src/site_packages.rs

@@ -2,12 +2,11 @@ use std::collections::BTreeMap;
 
 use anyhow::Result;
 use fs_err as fs;
+
+use puffin_distribution::InstalledDistribution;
 use puffin_interpreter::Virtualenv;
-
 use puffin_package::package_name::PackageName;
 
-use crate::InstalledDistribution;
-
 #[derive(Debug, Default)]
 pub struct SitePackages(BTreeMap<PackageName, InstalledDistribution>);
 

crates/puffin-installer/src/uninstall.rs

@@ -1,6 +1,6 @@
 use anyhow::Result;
 
-use crate::InstalledDistribution;
+use puffin_distribution::InstalledDistribution;
 
 /// Uninstall a package from the specified Python environment.
 pub async fn uninstall(

crates/puffin-installer/src/unzipper.rs

@@ -8,12 +8,12 @@ use tracing::debug;
 use zip::ZipArchive;
 
 use pep440_rs::Version;
+use puffin_distribution::CachedDistribution;
 use puffin_package::package_name::PackageName;
 
 use crate::cache::WheelCache;
 use crate::downloader::InMemoryDistribution;
 use crate::vendor::CloneableSeekableReader;
-use crate::CachedDistribution;
 
 #[derive(Default)]
 pub struct Unzipper {

crates/puffin-resolver/Cargo.toml

@@ -17,6 +17,7 @@ platform-host = { path = "../platform-host" }
 platform-tags = { path = "../platform-tags" }
 pubgrub = { path = "../../vendor/pubgrub" }
 puffin-client = { path = "../puffin-client" }
+puffin-distribution = { path = "../puffin-distribution" }
 puffin-package = { path = "../puffin-package" }
 puffin-traits = { path = "../puffin-traits" }
 distribution-filename = { path = "../distribution-filename" }

crates/puffin-resolver/src/lib.rs

@@ -1,7 +1,7 @@
 pub use error::ResolveError;
 pub use manifest::Manifest;
 pub use prerelease_mode::PreReleaseMode;
-pub use resolution::{Graph, PinnedPackage};
+pub use resolution::Graph;
 pub use resolution_mode::ResolutionMode;
 pub use resolver::{Reporter as ResolverReporter, Resolver};
 pub use source_distribution::BuiltSourceDistributionCache;

crates/puffin-resolver/src/resolution.rs

@@ -9,71 +9,33 @@ use pubgrub::type_aliases::SelectedDependencies;
 
 use pep440_rs::{Version, VersionSpecifier, VersionSpecifiers};
 use pep508_rs::{Requirement, VersionOrUrl};
+use puffin_distribution::RemoteDistribution;
 use puffin_package::package_name::PackageName;
 use puffin_package::pypi_types::File;
 
 use crate::pubgrub::{PubGrubPackage, PubGrubPriority, PubGrubVersion};
 
-/// A package pinned at a specific version.
-#[derive(Debug)]
-pub struct PinnedPackage {
-    name: PackageName,
-    version: Version,
-    file: File,
-}
-
-impl PinnedPackage {
-    /// Initialize a new pinned package.
-    pub fn new(name: PackageName, version: Version, file: File) -> Self {
-        Self {
-            name,
-            version,
-            file,
-        }
-    }
-
-    /// Return the name of the pinned package.
-    pub fn name(&self) -> &PackageName {
-        &self.name
-    }
-
-    /// Return the version of the pinned package.
-    pub fn version(&self) -> &Version {
-        &self.version
-    }
-
-    /// Return the file of the pinned package.
-    pub fn file(&self) -> &File {
-        &self.file
-    }
-}
-
 /// A set of packages pinned at specific versions.
 #[derive(Debug, Default)]
-pub struct Resolution(FxHashMap<PackageName, PinnedPackage>);
+pub struct Resolution(FxHashMap<PackageName, RemoteDistribution>);
 
 impl Resolution {
     /// Create a new resolution from the given pinned packages.
-    pub(crate) fn new(packages: FxHashMap<PackageName, PinnedPackage>) -> Self {
+    pub(crate) fn new(packages: FxHashMap<PackageName, RemoteDistribution>) -> Self {
         Self(packages)
     }
 
-    /// Iterate over the pinned packages in this resolution.
+    /// Return the distribution for the given package name, if it exists.
-    pub fn iter(&self) -> impl Iterator<Item = (&PackageName, &PinnedPackage)> {
+    pub fn get(&self, package_name: &PackageName) -> Option<&RemoteDistribution> {
-        self.0.iter()
-    }
-
-    /// Iterate over the wheels in this resolution.
-    pub fn into_files(self) -> impl Iterator<Item = File> {
-        self.0.into_values().map(|package| package.file)
-    }
-
-    /// Return the pinned package for the given package name, if it exists.
-    pub fn get(&self, package_name: &PackageName) -> Option<&PinnedPackage> {
         self.0.get(package_name)
     }
 
-    /// Return the number of pinned packages in this resolution.
+    /// Iterate over the [`RemoteDistribution`] entities in this resolution.
+    pub fn into_distributions(self) -> impl Iterator<Item = RemoteDistribution> {
+        self.0.into_values()
+    }
+
+    /// Return the number of distributions in this resolution.
     pub fn len(&self) -> usize {
         self.0.len()
     }
@@ -87,7 +49,7 @@ impl Resolution {
 /// A complete resolution graph in which every node represents a pinned package and every edge
 /// represents a dependency between two pinned packages.
 #[derive(Debug)]
-pub struct Graph(petgraph::graph::Graph<PinnedPackage, (), petgraph::Directed>);
+pub struct Graph(petgraph::graph::Graph<RemoteDistribution, (), petgraph::Directed>);
 
 impl Graph {
     /// Create a new graph from the resolved `PubGrub` state.
@@ -113,7 +75,7 @@ impl Graph {
                 .and_then(|versions| versions.get(&version))
                 .unwrap()
                 .clone();
-            let pinned_package = PinnedPackage::new(package_name.clone(), version, file);
+            let pinned_package = RemoteDistribution::new(package_name.clone(), version, file);
             let index = graph.add_node(pinned_package);
 
             inverse.insert(package_name, index);
@@ -165,10 +127,10 @@ impl Graph {
         self.0
             .node_indices()
             .map(|node| Requirement {
-                name: self.0[node].name.to_string(),
+                name: self.0[node].name().to_string(),
                 extras: None,
                 version_or_url: Some(VersionOrUrl::VersionSpecifier(VersionSpecifiers::from(
-                    VersionSpecifier::equals_version(self.0[node].version.clone()),
+                    VersionSpecifier::equals_version(self.0[node].version().clone()),
                 ))),
                 marker: None,
             })

crates/puffin-resolver/src/wheel_finder.rs

@@ -15,11 +15,12 @@ use distribution_filename::WheelFilename;
 use pep508_rs::Requirement;
 use platform_tags::Tags;
 use puffin_client::RegistryClient;
+use puffin_distribution::RemoteDistribution;
 use puffin_package::package_name::PackageName;
 use puffin_package::pypi_types::{File, Metadata21, SimpleJson};
 
 use crate::error::ResolveError;
-use crate::resolution::{PinnedPackage, Resolution};
+use crate::resolution::Resolution;
 
 pub struct WheelFinder<'a> {
     tags: &'a Tags,
@@ -81,7 +82,7 @@ impl<'a> WheelFinder<'a> {
         }
 
         // Resolve the requirements.
-        let mut resolution: FxHashMap<PackageName, PinnedPackage> =
+        let mut resolution: FxHashMap<PackageName, RemoteDistribution> =
             FxHashMap::with_capacity_and_hasher(requirements.len(), BuildHasherDefault::default());
 
         while let Some(chunk) = package_stream.next().await {
@@ -113,7 +114,7 @@ impl<'a> WheelFinder<'a> {
                             metadata.name, metadata.version, file.filename
                         );
 
-                        let package = PinnedPackage::new(
+                        let package = RemoteDistribution::new(
                             PackageName::normalize(&metadata.name),
                             metadata.version,
                             file,
@@ -161,7 +162,7 @@ enum Response {
 
 pub trait Reporter: Send + Sync {
     /// Callback to invoke when a package is resolved to a wheel.
-    fn on_progress(&self, package: &PinnedPackage);
+    fn on_progress(&self, package: &RemoteDistribution);
 
     /// Callback to invoke when the resolution is complete.
     fn on_complete(&self);