Incorporate build tag into wheel prioritization (#3781)

## Summary

It turns out that in the
[spec](https://packaging.python.org/en/latest/specifications/binary-distribution-format/#file-name-convention),
if a wheel filename includes a build tag, then we need to use it to
break ties. This PR implements that behavior. (Previously, we dropped
the build tag entirely.)

Closes #3779.

## Test Plan

Run: `cargo run pip install -i https://pypi.anaconda.org/intel/simple
mkl_fft==1.3.8 --python-platform linux --python-version 3.10`. This now
resolves without error. Previously, we selected build tag 63 of
`mkl_fft==1.3.8`, which led to an incompatibility with NumPy. Now, we
select build tag 70.
This commit is contained in:
Charlie Marsh 2024-05-23 17:12:53 -04:00 committed by GitHub
parent 5bebaddc24
commit a9d9a6c13f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 132 additions and 29 deletions

View file

@ -0,0 +1,63 @@
use std::num::ParseIntError;
use std::str::FromStr;
use std::sync::Arc;
#[derive(thiserror::Error, Debug)]
pub enum BuildTagError {
#[error("must not be empty")]
Empty,
#[error("must start with a digit")]
NoLeadingDigit,
#[error(transparent)]
ParseInt(#[from] ParseIntError),
}
/// The optional build tag for a wheel:
///
/// > Must start with a digit. Acts as a tie-breaker if two wheel file names are the same in all
/// > other respects (i.e. name, version, and other tags). Sort as an empty tuple if unspecified,
/// > else sort as a two-item tuple with the first item being the initial digits as an int, and the
/// > second item being the remainder of the tag as a str.
///
/// See: <https://packaging.python.org/en/latest/specifications/binary-distribution-format/#file-name-convention>
#[derive(
Debug,
Clone,
Eq,
PartialEq,
Hash,
Ord,
PartialOrd,
rkyv::Archive,
rkyv::Deserialize,
rkyv::Serialize,
)]
#[archive(check_bytes)]
#[archive_attr(derive(Debug))]
pub struct BuildTag(u32, Option<Arc<str>>);
impl FromStr for BuildTag {
type Err = BuildTagError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
// A build tag must not be empty.
if s.is_empty() {
return Err(BuildTagError::Empty);
}
// A build tag must start with a digit.
let (prefix, suffix) = match s.find(|c: char| !c.is_ascii_digit()) {
// Ex) `abc`
Some(0) => return Err(BuildTagError::NoLeadingDigit),
// Ex) `123abc`
Some(split) => {
let (prefix, suffix) = s.split_at(split);
(prefix, Some(suffix))
}
// Ex) `123`
None => (s, None),
};
Ok(BuildTag(prefix.parse::<u32>()?, suffix.map(Arc::from)))
}
}

View file

@ -3,9 +3,11 @@ use std::fmt::{Display, Formatter};
use std::str::FromStr;
use uv_normalize::PackageName;
pub use build_tag::{BuildTag, BuildTagError};
pub use source_dist::{SourceDistExtension, SourceDistFilename, SourceDistFilenameError};
pub use wheel::{WheelFilename, WheelFilenameError};
mod build_tag;
mod source_dist;
mod wheel;

View file

@ -1,6 +1,6 @@
---
source: crates/distribution-filename/src/wheel.rs
expression: "WheelFilename::from_str(\"foo-1.2.3-build-python-abi-platform.whl\")"
expression: "WheelFilename::from_str(\"foo-1.2.3-12-python-abi-platform.whl\")"
---
Ok(
WheelFilename {
@ -8,6 +8,12 @@ Ok(
"foo",
),
version: "1.2.3",
build_tag: Some(
BuildTag(
12,
None,
),
),
python_tag: [
"python",
],

View file

@ -8,6 +8,7 @@ Ok(
"foo",
),
version: "1.2.3",
build_tag: None,
python_tag: [
"ab",
"cd",

View file

@ -8,6 +8,7 @@ Ok(
"foo",
),
version: "1.2.3",
build_tag: None,
python_tag: [
"foo",
],

View file

@ -9,12 +9,15 @@ use pep440_rs::{Version, VersionParseError};
use platform_tags::{TagCompatibility, Tags};
use uv_normalize::{InvalidNameError, PackageName};
use crate::{BuildTag, BuildTagError};
#[derive(Debug, Clone, Eq, PartialEq, Hash, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[archive(check_bytes)]
#[archive_attr(derive(Debug))]
pub struct WheelFilename {
pub name: PackageName,
pub version: Version,
pub build_tag: Option<BuildTag>,
pub python_tag: Vec<String>,
pub abi_tag: Vec<String>,
pub platform_tag: Vec<String>,
@ -57,16 +60,6 @@ impl WheelFilename {
compatible_tags.compatibility(&self.python_tag, &self.abi_tag, &self.platform_tag)
}
/// Get the tag for this wheel.
pub fn get_tag(&self) -> String {
format!(
"{}-{}-{}",
self.python_tag.join("."),
self.abi_tag.join("."),
self.platform_tag.join(".")
)
}
/// The wheel filename without the extension.
pub fn stem(&self) -> String {
format!(
@ -82,6 +75,16 @@ impl WheelFilename {
Self::parse(stem, stem)
}
/// Get the tag for this wheel.
fn get_tag(&self) -> String {
format!(
"{}-{}-{}",
self.python_tag.join("."),
self.abi_tag.join("."),
self.platform_tag.join(".")
)
}
/// Parse a wheel filename from the stem (e.g., `foo-1.2.3-py3-none-any`).
///
/// The originating `filename` is used for high-fidelity error messages.
@ -129,7 +132,7 @@ impl WheelFilename {
));
};
let (name, version, python_tag, abi_tag, platform_tag) =
let (name, version, build_tag, python_tag, abi_tag, platform_tag) =
if let Some(platform_tag) = parts.next() {
if parts.next().is_some() {
return Err(WheelFilenameError::InvalidWheelFileName(
@ -140,6 +143,7 @@ impl WheelFilename {
(
name,
version,
Some(build_tag_or_python_tag),
python_tag_or_abi_tag,
abi_tag_or_platform_tag,
platform_tag,
@ -148,6 +152,7 @@ impl WheelFilename {
(
name,
version,
None,
build_tag_or_python_tag,
python_tag_or_abi_tag,
abi_tag_or_platform_tag,
@ -158,9 +163,16 @@ impl WheelFilename {
.map_err(|err| WheelFilenameError::InvalidPackageName(filename.to_string(), err))?;
let version = Version::from_str(version)
.map_err(|err| WheelFilenameError::InvalidVersion(filename.to_string(), err))?;
let build_tag = build_tag
.map(|build_tag| {
BuildTag::from_str(build_tag)
.map_err(|err| WheelFilenameError::InvalidBuildTag(filename.to_string(), err))
})
.transpose()?;
Ok(Self {
name,
version,
build_tag,
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(),
@ -214,10 +226,12 @@ impl Serialize for WheelFilename {
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}")]
#[error("The wheel filename \"{0}\" has an invalid version: {1}")]
InvalidVersion(String, VersionParseError),
#[error("The wheel filename \"{0}\" has an invalid package name")]
InvalidPackageName(String, InvalidNameError),
#[error("The wheel filename \"{0}\" has an invalid build tag: {1}")]
InvalidBuildTag(String, BuildTagError),
}
#[cfg(test)]
@ -276,7 +290,13 @@ mod tests {
#[test]
fn err_invalid_version() {
let err = WheelFilename::from_str("foo-x.y.z-python-abi-platform.whl").unwrap_err();
insta::assert_snapshot!(err, @r###"The wheel filename "foo-x.y.z-python-abi-platform.whl" has an invalid version part: expected version to start with a number, but no leading ASCII digits were found"###);
insta::assert_snapshot!(err, @r###"The wheel filename "foo-x.y.z-python-abi-platform.whl" has an invalid version: expected version to start with a number, but no leading ASCII digits were found"###);
}
#[test]
fn err_invalid_build_tag() {
let err = WheelFilename::from_str("foo-1.2.3-tag-python-abi-platform.whl").unwrap_err();
insta::assert_snapshot!(err, @r###"The wheel filename "foo-1.2.3-tag-python-abi-platform.whl" has an invalid build tag: must start with a digit"###);
}
#[test]
@ -294,7 +314,7 @@ mod tests {
#[test]
fn ok_build_tag() {
insta::assert_debug_snapshot!(WheelFilename::from_str(
"foo-1.2.3-build-python-abi-platform.whl"
"foo-1.2.3-12-python-abi-platform.whl"
));
}

View file

@ -1,3 +1,4 @@
use distribution_filename::BuildTag;
use std::fmt::{Display, Formatter};
use pep440_rs::VersionSpecifiers;
@ -134,7 +135,7 @@ impl Display for IncompatibleDist {
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum WheelCompatibility {
Incompatible(IncompatibleWheel),
Compatible(HashComparison, TagPriority),
Compatible(HashComparison, TagPriority, Option<BuildTag>),
}
#[derive(Debug, PartialEq, Eq, Clone)]
@ -245,7 +246,7 @@ impl PrioritizedDist {
// source distribution with a matching hash over a wheel with a mismatched hash. When
// the outcomes are equivalent (e.g., both have a matching hash), prefer the wheel.
(
Some((wheel, WheelCompatibility::Compatible(wheel_hash, tag_priority))),
Some((wheel, WheelCompatibility::Compatible(wheel_hash, tag_priority, ..))),
Some((sdist, SourceDistCompatibility::Compatible(sdist_hash))),
) => {
if sdist_hash > wheel_hash {
@ -262,7 +263,7 @@ impl PrioritizedDist {
}
}
// Prefer the highest-priority, platform-compatible wheel.
(Some((wheel, WheelCompatibility::Compatible(_, tag_priority))), _) => {
(Some((wheel, WheelCompatibility::Compatible(_, tag_priority, ..))), _) => {
Some(CompatibleDist::CompatibleWheel {
wheel,
priority: *tag_priority,
@ -309,7 +310,7 @@ impl PrioritizedDist {
.best_wheel_index
.map(|i| &self.0.wheels[i])
.and_then(|(_, compatibility)| match compatibility {
WheelCompatibility::Compatible(_, _) => None,
WheelCompatibility::Compatible(_, _, _) => None,
WheelCompatibility::Incompatible(incompatibility) => Some(incompatibility),
})
}
@ -415,7 +416,7 @@ impl<'a> CompatibleDist<'a> {
impl WheelCompatibility {
pub fn is_compatible(&self) -> bool {
matches!(self, Self::Compatible(_, _))
matches!(self, Self::Compatible(_, _, _))
}
/// Return `true` if the current compatibility is more compatible than another.
@ -424,12 +425,14 @@ impl WheelCompatibility {
/// Compatible wheel ordering is determined by tag priority.
pub fn is_more_compatible(&self, other: &Self) -> bool {
match (self, other) {
(Self::Compatible(_, _), Self::Incompatible(_)) => true,
(Self::Compatible(_, _, _), Self::Incompatible(_)) => true,
(
Self::Compatible(hash, tag_priority),
Self::Compatible(other_hash, other_tag_priority),
) => (hash, tag_priority) > (other_hash, other_tag_priority),
(Self::Incompatible(_), Self::Compatible(_, _)) => false,
Self::Compatible(hash, tag_priority, build_tag),
Self::Compatible(other_hash, other_tag_priority, other_build_tag),
) => {
(hash, tag_priority, build_tag) > (other_hash, other_tag_priority, other_build_tag)
}
(Self::Incompatible(_), Self::Compatible(_, _, _)) => false,
(Self::Incompatible(incompatibility), Self::Incompatible(other_incompatibility)) => {
incompatibility.is_more_compatible(other_incompatibility)
}

View file

@ -614,7 +614,7 @@ impl CacheBucket {
Self::FlatIndex => "flat-index-v0",
Self::Git => "git-v0",
Self::Interpreter => "interpreter-v1",
Self::Simple => "simple-v7",
Self::Simple => "simple-v8",
Self::Wheels => "wheels-v1",
Self::Archive => "archive-v0",
}

View file

@ -175,7 +175,7 @@ impl FlatIndex {
TagCompatibility::Compatible(priority) => priority,
};
// Check if hashes line up
// Check if hashes line up.
let hash = if let HashPolicy::Validate(required) = hasher.get_package(&filename.name) {
if hashes.is_empty() {
HashComparison::Missing
@ -188,7 +188,10 @@ impl FlatIndex {
HashComparison::Matched
};
WheelCompatibility::Compatible(hash, priority)
// Break ties with the build tag.
let build_tag = filename.build_tag.clone();
WheelCompatibility::Compatible(hash, priority, build_tag)
}
/// Get the [`FlatDistributions`] for the given package name.

View file

@ -56,6 +56,7 @@ Ok(
"anyio",
),
version: "4.3.0",
build_tag: None,
python_tag: [
"py3",
],

View file

@ -554,7 +554,10 @@ impl VersionMapLazy {
}
};
WheelCompatibility::Compatible(hash, priority)
// Break ties with the build tag.
let build_tag = filename.build_tag.clone();
WheelCompatibility::Compatible(hash, priority, build_tag)
}
}