From 22aedbe97fcc9a858523da4f3c1dc0b86cc896b6 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 3 Nov 2026 14:10:08 +0100 Subject: [PATCH] =?UTF-8?q?A=20recompute=20with=20no=20merge=20writes=20th?= =?UTF-8?q?e=20fingerprint's=20source=20revision=20as=20not=20known,=20so?= =?UTF-8?q?=20no=20release's=20Approve=20answers=20a=20build=20it=20never?= =?UTF-8?q?=20carried,=20though=20the=20source=20revision=20is=20still=20t?= =?UTF-8?q?hat=20release's=20(ADR=200601=20=C2=A71;=20review=20round=202)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery-assigner/pass_test.go | 9 ++++--- .../internal/index/source.go | 7 ----- .../internal/index/toolchains.go | 8 +++--- .../internal/index/toolchains_test.go | 26 ++++++++++++++----- 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go index 799ef530..d9912d36 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go @@ -811,10 +811,11 @@ func aReleasesApproveAnswersTheMoveOfItsBuildAlone(t *testing.T) { {"a setting changed at the build it runs", true, fpTest, commit, 7, false, true}, {"no approved release carries it", true, fpOld, "", 7, false, true}, {"the release merged another commit", true, fpOld, strings.Repeat("2", 40), 7, false, true}, - // Review round 1: its fingerprint moved by a recompute after the release (a toolchain, a package), with no merge of - // web: stamped with that moment's revision, whose release carries nothing of web; the earlier Approve answers nothing. - {"a fingerprint moved by a recompute after the release", true, fpOld, commit, 8, false, true}, - {"a fingerprint whose revision is not known", true, fpOld, commit, 0, false, true}, + // Its fingerprint last moved at another release's revision, which merged nothing of web: that Approve answers nothing. + {"a fingerprint another release moved", true, fpOld, commit, 8, false, true}, + // Review rounds 1 and 2: moved by a recompute after the release (a toolchain, a package, a base) with no merge, the + // indexer writes fp-src 0, not known, though Src is still the release's: asked of a person. + {"a fingerprint moved by a recompute after the release", true, fpOld, commit, 0, false, true}, } { asksKV := &memKV{values: map[string][]byte{}} p := newPasser(buckets{delivery.AssignerAsksBucket: asksKV}, time.Now()) diff --git a/modules/mesh-delivery-indexer/internal/index/source.go b/modules/mesh-delivery-indexer/internal/index/source.go index b7318bf4..7dd92d34 100644 --- a/modules/mesh-delivery-indexer/internal/index/source.go +++ b/modules/mesh-delivery-indexer/internal/index/source.go @@ -14,13 +14,6 @@ import ( // with it (Module.Src) when the head the source names for its repository is the merge commit the module's fingerprint // was computed at: from the modules bucket when the merge was heard first, and when the merge is heard after the source. -// sourceRev is the latest source revision seen, 0 before any. -func (ix *Indexer) sourceRev() uint64 { - ix.mu.Lock() - defer ix.mu.Unlock() - return ix.source.rev -} - // sourceNow is the latest source revision seen. type sourceNow struct { rev uint64 diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains.go b/modules/mesh-delivery-indexer/internal/index/toolchains.go index c8cee117..269f8fd1 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains.go @@ -313,9 +313,11 @@ func (ix *Indexer) recomputeModules(ctx context.Context, overlay map[string]deli return nil, false, nil } if cur.FP2 != r.fp { - // Moved with no merge of its own repository (a toolchain, a package, a base): the source revision of now, whose - // release does not carry it, never the Src of the merge it was last merged at (ADR 0601 §1). - cur.FPSrc = ix.sourceRev() + // Moved with no merge (a toolchain, a package, a base): no release carries this build, and the source revision + // moves only at a merge, so it is not known (0) and the move is asked of a person; never the revision of now, + // which may be the very release that last merged the module (ADR 0601 §1). The source stamp leaves it, since + // Src is set. + cur.FPSrc = 0 } cur.FP2, cur.None2, cur.Toolchains, cur.Packages = r.fp, r.none, r.tcs, r.pkgs cur.Inputs2, cur.ArtifactPaths = inputs, paths diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains_test.go b/modules/mesh-delivery-indexer/internal/index/toolchains_test.go index 368f0602..5ee8da5b 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains_test.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains_test.go @@ -113,19 +113,31 @@ func TestAgreedToolchainChangesAGoModulesReasonWithNoMerge(t *testing.T) { // 14 if m, _ := moduleOf(t, mods, "avahi"); m.None2 != delivery.ToolchainNoReport { t.Fatalf("before: none2 = %q", m.None2) } - ix.mu.Lock() - ix.source.rev = 11 // the source revision of now, whose release merged nothing of avahi's - ix.mu.Unlock() + // The release that last merged avahi: its source revision names avahi's commit, so Src (and FPSrc) are 7. + merged, _ := moduleOf(t, mods, "avahi") + src := delivery.Source{Group: "g", Heads: map[string]string{merged.Repository: merged.Commit}} + if err := ix.ApplySource(context.Background(), 7, src); err != nil { + t.Fatal(err) + } + if m, _ := moduleOf(t, mods, "avahi"); m.Src != 7 || m.FPSrc != 7 { + t.Fatalf("before: src %d fp-src %d, want 7 and 7", m.Src, m.FPSrc) + } reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.1") reportToolchain(t, ix, "mesh-build-go", "ace", "go", "go1.27.1") m, _ := moduleOf(t, mods, "avahi") if m.None2 != "" || m.FP2 == "" { t.Errorf("none2 = %q fp2 = %q, want a fingerprint: no builder input is waited for (ADR 0530 §1)", m.None2, m.FP2) } - // ADR 0601 §1: a fingerprint moved with no merge is stamped with the source revision of the moment, never the Src of - // the merge it was last merged at, so no earlier release is taken to carry it. - if m.FPSrc != 11 || m.Src == 11 { - t.Errorf("a fingerprint moved by a recompute: fp-src %d, src %d; want fp-src 11", m.FPSrc, m.Src) + // ADR 0601 §1 (review round 2): a fingerprint moved with no merge has no release that carries it: fp-src 0, not known, + // though the source revision is still the release's at the module's own Src; a source revision heard again stamps nothing. + if m.FPSrc != 0 { + t.Errorf("a fingerprint moved by a recompute: fp-src %d, want 0", m.FPSrc) + } + if err := ix.ApplySource(context.Background(), 8, src); err != nil { + t.Fatal(err) + } + if again, _ := moduleOf(t, mods, "avahi"); again.FPSrc != 0 { + t.Errorf("a source revision stamped the recomputed fingerprint: fp-src %d", again.FPSrc) } }