From 96005a8bfdb7c4427de58526962a53228f82ab87 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 02:04:38 +0200 Subject: [PATCH 1/2] Say what relates two changes, so unrelated ones can walk beside each other The helper decides, by repository, module (widened to dependents), delivery group head or module graph edge, whether two changes are related, as novox/hq ADR 0304 decision 1 reads in ADR 0306 decision 3. Nothing calls it yet; it is the first step of issue 525's parallel walks. --- cmd/mesh-controller/related.go | 98 +++++++++++++++++++++++++++++ cmd/mesh-controller/related_test.go | 92 +++++++++++++++++++++++++++ 2 files changed, 190 insertions(+) create mode 100644 cmd/mesh-controller/related.go create mode 100644 cmd/mesh-controller/related_test.go diff --git a/cmd/mesh-controller/related.go b/cmd/mesh-controller/related.go new file mode 100644 index 00000000..0180e6bd --- /dev/null +++ b/cmd/mesh-controller/related.go @@ -0,0 +1,98 @@ +package main + +import ( + "sort" + "strings" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// What relates two changes (novox/hq ADR 0304 decision 1, as ADR 0306 decision 3 reads it): two changes are +// related when they share a repository, a module, a delivery group's head, or an edge of the module graph +// between their modules. A module is every one the walk builds or sends: the modules its merges moved and the +// modules that depend on them, not only the ones the merges touched. Unrelated changes may walk beside each +// other (ADR 0304 decision 2, ADR 0282 decision 3); nothing walks so yet, and nothing but tests reads this. + +// footprint is what one change reaches: its repositories (lower case), its modules, and its delivery group +// heads (the head branch names of its merges). +type footprint struct { + repos, modules, heads map[string]bool +} + +func newFootprint() footprint { + return footprint{repos: map[string]bool{}, modules: map[string]bool{}, heads: map[string]bool{}} +} + +// footprintOfWalk is a walk's footprint: the repositories it carries and the modules it plans. A walk keeps no +// head of its merges; kept are the merges the record holds for it, whose announcements say their heads, and +// may be nil. +func footprintOfWalk(p inventory.Plan, kept []inventory.BatchedMerge) footprint { + f := newFootprint() + for _, c := range p.Carried() { + f.repos[strings.ToLower(c.Repository)] = true + } + for name := range p.Modules { + f.modules[name] = true + } + for _, m := range kept { + f.repos[strings.ToLower(m.Repository)] = true + if h := announcedOf(m).Head; h != "" { + f.heads[h] = true + } + } + return f +} + +// footprintOfBatch is an assembling batch's footprint: each merge's repository and head, and the modules its +// merges moved when heard, widened to their dependents as the walk cut from it would be (planOfMoves). +func footprintOfBatch(merges []inventory.BatchedMerge, edges []inventory.Edge) footprint { + f := newFootprint() + var moved []string + for _, m := range merges { + k := announcedOf(m) + f.repos[strings.ToLower(m.Repository)] = true + if k.Head != "" { + f.heads[k.Head] = true + } + moved = append(moved, k.Moves...) + } + for _, name := range reachableFrom(moved, edges) { + f.modules[name] = true + } + return f +} + +// related says whether two changes are related, and the first reason in plain words; false and empty when +// they are not. +func related(a, b footprint, edges []inventory.Edge) (bool, string) { + if r := firstShared(a.repos, b.repos); r != "" { + return true, "both change " + r + } + if m := firstShared(a.modules, b.modules); m != "" { + return true, "both move " + m + } + if h := firstShared(a.heads, b.heads); h != "" { + return true, "both are members of group " + h + } + for _, e := range edges { + if a.modules[e.From] && b.modules[e.To] || b.modules[e.From] && a.modules[e.To] { + return true, e.From + " depends on " + e.To + " (" + e.Kind + ")" + } + } + return false, "" +} + +// firstShared is the first key, in order, both sets hold; empty when none. +func firstShared(a, b map[string]bool) string { + var both []string + for k := range a { + if b[k] { + both = append(both, k) + } + } + if len(both) == 0 { + return "" + } + sort.Strings(both) + return both[0] +} diff --git a/cmd/mesh-controller/related_test.go b/cmd/mesh-controller/related_test.go new file mode 100644 index 00000000..bff1ec3b --- /dev/null +++ b/cmd/mesh-controller/related_test.go @@ -0,0 +1,92 @@ +package main + +import ( + "encoding/json" + "testing" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// heardMerge is a kept merge of a repository from a head branch that moved modules. +func heardMerge(repository, head string, moves ...string) inventory.BatchedMerge { + m := inventory.BatchedMerge{Repository: repository, Branch: "main", Commit: "c-" + repository} + m.Event, _ = json.Marshal(keptMerge{SourceMoved: link.SourceMoved{Head: head}, Moves: moves}) + return m +} + +// walkOf is a walk carrying one repository's commit and planning modules. +func walkOf(repository string, modules ...string) inventory.Plan { + p := inventory.Plan{Repository: repository, Commit: "w-" + repository, Modules: map[string]*inventory.PlanModule{}} + for _, name := range modules { + p.Modules[name] = &inventory.PlanModule{} + } + return p +} + +// TestRelatedChanges: two changes are related by a repository, a module, a group head or a graph edge between +// their modules, and by a module reached only through dependents (novox/hq ADR 0304 decision 1, ADR 0306 +// decision 3); otherwise they are not. +func TestRelatedChanges(t *testing.T) { + edges := []inventory.Edge{ + {From: "nzbget", To: "base-image", Kind: inventory.EdgeStandsOn}, + {From: "app", To: "build-machine", Kind: inventory.EdgeBuiltBy}, + } + cases := []struct { + name string + a, b footprint + why string + }{ + {"repository, case aside", + footprintOfWalk(walkOf("novox/Mesh-Catalog", "notes"), nil), + footprintOfBatch([]inventory.BatchedMerge{heardMerge("NOVOX/mesh-catalog", "fix/a", "app")}, nil), + "both change novox/mesh-catalog"}, + {"module", + footprintOfWalk(walkOf("novox/one", "nzbget"), nil), + footprintOfBatch([]inventory.BatchedMerge{heardMerge("novox/two", "fix/b", "nzbget")}, nil), + "both move nzbget"}, + {"edge, walk depends on batch", + footprintOfWalk(walkOf("novox/one", "nzbget"), nil), + footprintOfBatch([]inventory.BatchedMerge{heardMerge("novox/two", "fix/b", "base-image")}, nil), + "nzbget depends on base-image (stands-on)"}, + {"edge, batch depends on walk", + footprintOfWalk(walkOf("novox/one", "build-machine"), nil), + footprintOfBatch([]inventory.BatchedMerge{heardMerge("novox/two", "fix/b", "app")}, nil), + "app depends on build-machine (built-by)"}, + {"group head", + footprintOfWalk(walkOf("novox/one", "notes"), []inventory.BatchedMerge{heardMerge("novox/one", "feat/x")}), + footprintOfBatch([]inventory.BatchedMerge{heardMerge("novox/two", "feat/x", "app")}, nil), + "both are members of group feat/x"}, + {"module reached through dependents", + footprintOfWalk(walkOf("novox/one", "nzbget"), nil), + footprintOfBatch([]inventory.BatchedMerge{heardMerge("novox/two", "fix/b", "base-image")}, edges), + "both move nzbget"}, + {"unrelated", + footprintOfWalk(walkOf("novox/one", "notes"), []inventory.BatchedMerge{heardMerge("novox/one", "feat/x")}), + footprintOfBatch([]inventory.BatchedMerge{heardMerge("novox/two", "feat/y", "app")}, edges), + ""}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + for _, pair := range [][2]footprint{{c.a, c.b}, {c.b, c.a}} { + is, why := related(pair[0], pair[1], edges) + if is != (c.why != "") || why != c.why { + t.Errorf("related = %v, %q; want %q", is, why, c.why) + } + } + }) + } +} + +// TestFootprintOfBatchWidensToDependents: a batch's modules are what its merges moved and every module that +// depends on them, along the edges that widen a walk. +func TestFootprintOfBatchWidensToDependents(t *testing.T) { + edges := []inventory.Edge{ + {From: "nzbget", To: "base-image", Kind: inventory.EdgeStandsOn}, + {From: "base-image", To: "build-machine", Kind: inventory.EdgeBuiltBy}, + } + f := footprintOfBatch([]inventory.BatchedMerge{heardMerge("novox/two", "", "base-image")}, edges) + if !f.modules["base-image"] || !f.modules["nzbget"] || f.modules["build-machine"] || len(f.heads) != 0 { + t.Errorf("footprint = %+v; want base-image and nzbget, no head", f) + } +} From 9460689fffb4d52299b1e4db7edaaeed7cbb7175 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 02:06:54 +0200 Subject: [PATCH 2/2] Relate a batch whose moves are not known to every change The review found that a merge kept without its moves, judged again at the cut, gave its batch no modules, so it could read as unrelated to a walk it will build beside. An unknown change is never unrelated. --- cmd/mesh-controller/related.go | 17 +++++++++++++++-- cmd/mesh-controller/related_test.go | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/cmd/mesh-controller/related.go b/cmd/mesh-controller/related.go index 0180e6bd..fdaefed0 100644 --- a/cmd/mesh-controller/related.go +++ b/cmd/mesh-controller/related.go @@ -14,9 +14,11 @@ import ( // other (ADR 0304 decision 2, ADR 0282 decision 3); nothing walks so yet, and nothing but tests reads this. // footprint is what one change reaches: its repositories (lower case), its modules, and its delivery group -// heads (the head branch names of its merges). +// heads (the head branch names of its merges). unknown says a merge of it was kept without the modules it +// moves, so what it reaches is not known: it is related to every change, never wrongly to none. type footprint struct { repos, modules, heads map[string]bool + unknown string } func newFootprint() footprint { @@ -44,7 +46,9 @@ func footprintOfWalk(p inventory.Plan, kept []inventory.BatchedMerge) footprint } // footprintOfBatch is an assembling batch's footprint: each merge's repository and head, and the modules its -// merges moved when heard, widened to their dependents as the walk cut from it would be (planOfMoves). +// merges moved when heard, widened to their dependents (planOfMoves). A merge kept with no moves (one owed a +// later look, or kept for its packaging) is judged again at the cut and may move anything then: its batch is +// unknown, related to everything. func footprintOfBatch(merges []inventory.BatchedMerge, edges []inventory.Edge) footprint { f := newFootprint() var moved []string @@ -54,6 +58,9 @@ func footprintOfBatch(merges []inventory.BatchedMerge, edges []inventory.Edge) f if k.Head != "" { f.heads[k.Head] = true } + if len(k.Moves) == 0 && f.unknown == "" { + f.unknown = "the modules a merge of " + m.Repository + " moves are not known until its batch is cut" + } moved = append(moved, k.Moves...) } for _, name := range reachableFrom(moved, edges) { @@ -65,6 +72,12 @@ func footprintOfBatch(merges []inventory.BatchedMerge, edges []inventory.Edge) f // related says whether two changes are related, and the first reason in plain words; false and empty when // they are not. func related(a, b footprint, edges []inventory.Edge) (bool, string) { + if a.unknown != "" { + return true, a.unknown + } + if b.unknown != "" { + return true, b.unknown + } if r := firstShared(a.repos, b.repos); r != "" { return true, "both change " + r } diff --git a/cmd/mesh-controller/related_test.go b/cmd/mesh-controller/related_test.go index bff1ec3b..764df1d3 100644 --- a/cmd/mesh-controller/related_test.go +++ b/cmd/mesh-controller/related_test.go @@ -2,6 +2,7 @@ package main import ( "encoding/json" + "strings" "testing" "github.com/novox/mesh-controller/internal/inventory" @@ -90,3 +91,20 @@ func TestFootprintOfBatchWidensToDependents(t *testing.T) { t.Errorf("footprint = %+v; want base-image and nzbget, no head", f) } } + +// A batch holding a merge kept with no moves (one owed a later look, judged again at the cut) is related to +// every change, in both orders: what it will move is not known, and an unknown change is never unrelated +// (issue 525 review). +func TestABatchWhoseMovesAreNotKnownIsRelatedToEverything(t *testing.T) { + unknown := footprintOfBatch([]inventory.BatchedMerge{heardMerge("novox/one", "fix/a", "app"), heardMerge("novox/two", "fix/b")}, nil) + other := footprintOfWalk(walkOf("novox/three", "nzbget"), nil) + for _, pair := range [][2]footprint{{unknown, other}, {other, unknown}} { + if ok, why := related(pair[0], pair[1], nil); !ok || !strings.Contains(why, "not known") { + t.Fatalf("an unknown batch read as %v (%q)", ok, why) + } + } + known := footprintOfBatch([]inventory.BatchedMerge{heardMerge("novox/one", "fix/a", "app")}, nil) + if ok, why := related(known, other, nil); ok { + t.Fatalf("a known, unrelated batch read as related: %q", why) + } +}