A changed file touches exactly the modules whose build reads it (hq issue 280, ADR 0237)

The builder reads a module's own directory (the repository for one built from its root)
and a repository its recipe packages, nothing else. A file in no module's directory was
read as shared code and rebuilt everything built from the repository: 103 modules for a
merge-check.sh added at the catalogue's root. It now touches nothing, in the merge
handler, the release planner and the pull request's check alike, and the gate says so.
This commit is contained in:
jochen
2026-10-06 22:34:57 +02:00
parent 327654e57b
commit b24bb030ec
7 changed files with 71 additions and 116 deletions
+5 -4
View File
@@ -22,10 +22,11 @@ import (
//
// **The mesh's module graph decides, not the repository.** The controller holds the graph — every module,
// the repository and directory it is built from — and maps the pull request's changed paths onto it by
// the rule the merge handler uses (issue 278): a changed file inside a directory holding a module.json at
// the head is that module's, held or not; a file in no such directory, in a repository modules are built
// from into that branch, is shared code, and touches every module built from it there. A directory the
// change adds a module in, which the graph does not hold yet, is a new module and is checked too.
// the rule the merge handler and the release planner use (whatTheMergeTouched, issue 280): **a changed file
// touches exactly the modules whose build reads it** — a module's own directory (the whole repository for
// one built from its root), or a repository its recipe packages. A file no build reads — a script at the
// root, a README — touches no module. A directory the change adds a module.json in, which the graph does
// not hold yet (said by the head, issue 278), is a new module and is checked too.
//
// - touches a module: the build seat runs **the gate** — `mesh/merge-gate`, the touched manifests, every
// machine composed with the change, the replays — and the repository's own merge-check.sh beside it;
+5 -2
View File
@@ -41,8 +41,11 @@ func TestThePullRequestIsMappedOntoTheModuleGraph(t *testing.T) {
}{
{"one module's own files", pull("novox", "mesh-catalog", "main", []string{"modules/gitea/index.ts"}, "modules/gitea"),
"gitea", "", "modules/gitea/module.json", true, ""},
{"shared code touches every module built from the repository",
pull("novox", "mesh-catalog", "main", []string{"tsconfig.json"}), "gitea,keycloak,nats", "",
{"a file no module's build reads touches no module (issue 280)",
pull("novox", "mesh-catalog", "main", []string{"merge-check.sh", "README.md"}), "", "", "", true, ""},
{"files the announcer could not list: everything built from it",
link.PullUpdated{Owner: "novox", Repo: "mesh-catalog", Base: "main", PathsTruncated: true,
Paths: []string{"README.md"}}, "gitea,keycloak,nats", "",
"modules/gitea/module.json,modules/keycloak/module.json,modules/nats/module.json", true, ""},
{"a new module, said by the head", pull("novox", "mesh-catalog", "main",
[]string{"modules/newmod/index.ts", "modules/newmod/module.json"}, "modules/newmod"),
+11 -10
View File
@@ -108,9 +108,9 @@ type mergeComposed struct {
type mergeWidthOf struct {
Modules []string `json:"modules"`
Tiers int `json:"tiers"`
// Shared is the changed paths read as shared code: in no directory of a module the mesh holds, so
// everything built from the repository is rebuilt for them.
Shared []string `json:"shared,omitempty"`
// Unread is the changed paths no module's build reads — in no module's directory — which rebuild
// nothing (issue 280): said, so a reader sees why a change to them moves no module.
Unread []string `json:"unread,omitempty"`
}
// wideRebuild is how many modules a rebuild may take before the gate says so as a warning.
@@ -312,11 +312,11 @@ func judgeChange(ctx context.Context, in mergeCheckInput) (mergeVerdict, error)
v.Notes = append(v.Notes, "the width of the rebuild could not be worked out: "+err.Error())
} else {
v.Width = &w
if len(w.Shared) > 0 && len(w.Modules) > wideRebuild {
v.Warnings = append(v.Warnings, fmt.Sprintf("a merge rebuilds %d module(s), because %s read as shared "+
"code: in no directory of a module the mesh holds (issue 278)", len(w.Modules),
readableList(w.Shared)))
} else if len(w.Modules) > wideRebuild {
if len(w.Unread) > 0 {
v.Notes = append(v.Notes, fmt.Sprintf("%s read by no module's build, and rebuild nothing (issue 280)",
readableList(w.Unread)))
}
if len(w.Modules) > wideRebuild {
v.Warnings = append(v.Warnings, fmt.Sprintf("a merge rebuilds %d module(s) in %d tier(s)",
len(w.Modules), w.Tiers))
}
@@ -1016,7 +1016,8 @@ func rebuildWidth(f snapshot.Facts, repository string, paths []string, tree stri
}
sort.Strings(w.Modules)
}
// The paths read as shared: in no directory of a module the mesh holds from this repository.
// The paths no build reads: in no directory of a module the mesh holds from this repository, or one
// the tree holds.
var dirs []string
for _, e := range from {
if d := strings.Trim(e.Source.Path, "/"); d != "" {
@@ -1032,7 +1033,7 @@ func rebuildWidth(f snapshot.Facts, repository string, paths []string, tree stri
inModule = inModule || strings.HasPrefix(p, d)
}
if !inModule && len(dirs) > 0 {
w.Shared = append(w.Shared, p)
w.Unread = append(w.Unread, p)
}
}
return w, nil
+17 -11
View File
@@ -180,26 +180,32 @@ func TestAModuleAMachineRunsRemovedFromItsSourceFails(t *testing.T) {
}
}
// **Issue 278**: a file of a module nobody holds read as shared code, and the merge rebuilt the
// catalogue. The gate says how wide a merge's rebuild is, and warns when shared code makes it wide.
// **Issues 278 and 280**: a file in no held module's directory read as shared code, and a merge rebuilt the
// catalogue. A file is a module's only by being inside it — no build reads one outside — so it rebuilds
// nothing, and the gate says why; a merge that does rebuild widely is warned of.
func TestIssue278AWideRebuildIsSaidBeforeTheMerge(t *testing.T) {
f, manifests := catalogueMesh(t)
was := wideRebuild
wideRebuild = 2
t.Cleanup(func() { wideRebuild = was })
v := gateJudged(t, f, aTree(t, manifests), "modules/showcase/index.ts")
if v.Width == nil || len(v.Width.Modules) < 5 || len(v.Width.Shared) != 1 {
t.Fatalf("the width reads %+v", v.Width)
for _, file := range []string{"modules/showcase/index.ts", "merge-check.sh", "README.md"} {
v := gateJudged(t, f, aTree(t, manifests), file)
if v.Width == nil || len(v.Width.Modules) != 0 || len(v.Width.Unread) != 1 || v.Verdict != "pass" {
t.Fatalf("%s, read by no build, reads %+v, %s", file, v.Width, v.Verdict)
}
if !strings.Contains(v.Report(), "read by no module's build") {
t.Errorf("why %s rebuilds nothing is not said:\n%s", file, v.Report())
}
}
if v.Verdict != "warning" || !strings.Contains(v.Report(), "shared") {
t.Fatalf("a rebuild of everything for one shared file is not said:\n%s", v.Report())
v := gateJudged(t, f, aTree(t, manifests), "modules/album/x.ts", "modules/objects/x.go", "modules/resolver/x.go")
if v.Width == nil || len(v.Width.Modules) < 3 || v.Verdict != "warning" {
t.Fatalf("a rebuild wider than the bound is not warned of: %+v, %s", v.Width, v.Verdict)
}
// The same file, with the reference module's definition in the tree: its directory is a module, held
// or not, and the file is its business alone (the fix of 278, read from the tree as the announcer does).
// With the reference module's definition in the tree, its directory is a module, held or not.
withShowcase := withEdit(manifests, "showcase", `{"module":"showcase","version":"1"}`)
v = gateJudged(t, f, aTree(t, withShowcase), "modules/showcase/index.ts")
if v.Width == nil || len(v.Width.Modules) != 0 || len(v.Width.Shared) != 0 {
t.Fatalf("a file of a module nobody holds still reads as shared: %+v", v.Width)
if v.Width == nil || len(v.Width.Modules) != 0 || len(v.Width.Unread) != 0 {
t.Fatalf("a file of a module nobody holds reads %+v", v.Width)
}
v = gateJudged(t, f, aTree(t, manifests), "modules/album/module.json")
if v.Width == nil || strings.Join(v.Width.Modules, ",") != "album" || v.Verdict != "pass" {
+13 -14
View File
@@ -144,7 +144,7 @@ func TestAMergeRebuildsTheModulesItChanged(t *testing.T) {
}{
{"one module's own files", merge([]string{"modules/gitea/index.ts", "modules/gitea/client.ts"}, false), "gitea"},
{"two modules' files", merge([]string{"modules/gitea/index.ts", "modules/keycloak/module.json"}, false), "gitea,keycloak"},
{"a file they share", merge([]string{"tsconfig.json"}, false), "gitea,keycloak"},
{"a file at the root no build reads (issue 280)", merge([]string{"tsconfig.json"}, false), ""},
{"a module the mesh does not hold", merge([]string{"modules/plex/index.ts"}, false), ""},
{"nothing said about the files", merge(nil, false), "gitea,keycloak"},
{"more files than were listed", merge([]string{"modules/gitea/index.ts"}, true), "gitea,keycloak"},
@@ -153,9 +153,9 @@ func TestAMergeRebuildsTheModulesItChanged(t *testing.T) {
{"a new module beside a held one", merge([]string{"modules/gitea/x", "modules/newmod/module.json"}, false), "gitea"},
{"a new module's other files", merge([]string{"modules/newmod/index.ts", "modules/newmod/module.json"}, false), ""},
{"a module removed", merge([]string{"modules/gone/module.json"}, false), ""},
{"a directory with no manifest", merge([]string{"modules/lib/x.go"}, false), "gitea,keycloak"},
{"a file directly among the modules", merge([]string{"modules/README.md"}, false), "gitea,keycloak"},
{"the root's files still", merge([]string{"tsconfig.json"}, false), "gitea,keycloak"},
{"a directory with no manifest", merge([]string{"modules/lib/x.go"}, false), ""},
{"a file directly among the modules", merge([]string{"modules/README.md"}, false), ""},
{"a root file beside a module's", merge([]string{"merge-check.sh", "modules/keycloak/x.ts"}, false), "keycloak"},
} {
if got := named(whatTheMergeTouched(candidates, known, c.m)); got != c.want {
t.Errorf("%s: rebuilt %q, wanted %q", c.what, got, c.want)
@@ -245,12 +245,11 @@ func TestAModuleBuiltAtACommitStillFollowsItsBranch(t *testing.T) {
}
}
// novox/hq issue 278: whether a directory is a module is a fact of the repository at the merge commit,
// and the announcer says it. A merge touching the code of a module the mesh does not hold — the
// novox/hq issues 278 and 280: a merge touching the code of a module the mesh does not hold — the
// catalogue's reference module, whose manifest it left alone — read as shared and rebuilt every module
// built from the repository (103 of them on 2026-10-06, 88 byte-identical). Said by the announcer, it
// rebuilds nothing; a directory holding no manifest is shared as before; and an announcer that does not
// say keeps the old rule.
// built from the repository (103 of them on 2026-10-06, 88 byte-identical); so did a merge-check.sh added at
// the root. No build reads a file outside its module's directory, so neither rebuilds anything, said by the
// announcer or not.
func TestAChangeInsideAModuleIsThatModulesHeldOrNot(t *testing.T) {
const repo = "http://forge.internal:20000/novox/mesh-catalog.git"
gitea := fromRepo("gitea", repo, "modules/gitea")
@@ -278,12 +277,12 @@ func TestAChangeInsideAModuleIsThatModulesHeldOrNot(t *testing.T) {
{"beside a held one's change", merge(append([]string{"modules/gitea/index.ts"}, showcase...),
[]string{"modules/gitea", "modules/showcase"}, true), "gitea"},
{"deeper inside it", merge([]string{"modules/showcase/daemon/index.ts"}, []string{"modules/showcase"}, true), ""},
{"a directory holding no manifest is still shared", merge([]string{"modules/lib/x.go"}, nil, true), "gitea,keycloak"},
{"a directory holding no manifest, read by no build", merge([]string{"modules/lib/x.go"}, nil, true), ""},
{"one of two files in no module", merge([]string{"modules/showcase/index.ts", "modules/lib/x.go"},
[]string{"modules/showcase"}, true), "gitea,keycloak"},
{"the root is never a module directory", merge([]string{"tsconfig.json"}, []string{"", "/", "."}, true), "gitea,keycloak"},
{"not said: the old rule", merge(showcase, []string{"modules/showcase"}, false), "gitea,keycloak"},
{"an old announcer saying nothing", merge(showcase, nil, false), "gitea,keycloak"},
[]string{"modules/showcase"}, true), ""},
{"the root is never a module directory", merge([]string{"tsconfig.json"}, []string{"", "/", "."}, true), ""},
{"not said: still no build reads it", merge(showcase, []string{"modules/showcase"}, false), ""},
{"an old announcer saying nothing", merge(showcase, nil, false), ""},
{"a manifest the merge removed, said or not", merge([]string{"modules/gone/module.json", "modules/gone/x.ts"}, nil, true), ""},
} {
if got := named(whatTheMergeTouched(candidates, known, c.m)); got != c.want {
+18 -73
View File
@@ -5,7 +5,6 @@ import (
"errors"
"flag"
"fmt"
"path"
"regexp"
"sort"
"strings"
@@ -616,65 +615,33 @@ func lastLookAt(entries []inventory.Entry, m link.SourceMoved) time.Time {
return newest
}
// whatTheMergeTouched narrows the modules built from a repository to the ones the merge changed.
// whatTheMergeTouched narrows the modules built from a repository to the ones the merge changed: **a
// changed file touches exactly the modules whose build reads it** (novox/hq issue 280, ADR 0237 as amended).
//
// **A change inside no module's own directory is a change to what they share.** The forge lists the
// files a merge changed; a module is affected when one of them is inside its own directory, when it
// is built from the repository's root — everything there is its source — or when some changed file
// belongs to no module's directory at all, which is how a shared file, a build recipe or a
// dependency at the root rebuilds everything built from that repository.
// What a build reads is the module's own directory — the builder clones the repository and builds within
// that directory alone: the manifest, the recipes, the bundles' sources, the Docker context — or the whole
// repository for a module built from its root. A second repository a recipe packages (an artifact's
// `context`) is read too, and that is the build record's `read`, answered by readsFrom beside this. So a
// changed file inside a module's directory is that module's; a file in no module's directory — a script
// at the root, a README, CI configuration, a directory of notes — is read by no build and touches nothing.
//
// A change inside *another* module's directory is that module's business and not this one's, even
// when the mesh does not hold that module: `known` is every module this repository is known to hold,
// whatever branch it was registered from.
// **It used to be read as shared code**, rebuilding everything built from the repository, on the theory
// that a root file might be a build input (04-ISSUES/131). No build reads one: the theory cost a rebuild
// of 103 modules for a merge-check.sh added at the catalogue's root (issue 280), as it had for a module
// held by no machine (278) and a new module's directory (252), each patched as an exception to a rule that
// was wrong. Rebuilding too much is not the safe direction when every rebuild is a rollout.
//
// **And a module the mesh has never seen is still a module** (novox/hq issue 252). A merge adding a
// new module to the catalogue repository — `modules/newmod/module.json` and its files — read as a
// change to shared code, because `modules/newmod` was nobody's known directory, and every module
// built from the repository was rebuilt and rolled out for a module none of them is. So the
// directories that hold modules are known too: the parents of the known modules' directories
// (`modules`, never the root). A changed path `<parent>/<name>/…` belongs to the module at
// `<parent>/<name>` — held or not — and rebuilds nothing else, **provided it is shown to be a
// module**: its `module.json` is among the changed files (added, changed, or removed with it). A
// directory under the same parent whose manifest the merge did not touch may as well be a shared
// library (`modules/lib`), and that is still read as shared. Rebuilding too much remains the safe
// direction: the fault this whole path exists for is a mesh that believes it is current and is not
// (novox/hq 04-ISSUES/131). A file at the root, or directly in a parent, is shared as it always was.
//
// **Whether a directory is a module is a fact of the repository at the merge commit** (novox/hq issue
// 278), and the announcer now says it: a changed file inside a directory holding a `module.json` there
// is that module's, whatever the merge did to the manifest; only a file in no such directory is shared.
// From an announcer that does not say, the rule above stands.
// Nothing said about the files, or not all of them said, is still everything: what is not known cannot be
// narrowed. `known` is kept for the callers; which directories hold a module is no longer needed to read a
// change, because a file is a module's only by being inside it.
func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved) []inventory.Entry {
// Nothing said about the files, or not all of them said: everything built from it is affected.
_ = known
if len(m.Paths) == 0 || m.PathsTruncated {
return candidates
}
var dirs []string
parents := map[string]bool{}
for _, e := range known {
if e.Source.Path != "" && sameRepository(e.Source.Repository, m) {
dir := strings.Trim(e.Source.Path, "/")
dirs = append(dirs, dir)
if parent := path.Dir(dir); parent != "." && parent != "/" {
parents[parent] = true
}
}
}
changed := map[string]bool{}
for _, p := range m.Paths {
changed[strings.TrimPrefix(p, "/")] = true
}
modules := saidModuleDirs(m)
for _, p := range m.Paths {
if insideAny(p, dirs) || inAModuleOfItsOwn(p, parents, changed) || insideAny(p, modules) {
continue
}
return candidates
}
var out []inventory.Entry
for _, e := range candidates {
if e.Source.Path == "" || anyInside(m.Paths, e.Source.Path) {
if strings.Trim(e.Source.Path, "/") == "" || anyInside(m.Paths, e.Source.Path) {
out = append(out, e)
}
}
@@ -700,28 +667,6 @@ func saidModuleDirs(m link.SourceMoved) []string {
return out
}
// inAModuleOfItsOwn is whether a changed file is inside a module directory the mesh does not know —
// `<parent>/<name>/…` under a directory known to hold modules, whose `module.json` the same merge
// changed (novox/hq issue 252). Such a file is that module's business and nobody else's.
func inAModuleOfItsOwn(p string, parents, changed map[string]bool) bool {
p = strings.TrimPrefix(p, "/")
for parent := range parents {
rest, under := strings.CutPrefix(p, parent+"/")
if !under {
continue
}
name, _, inADirectory := strings.Cut(rest, "/")
if !inADirectory || name == "" {
// A file directly in the parent — `modules/README.md` — is about all of them.
continue
}
if changed[parent+"/"+name+"/module.json"] {
return true
}
}
return false
}
// inside is whether a changed file is in a directory: that directory itself, or under it.
func inside(path, dir string) bool {
dir = strings.Trim(dir, "/")
+2 -2
View File
@@ -223,8 +223,8 @@ type PullUpdated struct {
PathsTruncated bool `json:"paths_truncated,omitempty"`
// ModuleDirs are the directories above the changed files that hold a `module.json` at the head, read
// as the merge announcer reads them at a merge commit (issue 278); ModuleDirsSaid says it looked.
// What the controller maps a pull request onto the mesh's module graph with: a file inside one is
// that module's, held or not, and one inside none, in a repository modules are built from, is shared.
// What the controller finds a module the graph does not hold yet with: a directory the change adds a
// module.json in is a new module, checked before it merges.
ModuleDirs []string `json:"module_dirs,omitempty"`
ModuleDirsSaid bool `json:"module_dirs_said,omitempty"`
// MergeCheck says the head holds a merge-check.sh at its root — the repository's own tests, the