Add source distribution filename abstraction (#154)

The need for this became clear when working on the source distribution
integration into the resolver.

While at it i also switch the `WheelFilename` version to the parsed
`pep440_rs` version now that we have this crate.

Cargo.lock

@@ -676,6 +676,16 @@ dependencies = [
  "windows-sys 0.48.0",
 ]
 
+[[package]]
+name = "distribution-filename"
+version = "0.0.1"
+dependencies = [
+ "pep440_rs 0.3.12",
+ "platform-tags",
+ "puffin-package",
+ "thiserror",
+]
+
 [[package]]
 name = "doc-comment"
 version = "0.3.3"
@@ -990,6 +1000,7 @@ dependencies = [
  "clap",
  "configparser",
  "dirs",
+ "distribution-filename",
  "fs-err",
  "install-wheel-rs",
  "rayon",
@@ -1001,7 +1012,6 @@ dependencies = [
  "thiserror",
  "tracing",
  "tracing-subscriber",
- "wheel-filename",
  "which",
 ]
 
@@ -1322,6 +1332,7 @@ dependencies = [
  "configparser",
  "csv",
  "data-encoding",
+ "distribution-filename",
  "fs-err",
  "fs2",
  "fxhash",
@@ -1346,7 +1357,6 @@ dependencies = [
  "tracing",
  "tracing-subscriber",
  "walkdir",
- "wheel-filename",
  "zip",
 ]
 
@@ -2101,6 +2111,7 @@ version = "0.0.1"
 dependencies = [
  "anyhow",
  "cacache",
+ "distribution-filename",
  "fs-err",
  "install-wheel-rs",
  "pep440_rs 0.3.12",
@@ -2113,7 +2124,6 @@ dependencies = [
  "tokio-util",
  "tracing",
  "url",
- "wheel-filename",
  "zip",
 ]
 
@@ -2164,6 +2174,7 @@ dependencies = [
  "anyhow",
  "bitflags 2.4.1",
  "colored",
+ "distribution-filename",
  "futures",
  "fxhash",
  "insta",
@@ -2180,7 +2191,6 @@ dependencies = [
  "tokio",
  "tracing",
  "waitmap",
- "wheel-filename",
 ]
 
 [[package]]
@@ -3557,14 +3567,6 @@ dependencies = [
  "wasm-bindgen",
 ]
 
-[[package]]
-name = "wheel-filename"
-version = "0.0.1"
-dependencies = [
- "platform-tags",
- "thiserror",
-]
-
 [[package]]
 name = "which"
 version = "4.4.2"

crates/README.md

@@ -40,6 +40,6 @@ Types and functionality for working with Python packages, e.g., parsing wheel fi
 
 Functionality for resolving Python packages and their dependencies.
 
-## [wheel-filename](./wheel-filename)
+## [distribution-filename](./distribution-filename)
 
 Functionality for parsing wheel filenames as per [PEP 427](https://peps.python.org/pep-0427/).

crates/distribution-filename/Cargo.toml

@@ -1,5 +1,5 @@
 [package]
-name = "wheel-filename"
+name = "distribution-filename"
 version = "0.0.1"
 edition = { workspace = true }
 rust-version = { workspace = true }
@@ -11,5 +11,7 @@ license = { workspace = true }
 
 [dependencies]
 platform-tags = { path = "../platform-tags" }
+puffin-package = { path = "../puffin-package" }
+pep440_rs = { path = "../pep440-rs" }
 
 thiserror = { workspace = true }

crates/distribution-filename/src/lib.rs

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

crates/distribution-filename/src/source_distribution.rs

@@ -0,0 +1,142 @@
+use pep440_rs::Version;
+use puffin_package::package_name::PackageName;
+use std::fmt::{Display, Formatter};
+use std::str::FromStr;
+use thiserror::Error;
+
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum SourceDistributionExtension {
+    Zip,
+    TarGz,
+}
+
+impl FromStr for SourceDistributionExtension {
+    type Err = SourceDistributionFilenameError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(match s {
+            "zip" => Self::Zip,
+            "tar.gz" => Self::TarGz,
+            other => {
+                return Err(SourceDistributionFilenameError::InvalidExtension(
+                    other.to_string(),
+                ))
+            }
+        })
+    }
+}
+
+impl Display for SourceDistributionExtension {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            SourceDistributionExtension::Zip => f.write_str("zip"),
+            SourceDistributionExtension::TarGz => f.write_str("tar.gz"),
+        }
+    }
+}
+
+impl SourceDistributionExtension {
+    pub fn from_filename(filename: &str) -> Option<(&str, Self)> {
+        if let Some(stem) = filename.strip_suffix(".zip") {
+            return Some((stem, Self::Zip));
+        }
+        if let Some(stem) = filename.strip_suffix(".tar.gz") {
+            return Some((stem, Self::TarGz));
+        }
+        None
+    }
+}
+
+/// Note that this is a normalized and not an exact representation, keep the original string if you
+/// need the latter.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct SourceDistributionFilename {
+    pub name: PackageName,
+    pub version: Version,
+    pub extension: SourceDistributionExtension,
+}
+
+impl SourceDistributionFilename {
+    /// No `FromStr` impl since we need to know the package name to be able to reasonable parse
+    /// these (consider e.g. `a-1-1.zip`)
+    pub fn parse(
+        filename: &str,
+        package_name: &PackageName,
+    ) -> Result<Self, SourceDistributionFilenameError> {
+        let Some((stem, extension)) = SourceDistributionExtension::from_filename(filename) else {
+            return Err(SourceDistributionFilenameError::InvalidExtension(
+                filename.to_string(),
+            ));
+        };
+
+        if stem.len() <= package_name.as_ref().len() + "-".len()
+            || &PackageName::normalize(&stem[..package_name.as_ref().len()]) != package_name
+        {
+            return Err(SourceDistributionFilenameError::InvalidFilename {
+                filename: filename.to_string(),
+                package_name: package_name.to_string(),
+            });
+        }
+
+        // We checked the length above
+        let version = Version::from_str(&stem[package_name.as_ref().len() + "-".len()..])
+            .map_err(SourceDistributionFilenameError::InvalidVersion)?;
+
+        Ok(Self {
+            name: package_name.clone(),
+            version,
+            extension,
+        })
+    }
+}
+
+impl Display for SourceDistributionFilename {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}-{}.{}", self.name, self.version, self.extension)
+    }
+}
+
+#[derive(Error, Debug, Clone)]
+pub enum SourceDistributionFilenameError {
+    #[error("Source distribution name {filename} doesn't start with package name {package_name}")]
+    InvalidFilename {
+        filename: String,
+        package_name: String,
+    },
+    #[error("Source distributions filenames must end with .zip or .tar.gz, not {0}")]
+    InvalidExtension(String),
+    #[error("Source distribution filename version section is invalid: {0}")]
+    InvalidVersion(String),
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::SourceDistributionFilename;
+    use puffin_package::package_name::PackageName;
+
+    /// Only test already normalized names since the parsing is lossy
+    #[test]
+    fn roundtrip() {
+        for normalized in [
+            "foo-lib-1.2.3.zip",
+            "foo-lib-1.2.3a3.zip",
+            "foo-lib-1.2.3.tar.gz",
+        ] {
+            assert_eq!(
+                SourceDistributionFilename::parse(normalized, &PackageName::normalize("foo_lib"))
+                    .unwrap()
+                    .to_string(),
+                normalized
+            );
+        }
+    }
+
+    #[test]
+    fn errors() {
+        for invalid in ["b-1.2.3.zip", "a-1.2.3-gamma.3.zip", "a-1.2.3.tar.zstd"] {
+            assert!(
+                SourceDistributionFilename::parse(invalid, &PackageName::normalize("a")).is_err()
+            );
+        }
+    }
+}

crates/distribution-filename/src/wheel_filename.rs

@@ -1,5 +1,7 @@
+use std::fmt::{Display, Formatter};
 use std::str::FromStr;
 
+use pep440_rs::Version;
 use thiserror::Error;
 
 use platform_tags::Tags;
@@ -7,31 +9,38 @@ use platform_tags::Tags;
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct WheelFilename {
     pub distribution: String,
-    pub version: String,
+    pub version: Version,
     pub python_tag: Vec<String>,
     pub abi_tag: Vec<String>,
     pub platform_tag: Vec<String>,
 }
 
 impl FromStr for WheelFilename {
-    type Err = Error;
+    type Err = WheelFilenameError;
 
     fn from_str(filename: &str) -> Result<Self, Self::Err> {
         let basename = filename.strip_suffix(".whl").ok_or_else(|| {
-            Error::InvalidWheelFileName(filename.to_string(), "Must end with .whl".to_string())
+            WheelFilenameError::InvalidWheelFileName(
+                filename.to_string(),
+                "Must end with .whl".to_string(),
+            )
         })?;
         // https://www.python.org/dev/peps/pep-0427/#file-name-convention
         match basename.split('-').collect::<Vec<_>>().as_slice() {
             // TODO(charlie): Build tag precedence
             &[distribution, version, _, python_tag, abi_tag, platform_tag]
-            | &[distribution, version, python_tag, abi_tag, platform_tag] => Ok(WheelFilename {
-                distribution: distribution.to_string(),
-                version: version.to_string(),
-                python_tag: python_tag.split('.').map(String::from).collect(),
-                abi_tag: abi_tag.split('.').map(String::from).collect(),
-                platform_tag: platform_tag.split('.').map(String::from).collect(),
-            }),
-            _ => Err(Error::InvalidWheelFileName(
+            | &[distribution, version, python_tag, abi_tag, platform_tag] => {
+                let version = Version::from_str(version)
+                    .map_err(|err| WheelFilenameError::InvalidVersion(filename.to_string(), err))?;
+                Ok(WheelFilename {
+                    distribution: distribution.to_string(),
+                    version,
+                    python_tag: python_tag.split('.').map(String::from).collect(),
+                    abi_tag: abi_tag.split('.').map(String::from).collect(),
+                    platform_tag: platform_tag.split('.').map(String::from).collect(),
+                })
+            }
+            _ => Err(WheelFilenameError::InvalidWheelFileName(
                 filename.to_string(),
                 "Expected four \"-\" in the filename".to_string(),
             )),
@@ -39,6 +48,18 @@ impl FromStr for WheelFilename {
     }
 }
 
+impl Display for WheelFilename {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "{}-{}-{}.whl",
+            self.distribution,
+            self.version,
+            self.get_tag()
+        )
+    }
+}
+
 impl WheelFilename {
     /// Returns `true` if the wheel is compatible with the given tags.
     pub fn is_compatible(&self, compatible_tags: &Tags) -> bool {
@@ -65,7 +86,9 @@ impl WheelFilename {
 }
 
 #[derive(Error, Debug)]
-pub enum Error {
+pub enum WheelFilenameError {
     #[error("The wheel filename \"{0}\" is invalid: {1}")]
     InvalidWheelFileName(String, String),
+    #[error("The wheel filename \"{0}\" has an invalid version part: {1}")]
+    InvalidVersion(String, String),
 }

crates/gourgeist/Cargo.toml

@@ -15,7 +15,7 @@ license = { workspace = true }
 
 [dependencies]
 install-wheel-rs = { path = "../install-wheel-rs", optional = true }
-wheel-filename = { path = "../wheel-filename" }
+distribution-filename = { path = "../distribution-filename" }
 
 camino = { workspace = true }
 clap = { workspace = true }

crates/gourgeist/src/packages.rs

@@ -10,8 +10,8 @@ use rayon::iter::{IntoParallelIterator, ParallelIterator};
 use tempfile::NamedTempFile;
 use tracing::debug;
 
+use distribution_filename::WheelFilename;
 use install_wheel_rs::{install_wheel, InstallLocation};
-use wheel_filename::WheelFilename;
 
 use crate::bare::VenvPaths;
 use crate::interpreter::InterpreterInfo;

crates/install-wheel-rs/Cargo.toml

@@ -18,7 +18,7 @@ name = "install_wheel_rs"
 
 [dependencies]
 platform-host = { path = "../platform-host" }
-wheel-filename = { path = "../wheel-filename" }
+distribution-filename = { path = "../distribution-filename" }
 
 clap = { workspace = true, optional = true, features = ["derive", "env"] }
 configparser = { workspace = true }

crates/install-wheel-rs/src/lib.rs

@@ -42,7 +42,7 @@ pub enum Error {
     InvalidPoetry(String),
     /// Doesn't follow file name schema
     #[error(transparent)]
-    InvalidWheelFileName(#[from] wheel_filename::Error),
+    InvalidWheelFileName(#[from] distribution_filename::WheelFilenameError),
     #[error("Failed to read the wheel file {0}")]
     Zip(String, #[source] ZipError),
     #[error("Failed to run python subcommand")]

crates/install-wheel-rs/src/main.rs

@@ -6,8 +6,8 @@ use fs_err::File;
 #[cfg(feature = "rayon")]
 use rayon::iter::{IntoParallelIterator, ParallelIterator};
 
+use distribution_filename::WheelFilename;
 use install_wheel_rs::{install_wheel, Error, InstallLocation};
-use wheel_filename::WheelFilename;
 
 /// Low level install CLI, mainly used for testing
 #[derive(Parser)]

crates/install-wheel-rs/src/python_bindings.rs

@@ -1,6 +1,7 @@
 #![allow(clippy::format_push_string)] // I will not replace clear and infallible with fallible, io looking code
 
 use crate::{install_wheel, Error, InstallLocation, LockedDir};
+use distribution_filename::WheelFilename;
 use fs_err::File;
 use pyo3::create_exception;
 use pyo3::types::PyModule;
@@ -8,7 +9,6 @@ use pyo3::{pyclass, pymethods, pymodule, PyErr, PyResult, Python};
 use std::env;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
-use wheel_filename::WheelFilename;
 
 create_exception!(
     install_wheel_rs,

crates/install-wheel-rs/src/wheel.rs

@@ -19,7 +19,7 @@ use zip::result::ZipError;
 use zip::write::FileOptions;
 use zip::{ZipArchive, ZipWriter};
 
-use wheel_filename::WheelFilename;
+use distribution_filename::WheelFilename;
 
 use crate::install_location::{InstallLocation, LockedDir};
 use crate::record::RecordEntry;

crates/puffin-build/src/lib.rs

@@ -326,7 +326,7 @@ impl SourceDistributionBuilder {
         let wheel = stdout
             .lines()
             .last()
-            .map(|wheel_filename| wheel_dir.join(wheel_filename));
+            .map(|distribution_filename| wheel_dir.join(distribution_filename));
         let Some(wheel) = wheel.filter(|wheel| wheel.is_file()) else {
             return Err(Error::from_command_output(
                 "Build backend did not return the wheel filename through `build_wheel()`"

crates/puffin-installer/Cargo.toml

@@ -15,7 +15,7 @@ pep440_rs = { path = "../pep440-rs" }
 puffin-client = { path = "../puffin-client" }
 puffin-interpreter = { path = "../puffin-interpreter" }
 puffin-package = { path = "../puffin-package" }
-wheel-filename = { path = "../wheel-filename" }
+distribution-filename = { path = "../distribution-filename" }
 
 anyhow = { workspace = true }
 cacache = { workspace = true }

crates/puffin-installer/src/distribution.rs

@@ -3,11 +3,11 @@ use std::str::FromStr;
 
 use anyhow::{anyhow, Result};
 
+use distribution_filename::WheelFilename;
 use pep440_rs::Version;
 use puffin_client::File;
 use puffin_package::dist_info_name::DistInfoName;
 use puffin_package::package_name::PackageName;
-use wheel_filename::WheelFilename;
 
 /// A built distribution (wheel), which either exists remotely or locally.
 #[derive(Debug, Clone)]
@@ -79,10 +79,9 @@ impl RemoteDistribution {
     pub fn from_file(file: File) -> Result<Self> {
         let filename = WheelFilename::from_str(&file.filename)?;
         let name = PackageName::normalize(&filename.distribution);
-        let version = Version::from_str(&filename.version).map_err(|err| anyhow!(err))?;
         Ok(Self {
             name,
-            version,
+            version: filename.version.clone(),
             file,
         })
     }

crates/puffin-resolver/Cargo.toml

@@ -17,7 +17,7 @@ platform-tags = { path = "../platform-tags" }
 pubgrub = { path = "../../vendor/pubgrub" }
 puffin-client = { path = "../puffin-client" }
 puffin-package = { path = "../puffin-package" }
-wheel-filename = { path = "../wheel-filename" }
+distribution-filename = { path = "../distribution-filename" }
 
 anyhow = { workspace = true }
 bitflags = { workspace = true }

crates/puffin-resolver/src/resolver.rs

@@ -18,13 +18,13 @@ use tokio::select;
 use tracing::{debug, trace};
 use waitmap::WaitMap;
 
+use distribution_filename::WheelFilename;
 use pep508_rs::{MarkerEnvironment, Requirement};
 use platform_tags::Tags;
 use puffin_client::{File, PypiClient, SimpleJson};
 use puffin_package::dist_info_name::DistInfoName;
 use puffin_package::metadata::Metadata21;
 use puffin_package::package_name::PackageName;
-use wheel_filename::WheelFilename;
 
 use crate::error::ResolveError;
 use crate::pubgrub::package::PubGrubPackage;
@@ -268,17 +268,13 @@ impl<'a> Resolver<'a> {
                     return false;
                 };
 
-                let Ok(version) = pep440_rs::Version::from_str(&name.version) else {
-                    return false;
-                };
-
                 if !name.is_compatible(self.tags) {
                     return false;
                 }
 
                 if !range
                     .borrow()
-                    .contains(&PubGrubVersion::from(version.clone()))
+                    .contains(&PubGrubVersion::from(name.version.clone()))
                 {
                     return false;
                 };
@@ -322,17 +318,13 @@ impl<'a> Resolver<'a> {
                         return None;
                     };
 
-                    let Ok(version) = pep440_rs::Version::from_str(&name.version) else {
-                        return None;
-                    };
-
                     if !name.is_compatible(self.tags) {
                         return None;
                     }
 
                     if !range
                         .borrow()
-                        .contains(&PubGrubVersion::from(version.clone()))
+                        .contains(&PubGrubVersion::from(name.version.clone()))
                     {
                         return None;
                     };
@@ -340,7 +332,7 @@ impl<'a> Resolver<'a> {
                     Some(Wheel {
                         file: file.clone(),
                         name: package_name.clone(),
-                        version: version.clone(),
+                        version: name.version.clone(),
                     })
                 }) else {
                     // Short circuit: we couldn't find _any_ compatible versions for a package.

crates/puffin-resolver/src/wheel_finder.rs

@@ -11,13 +11,12 @@ use futures::{StreamExt, TryFutureExt};
 use fxhash::FxHashMap;
 use tracing::debug;
 
-use pep440_rs::Version;
+use distribution_filename::WheelFilename;
 use pep508_rs::Requirement;
 use platform_tags::Tags;
 use puffin_client::{File, PypiClient, SimpleJson};
 use puffin_package::metadata::Metadata21;
 use puffin_package::package_name::PackageName;
-use wheel_filename::WheelFilename;
 
 use crate::error::ResolveError;
 use crate::resolution::{PinnedPackage, Resolution};
@@ -97,15 +96,11 @@ impl<'a> WheelFinder<'a> {
                                 return false;
                             };
 
-                            let Ok(version) = Version::from_str(&name.version) else {
-                                return false;
-                            };
-
                             if !name.is_compatible(self.tags) {
                                 return false;
                             }
 
-                            requirement.is_satisfied_by(&version)
+                            requirement.is_satisfied_by(&name.version)
                         }) else {
                             return Err(ResolveError::NotFound(requirement));
                         };