From 1e1bc3cffac4cecfe0a818fb070de6c3a4c40952 Mon Sep 17 00:00:00 2001 From: Jonathan Yoder Date: Fri, 18 Sep 2026 13:58:10 -0400 Subject: [PATCH] feat(wheelname): add ParseTags, which does not require a parseable version Parse rejects a whole filename when only the version fails PEP 440, and the version contributes nothing to a PEP 425 tag. PEP 427 escapes non-alphanumeric runs to "_", so a legal 1.0+cpu reaches disk as 1.0_cpu and does not parse. Over a sweep of 14.3M mirrored PyPI wheels, 221 of 299 rejected filenames failed on the version alone with readable tags, understating real platform compatibility. Un-escaping inside Parse was rejected: the escaping is not losslessly invertible. Both 2.5.0+post1_cpu and 2.5.0_post1+cpu parse from the escaped 2.5.0_post1_cpu and only the second is right, so it would return a wrong version with no error. Both entry points share parseExceptVersion so they cannot disagree about what a wheel filename is. WheelTags is a separate type rather than a WheelFilename with an empty Version, so no caller can read a zero version as a real one. Fixes #55 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 25 +++++++++++++++ wheelname/wheelname.go | 57 +++++++++++++++++++++++++++------ wheelname/wheelname_test.go | 64 +++++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b4612f..fd46a31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,31 @@ mistaken for a safe patch upgrade. ## [Unreleased] +### Added + +- `wheelname`: **`ParseTags` reads a wheel's compatibility tags without requiring + its version to parse**, returning a `WheelTags` of name, build tag and tags. + + `Parse` rejects the whole filename when only the version segment fails PEP 440, + and the version contributes nothing to a PEP 425 tag. PEP 427 escapes runs of + non-alphanumeric characters to `_`, so a legal local version like `1.0+cpu` + reaches disk as `1.0_cpu` and does not parse. Measured over a full-corpus sweep + of 14.3M mirrored PyPI wheels, 221 of the 299 rejected filenames failed on the + version alone with their tags fully readable — and that silently understated + real platform compatibility for the affected releases. + + Un-escaping inside `Parse` was considered and rejected: the escaping is not + losslessly invertible. Both `2.5.0+post1_cpu` and `2.5.0_post1+cpu` parse from + the escaped `2.5.0_post1_cpu` and only the second is correct, so un-escaping + would return a plausible wrong version with no error. `Parse`'s behaviour is + unchanged. + + ⚠️ One trade-off worth knowing: dropping version validation also drops the only + signal that rejected some malformed names. `too-few-py3-none-any.whl` has five + fields and `few` where a version belongs, so `ParseTags` accepts it while + `Parse` does not. Re-validating the version to close that gap would defeat the + function's purpose. Use `Parse` when you need the version. + ## [0.9.0] - 2026-08-28 ### Added diff --git a/wheelname/wheelname.go b/wheelname/wheelname.go index 0efd1d5..4d5beec 100644 --- a/wheelname/wheelname.go +++ b/wheelname/wheelname.go @@ -20,14 +20,57 @@ type WheelFilename struct { Tags []tags.Tag } +// WheelTags is Parse's result without the version: the fields a PEP 425 +// compatibility decision actually needs. It is a distinct type rather than a +// WheelFilename with an empty Version, so a caller cannot read a zero version +// and mistake it for a real one. +type WheelTags struct { + Name string + Build string + Tags []tags.Tag +} + func Parse(filename string) (WheelFilename, error) { + w, verStr, err := parseExceptVersion(filename) + if err != nil { + return WheelFilename{}, err + } + // Parsed as-is. PEP 427 escaping is NOT losslessly invertible -- both + // 2.5.0+post1_cpu and 2.5.0_post1+cpu parse from the escaped + // 2.5.0_post1_cpu, and only the second is right -- so un-escaping here would + // return a plausible wrong version with no error. Callers that want the tags + // of such a wheel should use ParseTags. + ver, err := version.Parse(verStr) + if err != nil { + return WheelFilename{}, fmt.Errorf("wheel %q: version: %w", filename, err) + } + return WheelFilename{Name: w.Name, Version: ver, Build: w.Build, Tags: w.Tags}, nil +} + +// ParseTags is Parse without the version requirement, for callers that want a +// wheel's compatibility tags. The version segment is not parsed and not +// returned; every other field is validated exactly as Parse validates it. +// +// This exists because the version contributes nothing to a PEP 425 tag, yet a +// version that fails PEP 440 made Parse discard tags it had already read +// successfully. PEP 427 escapes a local version's "+" to "_", so a legal +// 1.0+cpu reaches disk as 1.0_cpu and does not parse. +func ParseTags(filename string) (WheelTags, error) { + w, _, err := parseExceptVersion(filename) + return w, err +} + +// parseExceptVersion splits and validates everything but the version, returning +// the unparsed version string alongside. Both entry points go through it so they +// cannot disagree about what a wheel filename is. +func parseExceptVersion(filename string) (WheelTags, string, error) { base, ok := strings.CutSuffix(filename, ".whl") if !ok { - return WheelFilename{}, fmt.Errorf("%w: missing .whl suffix", ErrInvalidWheelFilename) + return WheelTags{}, "", fmt.Errorf("%w: missing .whl suffix", ErrInvalidWheelFilename) } parts := strings.Split(base, "-") if len(parts) != 5 && len(parts) != 6 { - return WheelFilename{}, fmt.Errorf("%w: expected 5 or 6 dash fields, got %d", ErrInvalidWheelFilename, len(parts)) + return WheelTags{}, "", fmt.Errorf("%w: expected 5 or 6 dash fields, got %d", ErrInvalidWheelFilename, len(parts)) } name := strings.ToLower(parts[0]) verStr := parts[1] @@ -36,20 +79,16 @@ func Parse(filename string) (WheelFilename, error) { if len(parts) == 6 { build = parts[2] if build == "" || build[0] < '0' || build[0] > '9' { - return WheelFilename{}, fmt.Errorf("%w: build tag %q must start with a digit", ErrInvalidWheelFilename, build) + return WheelTags{}, "", fmt.Errorf("%w: build tag %q must start with a digit", ErrInvalidWheelFilename, build) } tagStart = 3 } - ver, err := version.Parse(verStr) // as-is; no _->+ un-escape - if err != nil { - return WheelFilename{}, fmt.Errorf("wheel %q: version: %w", filename, err) - } tagComponent := strings.Join(parts[tagStart:tagStart+3], "-") tg, err := tags.ParseTag(tagComponent) if err != nil { - return WheelFilename{}, fmt.Errorf("wheel %q: tag: %w", filename, err) + return WheelTags{}, "", fmt.Errorf("wheel %q: tag: %w", filename, err) } - return WheelFilename{Name: name, Version: ver, Build: build, Tags: tg}, nil + return WheelTags{Name: name, Build: build, Tags: tg}, verStr, nil } // CompareBuildTags orders PEP 427 build tags; absent ("") sorts first; present diff --git a/wheelname/wheelname_test.go b/wheelname/wheelname_test.go index 5a6b20c..55765bd 100644 --- a/wheelname/wheelname_test.go +++ b/wheelname/wheelname_test.go @@ -46,6 +46,70 @@ func TestParse_Malformed(t *testing.T) { } } +// TestParseTags_RecoversTagsFromAnUnparseableVersion is the point of ParseTags. +// PEP 427 escapes a local version's "+" to "_", so a legal 2.5.0.post1+cpu is +// stored as 2.5.0_post1_cpu and fails PEP 440 -- and Parse then discards tags it +// read perfectly well. The version contributes nothing to a PEP 425 tag. +func TestParseTags_RecoversTagsFromAnUnparseableVersion(t *testing.T) { + for _, name := range []string{ + "torch-2.5.0_post1_cpu-cp39-cp39-manylinux1_x86_64.whl", + "dcnnt-0.3.8fix-py3-none-any.whl", + } { + _, err := Parse(name) + require.Error(t, err, "precondition: Parse must reject %s, or this test proves nothing", name) + + w, err := ParseTags(name) + require.NoError(t, err, name) + assert.NotEmpty(t, w.Tags, name) + } +} + +// TestParseTags_AgreesWithParse guards against the two entry points drifting: +// they must split a filename identically, differing only in the version. +func TestParseTags_AgreesWithParse(t *testing.T) { + const name = "foo-1.0-1-py2.py3-none-any.whl" + + full, err := Parse(name) + require.NoError(t, err) + w, err := ParseTags(name) + require.NoError(t, err) + + assert.Equal(t, full.Name, w.Name) + assert.Equal(t, full.Build, w.Build) + assert.Equal(t, full.Tags, w.Tags) +} + +// TestParseTags_StillRejectsRealDefects keeps ParseTags from degenerating into a +// parse-anything function. Only the version becomes optional. +func TestParseTags_StillRejectsRealDefects(t *testing.T) { + for _, s := range []string{ + "nowhlsuffix-1.0-py3-none-any", // no .whl + "foo-1.0-x-py3-none-any.whl", // build tag not digit-led + "foo-1.0-py3--any.whl", // empty ABI field + } { + _, err := ParseTags(s) + require.Error(t, err, s) + } +} + +// TestParseTags_AcceptsWhatOnlyTheVersionRejected records the cost of the +// trade-off, so it is a decision on the record rather than a surprise. A +// five-field name whose second field is not a version is indistinguishable from +// a wheel whose version this library cannot parse, so ParseTags accepts it where +// Parse does not. Re-validating the version to close that gap would defeat the +// function's entire purpose. +func TestParseTags_AcceptsWhatOnlyTheVersionRejected(t *testing.T) { + const name = "too-few-py3-none-any.whl" // 5 fields; "few" sits where a version goes + + _, err := Parse(name) + require.Error(t, err, "Parse rejects it, on the version") + + w, err := ParseTags(name) + require.NoError(t, err) + assert.Equal(t, "too", w.Name) + assert.Equal(t, []tags.Tag{{Interpreter: "py3", ABI: "none", Platform: "any"}}, w.Tags) +} + func TestCompareBuildTags(t *testing.T) { assert.Negative(t, CompareBuildTags("", "1")) // absent < present assert.Negative(t, CompareBuildTags("2", "10")) // numeric, not lexical