From 9460689fffb4d52299b1e4db7edaaeed7cbb7175 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 02:06:54 +0200 Subject: [PATCH] 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) + } +}