From d7f359c4980a01e9fbcd655381d995b23e058d27 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 5 Oct 2026 17:53:03 +0200 Subject: [PATCH] Read a new module's directory as its own, not as shared code (hq issue 252) A merge adding a module the mesh has not registered rebuilt every module built from the repository. A path under a directory known to hold modules belongs to that module when its module.json is among the changed files. --- cmd/mesh-controller/order_test.go | 8 +++++ cmd/mesh-controller/upgrades.go | 57 +++++++++++++++++++++++++++---- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/cmd/mesh-controller/order_test.go b/cmd/mesh-controller/order_test.go index 2acac30..f4a2c62 100644 --- a/cmd/mesh-controller/order_test.go +++ b/cmd/mesh-controller/order_test.go @@ -147,6 +147,14 @@ func TestAMergeRebuildsTheModulesItChanged(t *testing.T) { {"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"}, + // novox/hq issue 252: a module the mesh has never registered is still a module, when the merge + // shows it is one — and a directory that may be shared code is still shared. + {"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"}, } { if got := named(whatTheMergeTouched(candidates, known, c.m)); got != c.want { t.Errorf("%s: rebuilt %q, wanted %q", c.what, got, c.want) diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index b16542b..f82ccb2 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -5,6 +5,7 @@ import ( "errors" "flag" "fmt" + "path" "regexp" "strings" "time" @@ -423,25 +424,45 @@ func lastLookAt(entries []inventory.Entry, m link.SourceMoved) time.Time { // // 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. That is also the limit of this — a repository whose shared -// code sits inside a directory the mesh has never seen a module in reads as shared, and everything -// is rebuilt. Rebuilding too much is 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). +// whatever branch it was registered from. +// +// **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 `//…` belongs to the module at +// `/` — 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. 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. 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) { - dirs = append(dirs, e.Source.Path) + 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 { - if !insideAny(p, dirs) { - return candidates + changed[strings.TrimPrefix(p, "/")] = true + } + for _, p := range m.Paths { + if insideAny(p, dirs) || inAModuleOfItsOwn(p, parents, changed) { + continue } + return candidates } var out []inventory.Entry for _, e := range candidates { @@ -452,6 +473,28 @@ func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved return out } +// inAModuleOfItsOwn is whether a changed file is inside a module directory the mesh does not know — +// `//…` 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, "/")