From d9bfba1e6253d4f9cdb9d5ef3e934b3450ae948c Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:36:22 +0100 Subject: [PATCH] Indexer: read one directory of a registered module's repository, leave a repository the source names to its merges, accept master as a trunk --- .../cmd/mesh-delivery-indexer/named.go | 12 ++-- .../cmd/mesh-delivery-indexer/named_test.go | 9 ++- .../internal/index/builds.go | 4 +- .../internal/index/candidates.go | 8 +-- .../internal/index/registered.go | 39 +++++++----- .../internal/index/registered_test.go | 62 +++++++++++++------ 6 files changed, 86 insertions(+), 48 deletions(-) diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named.go index 0678740..796cfba 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named.go @@ -106,11 +106,11 @@ const getFileTool = "gitea.gitea_get_file" // lockFiles are read beside a manifest, as the forge bridge reads them (bridge.LockFiles). var lockFiles = []string{"go.sum", "package-lock.json"} -// readVia reads a repository at a commit as the forge bridge does for a merged message: the module directories by -// gitea_commit_trees, then each one's module.json and lock files by gitea_get_file. A manifest the forge does not have is -// an error; a lock file it does not have is left out. -func readVia(call toolCaller) func(ctx context.Context, repository, sha string) (index.RepoRead, error) { - return func(_ context.Context, repository, sha string) (index.RepoRead, error) { +// readVia reads a repository at a commit as the forge bridge does for a merged message: the trees by gitea_commit_trees, +// then the module.json and lock files of the one directory dir by gitea_get_file (a repository can hold hundreds). A manifest the +// forge does not have is an error; a directory with no module.json in the trees is answered with no manifest; a lock file it does not have is left out. +func readVia(call toolCaller) func(ctx context.Context, repository, sha, dir string) (index.RepoRead, error) { + return func(_ context.Context, repository, sha, dir string) (index.RepoRead, error) { owner, repo, ok := strings.Cut(delivery.RepositoryName(repository), "/") if !ok { return index.RepoRead{}, fmt.Errorf("%q is not /", repository) @@ -146,7 +146,7 @@ func readVia(call toolCaller) func(ctx context.Context, repository, sha string) } return *f.File.Content, nil } - for dir := range a.Trees { + if _, held := a.Trees[dir]; held { m, err := file(dir, "module.json") if err != nil { return index.RepoRead{}, fmt.Errorf("%s: %w", path.Join(dir, "module.json"), err) diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go index 2815aae..b3040ed 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go @@ -53,11 +53,13 @@ func TestTheTrunkIsTheDirectoriesTheForgeListsTrees(t *testing.T) { // 1900/AC3 } func TestReadViaReadsManifestsAndLocks(t *testing.T) { + var files []string call := func(key string, body any) (json.RawMessage, error) { switch key { case commitTreesTool: - return json.RawMessage(`{"root":"r0","trees":{"":"t1"}}`), nil + return json.RawMessage(`{"root":"r0","trees":{"":"t1","modules/other":"t2"}}`), nil case getFileTool: + files = append(files, body.(map[string]string)["path"]) switch body.(map[string]string)["path"] { case "module.json": return json.RawMessage(`{"file":{"content":"{\"module\":\"snake\"}"}}`), nil @@ -68,10 +70,13 @@ func TestReadViaReadsManifestsAndLocks(t *testing.T) { } return nil, errors.New("unexpected " + key) } - got, err := readVia(call)(context.Background(), "http://novox.internal:20000/jschoubben/snake.git", "abc") + got, err := readVia(call)(context.Background(), "http://novox.internal:20000/jschoubben/snake.git", "abc", "") if err != nil { t.Fatal(err) } + if len(got.Manifests) != 1 || len(files) != 3 { // the one directory asked: its manifest and two lock files, never the others + t.Fatalf("manifests = %v, files asked = %v", got.Manifests, files) + } if got.Root != "r0" || got.Manifests[""] != `{"module":"snake"}` || len(got.Locks[""]) != 1 || got.Locks[""]["go.sum"] == "" { t.Fatalf("read = %+v", got) } diff --git a/modules/mesh-delivery-indexer/internal/index/builds.go b/modules/mesh-delivery-indexer/internal/index/builds.go index 38376a6..8656664 100644 --- a/modules/mesh-delivery-indexer/internal/index/builds.go +++ b/modules/mesh-delivery-indexer/internal/index/builds.go @@ -98,10 +98,10 @@ type Registration struct { Module, Repository, Path, Commit string } -// registered is whether the record is an outcome of the trunk (or of no named branch) that did not fail: what registers a +// 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. func registered(r delivery.BuildRecord) bool { - return r.State == "" && r.Failed == "" && (r.Ref == "" || r.Ref == "main") + return r.State == "" && r.Failed == "" && trunkLike[r.Ref] } // RegistrationOf is the registration of a module by its newest registering record; false when there is none. diff --git a/modules/mesh-delivery-indexer/internal/index/candidates.go b/modules/mesh-delivery-indexer/internal/index/candidates.go index 2cc52d0..a40eeb1 100644 --- a/modules/mesh-delivery-indexer/internal/index/candidates.go +++ b/modules/mesh-delivery-indexer/internal/index/candidates.go @@ -53,9 +53,9 @@ type Indexer struct { // has none. Named func(ctx context.Context, repository, sha string, paths map[string][]string) (map[string]map[string]string, error) Trunk func(ctx context.Context, repository, sha string) ([]string, error) - // Read asks the forge for a repository's module directories at a commit, with their manifests and lock-file hashes, as - // the forge bridge reads them for a merged message (registered.go). Nil indexes no registered module. - Read func(ctx context.Context, repository, sha string) (RepoRead, error) + // Read asks the forge for a repository's trees at a commit and the manifest and lock-file hashes of the one module directory dir (the + // root spelled ""), as the forge bridge reads them for a merged message (registered.go). Nil indexes no registered module. + Read func(ctx context.Context, repository, sha, dir string) (RepoRead, error) // Contains says whether a commit is the second or an ancestor of it (gitea.gitea_contains). Contains func(ctx context.Context, repository, commit, in string) (bool, error) Logf func(format string, args ...any) @@ -64,7 +64,7 @@ type Indexer struct { source sourceNow previews map[string]delivery.Preview // by previews key (previews.go) toolchains map[string]map[string]delivery.Toolchain // language, then node (toolchains.go) - tried map[string]string // registered module to the commit its manifest was last tried at (registered.go) + tried map[string]bool // repository, commit and directory a registered module's manifest was tried at (registered.go) recompute sync.Mutex // one Recompute at a time } diff --git a/modules/mesh-delivery-indexer/internal/index/registered.go b/modules/mesh-delivery-indexer/internal/index/registered.go index 09ddd50..248bc81 100644 --- a/modules/mesh-delivery-indexer/internal/index/registered.go +++ b/modules/mesh-delivery-indexer/internal/index/registered.go @@ -11,7 +11,7 @@ import ( // A module registered in the catalogue from its own repository (the operator registers it with the repository and the // path of its manifest) had no manifest here: the indexer learns a repository only from a merged message, and no merge // followed the registration. The controller's outcome records name each registered module's repository, directory and -// commit (builds.go Registered), so the indexer reads that module's manifest at the repository's trunk head and writes the +// commit (builds.go Registered), so the indexer reads that module's manifest writes the // same keys a merge writes. A later merge to the repository's trunk reaches the module through the forge bridge's merged // message as for every other repository. @@ -25,7 +25,7 @@ type RepoRead struct { } // IndexRegistered writes the module of a registration as a merge of its repository would, when the modules bucket holds -// none of that name. Its commit is the head the gate's source names for the repository, else the registration's. The +// none of that name. Its commit is the registration's; a repository the gate's source names is left to the merges. The // repository's other modules are neither written nor dropped: they are the merges'. A manifest that is not at the path or // names another module is said and left. func (ix *Indexer) IndexRegistered(ctx context.Context, r Registration) error { @@ -44,40 +44,47 @@ func (ix *Indexer) IndexRegistered(ctx context.Context, r Registration) error { } repository := delivery.RepositoryName(r.Repository) commit := r.Commit + dir := strings.Trim(r.Path, "/") + if dir == delivery.RootPath { + dir = "" + } + // A repository the gate's source names is the merges': an unheld module there was removed by a merge, or its record is + // stale, and reading the repository for it would cost a forge call for each such module at every start. ix.mu.Lock() - for repo, head := range ix.source.heads { - if delivery.SameRepository(repo, r.Repository) && head != "" { - commit = head + owned := false + for repo := range ix.source.heads { + if delivery.SameRepository(repo, r.Repository) { + owned = true } } ix.mu.Unlock() + if owned { + return nil + } if repository == "" || commit == "" { ix.logf("%s: registered with no repository or commit, so its manifest is not read", r.Module) return nil } - // One try for each commit of a registration: a repository with no manifest where it was registered is said once, not at - // every record of the controller's builds. + // One try for each repository, commit and directory: a repository with no manifest where it was registered is said once, + // not at every record of the controller's builds. + tried := repository + "@" + commit + ":" + dir ix.mu.Lock() - if ix.tried[r.Module] == commit { + if ix.tried[tried] { ix.mu.Unlock() return nil } if ix.tried == nil { - ix.tried = map[string]string{} + ix.tried = map[string]bool{} } - ix.tried[r.Module] = commit + ix.tried[tried] = true ix.mu.Unlock() - read, err := ix.Read(ctx, repository, commit) + read, err := ix.Read(ctx, repository, commit, dir) if err != nil { ix.mu.Lock() - delete(ix.tried, r.Module) // a forge that could not say is asked again + delete(ix.tried, tried) // a forge that could not say is asked again ix.mu.Unlock() return fmt.Errorf("%s: reading %s at %s: %w", r.Module, repository, commit, err) } - dir := strings.Trim(r.Path, "/") - if dir == delivery.RootPath { - dir = "" - } raw, ok := read.Manifests[dir] if !ok || read.Trees[dir] == "" { ix.logf("%s: %s at %s holds no module.json at %q, so the registered module is not indexed", r.Module, repository, commit, dir) diff --git a/modules/mesh-delivery-indexer/internal/index/registered_test.go b/modules/mesh-delivery-indexer/internal/index/registered_test.go index 70e9779..a6bca70 100644 --- a/modules/mesh-delivery-indexer/internal/index/registered_test.go +++ b/modules/mesh-delivery-indexer/internal/index/registered_test.go @@ -12,32 +12,41 @@ func TestARegisteredModuleIsIndexedFromItsRepository(t *testing.T) { // the gran const forge = "http://novox.internal:20000/jschoubben/snake.git" snake := `{"module":"snake","version":"1"}` for _, tc := range []struct { - name string - path string - read RepoRead - err error - held bool // modules.snake exists already - other bool // modules.other of the same repository exists - want bool // modules.snake is written + name string + path string + read RepoRead + err error + ref string // the build record's ref + heads map[string]string // the gate source's heads + wantRead bool // the forge is read + held bool // modules.snake exists already + other bool // modules.other of the same repository exists + want bool // modules.snake is written }{ - {name: "a module at the root of its repository", path: "", want: true, + {wantRead: true, name: "a module at the root of its repository", path: "", want: true, read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": snake}}}, - {name: "a root spelled as a dot", path: ".", want: true, + {wantRead: true, name: "a root spelled as a dot", path: ".", want: true, read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": snake}}}, - {name: "a module in a directory", path: "modules/snake", want: true, + {wantRead: true, name: "a module in a directory", path: "modules/snake", want: true, read: RepoRead{Root: "r0", Trees: map[string]string{"modules/snake": "t1"}, Manifests: map[string]string{"modules/snake": snake}}}, - {name: "another module of the repository is neither written nor dropped", path: "", want: true, other: true, + {wantRead: true, name: "another module of the repository is neither written nor dropped", path: "", want: true, other: true, read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": snake}}}, {name: "a module the bucket holds is left", path: "", held: true, read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": snake}}}, - {name: "no manifest at the path", path: "modules/x", + {wantRead: true, name: "no manifest at the path", path: "modules/x", read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": snake}}}, - {name: "a manifest of another module", path: "", + {wantRead: true, name: "a manifest of another module", path: "", read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": `{"module":"other","version":"1"}`}}}, - {name: "a forge that cannot say", path: "", err: errors.New("down")}, + {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", + read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": snake}}}, + {name: "a repository the gate's source names is the merges'", path: "", heads: map[string]string{"jschoubben/snake": h2}, + read: RepoRead{Root: "r0", Trees: map[string]string{"": "t1"}, Manifests: map[string]string{"": snake}}}, + {wantRead: true, name: "a forge that cannot say", path: "", err: errors.New("down")}, } { t.Run(tc.name, func(t *testing.T) { - ix, mods := modulesIndexer(t, rec(t, "build-1", "snake", forge, tc.path, "", "", "2026-10-19T09:00:00Z", "")) + ix, mods := modulesIndexer(t, rec(t, "build-1", "snake", forge, tc.path, tc.ref, "", "2026-10-19T09:00:00Z", "")) if tc.held { if err := mods.Create(context.Background(), "snake", mustModule(t, delivery.Module{Repository: "jschoubben/snake", Commit: h1, Path: ".", None2: "x"})); err != nil { t.Fatal(err) @@ -49,15 +58,32 @@ func TestARegisteredModuleIsIndexedFromItsRepository(t *testing.T) { // the gran } } ix.Manifests = newMem() - var asked struct{ repo, sha string } - ix.Read = func(_ context.Context, repo, sha string) (RepoRead, error) { - asked.repo, asked.sha = repo, sha + var asked struct { + repo, sha, dir string + n int + } + ix.Read = func(_ context.Context, repo, sha, dir string) (RepoRead, error) { + asked.repo, asked.sha, asked.dir = repo, sha, dir + asked.n++ return tc.read, tc.err } + if tc.heads != nil { + ix.source = sourceNow{rev: 1, heads: tc.heads} + } err := ix.IndexRegisteredAll(context.Background()) if (err != nil) != (tc.err != nil) { t.Fatalf("error = %v", err) } + wantReads := 1 + if !tc.wantRead { + wantReads = 0 + } + if tc.wantRead && asked.dir != dirOf(tc.path) { + t.Errorf("the forge was asked for directory %q", asked.dir) + } + if asked.n != wantReads { + t.Errorf("forge reads = %d, want %d", asked.n, wantReads) + } m, ok := moduleOf(t, mods, "snake") if tc.want { if !ok || m.Manifest == "" || m.Repository != "jschoubben/snake" || m.Commit != "7a74cee83f88e6585a5511c3cc53bdf0f5303d20" || m.Path != pathOf(dirOf(tc.path)) {