diff --git a/internal/freshness/freshness.go b/internal/freshness/freshness.go index bea86b5..7d0436d 100644 --- a/internal/freshness/freshness.go +++ b/internal/freshness/freshness.go @@ -16,6 +16,12 @@ // is satisfied by one line while the other has silently gone missing, and a // server on the missing line sees a plugin with no installable version at all. // +// A release is named here by the version its tag carries and not by the whole +// tag. One version is released once per live server line, each under its own +// tag, so the newest finished release reaches the catalogue under a different +// tag on each line and a comparison of whole tags refuses a catalogue that is +// current. releaseVersion is where that is decided and what it costs. +// // It fails closed everywhere. A body that does not parse, a catalogue with no // plugins, a plugin the catalogue does not list: each is a refusal and none is a // pass with a shrug. The likeliest real failure of this check is a network blip, @@ -41,6 +47,11 @@ type Expected struct { // Tag is the newest finished release's tag. The finished set is // decisions/channel-model.md's, read from the tag rather than from the // release's pre-release flag. + // + // What is compared against a catalogue entry is the release this tag names + // rather than the tag itself, which releaseVersion below decides and says + // why. The whole tag is still carried, because it is what a refusal quotes + // and what somebody repairing one goes looking for. Tag string } @@ -124,9 +135,10 @@ func judgeOne(plugins []plugin, want Expected) error { want.Slug, want.Path) } + wanted := releaseVersion(want.Tag) var missing []string for target, tags := range tagsPerTarget { - if !tags[want.Tag] { + if !carries(tags, wanted) { missing = append(missing, fmt.Sprintf("%s (it carries %s)", target, joinSorted(tags))) } } @@ -138,6 +150,64 @@ func judgeOne(plugins []plugin, want Expected) error { want.Slug, want.Tag, strings.Join(missing, ", ")) } +// carries says whether a target line offers the release wanted names. +// +// The comparison is per tag rather than over a set of stripped tags, because +// two tags naming one release differ only in segments releaseVersion drops, and +// a set of those would have to be built to be thrown away. +func carries(tags map[string]bool, wanted string) bool { + for tag := range tags { + if releaseVersion(tag) == wanted { + return true + } + } + return false +} + +// releaseVersion is the part of a tag that says WHICH release it is, with the +// parts that say where the release goes removed. +// +// A tag admitted by a declaration's stable_tags pattern carries three kinds of +// thing, and only one of them identifies the release. An optional leading v is +// spelling. A trailing -stable names the channel, which +// decisions/channel-model.md reads out of the tag rather than out of the +// release's flag. And a -jf12 segment names the server line a file was cut for, +// under the one-version-many-lines decision the requests declaration's note +// carries: one version is released once per live line, each release under its +// own tag, and the catalogue tells the lines apart by targetAbi rather than by +// that segment. +// +// So one release version reaches the catalogue under several tags, and a check +// asking for one tag under every target line asks for a tag that was never cut +// for most of them. That is what this repairs, and it is the whole of the +// change: the numeric components are compared and the decoration is not. +// +// What it gives up is the ability to tell two finished releases apart that +// share a version and differ only in a suffix. Under the decision above that is +// the intended reading, because such a pair IS one version cut for two lines. +// A pair that is not - one version retagged for some other reason - is read as +// current here when only one of the two is published, and no reading of a tag +// separates those two cases. +// +// It refuses nothing this rule did not refuse before. Equal tags have equal +// versions, so every catalogue the whole-tag comparison passed is passed here +// too, and the change can only stop refusals rather than start them. +// +// A tag with no numeric run at its front is handed back whole. Returning the +// empty string for it would make every such tag equal to every other, which is +// the one direction a comparison here must not fail in. +func releaseVersion(tag string) string { + rest := strings.TrimPrefix(tag, "v") + end := 0 + for end < len(rest) && (rest[end] == '.' || (rest[end] >= '0' && rest[end] <= '9')) { + end++ + } + if end == 0 { + return tag + } + return rest[:end] +} + func joinSorted(set map[string]bool) string { out := make([]string, 0, len(set)) for k := range set { diff --git a/internal/freshness/freshness_test.go b/internal/freshness/freshness_test.go index b5d3e22..f8ea3b9 100644 --- a/internal/freshness/freshness_test.go +++ b/internal/freshness/freshness_test.go @@ -151,3 +151,80 @@ func TestEveryFailureIsNamedRatherThanTheFirst(t *testing.T) { } } } + +// TestOneVersionCutPerLineIsCurrent is the failure this file was changed for. +// The catalogue is current: the newest finished release is published on both +// lines, each under the tag that line's file was cut from. A comparison of +// whole tags refuses it, and every server on the newer line is offered a build +// the check has just called missing. +// +// The tags are the ones the scheduled run of 2026-09-05 read, with the plugin +// name this file uses everywhere else. +func TestOneVersionCutPerLineIsCurrent(t *testing.T) { + body := published( + versionEntry(declared, "0.3.0.0-jf12-stable", "12.0.0.0"), + versionEntry(declared, "0.3.0.0-stable", "10.11.0.0"), + versionEntry(declared, "0.2.0.0-stable", "10.11.0.0"), + ) + for _, newest := range []string{"0.3.0.0-stable", "0.3.0.0-jf12-stable"} { + // Either tag may be the newest one the release list answers with: + // the two were published thirteen seconds apart and the order the + // API returns them in is not this check's to decide. + if err := Judge(body, []Expected{{Slug: "widget", Path: declared, Tag: newest}}); err != nil { + t.Errorf("a catalogue carrying the newest release on both lines was refused, newest %s: %v", newest, err) + } + } +} + +// TestALineBehindTheNewestVersionIsStillRefused is the other half, and it is +// the property the change must not have bought its way out of. The line tags +// are the same shape as above and the newer line is a version behind, which is +// exactly what the check exists to catch. +func TestALineBehindTheNewestVersionIsStillRefused(t *testing.T) { + body := published( + versionEntry(declared, "0.3.0.0-jf12-stable", "12.0.0.0"), + versionEntry(declared, "0.4.0.0-stable", "10.11.0.0"), + ) + err := Judge(body, []Expected{{Slug: "widget", Path: declared, Tag: "0.4.0.0-stable"}}) + if err == nil { + t.Fatal("a target line a version behind was read as current") + } + for _, want := range []string{"widget", "0.4.0.0-stable", "12.0.0.0"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the refusal does not carry %q: %v", want, err) + } + } + if strings.Contains(err.Error(), "10.11.0.0 (") { + t.Errorf("the refusal blames the target line that is current: %v", err) + } +} + +// TestTagsThatNameDifferentReleasesAreNotConflated holds the edge the version +// comparison could have flattened. A leading v is spelling and is dropped; a +// numeric run that differs is a different release whatever follows it; and a +// tag with no numeric run at its front is compared whole rather than being made +// equal to every other such tag. +func TestTagsThatNameDifferentReleasesAreNotConflated(t *testing.T) { + for _, c := range []struct { + published string + newest string + current bool + why string + }{ + {"v1.2.3.4", "1.2.3.4-stable", true, "a leading v is spelling, not a different release"}, + {"1.2.3.4-jf13-stable", "1.2.3.4-jf12-stable", true, "two lines of one version"}, + {"1.2.3.0-stable", "1.2.3.4-stable", false, "a fourth component is part of the version"}, + {"1.2.3.4-stable", "1.2.4.0-stable", false, "a newer version is a different release"}, + {"nightly", "snapshot", false, "a tag with no version is compared whole"}, + {"nightly", "nightly", true, "a tag with no version still matches itself"}, + } { + body := published(versionEntry(declared, c.published, "10.11.0.0")) + err := Judge(body, []Expected{{Slug: "widget", Path: declared, Tag: c.newest}}) + if c.current && err != nil { + t.Errorf("%s: %s against %s was refused: %v", c.why, c.newest, c.published, err) + } + if !c.current && err == nil { + t.Errorf("%s: %s against %s was read as current", c.why, c.newest, c.published) + } + } +}