From 319d56be7ee4ca5ccaa400003cda40bd761c78dd Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 12:45:35 +0100 Subject: [PATCH 1/2] Indexer: a registered module is read from every repository it was built from, whatever the branch, and at the registered commit when the head is gone --- .../internal/index/builds.go | 57 +++++++++++++------ .../internal/index/registered.go | 15 ++++- .../internal/index/registered_test.go | 49 +++++++++++++++- 3 files changed, 100 insertions(+), 21 deletions(-) diff --git a/modules/mesh-delivery-indexer/internal/index/builds.go b/modules/mesh-delivery-indexer/internal/index/builds.go index 8656664..2d52daa 100644 --- a/modules/mesh-delivery-indexer/internal/index/builds.go +++ b/modules/mesh-delivery-indexer/internal/index/builds.go @@ -98,30 +98,55 @@ type Registration struct { Module, Repository, Path, Commit string } -// registered is whether the record is an outcome of the trunk (no named branch, or main, master or trunk, as a repository's default branch is named) that did not fail: what registers a -// module. Unlike IsTrunkBuild it asks for no recorded source inputs, which no build records since ADR 0530 clause 1. +// registered is whether the record is an outcome that did not fail: what registers a module. Unlike IsTrunkBuild it +// asks for no recorded source inputs, which no build records since ADR 0530 clause 1, and for no particular branch: a +// repository's trunk is named main, master, or what its owner chose (the registered photos, de-spiegel and link2pay were +// built from nox-mesh, and every record of them names that ref). func registered(r delivery.BuildRecord) bool { - return r.State == "" && r.Failed == "" && trunkLike[r.Ref] + return r.State == "" && r.Failed == "" } -// RegistrationOf is the registration of a module by its newest registering record; false when there is none. -func (b *Builds) RegistrationOf(module string) (Registration, bool) { - var best *delivery.BuildRecord +// RegistrationsOf are the places a module was registered from, by its outcome records: the newest record of each +// repository and directory (a module's name was once another repository's, as photos was a directory of the catalogue's). +// Records of a trunk's ref (main, master, trunk, none) come before those of another branch, then the newer first, so the +// first that holds the module's manifest is the one indexed. +func (b *Builds) RegistrationsOf(module string) []Registration { + type place struct{ repository, path string } + best := map[place]delivery.BuildRecord{} b.mu.Lock() - defer b.mu.Unlock() for _, r := range b.records { - if r.Module == module && registered(r) && (best == nil || newer(r, *best)) { - c := r - best = &c + if r.Module != module || !registered(r) { + continue + } + k := place{delivery.RepositoryName(r.Repository), pathOf(strings.Trim(r.Path, "/"))} + if cur, ok := best[k]; !ok || newer(r, cur) { + best[k] = r } } - if best == nil { - return Registration{}, false + b.mu.Unlock() + recs := slices.Collect(maps.Values(best)) + slices.SortFunc(recs, func(x, y delivery.BuildRecord) int { + switch { + case trunkLike[x.Ref] != trunkLike[y.Ref]: + if trunkLike[x.Ref] { + return -1 + } + return 1 + case newer(x, y): + return -1 + case newer(y, x): + return 1 + } + return strings.Compare(x.Repository+x.Path, y.Repository+y.Path) + }) + out := make([]Registration, 0, len(recs)) + for _, r := range recs { + out = append(out, Registration{Module: r.Module, Repository: r.Repository, Path: r.Path, Commit: r.Commit}) } - return Registration{Module: best.Module, Repository: best.Repository, Path: best.Path, Commit: best.Commit}, true + return out } -// Registered is the registration of every module a record names, by module name. +// Registered is the registrations of every module a record names, by module name. func (b *Builds) Registered() []Registration { b.mu.Lock() names := map[string]bool{} @@ -131,9 +156,7 @@ func (b *Builds) Registered() []Registration { b.mu.Unlock() var out []Registration for _, name := range slices.Sorted(maps.Keys(names)) { - if reg, ok := b.RegistrationOf(name); ok { - out = append(out, reg) - } + out = append(out, b.RegistrationsOf(name)...) } return out } diff --git a/modules/mesh-delivery-indexer/internal/index/registered.go b/modules/mesh-delivery-indexer/internal/index/registered.go index 2ed1f30..08e067e 100644 --- a/modules/mesh-delivery-indexer/internal/index/registered.go +++ b/modules/mesh-delivery-indexer/internal/index/registered.go @@ -75,6 +75,12 @@ func (ix *Indexer) IndexRegistered(ctx context.Context, r Registration) error { ix.tried[tried] = true ix.mu.Unlock() read, err := ix.Read(ctx, repository, commit, dir) + if err != nil && r.Commit != "" && r.Commit != commit { + // A head the forge no longer holds (a history that was rewritten) leaves the commit the module was registered at. + ix.logf("%s: reading %s at the head %s failed (%v), so at the registered commit %s", r.Module, repository, commit, err, r.Commit) + commit = r.Commit + read, err = ix.Read(ctx, repository, commit, dir) + } if err != nil { ix.mu.Lock() delete(ix.tried, tried) // a forge that could not say is asked again @@ -104,10 +110,13 @@ func (ix *Indexer) IndexRegisteredModule(ctx context.Context, module string) err if ix.Builds == nil { return nil } - if r, ok := ix.Builds.RegistrationOf(module); ok { - return ix.IndexRegistered(ctx, r) + var first error + for _, r := range ix.Builds.RegistrationsOf(module) { + if err := ix.IndexRegistered(ctx, r); err != nil && first == nil { + first = err + } } - return nil + return first } // IndexRegisteredAll indexes every registered module the modules bucket does not hold. A module that cannot be read is diff --git a/modules/mesh-delivery-indexer/internal/index/registered_test.go b/modules/mesh-delivery-indexer/internal/index/registered_test.go index 9098816..efd4fd3 100644 --- a/modules/mesh-delivery-indexer/internal/index/registered_test.go +++ b/modules/mesh-delivery-indexer/internal/index/registered_test.go @@ -39,7 +39,7 @@ func TestARegisteredModuleIsIndexedFromItsRepository(t *testing.T) { // the gran read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": `{"module":"other","version":"1"}`}}}, {wantRead: true, name: "a repository whose default branch is master", path: "", ref: "master", want: true, read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": snake}}}, - {name: "a record of another branch", path: "", ref: "feat/x", + {wantRead: true, name: "a repository built from a branch of its own (nox-mesh)", path: "", ref: "nox-mesh", want: true, read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": snake}}}, {wantRead: true, name: "a repository the source names, the module unheld and present at the head", path: "", want: true, heads: map[string]string{"jschoubben/snake": h2}, read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": snake}}}, @@ -127,3 +127,50 @@ func dirOf(p string) string { } return p } + +func TestARegistrationOfAnotherRepositoryDoesNotHideTheRealOne(t *testing.T) { // photos: its newest trunk record was a directory of the catalogue's, then novox/photos at nox-mesh + const catalogue, own = "http://novox.internal:20000/novox/mesh-catalog.git", "http://novox.internal:20000/novox/Link2Pay.git" + ix, mods := modulesIndexer(t, + rec(t, "build-1", "link2pay", own, "", "nox-mesh", "", "2026-10-06T23:02:00Z", ""), + rec(t, "build-2", "link2pay", catalogue, "modules/link2pay", "main", "", "2026-10-04T16:02:00Z", ""), + rec(t, "build-3", "link2pay", catalogue, "modules/link2pay", "main", "", "2026-10-03T16:02:00Z", "")) + ix.Manifests = newMem() + var asked []string + ix.Read = func(_ context.Context, repo, _, dir string) (RepoRead, error) { + asked = append(asked, repo+":"+dir) + if repo == "novox/Link2Pay" { + return RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": `{"module":"link2pay","version":"1"}`}}, nil + } + return RepoRead{Root: "r0"}, nil + } + if err := ix.IndexRegisteredAll(context.Background()); err != nil { + t.Fatal(err) + } + m, ok := moduleOf(t, mods, "link2pay") + if !ok || m.Repository != "novox/Link2Pay" { + t.Fatalf("modules.link2pay = %+v, found %v", m, ok) + } + if len(asked) != 2 || asked[0] != "novox/mesh-catalog:modules/link2pay" || asked[1] != "novox/Link2Pay:." && asked[1] != "novox/Link2Pay:" { + t.Errorf("the forge was asked %v: want the catalogue's trunk record first, once, then the repository's own", asked) + } +} + +func TestAHeadTheForgeLacksFallsBackToTheRegisteredCommit(t *testing.T) { // car-hunter: the source's head read as a 404 + ix, mods := modulesIndexer(t, rec(t, "build-1", "snake", "http://novox.internal:20000/jschoubben/snake.git", "", "master", "", "2026-10-19T09:00:00Z", "")) + ix.Manifests = newMem() + ix.source = sourceNow{rev: 1, heads: map[string]string{"jschoubben/snake": h2}} + var asked []string + ix.Read = func(_ context.Context, _, sha, _ string) (RepoRead, error) { + asked = append(asked, sha) + if sha == h2 { + return RepoRead{}, errors.New("404") + } + return RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": `{"module":"snake","version":"1"}`}}, nil + } + if err := ix.IndexRegisteredAll(context.Background()); err != nil { + t.Fatal(err) + } + if m, ok := moduleOf(t, mods, "snake"); !ok || m.Commit != "7a74cee83f88e6585a5511c3cc53bdf0f5303d20" || len(asked) != 2 { + t.Fatalf("modules.snake = %+v, found %v, asked %v", m, ok, asked) + } +} From 699cbfb7c1311f683707fb9b0621be575408bd66 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 12:52:29 +0100 Subject: [PATCH 2/2] Indexer: a registered module takes its registered ref, not a newer branch's record; no read at an older commit after a failure --- .../internal/index/builds.go | 47 +++++++++++++---- .../internal/index/registered.go | 6 --- .../internal/index/registered_test.go | 51 ++++++++++++++----- 3 files changed, 75 insertions(+), 29 deletions(-) diff --git a/modules/mesh-delivery-indexer/internal/index/builds.go b/modules/mesh-delivery-indexer/internal/index/builds.go index 2d52daa..7c3eacd 100644 --- a/modules/mesh-delivery-indexer/internal/index/builds.go +++ b/modules/mesh-delivery-indexer/internal/index/builds.go @@ -95,7 +95,7 @@ func (b *Builds) PathOf(module, repository string) (string, bool) { // of it names it: the repository, the directory of its manifest there ("" or "." for the root) and the commit it was // built at. type Registration struct { - Module, Repository, Path, Commit string + Module, Repository, Path, Commit, Ref string } // registered is whether the record is an outcome that did not fail: what registers a module. Unlike IsTrunkBuild it @@ -112,18 +112,47 @@ func registered(r delivery.BuildRecord) bool { // first that holds the module's manifest is the one indexed. func (b *Builds) RegistrationsOf(module string) []Registration { type place struct{ repository, path string } - best := map[place]delivery.BuildRecord{} + byPlace := map[place][]delivery.BuildRecord{} b.mu.Lock() for _, r := range b.records { - if r.Module != module || !registered(r) { - continue - } - k := place{delivery.RepositoryName(r.Repository), pathOf(strings.Trim(r.Path, "/"))} - if cur, ok := best[k]; !ok || newer(r, cur) { - best[k] = r + if r.Module == module && registered(r) { + k := place{delivery.RepositoryName(r.Repository), pathOf(strings.Trim(r.Path, "/"))} + byPlace[k] = append(byPlace[k], r) } } b.mu.Unlock() + // Within one place the record is that of the ref a registration comes from, never a newer pull request's or try's: a + // trunk's ref (main, master, trunk, none), else the ref most of the place's records name (the registered photos, de-spiegel + // and link2pay were built from nox-mesh), the newer of two equal; then its newest record. + best := map[place]delivery.BuildRecord{} + for k, recs := range byPlace { + count, latest := map[string]int{}, map[string]delivery.BuildRecord{} + for _, r := range recs { + count[r.Ref]++ + if cur, ok := latest[r.Ref]; !ok || newer(r, cur) { + latest[r.Ref] = r + } + } + var pick string + first := true + for ref, rec := range latest { + switch { + case first: + case trunkLike[ref] != trunkLike[pick]: + if !trunkLike[ref] { + continue + } + case count[ref] != count[pick]: + if count[ref] < count[pick] { + continue + } + case !newer(rec, latest[pick]): + continue + } + pick, first = ref, false + } + best[k] = latest[pick] + } recs := slices.Collect(maps.Values(best)) slices.SortFunc(recs, func(x, y delivery.BuildRecord) int { switch { @@ -141,7 +170,7 @@ func (b *Builds) RegistrationsOf(module string) []Registration { }) out := make([]Registration, 0, len(recs)) for _, r := range recs { - out = append(out, Registration{Module: r.Module, Repository: r.Repository, Path: r.Path, Commit: r.Commit}) + out = append(out, Registration{Module: r.Module, Repository: r.Repository, Path: r.Path, Commit: r.Commit, Ref: r.Ref}) } return out } diff --git a/modules/mesh-delivery-indexer/internal/index/registered.go b/modules/mesh-delivery-indexer/internal/index/registered.go index 08e067e..3223844 100644 --- a/modules/mesh-delivery-indexer/internal/index/registered.go +++ b/modules/mesh-delivery-indexer/internal/index/registered.go @@ -75,12 +75,6 @@ func (ix *Indexer) IndexRegistered(ctx context.Context, r Registration) error { ix.tried[tried] = true ix.mu.Unlock() read, err := ix.Read(ctx, repository, commit, dir) - if err != nil && r.Commit != "" && r.Commit != commit { - // A head the forge no longer holds (a history that was rewritten) leaves the commit the module was registered at. - ix.logf("%s: reading %s at the head %s failed (%v), so at the registered commit %s", r.Module, repository, commit, err, r.Commit) - commit = r.Commit - read, err = ix.Read(ctx, repository, commit, dir) - } if err != nil { ix.mu.Lock() delete(ix.tried, tried) // a forge that could not say is asked again diff --git a/modules/mesh-delivery-indexer/internal/index/registered_test.go b/modules/mesh-delivery-indexer/internal/index/registered_test.go index efd4fd3..06e2838 100644 --- a/modules/mesh-delivery-indexer/internal/index/registered_test.go +++ b/modules/mesh-delivery-indexer/internal/index/registered_test.go @@ -155,22 +155,45 @@ func TestARegistrationOfAnotherRepositoryDoesNotHideTheRealOne(t *testing.T) { / } } -func TestAHeadTheForgeLacksFallsBackToTheRegisteredCommit(t *testing.T) { // car-hunter: the source's head read as a 404 - ix, mods := modulesIndexer(t, rec(t, "build-1", "snake", "http://novox.internal:20000/jschoubben/snake.git", "", "master", "", "2026-10-19T09:00:00Z", "")) - ix.Manifests = newMem() - ix.source = sourceNow{rev: 1, heads: map[string]string{"jschoubben/snake": h2}} - var asked []string - ix.Read = func(_ context.Context, _, sha, _ string) (RepoRead, error) { - asked = append(asked, sha) - if sha == h2 { - return RepoRead{}, errors.New("404") +func TestAForgeThatCannotSayLeavesTheModuleUnwrittenAndAsksAgain(t *testing.T) { // a timeout or a 404 at the head: no read at an older commit + for _, failure := range []string{"timeout", "Gitea API /repos/x/git/trees/y: 404 not found"} { + ix, mods := modulesIndexer(t, rec(t, "build-1", "snake", "http://novox.internal:20000/jschoubben/snake.git", "", "master", "", "2026-10-19T09:00:00Z", "")) + ix.Manifests = newMem() + ix.source = sourceNow{rev: 1, heads: map[string]string{"jschoubben/snake": h2}} + var asked []string + ix.Read = func(_ context.Context, _, sha, _ string) (RepoRead, error) { + asked = append(asked, sha) + return RepoRead{}, errors.New(failure) + } + if err := ix.IndexRegisteredAll(context.Background()); err == nil { + t.Errorf("%q: no error", failure) + } + if _, ok := moduleOf(t, mods, "snake"); ok || len(asked) != 1 || asked[0] != h2 { + t.Errorf("%q: written %v, asked %v", failure, ok, asked) + } + if err := ix.IndexRegisteredAll(context.Background()); err == nil || len(asked) != 2 { + t.Errorf("%q: not asked again (asked %v)", failure, asked) } - return RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": `{"module":"snake","version":"1"}`}}, nil } - if err := ix.IndexRegisteredAll(context.Background()); err != nil { - t.Fatal(err) +} + +func TestARecordOfAPullRequestDoesNotReplaceTheRegisteredRef(t *testing.T) { // a newer feat/x record of the same place + const repo = "http://novox.internal:20000/novox/photos.git" + b := NewBuilds() + for _, r := range [][2]any{ + rec(t, "b1", "photos", repo, "", "nox-mesh", "", "2026-10-01T09:00:00Z", ""), + rec(t, "b2", "photos", repo, "", "nox-mesh", "", "2026-10-02T09:00:00Z", ""), + rec(t, "b3", "photos", repo, "", "feat/x", "", "2026-10-09T09:00:00Z", ""), + } { + b.Put(r[0].(string), r[1].([]byte)) } - if m, ok := moduleOf(t, mods, "snake"); !ok || m.Commit != "7a74cee83f88e6585a5511c3cc53bdf0f5303d20" || len(asked) != 2 { - t.Fatalf("modules.snake = %+v, found %v, asked %v", m, ok, asked) + got := b.RegistrationsOf("photos") + if len(got) != 1 || got[0].Ref != "nox-mesh" { + t.Fatalf("registrations = %+v, want the ref most records name, not the newer feat/x", got) + } + k, v := record(t, "b4", "photos", repo, "", "main", "", "2026-10-03T09:00:00Z", "") + b.Put(k, v) // a trunk's ref beats a branch of its own + if got := b.RegistrationsOf("photos"); len(got) != 1 || got[0].Ref != "main" { + t.Fatalf("registrations = %+v", got) } }