diff --git a/Cargo.lock b/Cargo.lock index 7bbaece65..024944a4b 100644 --- a/Cargo.lock +++ b/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" diff --git a/crates/README.md b/crates/README.md index 96dcfe776..53c5b22d3 100644 --- a/crates/README.md +++ b/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/). diff --git a/crates/wheel-filename/Cargo.toml b/crates/distribution-filename/Cargo.toml similarity index 75% rename from crates/wheel-filename/Cargo.toml rename to crates/distribution-filename/Cargo.toml index c5204ef0d..c0cd9ed4a 100644 --- a/crates/wheel-filename/Cargo.toml +++ b/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 } diff --git a/crates/distribution-filename/src/lib.rs b/crates/distribution-filename/src/lib.rs new file mode 100644 index 000000000..97b54a494 --- /dev/null +++ b/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; diff --git a/crates/distribution-filename/src/source_distribution.rs b/crates/distribution-filename/src/source_distribution.rs new file mode 100644 index 000000000..09003f996 --- /dev/null +++ b/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 { + 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 { + 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() + ); + } + } +} diff --git a/crates/wheel-filename/src/lib.rs b/crates/distribution-filename/src/wheel_filename.rs similarity index 57% rename from crates/wheel-filename/src/lib.rs rename to crates/distribution-filename/src/wheel_filename.rs index acebea2e2..3a8e68133 100644 --- a/crates/wheel-filename/src/lib.rs +++ b/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, pub abi_tag: Vec, pub platform_tag: Vec, } impl FromStr for WheelFilename { - type Err = Error; + type Err = WheelFilenameError; fn from_str(filename: &str) -> Result { 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::>().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), } diff --git a/crates/gourgeist/Cargo.toml b/crates/gourgeist/Cargo.toml index cd3f56589..72a4c09b9 100644 --- a/crates/gourgeist/Cargo.toml +++ b/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 } diff --git a/crates/gourgeist/src/packages.rs b/crates/gourgeist/src/packages.rs index 4a1ef2879..bfd6899e7 100644 --- a/crates/gourgeist/src/packages.rs +++ b/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; diff --git a/crates/install-wheel-rs/Cargo.toml b/crates/install-wheel-rs/Cargo.toml index 9b77e61f6..0325d4019 100644 --- a/crates/install-wheel-rs/Cargo.toml +++ b/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 } diff --git a/crates/install-wheel-rs/src/lib.rs b/crates/install-wheel-rs/src/lib.rs index a00d49df1..5c7c658fb 100644 --- a/crates/install-wheel-rs/src/lib.rs +++ b/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")] diff --git a/crates/install-wheel-rs/src/main.rs b/crates/install-wheel-rs/src/main.rs index d68410517..50ef7f5e2 100644 --- a/crates/install-wheel-rs/src/main.rs +++ b/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)] diff --git a/crates/install-wheel-rs/src/python_bindings.rs b/crates/install-wheel-rs/src/python_bindings.rs index 5d83c5371..5982c3efb 100644 --- a/crates/install-wheel-rs/src/python_bindings.rs +++ b/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, diff --git a/crates/install-wheel-rs/src/wheel.rs b/crates/install-wheel-rs/src/wheel.rs index 4e39ace76..a10e19341 100644 --- a/crates/install-wheel-rs/src/wheel.rs +++ b/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; diff --git a/crates/puffin-build/src/lib.rs b/crates/puffin-build/src/lib.rs index c7ed1aa57..e46d1a30f 100644 --- a/crates/puffin-build/src/lib.rs +++ b/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()`" diff --git a/crates/puffin-installer/Cargo.toml b/crates/puffin-installer/Cargo.toml index bdfba2576..55243c5f5 100644 --- a/crates/puffin-installer/Cargo.toml +++ b/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 } diff --git a/crates/puffin-installer/src/distribution.rs b/crates/puffin-installer/src/distribution.rs index 00db9e1b7..b1ad9a171 100644 --- a/crates/puffin-installer/src/distribution.rs +++ b/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 { 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, }) } diff --git a/crates/puffin-resolver/Cargo.toml b/crates/puffin-resolver/Cargo.toml index cc5b8616d..f9a75fa9b 100644 --- a/crates/puffin-resolver/Cargo.toml +++ b/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 } diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index c2b9976b6..c537bdb11 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/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. diff --git a/crates/puffin-resolver/src/wheel_finder.rs b/crates/puffin-resolver/src/wheel_finder.rs index bcc255f4a..fb47af87b 100644 --- a/crates/puffin-resolver/src/wheel_finder.rs +++ b/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)); };