From 477d0fdb7679c11f0a80788057fc2d3530982e7f Mon Sep 17 00:00:00 2001 From: Jonathan Yoder Date: Fri, 14 Aug 2026 13:12:14 -0400 Subject: [PATCH 1/3] version: ReleaseKey, a release-group key that needs no render Getting at a version's release from outside this package meant BaseVersion(), which renders it back to text -- a bytes.Buffer plus one math/big decimal conversion per segment -- and left the caller splitting the result apart again. go-pyresolver's PEP 440 set algebra did exactly that once per candidate version per containment test, where it cost 32% of a warm resolution. ReleaseKey derives the same (epoch, trailing-zero-stripped release) from the parsed fields instead: 16 ns and no allocations against 220 ns and 10 for the render-and-split, comparing in ~3 ns. Most keys pack into three words of 32-bit segment fields; the rest compare as arbitrary-precision integers, because PEP 440 puts no ceiling on an epoch or a segment and datestamped releases push real ones high. packVersion's release-word assembly moves into a shared packRelease, so the layout has one definition rather than two that could drift. The key is coarser than the version order on purpose -- 1.0, 1.00, 1.0a1 and 1.0+ubuntu1 are one position -- and coarser at the zero value, where it folds the uninitialized Version in with "0" rather than sorting it below. Both are asserted, the second alongside the Compare behaviour it deliberately differs from. Held to pypa/packaging 26.2 by a frozen fixture ranked on packaging's own release key, to the render-and-split derivation it replaces across 348,752 corpus versions, and to the general path pairwise so the packed fast path cannot hide a bug. --- CHANGELOG.md | 28 + version/packed.go | 93 ++- version/releasekey.go | 178 +++++ version/releasekey_test.go | 734 ++++++++++++++++++ .../testdata/pypa-26.2-releasekey.ranked.gz | Bin 0 -> 344 bytes 5 files changed, 1007 insertions(+), 26 deletions(-) create mode 100644 version/releasekey.go create mode 100644 version/releasekey_test.go create mode 100644 version/testdata/pypa-26.2-releasekey.ranked.gz diff --git a/CHANGELOG.md b/CHANGELOG.md index dc0c7b8..063313c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,34 @@ mistaken for a safe patch upgrade. ## [Unreleased] +### Added + +- `version`: `Version.ReleaseKey()` and the `ReleaseKey` type, for consumers + that need to group or bracket versions by their **release** — the epoch and + the release segments with trailing zeros stripped — ignoring the + pre/post/dev/local suffix. `ReleaseKey.Compare` orders two keys, and + `ReleaseKey.String` renders the smallest version string carrying a key. + + Until now the only way to reach a version's release from outside this + package was `BaseVersion()`, which renders the version back to text (a + `bytes.Buffer` plus one `math/big` decimal conversion per segment) and + leaves the caller to split the result apart again. `ReleaseKey` derives the + same answer from the parsed fields: **~16 ns and zero allocations, against + ~220 ns and 10 allocations** for the render-and-split it replaces, measured + on `2024.10.31`. Comparing two keys is ~3 ns. + + The ordering is exact at every magnitude — PEP 440 puts no ceiling on an + epoch or a release segment, and datestamped and calendar versions push real + segments high — so keys that do not fit the packed layout compare as + arbitrary-precision integers. It is pinned to pypa/packaging 26.2's own + release key (`Version._key[0:2]`) by a frozen fixture. + + ⚠️ `ReleaseKey` is deliberately coarser than the version order: `1.0`, + `1.0.0`, `1.00`, `1.0a1`, `1.0.post3.dev2` and `1.0+ubuntu1` all share one + key. It is also coarser at the zero value — the zero `ReleaseKey` compares + equal to the key of `"0"`, where `Version.Compare` sorts an uninitialized + `Version` strictly below every real version. + ## [0.6.0] - 2026-08-14 ### Notes diff --git a/version/packed.go b/version/packed.go index 9f93603..89a55c8 100644 --- a/version/packed.go +++ b/version/packed.go @@ -95,9 +95,10 @@ const ( packedDevMax = 1< 0 { + if bi := big.Int(release[n-1]); bi.Sign() != 0 { + break + } + n-- + } + return n +} + +// packRelease packs a release segment into three words of six 32-bit fields, +// most significant segment first, or ok=false when it does not fit: more than +// packedMaxSegments segments after stripping, or any segment at or above +// 2^32. +// +// Trailing zeros are stripped first and the remainder zero-padded to the fixed +// width, which preserves order: cmpkey strips the same zeros, and comparing +// two stripped tuples lexicographically gives the same answer as comparing +// their zero-padded fixed-width forms. +// +// An EMPTY release packs, to three zero words -- the same key version "0" +// gets, which is the right answer for a release-only key (see ReleaseKey) but +// NOT for a whole-version key. packVersion therefore rejects an empty release +// itself, before calling this; see the comment there. +// +// ⚠️ This is the single definition of the release word layout, shared by +// packVersion and ReleaseKey. Two copies could drift, and a drifted copy would +// order versions consistently within itself while disagreeing with the other, +// which is exactly the kind of split that no single-encoding test can see. +func packRelease(release []part.BigInt) (w [3]uint64, ok bool) { + n := releaseLen(release) + if n > packedMaxSegments { + return w, false + } + var segs [packedMaxSegments]uint64 + for i := 0; i < n; i++ { + v, ok := smallUint(release[i], packedSegMax) + if !ok { + return [3]uint64{}, false + } + segs[i] = v + } + w[0] = segs[0]< 0 { - if v, ok := smallUint(release[n-1], packedSegMax); ok && v == 0 { - n-- - continue - } - break - } - if n > packedMaxSegments { + w, relOK := packRelease(release) + if !relOK { return k, false } - var segs [packedMaxSegments]uint64 - for i := 0; i < n; i++ { - v, ok := smallUint(release[i], packedSegMax) - if !ok { - return k, false - } - segs[i] = v - } - k.w0 = segs[0]< o.w[0]) + case k.w[1] != o.w[1]: + return boolCmp(k.w[1] > o.w[1]) + case k.w[2] != o.w[2]: + return boolCmp(k.w[2] > o.w[2]) + } + return 0 + } + return k.compareGeneral(o) +} + +// compareGeneral is the arbitrary-precision path, taken whenever either side +// did not pack. It is also the reference the packed path is held to: +// TestReleaseKeyPackedAgreesWithGeneral runs both over every corpus pair, so a +// packing bug cannot hide behind the fast path. +func (k ReleaseKey) compareGeneral(o ReleaseKey) int { + // (*big.Int)(&…) rather than big.Int(…): part.BigInt IS a big.Int, so the + // pointer conversion is free, where the value conversion would copy a + // header per segment for no reason. Cmp only reads through both pointers. + if c := (*big.Int)(&k.epoch).Cmp((*big.Int)(&o.epoch)); c != 0 { + return c + } + n := min(len(k.release), len(o.release)) + for i := range n { + if c := (*big.Int)(&k.release[i]).Cmp((*big.Int)(&o.release[i])); c != 0 { + return c + } + } + // Both releases are stripped, so a shorter one is genuinely shorter and + // its missing segments are zeros: it is the smaller release. + switch { + case len(k.release) < len(o.release): + return -1 + case len(k.release) > len(o.release): + return 1 + } + return 0 +} + +// String renders the key as the smallest version string carrying it: the +// stripped release, dot-joined, with an "N!" epoch prefix when the epoch is +// nonzero. The empty release renders as "0", which is the version that shares +// its key, so the result always parses and always round-trips — +// Parse(k.String()).ReleaseKey() compares equal to k. +// +// This exists so that a key appears legibly in a test failure or a %v; it is +// not on any hot path, and nothing in this package's ordering goes through it. +func (k ReleaseKey) String() string { + var b strings.Builder + if bi := big.Int(k.epoch); bi.Sign() != 0 { + b.WriteString(bi.String()) + b.WriteByte('!') + } + if len(k.release) == 0 { + b.WriteByte('0') + return b.String() + } + for i := range k.release { + if i > 0 { + b.WriteByte('.') + } + b.WriteString((*big.Int)(&k.release[i]).String()) + } + return b.String() +} diff --git a/version/releasekey_test.go b/version/releasekey_test.go new file mode 100644 index 0000000..aff1fb6 --- /dev/null +++ b/version/releasekey_test.go @@ -0,0 +1,734 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT + +package version + +import ( + "fmt" + "math/big" + "math/rand" + "os" + "strings" + "testing" + + "github.com/rstudio/go-version/pkg/part" +) + +// releaseGridStrings is the constructed half of the release-key fixture: the +// cases that make a release key hard, which real corpus data does not reliably +// contain. Every string here must be accepted by BOTH Parse and pypa/packaging +// 26.2, since the fixture is generated by ranking them with the latter. +// +// The groups are, in order: trailing zeros and canonical form; segment counts +// on both sides of the six-segment packing limit; segment magnitudes at and +// beyond 2^32 (the packed field width) and 2^64 (the machine word); epochs +// including one too large to be a machine word; calendar and datestamped +// versions, which are what pushes real segments high; suffixes the key must +// ignore; and leading zeros inside a segment. +func releaseGridStrings() []string { + return []string{ + // Trailing zeros: these must all land in ONE group. + "1", "1.0", "1.0.0", "1.0.0.0", "1.00", "1.0.0.0.0.0.0.0", + "0", "0.0", "0.0.0", "00", "0!0", + // ...and these must not collapse into it. + "1.0.1", "1.0.0.1", "1.0.0.0.0.0.0.1", "0.1", "0.0.1", + + // Segment counts across the six-segment packing limit. The seventh + // segment is what makes a key unpackable while its neighbour packs, + // so mixed-path comparisons are exercised. + "2", "2.1", "2.1.3", "2.1.3.4", "2.1.3.4.5", "2.1.3.4.5.6", + "2.1.3.4.5.6.7", "2.1.3.4.5.6.7.8", "2.1.3.4.5.6.0", + "2.1.3.4.5.7", "2.1.3.4.5.6.1", + + // Magnitudes. 4294967295 is the largest packable segment, 4294967296 + // the smallest unpackable one, and the last two exceed uint64 — a key + // that truncated or overflowed would order them wrongly against their + // neighbours here. + "3.4294967294", "3.4294967295", "3.4294967296", "3.4294967297", + "3.18446744073709551615", "3.18446744073709551616", + "3.99999999999999999999", "3.999999999999999999990", + "18446744073709551616.0", "99999999999999999999.0", + "99999999999999999999.1", + + // Epochs, including one beyond a machine word: the epoch dominates + // every release segment, so an epoch compared as a truncated integer + // shows up as a gross misordering. + "1!0", "1!1.0", "1!2.3", "2!1.0", "10!1.0", + "99999999999999999999!1.0", "99999999999999999999!2.0", + + // Calendar and datestamped releases. + "2024.1", "2024.1.1", "2024.10", "2024.10.31", "2025.1", + "20240101", "20240102", "20240101.1", + + // Suffixes the key must ignore entirely: every one of these belongs to + // the same group as "5.1". + "5.1", "5.1a1", "5.1b2", "5.1rc3", "5.1.post1", "5.1.dev4", + "5.1.post2.dev3", "5.1+ubuntu1", "5.1a1.post1.dev1+local.7", + "5.1.0", "5.1.0a1", "1!5.1a1", + + // Leading zeros inside a segment normalize to the integer value. + "6.007", "6.7", "06.7", "6.0007.0", + } +} + +// TestDumpReleaseGrid writes releaseGridStrings() one per line to the path in +// GPP_RELEASE_GRID_OUT, which is how the fixture below is regenerated. It is +// skipped in every normal run. See TestReleaseKeyConformanceFixture. +func TestDumpReleaseGrid(t *testing.T) { + path := os.Getenv("GPP_RELEASE_GRID_OUT") + if path == "" { + t.Skip("set GPP_RELEASE_GRID_OUT to regenerate the release-key fixture input") + } + if err := os.WriteFile(path, []byte(strings.Join(releaseGridStrings(), "\n")+"\n"), 0o600); err != nil { + t.Fatal(err) + } +} + +// releaseKeyRef is the REFERENCE derivation, and it is deliberately the +// algorithm ReleaseKey replaces: render the base version to text with +// BaseVersion() and split it back into digit runs. go-pyresolver's pep440set +// derived its group key exactly this way, so holding ReleaseKey to it is what +// says the migration changes no answer. +// +// It is a reference, not a fallback: nothing in the package calls it. +func releaseKeyRef(v Version) (epoch string, release []string) { + base := v.BaseVersion() + epoch = "0" + if i := strings.Index(base, "!"); i >= 0 { + epoch = base[:i] + base = base[i+1:] + } + if base != "" { + release = strings.Split(base, ".") + } + for len(release) > 0 && release[len(release)-1] == "0" { + release = release[:len(release)-1] + } + return epoch, release +} + +// cmpDigitRuns orders two leading-zero-free decimal digit runs by value: the +// shorter run is the smaller number, and equal-length runs compare byte-wise. +// BaseVersion renders through math/big, so its output never has leading zeros. +func cmpDigitRuns(a, b string) int { + switch { + case len(a) < len(b): + return -1 + case len(a) > len(b): + return 1 + } + return strings.Compare(a, b) +} + +// releaseKeyRefCompare orders two reference keys. +func releaseKeyRefCompare(ae string, ar []string, be string, br []string) int { + if c := cmpDigitRuns(ae, be); c != 0 { + return c + } + for i := 0; i < len(ar) && i < len(br); i++ { + if c := cmpDigitRuns(ar[i], br[i]); c != 0 { + return c + } + } + switch { + case len(ar) < len(br): + return -1 + case len(ar) > len(br): + return 1 + } + return 0 +} + +func sign(x int) int { + switch { + case x < 0: + return -1 + case x > 0: + return 1 + } + return 0 +} + +// TestReleaseKeyGroups pins the canonicalization Equal-style consumers depend +// on: the versions listed together must occupy ONE position, and versions in +// different groups must not. +func TestReleaseKeyGroups(t *testing.T) { + groups := [][]string{ + {"1", "1.0", "1.0.0", "1.0.0.0", "1.00", "1.0.0.0.0.0.0.0", + "1.0a1", "1.0b2", "1.0rc3", "1.0.post1", "1.0.dev4", + "1.0+ubuntu1", "1.0.0a1.post2.dev3+x.y.7", "0!1.0"}, + {"0", "0.0", "0.0.0", "00", "0!0", "0.dev0", "0+local"}, + {"1.0.1", "1.0.1.0", "1.0.1.0.0"}, + {"1!1.0", "1!1.0.0", "1!1.0a1"}, + {"2.1.3.4.5.6.7", "2.1.3.4.5.6.7.0"}, + {"3.4294967296", "3.4294967296.0"}, + {"99999999999999999999.0", "99999999999999999999", "99999999999999999999.0.0"}, + } + + // Every member of a group agrees with every other member. + for gi, g := range groups { + for _, a := range g { + for _, b := range g { + ka := mustParse(t, a).ReleaseKey() + kb := mustParse(t, b).ReleaseKey() + if c := ka.Compare(kb); c != 0 { + t.Errorf("group %d: ReleaseKey(%q).Compare(ReleaseKey(%q)) = %d, want 0", gi, a, b, c) + } + } + } + } + + // No two groups collide. The zero Version is folded into the "0" group + // deliberately (see ReleaseKey's doc), so it is checked there too. + for gi := range groups { + for gj := range groups { + if gi == gj { + continue + } + ka := mustParse(t, groups[gi][0]).ReleaseKey() + kb := mustParse(t, groups[gj][0]).ReleaseKey() + if ka.Compare(kb) == 0 { + t.Errorf("groups %d (%q) and %d (%q) collide", gi, groups[gi][0], gj, groups[gj][0]) + } + } + } +} + +// TestReleaseKeyZeroValue pins the zero ReleaseKey to the "0" group, and to the +// key of the zero Version. +func TestReleaseKeyZeroValue(t *testing.T) { + var zeroKey ReleaseKey + var zeroVersion Version + + if c := zeroVersion.ReleaseKey().Compare(zeroKey); c != 0 { + t.Errorf("the zero Version's key vs the zero ReleaseKey = %d, want 0", c) + } + for _, s := range []string{"0", "0.0", "0!0.0.0"} { + if c := mustParse(t, s).ReleaseKey().Compare(zeroKey); c != 0 { + t.Errorf("ReleaseKey(%q) vs the zero ReleaseKey = %d, want 0", s, c) + } + } + if c := zeroKey.Compare(mustParse(t, "0.0.1").ReleaseKey()); c != -1 { + t.Errorf("the zero ReleaseKey vs 0.0.1 = %d, want -1", c) + } + + // ⚠️ Version.Compare sorts an uninitialized Version strictly below "0"; + // ReleaseKey deliberately does not, and cannot. Assert the difference so + // that anyone who "fixes" one to match the other reads why. + if c := zeroVersion.Compare(mustParse(t, "0")); c != -1 { + t.Fatalf("precondition: Version{}.Compare(0) = %d, want -1", c) + } +} + +// TestReleaseKeyOrdering pins a hand-checked strictly increasing chain, +// including the magnitudes a machine-word key would get wrong. +func TestReleaseKeyOrdering(t *testing.T) { + ascending := []string{ + "0", + "0.0.1", + "0.1", + "1", + "1.0.1", + "1.2", + "2.1.3.4.5.6", + "2.1.3.4.5.6.1", + "2.1.3.4.5.6.7", + "2.1.3.4.5.7", + "3.4294967295", + "3.4294967296", + "3.18446744073709551615", + "3.18446744073709551616", + "3.99999999999999999999", + "2024.1", + "2024.10", + "20240101", + "18446744073709551616.0", + "99999999999999999999.0", + "99999999999999999999.1", + "1!0", + "1!1.0", + "2!1.0", + "10!1.0", + "99999999999999999999!1.0", + } + keys := make([]ReleaseKey, len(ascending)) + for i, s := range ascending { + keys[i] = mustParse(t, s).ReleaseKey() + } + for i := range keys { + for j := range keys { + want := sign(i - j) + if got := keys[i].Compare(keys[j]); got != want { + t.Errorf("Compare(%q, %q) = %d, want %d", ascending[i], ascending[j], got, want) + } + } + } +} + +// TestReleaseKeyIgnoresSuffix asserts the coarseness directly: for every grid +// string, appending any pre/post/dev/local suffix must not move the key. +func TestReleaseKeyIgnoresSuffix(t *testing.T) { + suffixes := []string{"a1", "b2", "rc3", ".post1", ".dev4", ".post2.dev3", "+ubuntu1", + "a1.post1.dev1+local.7"} + for _, base := range []string{"1", "1.0", "2.1.3.4.5.6.7", "3.99999999999999999999", "1!2.3"} { + want := mustParse(t, base).ReleaseKey() + for _, sfx := range suffixes { + s := base + sfx + v, err := Parse(s) + if err != nil { + t.Fatalf("Parse(%q): %v", s, err) + } + if c := v.ReleaseKey().Compare(want); c != 0 { + t.Errorf("ReleaseKey(%q) vs ReleaseKey(%q) = %d, want 0", s, base, c) + } + } + } +} + +// TestReleaseKeyMatchesBaseVersionSplit is the migration differential: the new +// derivation must agree with the render-and-split reference on every corpus +// version, both on the key it produces and on the order it induces. +// +// It runs over the full grid fixture and the production corpus sample — +// 348,752 versions between them — so any input shape either fixture contains +// is covered. +func TestReleaseKeyMatchesBaseVersionSplit(t *testing.T) { + for _, name := range []string{"pypa-26.2-grid.ranked.gz", "pypa-26.2-corpus-sample.ranked.gz"} { + es := readRankedFixture(t, name) + + // Same key, component by component. + mismatch := 0 + for _, e := range es { + gotEpoch, gotRelease := releaseKeyRef(e.v) + k := e.v.ReleaseKey() + if got, want := (*big.Int)(&k.epoch).String(), gotEpoch; got != want { + mismatch++ + if mismatch <= 10 { + t.Errorf("%s: %q epoch = %s, reference %s", name, e.s, got, want) + } + continue + } + if len(k.release) != len(gotRelease) { + mismatch++ + if mismatch <= 10 { + t.Errorf("%s: %q release length = %d, reference %d", name, e.s, len(k.release), len(gotRelease)) + } + continue + } + for i := range k.release { + if got := (*big.Int)(&k.release[i]).String(); got != gotRelease[i] { + mismatch++ + if mismatch <= 10 { + t.Errorf("%s: %q release[%d] = %s, reference %s", name, e.s, i, got, gotRelease[i]) + } + break + } + } + } + if mismatch > 0 { + t.Fatalf("%s: %d versions whose key differs from the render-and-split reference", name, mismatch) + } + + // Same order, over a shuffled all-pairs sample. A shuffle rather than + // a stride so it cannot phase-lock onto the fixture's order. + sample := shuffledSample(es, 700, 7) + for i := range sample { + ae, ar := releaseKeyRef(sample[i].v) + ki := sample[i].v.ReleaseKey() + for j := range sample { + be, br := releaseKeyRef(sample[j].v) + want := releaseKeyRefCompare(ae, ar, be, br) + if got := ki.Compare(sample[j].v.ReleaseKey()); got != want { + t.Fatalf("%s: Compare(%q, %q) = %d, render-and-split reference %d", + name, sample[i].s, sample[j].s, got, want) + } + } + } + t.Logf("%s: %d versions, %d pairs agree with the render-and-split reference", + name, len(es), len(sample)*len(sample)) + } +} + +// TestReleaseKeyPackedAgreesWithGeneral holds the two encodings together. The +// packed path decides most comparisons, so without this a packing bug would be +// invisible to every other test that only calls Compare. +func TestReleaseKeyPackedAgreesWithGeneral(t *testing.T) { + for _, name := range []string{"pypa-26.2-grid.ranked.gz", "pypa-26.2-corpus-sample.ranked.gz"} { + es := readRankedFixture(t, name) + sample := shuffledSample(es, 700, 11) + packed := 0 + for i := range sample { + ki := sample[i].v.ReleaseKey() + if ki.packed { + packed++ + } + for j := range sample { + kj := sample[j].v.ReleaseKey() + if got, want := ki.Compare(kj), ki.compareGeneral(kj); got != want { + t.Fatalf("%s: Compare(%q, %q) = %d, general path %d", + name, sample[i].s, sample[j].s, got, want) + } + } + } + // A sample in which nothing packs would pass vacuously. + if packed == 0 { + t.Fatalf("%s: no sampled key packed; the fast path went untested", name) + } + t.Logf("%s: %d/%d sampled keys packed, %d pairs agree with the general path", + name, packed, len(sample), len(sample)*len(sample)) + } +} + +// TestReleaseKeyRefinesVersionOrder is a free cross-check against the frozen +// pypa/packaging 26.2 ranks: PEP 440's key is lexicographic with (epoch, +// release) first, so a version whose release key is strictly smaller MUST rank +// strictly below. It runs over every fixture entry, and it is independent of +// the render-and-split reference — a shared misreading of the release segment +// would show up here. +// +// The converse does not hold and is not asserted: equal release keys say +// nothing about the ranks, since the suffix breaks those ties. +func TestReleaseKeyRefinesVersionOrder(t *testing.T) { + for _, name := range []string{"pypa-26.2-grid.ranked.gz", "pypa-26.2-corpus-sample.ranked.gz"} { + es := readRankedFixture(t, name) + sample := shuffledSample(es, 900, 13) + for i := range sample { + ki := sample[i].v.ReleaseKey() + for j := range sample { + if ki.Compare(sample[j].v.ReleaseKey()) >= 0 { + continue + } + if sample[i].rank >= sample[j].rank { + t.Fatalf("%s: release key of %q is below %q, but packaging ranks them %d vs %d", + name, sample[i].s, sample[j].s, sample[i].rank, sample[j].rank) + } + } + } + } +} + +// TestReleaseKeyConformanceFixture is the only thing in CI that holds +// ReleaseKey directly to pypa/packaging. The other checks are transitive: they +// hold it to the render-and-split reference, or to the frozen full-version +// ranks, both of which route through Version's own parse. This one ranks the +// constructed grid by packaging's OWN release key — `Version._key[0:2]`, the +// epoch and the trailing-zero-stripped release — so a shared misreading of the +// release segment has nowhere to hide. +// +// ⚠️ DO NOT DELETE THE FIXTURE. It covers what the packed grid does not: +// epochs and segments beyond a machine word, eight-segment releases, and +// leading zeros inside a segment. +// +// To regenerate (needs pypa/packaging >= 26.2): +// +// GPP_RELEASE_GRID_OUT=/tmp/grid.txt go test ./version -run TestDumpReleaseGrid +// python3 - <<'EOF' < /tmp/grid.txt | gzip -9 > version/testdata/pypa-26.2-releasekey.ranked.gz +// import sys +// from packaging.version import Version, InvalidVersion +// seen = {} +// for line in sys.stdin: +// s = line.rstrip("\n") +// if not s or s in seen: continue +// try: v = Version(s) +// except InvalidVersion: +// print(f"REJECTED: {s!r}", file=sys.stderr); continue +// # _key[0:2] is packaging's own (epoch, stripped release) -- read from +// # the reference implementation rather than reimplemented here. +// seen[s] = v._key[0:2] +// rank, prev = 0, None +// for s, key in sorted(seen.items(), key=lambda kv: kv[1]): +// if prev is not None and key > prev: rank += 1 +// prev = key +// print(f"{s}\t{rank}") +// EOF +func TestReleaseKeyConformanceFixture(t *testing.T) { + es := readRankedFixture(t, "pypa-26.2-releasekey.ranked.gz") + + // The fixture must rank exactly the strings the grid generates: a stale + // fixture (missing a newly added case) is the failure mode that would + // otherwise pass silently. + want := map[string]bool{} + for _, s := range releaseGridStrings() { + want[s] = true + } + seen := map[string]bool{} + for _, e := range es { + if seen[e.s] { + t.Fatalf("fixture entry %q is duplicated; regenerate it", e.s) + } + seen[e.s] = true + if !want[e.s] { + t.Fatalf("fixture ranks %q, which releaseGridStrings() does not generate; regenerate it", e.s) + } + } + for s := range want { + if !seen[s] { + t.Fatalf("releaseGridStrings() generates %q but the fixture does not rank it; regenerate it", s) + } + } + + keys := make([]ReleaseKey, len(es)) + for i, e := range es { + keys[i] = e.v.ReleaseKey() + } + for i := range es { + for j := range es { + want := sign(es[i].rank - es[j].rank) + if got := keys[i].Compare(keys[j]); got != want { + t.Errorf("Compare(%q, %q) = %d, packaging ranks their release keys %d vs %d", + es[i].s, es[j].s, got, es[i].rank, es[j].rank) + } + } + } + t.Logf("%d entries, %d pairs consistent with pypa/packaging's release key", len(es), len(es)*len(es)) +} + +// TestReleaseKeyIsATotalOrder checks the three properties a strict total order +// needs, over the constructed grid where the hard magnitudes are dense. +func TestReleaseKeyIsATotalOrder(t *testing.T) { + ss := releaseGridStrings() + keys := make([]ReleaseKey, len(ss)) + for i, s := range ss { + keys[i] = mustParse(t, s).ReleaseKey() + } + for i := range keys { + if c := keys[i].Compare(keys[i]); c != 0 { + t.Errorf("reflexivity: %q vs itself = %d", ss[i], c) + } + for j := range keys { + if a, b := keys[i].Compare(keys[j]), keys[j].Compare(keys[i]); a != -b { + t.Errorf("antisymmetry: %q vs %q = %d, reverse = %d", ss[i], ss[j], a, b) + } + } + } + for i := range keys { + for j := range keys { + if keys[i].Compare(keys[j]) > 0 { + continue + } + for k := range keys { + if keys[j].Compare(keys[k]) <= 0 && keys[i].Compare(keys[k]) > 0 { + t.Fatalf("transitivity: %q <= %q <= %q but %q > %q", + ss[i], ss[j], ss[k], ss[i], ss[k]) + } + } + } + } +} + +// TestReleaseKeyAllocatesNothing pins the whole point of the type. A key that +// allocated would still be correct, and would have given back the churn the +// render cost in the first place. +func TestReleaseKeyAllocatesNothing(t *testing.T) { + cases := []struct { + name string + s string + }{ + {"packed", "1.2.3"}, + {"big epoch", "99999999999999999999!1.2.3"}, + {"seven segments", "1.2.3.4.5.6.7"}, + {"huge segment", "3.99999999999999999999"}, + {"local label", "1.2.3+ubuntu1"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + v := mustParse(t, tc.s) + if n := testing.AllocsPerRun(200, func() { sinkKey = v.ReleaseKey() }); n != 0 { + t.Errorf("ReleaseKey() allocated %.1f times per run, want 0", n) + } + other := mustParse(t, "4.5.6") + ka, kb := v.ReleaseKey(), other.ReleaseKey() + if n := testing.AllocsPerRun(200, func() { sinkInt = ka.Compare(kb) }); n != 0 { + t.Errorf("Compare() allocated %.1f times per run, want 0", n) + } + }) + } +} + +var ( + sinkKey ReleaseKey + sinkInt int +) + +// TestReleaseKeyDoesNotAliasIntoTheVersion pins the capacity clip. Without it +// the key's release slice keeps the stripped trailing zeros as spare capacity, +// and an append writes through into the Version's own segments — corrupting a +// value this package advertises as safe to share across goroutines. +func TestReleaseKeyDoesNotAliasIntoTheVersion(t *testing.T) { + v := mustParse(t, "1.2.0.0") + k := v.ReleaseKey() + if len(k.release) != 2 { + t.Fatalf("stripped release length = %d, want 2", len(k.release)) + } + if cap(k.release) != len(k.release) { + t.Fatalf("key release cap = %d, len = %d; the capacity clip is missing", + cap(k.release), len(k.release)) + } + + // The append must not be visible through v. + nine, err := part.NewBigInt("9") + if err != nil { + t.Fatal(err) + } + grown := append(k.release, nine) //nolint:gocritic // the point is that append must copy + if got := (*big.Int)(&grown[2]).String(); got != "9" { + t.Fatalf("precondition: the appended segment is %s, want 9", got) + } + if got := v.String(); got != "1.2.0.0" { + t.Errorf("after appending to the key's release, v renders %q, want %q -- "+ + "the append wrote through into the Version's own segments", got, "1.2.0.0") + } + if got := (*big.Int)(&v.release[2]).String(); got != "0" { + t.Errorf("v.release[2] = %s after the append, want 0", got) + } +} + +// TestReleaseKeyStringRoundTrips: String renders the smallest version carrying +// the key, so re-parsing it must land on the same key. +func TestReleaseKeyStringRoundTrips(t *testing.T) { + want := map[string]string{ + "1.0.0": "1", + "0": "0", + "0.0": "0", + "1!2.3": "1!2.3", + "1.0a1.post2.dev3+ubuntu1": "1", + "3.99999999999999999999": "3.99999999999999999999", + "6.007": "6.7", + } + var zero ReleaseKey + if got := zero.String(); got != "0" { + t.Errorf("the zero ReleaseKey renders %q, want %q", got, "0") + } + for _, s := range append(releaseGridStrings(), "1.0a1.post2.dev3+ubuntu1") { + k := mustParse(t, s).ReleaseKey() + got := k.String() + if w, ok := want[s]; ok && got != w { + t.Errorf("ReleaseKey(%q).String() = %q, want %q", s, got, w) + } + back, err := Parse(got) + if err != nil { + t.Errorf("ReleaseKey(%q).String() = %q, which does not parse: %v", s, got, err) + continue + } + if c := back.ReleaseKey().Compare(k); c != 0 { + t.Errorf("ReleaseKey(%q).String() = %q, whose key differs (%d)", s, got, c) + } + } +} + +// TestPackReleaseIsTheOnlyReleaseLayout holds the refactor that gave +// packVersion and ReleaseKey one packing function: for every packable version, +// the whole-version key's release words must be exactly what packRelease +// produces. If someone reintroduces a second copy of the layout, or changes +// packedKey so that w0-w2 stop being release-only, this fails. +func TestPackReleaseIsTheOnlyReleaseLayout(t *testing.T) { + es := readRankedFixture(t, "pypa-26.2-grid.ranked.gz") + checked := 0 + for _, e := range es { + if !e.v.packable { + continue + } + checked++ + w, ok := packRelease(e.v.release) + if !ok { + t.Fatalf("%q packs as a whole version but its release does not", e.s) + } + if w[0] != e.v.packed.w0 || w[1] != e.v.packed.w1 || w[2] != e.v.packed.w2 { + t.Fatalf("%q: packRelease gives %v, packedKey holds {%d %d %d}", + e.s, w, e.v.packed.w0, e.v.packed.w1, e.v.packed.w2) + } + } + if checked == 0 { + t.Fatal("no packable version in the grid fixture; the check was vacuous") + } + t.Logf("%d packable versions share one release layout", checked) +} + +// TestReleaseLen covers the strip directly, including the magnitudes that make +// a "is this segment zero" test written with smallUint accidentally right. +func TestReleaseLen(t *testing.T) { + cases := []struct { + s string + want int + }{ + {"1", 1}, {"1.0", 1}, {"1.0.0", 1}, {"0", 0}, {"0.0", 0}, + {"0.1", 2}, {"1.0.1", 3}, {"1.2.3.4.5.6.7", 7}, + {"3.99999999999999999999", 2}, + {"99999999999999999999.0", 1}, + {"1.0.99999999999999999999", 3}, + } + for _, tc := range cases { + v := mustParse(t, tc.s) + if got := releaseLen(v.release); got != tc.want { + t.Errorf("releaseLen(%q) = %d, want %d", tc.s, got, tc.want) + } + } + if got := releaseLen(nil); got != 0 { + t.Errorf("releaseLen(nil) = %d, want 0", got) + } +} + +// shuffledSample draws n entries with a fixed seed, so a failure reproduces. +func shuffledSample(es []rankedVersion, n int, seed int64) []rankedVersion { + idx := make([]int, len(es)) + for i := range idx { + idx[i] = i + } + rand.New(rand.NewSource(seed)).Shuffle(len(idx), func(i, j int) { idx[i], idx[j] = idx[j], idx[i] }) + out := make([]rankedVersion, 0, min(n, len(idx))) + for _, i := range idx[:min(n, len(idx))] { + out = append(out, es[i]) + } + return out +} + +func BenchmarkReleaseKey(b *testing.B) { + for _, s := range []string{"1.2.3", "2024.10.31", "1.2.3.4.5.6.7", "99999999999999999999!1.2"} { + v, err := Parse(s) + if err != nil { + b.Fatal(err) + } + b.Run(fmt.Sprintf("derive/%s", s), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + sinkKey = v.ReleaseKey() + } + }) + other, err := Parse("4.5.6") + if err != nil { + b.Fatal(err) + } + ka, kb := v.ReleaseKey(), other.ReleaseKey() + b.Run(fmt.Sprintf("compare/%s", s), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + sinkInt = ka.Compare(kb) + } + }) + } +} + +// BenchmarkReleaseKeyVsBaseVersionSplit is the before/after: the reference is +// the render-and-split derivation this type replaces. +func BenchmarkReleaseKeyVsBaseVersionSplit(b *testing.B) { + v, err := Parse("2024.10.31") + if err != nil { + b.Fatal(err) + } + b.Run("ReleaseKey", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + sinkKey = v.ReleaseKey() + } + }) + b.Run("BaseVersionSplit", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + sinkEpoch, sinkRelease = releaseKeyRef(v) + } + }) +} + +var ( + sinkEpoch string + sinkRelease []string +) diff --git a/version/testdata/pypa-26.2-releasekey.ranked.gz b/version/testdata/pypa-26.2-releasekey.ranked.gz new file mode 100644 index 0000000000000000000000000000000000000000..f55124bfb5811b802e80b138e42ccb97809bb0d6 GIT binary patch literal 344 zcmV-e0jK^SiwFpSOMhwt19D|-XL4y|E^=XRYh`2rbyLx9!ypWNHh;0_O2{?_!soQz z-l}!l+I|1dAvEchC6axgd$wlG3@ zMnWYh)u2F=+nA_zAB}_ujG08%AQlqchWOYo0)?k1qLSD%5hs$op^#Ampf+oz!jjZ& zj2fnGj3U}LuB41uCbLq_xTeZ2XDO0NWnU?z3Z?Blr0E}pNiAeQmF(^~z)2VIc}@$8 zAJ?zj(Btddw`~P=W~3hOr~CWuzV6!BdWsw_*Yoj$4lRrM0s6;r01UAN?=u`P-g&N` zj&o);uemPB@w8whv;kwnsIf=>i%Djlg@N^fT}dNSN*Ju2=K2igu-8IY%D#e~YY^Ok qG{May-vAV&X$;UiZm4S(4OfFZO1m3&&Gt2QEr%cGaF&aR0{{SH>7>p8 literal 0 HcmV?d00001 From ad7701c4c887efdc0e662f304a8c8e4b74058139 Mon Sep 17 00:00:00 2001 From: Jonathan Yoder Date: Fri, 14 Aug 2026 13:21:10 -0400 Subject: [PATCH 2/3] test(version): note which reference guards are unreachable, not forgotten --- version/releasekey_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/version/releasekey_test.go b/version/releasekey_test.go index aff1fb6..239f780 100644 --- a/version/releasekey_test.go +++ b/version/releasekey_test.go @@ -90,6 +90,14 @@ func TestDumpReleaseGrid(t *testing.T) { // says the migration changes no answer. // // It is a reference, not a fallback: nothing in the package calls it. +// +// Two guards from the original are dropped, and both are unreachable rather +// than forgotten: it refused a non-numeric epoch and stopped at a non-numeric +// release segment, and it stripped leading zeros from each run. BaseVersion +// renders every component through big.Int.String(), which produces only ASCII +// digits and never a leading zero, so neither guard can fire on a version this +// package parsed. Reinstating them would make the reference agree with itself +// on inputs it cannot receive, and would not change a single comparison below. func releaseKeyRef(v Version) (epoch string, release []string) { base := v.BaseVersion() epoch = "0" From 7a204aac35b5bce2dcce6a5828c63d2913dc46f9 Mon Sep 17 00:00:00 2001 From: Jonathan Yoder Date: Fri, 14 Aug 2026 13:31:18 -0400 Subject: [PATCH 3/3] version: correct what ReleaseKey's docs promise about String() An unbriefed review seat found the doc claim misusable, and it was right. String() renders the SHORTEST version string carrying a key, and the doc called it the smallest one -- while the type doc sold itself for "bracketing versions by release". Together those invite lo, _ := Parse(k.String()) as a group's lower bound, which silently drops every pre-release and dev release of that release: 1.0a1 and 1.0.dev0 carry the key of "1" and sort strictly BELOW Parse("1"). The group has no greatest member either, since .postN and +local extend it upward without limit. Both facts are now stated on the type, on String, in the changelog, and asserted by TestReleaseKeyStringIsNotABound. Also from that seat: - "group" reads as "map key", which a ReleaseKey cannot be. Say order or partition, and say why. - The "32% of a warm resolution" figure is not in the issue cited beside it, and a hard percentage in a public godoc rots. Replaced with the benchmark in this repository, which a reader can rerun. - The ~16 ns figure is the best case. The receiver is by value and the method does not inline, so a caller ranging over a []Version pays the struct copy too, ~24 ns. Documented rather than papered over; the value receiver stays, for consistency and because it cannot write through to the caller's Version. - The file explains why the per-segment loop uses a pointer conversion and then uses the value conversion twice elsewhere. Scoped the rule to the loop it is actually about. - packRelease now takes the already-stripped release, so its own strip finds nothing to redo. - TestReleaseKeyRefinesVersionOrder only asserted on strictly-below pairs, so it would have passed vacuously against a Compare that never returned one. It now counts them and fails at zero, like its neighbours. --- CHANGELOG.md | 14 +++++++- version/packed.go | 8 ++--- version/releasekey.go | 65 +++++++++++++++++++++++++++++++------- version/releasekey_test.go | 41 ++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 063313c..c59803c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,12 @@ mistaken for a safe patch upgrade. leaves the caller to split the result apart again. `ReleaseKey` derives the same answer from the parsed fields: **~16 ns and zero allocations, against ~220 ns and 10 allocations** for the render-and-split it replaces, measured - on `2024.10.31`. Comparing two keys is ~3 ns. + on `2024.10.31`. Comparing two packed keys is ~3 ns; ~8 ns when either side + did not pack. + + ⚠️ The ~16 ns is for a `Version` held in a local. The receiver is by value + and the method does not inline, so a caller ranging over a `[]Version` pays + the struct copy too — about 24 ns. The ordering is exact at every magnitude — PEP 440 puts no ceiling on an epoch or a release segment, and datestamped and calendar versions push real @@ -37,6 +42,13 @@ mistaken for a safe patch upgrade. equal to the key of `"0"`, where `Version.Compare` sorts an uninitialized `Version` strictly below every real version. + ⚠️ **`ReleaseKey.String()` is not a bound on the group.** It renders the + shortest version string carrying the key, not the smallest version carrying + it: `1.0a1` and `1.0.dev0` carry the key of `"1"` and sort strictly *below* + `Parse("1")`, and `.postN` and `+local` sort above it without limit. And + because a `ReleaseKey` holds a slice it is not comparable with `==` and + cannot be a map key — sort by `Compare` and scan the equal runs. + ## [0.6.0] - 2026-08-14 ### Notes diff --git a/version/packed.go b/version/packed.go index 89a55c8..d8504a6 100644 --- a/version/packed.go +++ b/version/packed.go @@ -183,10 +183,10 @@ func releaseLen(release []part.BigInt) int { return n } -// packRelease packs a release segment into three words of six 32-bit fields, -// most significant segment first, or ok=false when it does not fit: more than -// packedMaxSegments segments after stripping, or any segment at or above -// 2^32. +// packRelease packs a whole release — all of its segments — into three words +// of six 32-bit fields, most significant segment first, or ok=false when it +// does not fit: more than packedMaxSegments segments after stripping, or any +// segment at or above 2^32. // // Trailing zeros are stripped first and the remainder zero-padded to the fixed // width, which preserves order: cmpkey strips the same zeros, and comparing diff --git a/version/releasekey.go b/version/releasekey.go index 20cf23f..03f7cf3 100644 --- a/version/releasekey.go +++ b/version/releasekey.go @@ -16,19 +16,27 @@ import ( // 1.0a1, 1.0.post3.dev2 and 1.0+ubuntu1 all carry the same ReleaseKey, because // PEP 440 gives every one of them epoch 0 and, after stripping, the release // tuple (1). A consumer that needs the full order wants Compare on the Version; -// this key exists for consumers that need to group or bracket versions BY +// this key exists for consumers that need to ORDER or PARTITION versions by // release, where the pre/post/dev/local suffix must not participate. // +// ⚠️ "Partition", not "index": a ReleaseKey is not comparable with == and so +// cannot be a map key. Use Compare — sort by it and scan the runs of equal +// keys. String() is a stable scalar form, but it allocates, which is the cost +// this type exists to avoid. +// // # Why this is not just "read the release segments" // // The obvious way to get at a version's release from outside this package is // BaseVersion(), which renders "1!3.4.5" — and rendering means a bytes.Buffer // and one math/big decimal conversion per segment, followed by the caller // splitting the result back apart. go-pyresolver's PEP 440 set algebra did -// exactly that, once per candidate version per containment test, and it cost -// 32% of a warm resolution (rstudio/package-manager#19713 and its neighbours). +// exactly that, once per candidate version per containment test. +// // A key derived from the parsed fields costs no rendering, no parsing and no -// allocation, and answers the same question. +// allocation, and answers the same question: BenchmarkReleaseKeyVsBaseVersion- +// Split measures 16 ns and 0 allocations against 220 ns and 10 allocations on +// "2024.10.31". Deriving one costs more when the Version has to be copied out +// of a slice first (~24 ns), because the receiver is by value; see ReleaseKey(). // // # Ordering // @@ -58,6 +66,20 @@ import ( // and reading a math/big value does not mutate it — so a key is safe to share // across goroutines exactly as the Version it came from is. // +// # ⚠️ A key is not a bound on its own group +// +// String() renders the shortest version string carrying the key, but that +// version is NOT the least version in the group: 1.0a1 and 1.0.dev0 carry the +// key of "1" and sort strictly BELOW it. Neither is there a greatest — .postN +// and +local extend the group upward without limit. So +// +// lo, _ := Parse(k.String()) // ⚠️ NOT a lower bound on k's group +// +// silently excludes every pre-release and dev release of that release. A +// consumer bracketing a release group needs positions between versions, which +// is a different thing from a version; see go-pyresolver's pep440set for one +// treatment of it. +// // ReleaseKey is NOT comparable with ==; it holds a slice. Use Compare. type ReleaseKey struct { // w is the packed encoding of the stripped release, valid when packed is @@ -78,6 +100,15 @@ type ReleaseKey struct { // // It allocates nothing: the epoch is a by-value copy of a math/big header and // the release is a subslice of v's own. +// +// ⚠️ The receiver is by VALUE, so calling this copies the whole Version — a +// large struct, and this method does not inline. That is deliberate, for +// consistency with every other Version method and because a by-value receiver +// cannot write through to the caller's Version; but it means a caller ranging +// over a []Version pays ~50% more per key than one holding a single Version in +// a local. compareVersions takes pointers precisely to dodge that copy in +// sort's inner loop, and a caller with the same problem can hoist the Version +// out of the loop once. func (v Version) ReleaseKey() ReleaseKey { n := releaseLen(v.release) k := ReleaseKey{ @@ -96,8 +127,12 @@ func (v Version) ReleaseKey() ReleaseKey { // reasons that have nothing to do with the release (a local label, a large // dev number), and buying a few nanoseconds once per key with a standing // assumption about another key's layout is a bad trade. + // + // k.release rather than v.release: it is the same segments already + // stripped, so packRelease's own strip finds nothing left to do instead of + // rescanning the trailing zeros. if bi := big.Int(v.epoch); bi.Sign() == 0 { - k.w, k.packed = packRelease(v.release) + k.w, k.packed = packRelease(k.release) } return k } @@ -128,8 +163,11 @@ func (k ReleaseKey) Compare(o ReleaseKey) int { // packing bug cannot hide behind the fast path. func (k ReleaseKey) compareGeneral(o ReleaseKey) int { // (*big.Int)(&…) rather than big.Int(…): part.BigInt IS a big.Int, so the - // pointer conversion is free, where the value conversion would copy a - // header per segment for no reason. Cmp only reads through both pointers. + // pointer conversion is free, where the value conversion copies a header. + // That matters HERE, in a per-segment loop; elsewhere in this file the + // value conversion is used for a single Sign() or String() call, where one + // header copy reads more plainly than a pointer conversion. Cmp only reads + // through both pointers. if c := (*big.Int)(&k.epoch).Cmp((*big.Int)(&o.epoch)); c != 0 { return c } @@ -150,14 +188,19 @@ func (k ReleaseKey) compareGeneral(o ReleaseKey) int { return 0 } -// String renders the key as the smallest version string carrying it: the -// stripped release, dot-joined, with an "N!" epoch prefix when the epoch is -// nonzero. The empty release renders as "0", which is the version that shares -// its key, so the result always parses and always round-trips — +// String renders the key in canonical shortest form: the stripped release, +// dot-joined, with an "N!" epoch prefix when the epoch is nonzero. The empty +// release renders as "0", so the result always parses and always round-trips — // Parse(k.String()).ReleaseKey() compares equal to k. // +// ⚠️ THE RESULT IS NOT A BOUND ON THE GROUP. It is the shortest spelling +// carrying the key, not the smallest version carrying it: 1.0a1 and 1.0.dev0 +// carry the key of "1" and sort strictly below Parse("1"), and .postN and +// +local carry it and sort above with no upper limit. See the type doc. +// // This exists so that a key appears legibly in a test failure or a %v; it is // not on any hot path, and nothing in this package's ordering goes through it. +// It allocates, unlike everything else here. func (k ReleaseKey) String() string { var b strings.Builder if bi := big.Int(k.epoch); bi.Sign() != 0 { diff --git a/version/releasekey_test.go b/version/releasekey_test.go index 239f780..a6c58fc 100644 --- a/version/releasekey_test.go +++ b/version/releasekey_test.go @@ -399,18 +399,59 @@ func TestReleaseKeyRefinesVersionOrder(t *testing.T) { for _, name := range []string{"pypa-26.2-grid.ranked.gz", "pypa-26.2-corpus-sample.ranked.gz"} { es := readRankedFixture(t, name) sample := shuffledSample(es, 900, 13) + // The assertion below only fires on a strictly-below pair, so an + // implementation whose Compare never returned a negative would pass it + // without being checked at all. Count them and require some. + below := 0 for i := range sample { ki := sample[i].v.ReleaseKey() for j := range sample { if ki.Compare(sample[j].v.ReleaseKey()) >= 0 { continue } + below++ if sample[i].rank >= sample[j].rank { t.Fatalf("%s: release key of %q is below %q, but packaging ranks them %d vs %d", name, sample[i].s, sample[j].s, sample[i].rank, sample[j].rank) } } } + if below == 0 { + t.Fatalf("%s: no sampled pair had a strictly smaller release key; the check was vacuous", name) + } + t.Logf("%s: %d strictly-below pairs, all consistent with packaging's ranks", name, below) + } +} + +// TestReleaseKeyStringIsNotABound pins the trap the type doc warns about: the +// version String() renders is the shortest one carrying the key, NOT the least +// one, so a consumer must not reach for it as a lower bound. +// +// This is asserted rather than only documented because the wrong reading is the +// natural one and produces a silently narrow bracket -- it drops every +// pre-release and dev release of the release. +func TestReleaseKeyStringIsNotABound(t *testing.T) { + k := mustParse(t, "1.0").ReleaseKey() + lo := mustParse(t, k.String()) + + for _, s := range []string{"1.0a1", "1.0b2", "1.0rc3", "1.0.dev0", "1.0a1.dev0"} { + v := mustParse(t, s) + if c := v.ReleaseKey().Compare(k); c != 0 { + t.Fatalf("precondition: %q is not in the group (%d)", s, c) + } + if c := v.Compare(lo); c >= 0 { + t.Errorf("%q vs Parse(key.String()) = %d, want -1: String() must not be "+ + "documentable as the group's lower bound", s, c) + } + } + for _, s := range []string{"1.0.post1", "1.0+ubuntu1", "1.0.post99999"} { + v := mustParse(t, s) + if c := v.ReleaseKey().Compare(k); c != 0 { + t.Fatalf("precondition: %q is not in the group (%d)", s, c) + } + if c := v.Compare(lo); c <= 0 { + t.Errorf("%q vs Parse(key.String()) = %d, want +1", s, c) + } } }