From e31da597bbfcb924189b5e53502102d2a0309cf4 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 2 Nov 2026 19:35:58 +0100 Subject: [PATCH] mesh-delivery-indexer: the version line takes a directory-replaced sibling by its content, so it never goes stale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding 2 (ADR 0593 §2, §3): after the release no build has reported imports, and a module that replaces a mesh Go module of its own repository by a directory had no go.sum line and no version for it, so a change of buildstep or buildrun alone moved no importer. The indexer now reads go.mod's directory replaces, publishes those modules, and puts their content digest on the version line; builds are pinned to the other repositories' versions alone. Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL --- .../cmd/mesh-delivery-indexer/main.go | 2 +- .../cmd/mesh-delivery-indexer/named.go | 16 +++- .../internal/fingerprint/fingerprint.go | 74 +++++++++++++++++++ .../internal/fingerprint/fingerprint_test.go | 34 +++++++++ .../internal/index/candidates.go | 4 + .../internal/index/form.go | 15 ++-- .../internal/index/imports_test.go | 55 ++++++++++++++ .../internal/index/modules.go | 54 ++++++++++++-- .../internal/index/previews.go | 22 +++--- 9 files changed, 248 insertions(+), 28 deletions(-) diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/main.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/main.go index 4a12dd0..fdbaabb 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/main.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/main.go @@ -300,7 +300,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro Verdicts: awaitedStore{verdicts}, SayRefused: sayRefused(js, cred, logger.Printf), Predefined: awaitedStore{predefined}, ConfigCandidates: awaitedStore{configCandidates}, Logf: logger.Printf, Named: namedVia(calls.Call), Trunk: trunkVia(calls.Call), Contains: containsVia(calls.Call), Read: readVia(calls.Call), - ReadOpen: openVia(calls.Call), Packages: awaitedStore{packages}, Files: filesVia(calls.Call), GoSum: goSumVia(calls.Call)} + ReadOpen: openVia(calls.Call), Packages: awaitedStore{packages}, Files: filesVia(calls.Call), GoSum: goSumVia(calls.Call), GoMod: goModVia(calls.Call)} // The log's entries at the start are read before any fingerprint is computed or registered module indexed (below); each // new one indexes the module it names when that module is registered and the bucket holds none (registered.go). var started atomic.Bool 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 e3f31b2..f10cac8 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named.go @@ -110,14 +110,26 @@ func filesVia(call toolCaller) func(ctx context.Context, repository, sha string, // goSumVia reads a module directory's go.sum at a commit (gitea_get_file): the version line's packages of a module // whose build reported no imports (ADR 0593 §2). A directory with no go.sum answers nothing. func goSumVia(call toolCaller) func(ctx context.Context, repository, sha, dir string) ([]byte, error) { + return goFileVia(call, "go.sum") +} + +// goModVia reads a module directory's go.mod at a commit, as goSumVia reads its go.sum: the directory replaces of its own +// repository the version line takes by their content (fingerprint.ReplacedSiblings). +func goModVia(call toolCaller) func(ctx context.Context, repository, sha, dir string) ([]byte, error) { + return goFileVia(call, "go.mod") +} + +// goFileVia reads one Go file of a module directory at a commit through the forge's get-file tool; a file the commit does +// not hold is none. +func goFileVia(call toolCaller, name string) func(ctx context.Context, repository, sha, dir string) ([]byte, error) { return func(_ context.Context, repository, sha, dir string) ([]byte, error) { owner, repo, ok := strings.Cut(delivery.RepositoryName(repository), "/") if !ok { return nil, fmt.Errorf("%q is not /", repository) } - p := "go.sum" + p := name if dir != "" && dir != delivery.RootPath { - p = dir + "/go.sum" + p = dir + "/" + name } raw, err := call(getFileTool, map[string]string{"owner": owner, "repo": repo, "path": p, "ref": sha}) if err != nil { diff --git a/modules/mesh-delivery-indexer/internal/fingerprint/fingerprint.go b/modules/mesh-delivery-indexer/internal/fingerprint/fingerprint.go index 0935d7d..26bbb87 100644 --- a/modules/mesh-delivery-indexer/internal/fingerprint/fingerprint.go +++ b/modules/mesh-delivery-indexer/internal/fingerprint/fingerprint.go @@ -163,6 +163,80 @@ func OtherPackages(packages []string, repository string) []string { return out } +// ReplacedSiblings are the mesh Go modules of repository that a module's go.mod replaces by a directory (`replace +// => ../dir`, alone or in a block), sorted and each once; nil for none (review finding 2 of ADR 0593's release). Such a +// module is built from that directory of the same tree, so no go.sum line and no version moves with it: the version line +// takes it by the content its directory publishes (VersionLine). +func ReplacedSiblings(goMod []byte, repository string) []string { + seen := map[string]bool{} + inBlock := false + for _, line := range strings.Split(string(goMod), "\n") { + line = strings.TrimSpace(line) + if i := strings.Index(line, "//"); i >= 0 { + line = strings.TrimSpace(line[:i]) + } + switch { + case line == "replace (": + inBlock = true + continue + case inBlock && line == ")": + inBlock = false + continue + case strings.HasPrefix(line, "replace "): + line = strings.TrimSpace(strings.TrimPrefix(line, "replace ")) + case !inBlock: + continue + } + left, right, ok := strings.Cut(line, "=>") + if !ok { + continue + } + from, to := strings.Fields(left), strings.Fields(right) + if len(from) == 0 || len(to) != 1 || !(strings.HasPrefix(to[0], "./") || strings.HasPrefix(to[0], "../")) { + continue + } + if repo, _, ok := delivery.PackageRepository(from[0]); ok && delivery.SameRepository(repo, repository) { + seen[from[0]] = true + } + } + if len(seen) == 0 { + return nil + } + return slices.Sorted(maps.Keys(seen)) +} + +// VersionLine is the version line's value of each package (Inputs.Packages) of a module of repository: a mesh Go +// module of another repository by its version (versions), and one of repository itself, replaced by its directory +// (ReplacedSiblings), by the content its directory publishes, `content:` and the hex sha256 of its requirements' digest +// and each package's digest (empty while it is not published, which gives no fingerprint until it is, as a version +// none publishes does). +func VersionLine(repository string, packages []string, versions delivery.PackageVersions, published map[string]delivery.PublishedPackages) map[string]string { + if len(packages) == 0 { + return nil + } + out := map[string]string{} + for _, p := range packages { + repo, _, ok := delivery.PackageRepository(p) + if !ok || !delivery.SameRepository(repo, repository) { + out[p] = versions[p] + continue + } + pub, held := published[p] + if !held { + out[p] = "" + continue + } + lines := []string{"requires " + pub.Requires} + for _, d := range pub.Packages { + lines = append(lines, "package "+d.Package+" "+d.Digest) + } + sort.Strings(lines[1:]) + sum := sha256.Sum256([]byte(strings.Join(lines, "\n"))) + out[p] = "content:" + hex.EncodeToString(sum[:]) + } + return out +} + // Resolved is an import set resolved against the published packages: the digest of each import, the requirements' // digest of each mesh Go module holding one, the version to build each of another repository's against, and the // imports none publishes. diff --git a/modules/mesh-delivery-indexer/internal/fingerprint/fingerprint_test.go b/modules/mesh-delivery-indexer/internal/fingerprint/fingerprint_test.go index ddad203..3d9b433 100644 --- a/modules/mesh-delivery-indexer/internal/fingerprint/fingerprint_test.go +++ b/modules/mesh-delivery-indexer/internal/fingerprint/fingerprint_test.go @@ -15,6 +15,40 @@ func TestTheFingerprintComputation(t *testing.T) { t.Run("the moved computation gives the values it gave in mesh-sdk", movedComputationGivesTheValuesItGaveInMeshSDK) t.Run("the import form moves only with what the module imports", importFormMovesOnlyWithWhatTheModuleImports) t.Run("a package's digest is its own files", packageDigestIsItsOwnFiles) + t.Run("the version line takes a sibling directory by its content", versionLineTakesASiblingDirectoryByItsContent) +} + +// Review finding 2 (ADR 0593 §2 "never left stale", §3): a module that replaces a mesh Go module of its own repository by +// a directory names it on the version line by the content its directory publishes, since no version and no go.sum line +// moves with it: only the sibling's files change, and the importer's fingerprint moves; its build is pinned to no version +// of it. +func versionLineTakesASiblingDirectoryByItsContent(t *testing.T) { + const repo, buildrun = "novox/mesh-delivery", "git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/buildrun" + goMod := []byte("module mesh-delivery-checker\n\nrequire (\n\t" + buildrun + " v0.0.0-00010101000000-000000000000\n\t" + sdk + + " v0.1.14\n)\n\nreplace " + buildrun + " => ../mesh-delivery-builder/buildrun\n\nreplace (\n\t" + sdk + " => " + sdk + " v0.1.15\n)\n") + if got := ReplacedSiblings(goMod, repo); len(got) != 1 || got[0] != buildrun { + t.Fatalf("siblings %v, want buildrun alone (mesh-sdk is another repository, replaced by a version)", got) + } + published := func(digest string) map[string]delivery.PublishedPackages { + return map[string]delivery.PublishedPackages{buildrun: {Module: buildrun, Repository: repo, Requires: "r1", + Packages: []delivery.PackageDigest{{Package: buildrun, Digest: digest}}}} + } + versions := delivery.PackageVersions{sdk: "c1"} + line := func(digest string) string { + in := base() + in.Packages = VersionLine(repo, []string{sdk, buildrun}, versions, published(digest)) + fp, none := Of(in) + if fp == "" { + t.Fatalf("no fingerprint: %s", none) + } + return fp + } + if a, b := line("d1"), line("d2"); a == b { + t.Error("a change of the sibling directory alone left the importer's fingerprint where it was") + } + if got := VersionLine(repo, []string{buildrun}, nil, nil)[buildrun]; got != "" { + t.Errorf("an unpublished sibling is taken as %q, want none (no fingerprint until it is published)", got) + } } func base() Inputs { diff --git a/modules/mesh-delivery-indexer/internal/index/candidates.go b/modules/mesh-delivery-indexer/internal/index/candidates.go index 0ff54ad..68e0bb6 100644 --- a/modules/mesh-delivery-indexer/internal/index/candidates.go +++ b/modules/mesh-delivery-indexer/internal/index/candidates.go @@ -68,6 +68,10 @@ type Indexer struct { // GoSum reads a module directory's go.sum at a commit (gitea.gitea_get_file): the mesh Go modules a module with no // reported import set takes the version line of. Nil reads none: such a module keeps what it held. GoSum func(ctx context.Context, repository, sha, dir string) ([]byte, error) + // GoMod reads a module directory's go.mod at a commit (gitea.gitea_get_file): the mesh Go modules of its own + // repository it replaces by a directory (fingerprint.ReplacedSiblings), which the version line takes by their + // content; nil reads none. + GoMod func(ctx context.Context, repository, sha, dir string) ([]byte, error) // Now is the clock of a taken value; nil is time.Now. Now func() time.Time Builds *Builds diff --git a/modules/mesh-delivery-indexer/internal/index/form.go b/modules/mesh-delivery-indexer/internal/index/form.go index 4289b43..5fdc9c3 100644 --- a/modules/mesh-delivery-indexer/internal/index/form.go +++ b/modules/mesh-delivery-indexer/internal/index/form.go @@ -25,8 +25,10 @@ type form struct { // the build that reported the imports was built from. While the import form computes to it, nothing the build was // built from changed: the module keeps the fingerprint it holds (prev), in whatever form, and its versions. // Otherwise the import form is written, and ImportForm moves with it. -// - **The version line** otherwise (no import set, or an import none publishes): the version of each mesh Go module its -// go.sum names, read as the indexer always read it (fingerprint.HeldVersionsOf), so its value is the one it had. +// - **The version line** otherwise (no import set, or an import none publishes): the version of each mesh Go module of +// another repository its go.sum names, read as the indexer always read it (fingerprint.HeldVersionsOf), and the +// content of each mesh Go module of its own repository its go.mod replaces by a directory (fingerprint.VersionLine), +// so a change of that directory alone moves it (review finding 2). Its versions are the other repositories' alone. // // It sets kept.ImportForm when it writes the import form. func formOf(module, repository string, kept *delivery.KeptInputs2, toolchains map[string]delivery.Toolchain2, prev delivery.Module, held map[string]delivery.Module, published map[string]delivery.PublishedPackages, logf func(string, ...any)) form { @@ -49,16 +51,11 @@ func formOf(module, repository string, kept *delivery.KeptInputs2, toolchains ma logf("%s: no published package holds %s, so its fingerprint takes the version line", module, strings.Join(r.Missing, ", ")) } } - versions, unpublished := fingerprint.HeldVersionsOf(kept.Packages, held) + versions, unpublished := fingerprint.HeldVersionsOf(fingerprint.OtherPackages(kept.Packages, repository), held) if len(unpublished) > 0 && logf != nil { logf("%s: no module the indexer holds publishes %s, so its fingerprint takes no version of it", module, strings.Join(unpublished, ", ")) } - if len(kept.Packages) > 0 { - in.Packages = map[string]string{} - for _, p := range kept.Packages { - in.Packages[p] = versions[p] - } - } + in.Packages = fingerprint.VersionLine(repository, kept.Packages, versions, published) fp, none := fingerprint.Of(in) if fp == "" { return form{none: none} diff --git a/modules/mesh-delivery-indexer/internal/index/imports_test.go b/modules/mesh-delivery-indexer/internal/index/imports_test.go index ba8e703..ef512ed 100644 --- a/modules/mesh-delivery-indexer/internal/index/imports_test.go +++ b/modules/mesh-delivery-indexer/internal/index/imports_test.go @@ -48,6 +48,7 @@ func (m *missingDir) Error() string { return "no directory " + m.dir } // and the packages state holds one key with a digest per package. func TestImportsReportedMoveTheFingerprintOnlyWithAnImportedPackage(t *testing.T) { t.Run("a build of an older fingerprint keeps no imports", buildOfAnOlderFingerprintKeepsNoImports) + t.Run("a change of a sibling directory alone moves its importer", aSiblingDirectoryAloneMovesItsImporter) ctx := context.Background() ix, mods := newFormIndexer(t, mod{"avahi", catalogueManifest(t, "avahi")}) reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.1") @@ -130,6 +131,60 @@ func buildOfAnOlderFingerprintKeepsNoImports(t *testing.T) { } } +// Review finding 2 (ADR 0593 §2 "never left stale", §3): avahi, with no reported import set, replaces a mesh Go module +// of its own repository by a directory (its version line names it). A merge of its repository that changes only that +// sibling's files moves avahi's fingerprint; one that changes nothing it reaches leaves it; its build is pinned to no +// version of the sibling. +func aSiblingDirectoryAloneMovesItsImporter(t *testing.T) { + const sibling = "git.novox.be/novox/mesh-catalog/modules/mesh-build-go/buildstep" + ctx := context.Background() + ix, mods := newFormIndexer(t, mod{"avahi", catalogueManifest(t, "avahi")}) + reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.1") + listing := map[string]map[string]string{ + cat1: {"go.mod": "m1", "step.go": "s1"}, + strings.Repeat("b", 40): {"go.mod": "m1", "step.go": "s2"}, // only the sibling's file changed + strings.Repeat("c", 40): {"go.mod": "m1", "step.go": "s2"}, // nothing it reaches changed + } + ix.Packages = newMem() + ix.Files = func(_ context.Context, repository, sha string, dirs []string) (map[string]map[string]string, error) { + out := map[string]map[string]string{} + for _, d := range dirs { + if repository != catalogueRepo || d != "modules/mesh-build-go/buildstep" || listing[sha] == nil { + return nil, &missingDir{d} + } + out[d] = listing[sha] + } + return out, nil + } + setPackages(t, mods, "avahi", []string{sibling}) + fpAt := func(commit string) delivery.Module { + t.Helper() + held, _ := ix.heldModules(ctx) + if err := ix.publish(ctx, catalogueRepo, commit, held, false); err != nil { + t.Fatal(err) + } + if err := ix.Recompute(ctx); err != nil { + t.Fatal(err) + } + m, _ := moduleOf(t, mods, "avahi") + if m.FP2 == "" { + t.Fatalf("no fingerprint at %s: %q", commit, m.None2) + } + return m + } + first := fpAt(cat1) + moved := fpAt(strings.Repeat("b", 40)) + if moved.FP2 == first.FP2 { + t.Fatalf("a change of the sibling directory alone left avahi at %q", first.FP2) + } + if again := fpAt(strings.Repeat("c", 40)); again.FP2 != moved.FP2 { + t.Errorf("a merge changing nothing avahi reaches moved it from %q to %q", moved.FP2, again.FP2) + } + if _, pinned := moved.Packages[sibling]; pinned { + t.Errorf("the build is pinned to a version of its sibling directory: %v", moved.Packages) + } +} + func buildEnd(t *testing.T, module, fp string, imports ...string) []byte { t.Helper() raw, err := delivery.EncodeBuildLogEntry(delivery.BuildLogEntry{Event: delivery.BuildEndedEvent, Module: module, FP: fp, diff --git a/modules/mesh-delivery-indexer/internal/index/modules.go b/modules/mesh-delivery-indexer/internal/index/modules.go index 5fc9569..a8cc984 100644 --- a/modules/mesh-delivery-indexer/internal/index/modules.go +++ b/modules/mesh-delivery-indexer/internal/index/modules.go @@ -333,11 +333,51 @@ func unreadInputs(tree string, locks, paths map[string]string, packages []string return in } -// goSumPackages are the mesh Go modules of other repositories the go.sum of the merge's directory dir names (the version -// line's packages, ADR 0573 §2), read by the indexer itself (the bridge reads none since ADR 0593 §2): those the message -// carries (an older bridge), else the held module's while its go.sum's hash is unchanged, else the go.sum read from the -// forge (GoSum). A directory with no go.sum has none. +// goSumPackages are the version line's packages of the merge's directory dir (ADR 0573 §2, ADR 0593 §2): the mesh Go +// modules of other repositories its go.sum names, and those of its own repository its go.mod replaces by a directory +// (siblingPackages), which no go.sum line names. A directory with no go.sum is no Go module and has none. func (ix *Indexer) goSumPackages(ctx context.Context, f delivery.Forge, dir string, cur delivery.Module, has bool) []string { + others := ix.otherPackages(ctx, f, dir, cur, has) + if f.Locks[dir]["go.sum"] == "" && len(f.Packages[dir]) == 0 { + return others + } + siblings := ix.siblingPackages(ctx, f, dir, cur, has) + if len(siblings) == 0 { + return others + } + return slices.Compact(slices.Sorted(slices.Values(append(slices.Clone(others), siblings...)))) +} + +// siblingPackages are the mesh Go modules of the merge's own repository the go.mod of dir replaces by a directory, read +// from the forge (GoMod); where it cannot be read, the held module's own-repository packages stand, said. +func (ix *Indexer) siblingPackages(ctx context.Context, f delivery.Forge, dir string, cur delivery.Module, has bool) []string { + held := func() []string { + if !has || cur.Inputs2 == nil { + return nil + } + var out []string + for _, p := range cur.Inputs2.Packages { + if repo, _, ok := delivery.PackageRepository(p); ok && delivery.SameRepository(repo, f.Repository) { + out = append(out, p) + } + } + return out + } + if ix.GoMod == nil { + return held() + } + raw, err := ix.GoMod(ctx, f.Repository, f.Merge, dir) + if err != nil { + ix.logf("%s at %s: the go.mod of %q cannot be read, so its module keeps the directory replaces it held: %v", f.Repository, f.Merge, dir, err) + return held() + } + return fingerprint.ReplacedSiblings(raw, f.Repository) +} + +// otherPackages are the mesh Go modules of other repositories the go.sum of the merge's directory dir names: those the +// message carries (an older bridge), else the held module's while its go.sum's hash is unchanged, else the go.sum read +// from the forge (GoSum). +func (ix *Indexer) otherPackages(ctx context.Context, f delivery.Forge, dir string, cur delivery.Module, has bool) []string { if carried := f.Packages[dir]; len(carried) > 0 { return fingerprint.OtherPackages(carried, f.Repository) // its own repository's come with its tree } @@ -348,13 +388,13 @@ func (ix *Indexer) goSumPackages(ctx context.Context, f delivery.Forge, dir stri if has && cur.Inputs2 != nil && delivery.SameRepository(cur.Repository, f.Repository) { for _, h := range cur.Inputs2.Locks { if h == hash { - return cur.Inputs2.Packages + return fingerprint.OtherPackages(cur.Inputs2.Packages, f.Repository) } } } if ix.GoSum == nil { if has && cur.Inputs2 != nil { - return cur.Inputs2.Packages + return fingerprint.OtherPackages(cur.Inputs2.Packages, f.Repository) } return nil } @@ -362,7 +402,7 @@ func (ix *Indexer) goSumPackages(ctx context.Context, f delivery.Forge, dir stri if err != nil { ix.logf("%s at %s: the go.sum of %q cannot be read, so its module keeps the packages it held: %v", f.Repository, f.Merge, dir, err) if has && cur.Inputs2 != nil { - return cur.Inputs2.Packages + return fingerprint.OtherPackages(cur.Inputs2.Packages, f.Repository) } return nil } diff --git a/modules/mesh-delivery-indexer/internal/index/previews.go b/modules/mesh-delivery-indexer/internal/index/previews.go index 8bbb64a..bb367fb 100644 --- a/modules/mesh-delivery-indexer/internal/index/previews.go +++ b/modules/mesh-delivery-indexer/internal/index/previews.go @@ -261,8 +261,17 @@ func (ix *Indexer) candidateModule(ctx context.Context, m, dir string, pv delive // module still names them. trunk := held[m] packages := fingerprint.OtherPackages(pm.Packages, pv.Repository) - if len(pm.Packages) == 0 && trunk.Inputs2 != nil { - packages = trunk.Inputs2.Packages + if trunk.Inputs2 != nil { + if len(pm.Packages) == 0 { + packages = trunk.Inputs2.Packages + } else { + // The siblings its go.mod replaces by a directory are the trunk module's: no go.sum names them. + for _, p := range trunk.Inputs2.Packages { + if repo, _, ok := delivery.PackageRepository(p); ok && delivery.SameRepository(repo, pv.Repository) { + packages = append(packages, p) + } + } + } } in := inputsOfNamed(m, kept.JSON, pm.Tree, pm.Locks, pm.Paths, packages, ix.baseFP(ctx)) if in.Kept == nil { @@ -297,14 +306,9 @@ func (ix *Indexer) candidateModule(ctx context.Context, m, dir string, pv delive fp, none, versions = f.fp, f.none, f.versions } else { inputs.Imports, inputs.ImportForm = nil, "" - versions = candidatePackages(inputs.Packages, held, members) + versions = candidatePackages(fingerprint.OtherPackages(inputs.Packages, pv.Repository), held, members) in := inputsFrom(m, inputs, toolchains) - if len(inputs.Packages) > 0 { - in.Packages = map[string]string{} - for _, p := range inputs.Packages { - in.Packages[p] = versions[p] - } - } + in.Packages = fingerprint.VersionLine(pv.Repository, inputs.Packages, versions, published) fp, none = fingerprint.Of(in) } if fp == "" {