Deliver a merge whose walk said its modules matched what runs, instead of stopping it as stranded (hq issue 611)
mesh/merge-gate head 2f5e97030a5c merged into trunk bc3b59227f46 — pass: builds mesh-delivery → novox; no bus step; every machine composes with the change …
mesh/repo-check head 2f5e97030a5c merged into trunk bc3b59227f46 — pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-…
mesh/delivery delivered
mesh/delivery-group group fix/611-an-unchanged-build-is-delivered delivered: every member is delivered
mesh/merge-gate head 2f5e97030a5c merged into trunk bc3b59227f46 — pass: builds mesh-delivery → novox; no bus step; every machine composes with the change …
mesh/repo-check head 2f5e97030a5c merged into trunk bc3b59227f46 — pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-…
mesh/delivery delivered
mesh/delivery-group group fix/611-an-unchanged-build-is-delivered delivered: every member is delivered
A walk that moved nothing because every module's recorded build source held none of the changed files now says so per module (delivery.unchanged). Such a merge moves nothing (ADR 0270 §1): it is delivered, naming the modules, not stopped 20 minutes later with the operator asked about a change already on every machine. A module the walk neither built nor said unchanged still stops it (issue 474).
This commit is contained in:
@@ -0,0 +1,150 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"sort"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
// novox/hq issue 611: mesh-controller #293 changed one test file. Its check's plan, made while another walk built
|
||||||
|
// build-agent and route-proxy, read them whole and named them; at the cut their recorded build sources held none of
|
||||||
|
// its files, so its walk was done with no tier, "moved nothing the mesh holds: nothing to walk". The stranded rule
|
||||||
|
// (issue 474) read that as "ended without building build-agent, route-proxy", stopped it, and the operator was asked
|
||||||
|
// about a change already on every machine. The controller now says per module that the build matched what runs
|
||||||
|
// (the walk's delivery.unchanged), and such a delivery is delivered: its merge moves nothing (ADR 0270 §1). A walk
|
||||||
|
// that says neither — not built, not unchanged — still stops it.
|
||||||
|
|
||||||
|
const (
|
||||||
|
head293 = "94c928b4c7e1d2aa"
|
||||||
|
merge293 = "8f1aec0f2b9d6e11"
|
||||||
|
walk293 = "plan-1791867648768608552"
|
||||||
|
)
|
||||||
|
|
||||||
|
var at293 = time.Date(2026, 10, 13, 5, 10, 0, 0, time.UTC)
|
||||||
|
|
||||||
|
// matched is the why the controller gives a module it says unchanged.
|
||||||
|
const matched = "its build source in novox/mesh-controller holds none of the 1 changed file(s) " +
|
||||||
|
"(e.g. internal/builder/replay363_test.go): the build the mesh holds is its build"
|
||||||
|
|
||||||
|
// replay293 is #293 checked moving build-agent and route-proxy, merged, and its walk done with no tier, saying the
|
||||||
|
// modules named unchanged.
|
||||||
|
func replay293(t *testing.T, unchanged map[string]string) *world {
|
||||||
|
t.Helper()
|
||||||
|
w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), at293.Add(-time.Minute))
|
||||||
|
p := pr(ctl474, 293, head293, "fix/363-replay", "internal/builder/replay363_test.go")
|
||||||
|
w.h.PullUpdated(p)
|
||||||
|
w.h.Checked(verdictMoving(ctl474, 293, head293, "build-agent", "route-proxy"))
|
||||||
|
w.later(time.Minute)
|
||||||
|
w.h.PullMerged(mergedAt(p, merge293, at293))
|
||||||
|
w.later(90 * time.Second)
|
||||||
|
note := nothingNote
|
||||||
|
if len(unchanged) > 0 {
|
||||||
|
note += "; unchanged, the build the mesh holds is theirs: " + strings.Join(sortedKeys(unchanged), ", ")
|
||||||
|
}
|
||||||
|
w.h.WalkMoved(w.ctl.put(Walk{ID: walk293, Repository: ctl474, Branch: "main", Commit: merge293, Created: w.now,
|
||||||
|
Updated: w.now, State: walkDone, Tiers: [][]string{}, Modules: map[string]*WalkModule{}, Note: note,
|
||||||
|
Delivery: &WalkDelivery{Awaits: "mesh-delivery", Merges: []WalkMerge{{Repository: ctl474, Commit: merge293}},
|
||||||
|
Unchanged: unchanged}}))
|
||||||
|
w.settleAll()
|
||||||
|
return w
|
||||||
|
}
|
||||||
|
|
||||||
|
// The live case: every module its plan named, the walk said matched what runs. Delivered at once, naming them, a
|
||||||
|
// success on the pull request, and nothing listed by stalled or asked of the operator, then or past the grace.
|
||||||
|
func TestAMergeWhoseModulesMatchWhatRunsIsDelivered(t *testing.T) {
|
||||||
|
w := replay293(t, map[string]string{"build-agent": matched, "route-proxy": matched})
|
||||||
|
id := IDOf(ctl474, head293)
|
||||||
|
d := w.delivery(id)
|
||||||
|
if d.State != Delivered {
|
||||||
|
t.Fatalf("#293, its modules said unchanged by its walk, is %s, waiting for %q", d.State, w.h.waitsFor(d))
|
||||||
|
}
|
||||||
|
why := d.Transitions[len(d.Transitions)-1].Why
|
||||||
|
for _, want := range []string{walk293, "build-agent, route-proxy", "matched what runs"} {
|
||||||
|
if !strings.Contains(why, want) {
|
||||||
|
t.Fatalf("delivered as %q, not saying %q", why, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if d.Stranded != "" {
|
||||||
|
t.Fatalf("it was read as stranded: %q", d.Stranded)
|
||||||
|
}
|
||||||
|
status := w.forge.statuses["novox/mesh-controller@"+head293+" mesh/delivery"]
|
||||||
|
if !strings.HasPrefix(status, "success ") {
|
||||||
|
t.Fatalf("mesh/delivery says %q", status)
|
||||||
|
}
|
||||||
|
w.later(strandedAfter + time.Minute)
|
||||||
|
w.settleAll()
|
||||||
|
if s := w.state(id); s != Delivered {
|
||||||
|
t.Fatalf("past the grace it is %s", s)
|
||||||
|
}
|
||||||
|
if lines := w.h.Stalled(); len(lines) != 0 {
|
||||||
|
t.Fatalf("stalled lists %+v", lines)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A walk that said only one of its modules unchanged left the other unbuilt: it is still stopped past the grace,
|
||||||
|
// naming only the module nothing delivered.
|
||||||
|
func TestAMergeWithAModuleNeitherBuiltNorUnchangedIsStillStopped(t *testing.T) {
|
||||||
|
w := replay293(t, map[string]string{"build-agent": matched})
|
||||||
|
id := IDOf(ctl474, head293)
|
||||||
|
if s := w.state(id); s != Published {
|
||||||
|
t.Fatalf("within the grace it is %s", s)
|
||||||
|
}
|
||||||
|
w.later(strandedAfter + time.Minute)
|
||||||
|
w.settleAll()
|
||||||
|
d := w.delivery(id)
|
||||||
|
if d.State != Stopped {
|
||||||
|
t.Fatalf("route-proxy neither built nor unchanged, it is %s", d.State)
|
||||||
|
}
|
||||||
|
why := d.Transitions[len(d.Transitions)-1].Why
|
||||||
|
if !strings.Contains(why, "without building route-proxy (") {
|
||||||
|
t.Fatalf("its stop says %q", why)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A walk that says nothing unchanged — a controller from before, or one that built nothing at all — stops it as
|
||||||
|
// before (issue 474).
|
||||||
|
func TestAMergeWhoseWalkSaysNothingUnchangedIsStillStopped(t *testing.T) {
|
||||||
|
w := replay293(t, nil)
|
||||||
|
w.later(strandedAfter + time.Minute)
|
||||||
|
w.settleAll()
|
||||||
|
if d := w.delivery(IDOf(ctl474, head293)); d.State != Stopped ||
|
||||||
|
!strings.Contains(d.Stranded, "without building build-agent, route-proxy") {
|
||||||
|
t.Fatalf("it is %s: %q", d.State, d.Stranded)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Batched behind a later merge of its branch (issue 474's shape), a merge whose module the walk said unchanged moves
|
||||||
|
// nothing at either commit: delivered, not covered by a later merge that delivered nothing for it.
|
||||||
|
func TestABatchedMergeWhoseModuleMatchesWhatRunsIsDelivered(t *testing.T) {
|
||||||
|
w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), at215.Add(-time.Minute))
|
||||||
|
p215 := pr(ctl474, 215, head215, "fix/431-make-check-runs-what-the-gate-runs", "Makefile")
|
||||||
|
p217 := pr(ctl474, 217, head217, "fix/432-a-test-reads-what-it-carries", "cmd/mesh-controller/check_test.go")
|
||||||
|
w.h.PullUpdated(p215)
|
||||||
|
w.h.PullUpdated(p217)
|
||||||
|
w.h.Checked(verdictMoving(ctl474, 215, head215, "build-agent"))
|
||||||
|
w.h.Checked(verdictMoving(ctl474, 217, head217))
|
||||||
|
w.later(time.Minute)
|
||||||
|
w.h.PullMerged(mergedAt(p215, merge215, at215))
|
||||||
|
w.later(time.Minute)
|
||||||
|
w.h.PullMerged(mergedAt(p217, merge217, at215.Add(56*time.Second)))
|
||||||
|
w.later(90 * time.Second)
|
||||||
|
w.h.WalkMoved(w.ctl.put(Walk{ID: batchWalk, Repository: ctl474, Branch: "main", Commit: merge217, Created: w.now,
|
||||||
|
Updated: w.now, State: walkDone, Tiers: [][]string{}, Modules: map[string]*WalkModule{}, Note: nothingNote,
|
||||||
|
Delivery: &WalkDelivery{Awaits: "mesh-delivery", Merges: []WalkMerge{
|
||||||
|
{Repository: ctl474, Commit: merge215, Carried: merge217}, {Repository: ctl474, Commit: merge217}},
|
||||||
|
Unchanged: map[string]string{"build-agent": "its build source holds none of the 2 changed file(s)"}}}))
|
||||||
|
w.settleAll()
|
||||||
|
if d := w.delivery(IDOf(ctl474, head215)); d.State != Delivered {
|
||||||
|
t.Fatalf("#215, build-agent said unchanged by its batch's walk, is %s, waiting for %q", d.State, w.h.waitsFor(d))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func sortedKeys(m map[string]string) []string {
|
||||||
|
var out []string
|
||||||
|
for k := range m {
|
||||||
|
out = append(out, k)
|
||||||
|
}
|
||||||
|
sort.Strings(out)
|
||||||
|
return out
|
||||||
|
}
|
||||||
@@ -3,6 +3,7 @@ package main
|
|||||||
import (
|
import (
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"maps"
|
||||||
"regexp"
|
"regexp"
|
||||||
"sort"
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -460,6 +461,11 @@ type WalkDelivery struct {
|
|||||||
// cut, is the walk under the same id: its next record is `building`. A merge this owner reads in a batch is
|
// cut, is the walk under the same id: its next record is `building`. A merge this owner reads in a batch is
|
||||||
// not held as unwalked while the batch is heard of.
|
// not held as unwalked while the batch is heard of.
|
||||||
Batch *WalkBatch `json:"batch,omitempty"`
|
Batch *WalkBatch `json:"batch,omitempty"`
|
||||||
|
// Unchanged are the modules the walk did not build because their build source holds none of the files its
|
||||||
|
// merges changed, each with why (novox/hq issue 611, ADR 0267 §5): the build the mesh holds is theirs. A delivery
|
||||||
|
// plan that read them whole names them; this says they matched what runs, not that the walk left them unbuilt.
|
||||||
|
// Absent from a controller that does not say it.
|
||||||
|
Unchanged map[string]string `json:"unchanged,omitempty"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// WalkMerge is one merge a walk answers.
|
// WalkMerge is one merge a walk answers.
|
||||||
@@ -583,6 +589,9 @@ type WalkSeen struct {
|
|||||||
// from them.
|
// from them.
|
||||||
Moved []string `json:"moved,omitempty"`
|
Moved []string `json:"moved,omitempty"`
|
||||||
MovedSaid bool `json:"moved_said,omitempty"`
|
MovedSaid bool `json:"moved_said,omitempty"`
|
||||||
|
// Unchanged are the modules the walk said matched what runs, with why (novox/hq issue 611): not built, and
|
||||||
|
// nothing for it to deliver.
|
||||||
|
Unchanged map[string]string `json:"unchanged,omitempty"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// Seen is a walk as a delivery keeps it.
|
// Seen is a walk as a delivery keeps it.
|
||||||
@@ -594,6 +603,9 @@ func (w Walk) Seen() *WalkSeen {
|
|||||||
s.Waits = d.Awaits != "" && d.Go == nil
|
s.Waits = d.Awaits != "" && d.Go == nil
|
||||||
s.GoBy = d.By
|
s.GoBy = d.By
|
||||||
s.StoppedBy, s.StoppedWhy = d.Stopped, d.StoppedWhy
|
s.StoppedBy, s.StoppedWhy = d.Stopped, d.StoppedWhy
|
||||||
|
if len(d.Unchanged) > 0 {
|
||||||
|
s.Unchanged = maps.Clone(d.Unchanged)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
s.Branch = w.Branch
|
s.Branch = w.Branch
|
||||||
s.Created = w.Created
|
s.Created = w.Created
|
||||||
@@ -644,9 +656,10 @@ func (d *Delivery) stoppedWords() string {
|
|||||||
return s
|
return s
|
||||||
}
|
}
|
||||||
|
|
||||||
// walkMovedItsModules is whether the walk it follows built every module its plan moves, and whether that is known:
|
// walkMovedItsModules is whether the walk it follows answered every module its plan moves, and whether that is
|
||||||
// from the modules the walk's record names, or, for a record kept before they were, from a walk of no tier, which
|
// known: each built, or said unchanged — its build source holds none of the files the walk's merges changed, so the
|
||||||
// moved nothing. Unknown when its plan is not known.
|
// build the mesh holds is its build (novox/hq issue 611) — from the modules the walk's record names, or, for a
|
||||||
|
// record kept before they were, from a walk of no tier, which moved nothing. Unknown when its plan is not known.
|
||||||
func (d *Delivery) walkMovedItsModules() (moved, known bool) {
|
func (d *Delivery) walkMovedItsModules() (moved, known bool) {
|
||||||
if d.Walk == nil || d.Plan == nil {
|
if d.Walk == nil || d.Plan == nil {
|
||||||
return false, false
|
return false, false
|
||||||
@@ -654,28 +667,46 @@ func (d *Delivery) walkMovedItsModules() (moved, known bool) {
|
|||||||
if !d.Walk.MovedSaid && d.Walk.Tiers > 0 {
|
if !d.Walk.MovedSaid && d.Walk.Tiers > 0 {
|
||||||
return false, false
|
return false, false
|
||||||
}
|
}
|
||||||
for _, m := range d.Plan.Moved {
|
return len(d.notMovedBy()) == 0, true
|
||||||
if !containsString(d.Walk.Moved, m) {
|
|
||||||
return false, true
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return true, true
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// notMovedBy is the modules its plan moves that the walk it follows did not build.
|
// notMovedBy is the modules its plan moves that the walk it follows neither built nor said unchanged: what a walk
|
||||||
|
// that ended without building them left undelivered.
|
||||||
func (d *Delivery) notMovedBy() []string {
|
func (d *Delivery) notMovedBy() []string {
|
||||||
var out []string
|
var out []string
|
||||||
if d.Walk == nil || d.Plan == nil {
|
if d.Walk == nil || d.Plan == nil {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
for _, m := range d.Plan.Moved {
|
for _, m := range d.Plan.Moved {
|
||||||
if !containsString(d.Walk.Moved, m) {
|
if _, unchanged := d.Walk.Unchanged[m]; !containsString(d.Walk.Moved, m) && !unchanged {
|
||||||
out = append(out, m)
|
out = append(out, m)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// unchangedOfIts is the modules its plan moves that the walk it follows said matched what runs, sorted (novox/hq
|
||||||
|
// issue 611).
|
||||||
|
func (d *Delivery) unchangedOfIts() []string {
|
||||||
|
var out []string
|
||||||
|
if d.Walk == nil || d.Plan == nil {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
for _, m := range d.Plan.Moved {
|
||||||
|
if _, unchanged := d.Walk.Unchanged[m]; unchanged && !containsString(d.Walk.Moved, m) {
|
||||||
|
out = append(out, m)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
sort.Strings(out)
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
// builtNoneOfIts is whether the walk it follows built none of the modules its plan moves and said each unchanged:
|
||||||
|
// its merge moves nothing, whichever commit the walk carried (ADR 0270 §1).
|
||||||
|
func (d *Delivery) builtNoneOfIts() bool {
|
||||||
|
return d.Plan != nil && len(d.Plan.Moved) > 0 && len(d.unchangedOfIts()) == len(d.Plan.Moved)
|
||||||
|
}
|
||||||
|
|
||||||
// Started is whether the walk is past its wait: it waited for nobody, or was let go — whatever came of it
|
// Started is whether the walk is past its wait: it waited for nobody, or was let go — whatever came of it
|
||||||
// since. A walk superseded or stopped while it waited never started.
|
// since. A walk superseded or stopped while it waited never started.
|
||||||
func (s *WalkSeen) Started() bool { return s != nil && !s.Waits && (s.WaitedFor == "" || s.GoBy != "") }
|
func (s *WalkSeen) Started() bool { return s != nil && !s.Waits && (s.WaitedFor == "" || s.GoBy != "") }
|
||||||
|
|||||||
@@ -249,19 +249,31 @@ var Table = []Row{
|
|||||||
}},
|
}},
|
||||||
{From: []State{Published}, Event: EvDone, To: Delivered,
|
{From: []State{Published}, Event: EvDone, To: Delivered,
|
||||||
Guard: "nothing for a walk to move; or its walk, carrying its own commit, ended done before its word having built " +
|
Guard: "nothing for a walk to move; or its walk, carrying its own commit, ended done before its word having built " +
|
||||||
"every module it moves (novox/hq issue 474)",
|
"every module it moves or said it unchanged (novox/hq issue 474); or its walk said every module it moves " +
|
||||||
|
"unchanged, its build source holding none of the files changed, so it moves nothing (novox/hq issue 611, " +
|
||||||
|
"ADR 0270 §1)",
|
||||||
Holds: func(d *Delivery, f Facts) error {
|
Holds: func(d *Delivery, f Facts) error {
|
||||||
if d.Walk == nil && d.MovesNothing() {
|
if d.Walk == nil && d.MovesNothing() {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
moved, _ := d.walkMovedItsModules()
|
moved, _ := d.walkMovedItsModules()
|
||||||
return unless(d.endedUnstarted() && !d.carriedLater() && moved, "a walk moves it")
|
return unless(d.endedUnstarted() && moved && (!d.carriedLater() || d.builtNoneOfIts()), "a walk moves it")
|
||||||
},
|
},
|
||||||
Says: func(d *Delivery) string {
|
Says: func(d *Delivery) string {
|
||||||
if d.Walk == nil {
|
if d.Walk == nil {
|
||||||
return "nothing for a walk to move"
|
return "nothing for a walk to move"
|
||||||
}
|
}
|
||||||
return fmt.Sprintf("its walk %s ended done before its word, having built every module it moves", d.Walk.ID)
|
unchanged := d.unchangedOfIts()
|
||||||
|
switch {
|
||||||
|
case len(unchanged) == 0:
|
||||||
|
return fmt.Sprintf("its walk %s ended done before its word, having built every module it moves", d.Walk.ID)
|
||||||
|
case d.builtNoneOfIts():
|
||||||
|
return fmt.Sprintf("its walk %s moved nothing: %s matched what runs, its build source holding none of "+
|
||||||
|
"the files changed, so the build the mesh holds is its build (novox/hq issue 611)", d.Walk.ID,
|
||||||
|
strings.Join(unchanged, ", "))
|
||||||
|
}
|
||||||
|
return fmt.Sprintf("its walk %s ended done before its word, having built every module it moves but %s, "+
|
||||||
|
"which matched what runs (novox/hq issue 611)", d.Walk.ID, strings.Join(unchanged, ", "))
|
||||||
}},
|
}},
|
||||||
{From: []State{Published, Ready}, Event: EvHold, To: Held,
|
{From: []State{Published, Ready}, Event: EvHold, To: Held,
|
||||||
Guard: "something holds it for a person: its group's composed check did not pass for the heads that merged, " +
|
Guard: "something holds it for a person: its group's composed check did not pass for the heads that merged, " +
|
||||||
|
|||||||
Reference in New Issue
Block a user