From 8d38c6bac95929794d9b8840cb03bc4ae9a1581e Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 21:16:20 +0100 Subject: [PATCH] Write the source revision a candidate was previewed onto, so the builder merges its preview onto that trunk head Candidate.Base was never written non-zero (review round 2): the builder's base lookup would have found no trunk for any live candidate and built no preview at all. The indexer now writes, whenever it writes a candidate's fingerprints, the gate's source revision whose trunk heads it read the previews onto; that head of the pull request's repository is the preview's trunk. No mesh-sdk change: the field and its meaning were there. The builder's test now takes the candidate the indexer's own test writes (testdata/candidate-written.json, kept equal to it), not a hand-made one. --- .../internal/build/candidates_test.go | 64 +++++++++++++++++++ .../internal/index/previews.go | 22 +++++-- .../internal/index/previews_test.go | 46 ++++++++++++- .../index/testdata/candidate-written.json | 31 +++++++++ 4 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 modules/mesh-delivery-indexer/internal/index/testdata/candidate-written.json diff --git a/modules/mesh-delivery-builder/internal/build/candidates_test.go b/modules/mesh-delivery-builder/internal/build/candidates_test.go index af8d616..41ca4f1 100644 --- a/modules/mesh-delivery-builder/internal/build/candidates_test.go +++ b/modules/mesh-delivery-builder/internal/build/candidates_test.go @@ -2,9 +2,13 @@ package build import ( "context" + "encoding/json" + "fmt" + "os" "strings" "testing" + "git.novox.be/novox/mesh-sdk/go/catalogue" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -47,7 +51,67 @@ const ( headY = "2222222222222222222222222222222222222222" ) +// indexerCandidate is a candidate as the indexer writes it, with the gate's source revision it names (mesh-delivery-indexer +// internal/index testdata/candidate-written.json, kept equal to what the indexer writes by its own test). +func indexerCandidate(t *testing.T) (rev uint64, heads map[string]string, c delivery.Candidate, raw []byte) { + t.Helper() + data, err := os.ReadFile("../../../mesh-delivery-indexer/internal/index/testdata/candidate-written.json") + if err != nil { + t.Fatal(err) + } + var w struct { + Source struct { + Rev uint64 `json:"rev"` + Heads map[string]string `json:"heads"` + } `json:"source"` + Candidate json.RawMessage `json:"candidate"` + } + if err := json.Unmarshal(data, &w); err != nil { + t.Fatal(err) + } + if c, err = delivery.DecodeCandidate(w.Candidate); err != nil { + t.Fatal(err) + } + return w.Source.Rev, w.Source.Heads, c, w.Candidate +} + func TestACandidatesFingerprintIsBuiltOnceAsAKeyOfItsOwnFromTheHeadMergedOntoTheTrunk(t *testing.T) { // N7/AC1, N7/AC2 (issue 1684) + // The candidate the indexer writes is merged onto the trunk head of the source revision it names, never onto the module + // value's commit, which stays at the commit its fingerprint first appeared at (review of the indexer writing on change). + { + rev, heads, c, raw := indexerCandidate(t) + p := c.Pulls[0] + cm := p.Fps["avahi"] + ir := newRig(t, goBundle) + ir.mf.byDigest[cm.Manifest] = catalogue.Manifest{Module: "avahi", Build: &catalogue.Build{Artifacts: []catalogue.Artifact{goBundle}}} + ir.b.Trunk = func(_ context.Context, at uint64, repository string) (string, error) { + if at != rev { + return "", fmt.Errorf("source revision %d is not heard", at) + } + return heads[repository], nil + } + const valueCommit = "7a74cee83f88e6585a5511c3cc53bdf0f5303d20" + value, err := delivery.EncodeModule(delivery.Module{Repository: p.Repository, Commit: valueCommit, FP2: "src2:" + hex64('e'), + Manifest: cm.Manifest, Path: "modules/avahi", Toolchains: agreedToolchains}) + if err != nil { + t.Fatal(err) + } + ir.b.Module(context.Background(), "avahi", value) + ir.b.Wait() + ir.bm.mu.Lock() + ir.bm.calls = nil + ir.bm.mu.Unlock() + ir.b.Candidate(context.Background(), "feat%2Fx", raw) + ir.b.Wait() + fetches := ir.bm.callsOf("build_go_fetch") + if len(fetches) != 1 { + t.Fatalf("%d fetches of the indexer's candidate, want one", len(fetches)) + } + f := fetches[0].body.(map[string]any) + if want := heads[p.Repository]; want == "" || f["base"] != want || f["base"] == valueCommit || f["commit"] != p.Head.Head { + t.Errorf("fetch args = %+v, want the head merged onto the trunk head %.8s of source revision %d", f, want, rev) + } + } r := newRig(t, goBundle) trunkModule(t, r) r.b.Candidate(context.Background(), "feat%2Fx", candidateValue(t, "feat/x", 7, headX)) diff --git a/modules/mesh-delivery-indexer/internal/index/previews.go b/modules/mesh-delivery-indexer/internal/index/previews.go index c2913ea..25b0015 100644 --- a/modules/mesh-delivery-indexer/internal/index/previews.go +++ b/modules/mesh-delivery-indexer/internal/index/previews.go @@ -112,12 +112,12 @@ func (ix *Indexer) RecomputeCandidates(ctx context.Context) error { return nil } -// previewOf is the preview of a head merged onto the trunk head the source names now: found, or stale when only a preview +// previewOf is the preview of a head merged onto the trunk head the source heads name: found, or stale when only a preview // onto another trunk head exists, or neither (found false, stale false) when the source is not known or none exists. -func (ix *Indexer) previewOf(repository, head string) (pv delivery.Preview, found, stale bool) { +func (ix *Indexer) previewOf(heads map[string]string, repository, head string) (pv delivery.Preview, found, stale bool) { ix.mu.Lock() defer ix.mu.Unlock() - trunk := ix.source.heads[repository] + trunk := heads[repository] if trunk == "" { return delivery.Preview{}, false, false } @@ -137,14 +137,23 @@ func (ix *Indexer) previewOf(repository, head string) (pv delivery.Preview, foun // withPreviews gives each touched module of the candidate's pull requests the fingerprint its preview gives it, or the // reason it has none, and the candidate its fpset. It answers whether the candidate changed. A module of a pull request with // no preview keeps what it had. +// +// The candidate's Base is the source revision whose trunk heads the previews were read onto (novox/hq ADR 0437 ยง5): written +// whenever the fingerprints are, so the builder merges a preview onto the trunk head its fingerprint was computed on (the head +// of the pull request's repository in that revision), never onto a module value's commit, which stays at the commit its +// fingerprint first appeared at. func (ix *Indexer) withPreviews(ctx context.Context, c *delivery.Candidate) bool { - changed := false + ix.mu.Lock() + rev, heads := ix.source.rev, ix.source.heads + ix.mu.Unlock() + changed, read := false, false for i := range c.Pulls { p := &c.Pulls[i] - pv, found, stale := ix.previewOf(p.Repository, p.Head.Head) + pv, found, stale := ix.previewOf(heads, p.Repository, p.Head.Head) if !found && !stale { continue } + read = true // A touched module's directory: the one the modules bucket holds of the repository, else the preview's directory // whose manifest is the one the pull request gave the module (a module the pull request adds; ADR 0501 clause 6). dirs := map[string]string{} @@ -198,6 +207,9 @@ func (ix *Indexer) withPreviews(ctx context.Context, c *delivery.Candidate) bool if changed { c.FPSet = delivery.FingerprintSet(c.Pulls) } + if read && rev != 0 && c.Base != rev && (changed || c.Base == 0) { + c.Base, changed = rev, true + } return changed } diff --git a/modules/mesh-delivery-indexer/internal/index/previews_test.go b/modules/mesh-delivery-indexer/internal/index/previews_test.go index 5d32716..57e7b90 100644 --- a/modules/mesh-delivery-indexer/internal/index/previews_test.go +++ b/modules/mesh-delivery-indexer/internal/index/previews_test.go @@ -2,6 +2,8 @@ package index import ( "context" + "encoding/json" + "os" "strings" "testing" @@ -87,9 +89,16 @@ func TestACandidatesModuleGetsTheFingerprintATrunkModuleOfTheSameTreeHas(t *test if got.FP2 != trunk.FP2 || got.None2 != "" || got.Manifest != delivery.ManifestDigest([]byte(avahiOnTools(t))) { t.Fatalf("candidate avahi = %+v, want the trunk's fingerprint %s", got, trunk.FP2) } - if c := candidateOf(t, cands); c.FPSet != delivery.FingerprintSet(c.Pulls) || c.FPSet == "" { + c := candidateOf(t, cands) + if c.FPSet != delivery.FingerprintSet(c.Pulls) || c.FPSet == "" { t.Errorf("fpset = %q", c.FPSet) } + // The candidate names the source revision the preview was read onto (5, whose catalogue head is the preview's trunk): + // the builder merges the preview onto that head, never onto the module value's commit. + if c.Base != 5 { + t.Errorf("candidate base = %d, want the source revision 5 its preview was read onto", c.Base) + } + written(t, writtenCandidate{Source: sourceRev{Rev: 5, Heads: map[string]string{catalogueRepo: trunkNow}}, Candidate: c}) // Another tree gives another fingerprint. pv := avahiPreview(t, trunkNow, h1) m := pv.Modules["modules/avahi"] @@ -191,3 +200,38 @@ func TestAPreviewOfConflictsGivesTheCandidatesModuleTheReasonItDoesNotMerge(t *t t.Errorf("candidate avahi = %+v, want it not merging and naming the files", got) } } + +// writtenCandidate is what the builder's tests read of the indexer (mesh-delivery-builder internal/build): a candidate as the +// indexer writes it and the gate's source revision it names, kept in testdata/candidate-written.json so the builder is tested +// on what the indexer writes, never on a candidate made by hand. MESH_WRITE_TESTDATA=1 writes it again. +type writtenCandidate struct { + Source sourceRev `json:"source"` + Candidate delivery.Candidate `json:"candidate"` +} + +type sourceRev struct { + Rev uint64 `json:"rev"` + Heads map[string]string `json:"heads"` +} + +func written(t *testing.T, w writtenCandidate) { + t.Helper() + data, err := json.MarshalIndent(w, "", " ") + if err != nil { + t.Fatal(err) + } + data = append(data, '\n') + const file = "testdata/candidate-written.json" + if os.Getenv("MESH_WRITE_TESTDATA") == "1" { + if err := os.WriteFile(file, data, 0o644); err != nil { + t.Fatal(err) + } + } + kept, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + if string(kept) != string(data) { + t.Errorf("%s is not what the indexer writes now (MESH_WRITE_TESTDATA=1 writes it again):\n%s", file, data) + } +} diff --git a/modules/mesh-delivery-indexer/internal/index/testdata/candidate-written.json b/modules/mesh-delivery-indexer/internal/index/testdata/candidate-written.json new file mode 100644 index 0000000..966f47b --- /dev/null +++ b/modules/mesh-delivery-indexer/internal/index/testdata/candidate-written.json @@ -0,0 +1,31 @@ +{ + "source": { + "rev": 5, + "heads": { + "novox/mesh-catalog": "5555555555555555555555555555555555555555" + } + }, + "candidate": { + "v": 1, + "group": "feat/x", + "base": 5, + "heads": "dfe9fb8f79e8", + "pulls": [ + { + "repository": "novox/mesh-catalog", + "number": 301, + "head": "0123456789abcdef0123456789abcdef01234567", + "modules": [ + "avahi" + ], + "fps": { + "avahi": { + "fp2": "src2:e8d595aea1b13f1f5578ef1f245dab0d28a351c28079ecdcd2276c63a932d7be", + "manifest": "8a3856b6c0566d76f2556fc4f46c5251dc7935517bb6bd15809ded7e5546a0d8" + } + } + } + ], + "fpset": "01c840ec276b" + } +}