diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index 77522045..95550c73 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -1430,7 +1430,7 @@ func (h *Holder) plan() []func() { h.tellCovered(d, now) // covered before this holder told it, or before a restart (novox/hq issue 480) continue } - if strandedUncovered(d) { + if strandedOpen(d) { // Stopped and final, but still read for a later merge that delivered its modules: until one did, // `stalled` keeps listing it (review of novox/hq issue 474). var ask []string @@ -1606,6 +1606,12 @@ func (h *Holder) tellCovered(d *Delivery, now time.Time) { // merged change that never reached the machines, which `stalled` keeps listing until a later delivered merge // carries its modules (review of novox/hq issue 474). func strandedUncovered(d *Delivery) bool { + return strandedOpen(d) && !d.strandedEnded() +} + +// strandedOpen is a stop for a walk that built none of its modules that no merge was found to cover: still read for +// a cover, and listed unless it is ended (novox/hq issue 1581). +func strandedOpen(d *Delivery) bool { return d.State == Stopped && d.Stranded != "" && d.CoveredBy == "" } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1581_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1581_test.go new file mode 100644 index 00000000..3f89f04f --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1581_test.go @@ -0,0 +1,72 @@ +package main + +import ( + "strings" + "testing" + "time" +) + +// novox/hq issue 1581: a delivery stopped because its walk ended without building its modules (issue 474) waited for +// "a later delivered merge to carry" them, listed by `stalled` for good. When no later merge can: the pull request +// removed the module from the catalogue (mesh-catalog #384, scan), or the module is built for no machine (#391, "sent +// nowhere"), the stop reads ended, as covered reads covered (issue 480) — a reading of the stop, no transition — says +// why, and leaves `stalled`, so the condition that listed it ends by itself. + +func strandedWith(t *testing.T, change func(d *Delivery)) (*world, string) { + w, id := strandedNight(t) + if len(w.h.Stalled()) != 1 { + t.Fatalf("before, stalled lists %+v", w.h.Stalled()) + } + w.h.mu.Lock() + d := w.h.deliveries[id] + change(d) + if err := w.store.PutDelivery(d); err != nil { + t.Fatal(err) + } + w.h.mu.Unlock() + w.later(time.Minute) + w.settleAll() + return w, id +} + +func TestAStrandedStopWhoseModuleIsSentNowhereEndsWithoutALaterMerge(t *testing.T) { + w, id := strandedWith(t, func(d *Delivery) { + d.Plan.Summary = "builds build-agent, sent nowhere; no bus step" + }) + if s := w.h.Stalled(); len(s) != 0 { + t.Fatalf("sent nowhere, stalled still lists %+v", s) + } + shown, err := w.h.Show(id) + if err != nil { + t.Fatal(err) + } + reads, _ := shown.(map[string]any)["reads"].(string) + if !strings.HasPrefix(reads, "ended") || !strings.Contains(reads, "build-agent") { + t.Fatalf("show reads %q", reads) + } + if waits, _ := shown.(map[string]any)["waits"].(string); strings.Contains(waits, "later delivered merge") { + t.Fatalf("it still waits for %q", waits) + } + if d := w.delivery(id); d.State != Stopped { + t.Fatalf("a reading, not a transition: it is %s", d.State) + } +} + +func TestAStrandedStopWhoseModuleThePullRequestRemovedEndsWithoutALaterMerge(t *testing.T) { + w, _ := strandedWith(t, func(d *Delivery) { + d.Removed = []string{"modules/build-agent/module.json", "modules/build-agent/cmd/main.go"} + }) + if s := w.h.Stalled(); len(s) != 0 { + t.Fatalf("removed, stalled still lists %+v", s) + } +} + +// A stop whose module still runs somewhere keeps waiting for a later merge, and stays listed. +func TestAStrandedStopWhoseModuleCanStillBeCarriedStaysListed(t *testing.T) { + w, _ := strandedWith(t, func(d *Delivery) { + d.Removed = []string{"modules/other/module.json"} + }) + if s := w.h.Stalled(); len(s) != 1 { + t.Fatalf("stalled lists %+v", s) + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go index ada614f3..791d2c26 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go @@ -357,6 +357,44 @@ func (d *Delivery) coveredStop() bool { return false } +// strandedEnded is whether a stop for a walk that built none of its modules (novox/hq issue 474), found covered by no +// merge, waits for nothing that can come: every module its walk left unbuilt is one no later merge can carry, because +// the pull request removed it from the catalogue (its module.json is among the removed paths) or its plan sends it to +// no machine ("sent nowhere"). It is a reading of the stop, as covered is (issue 480): the state stays stopped, and +// `stalled` no longer lists it, so the condition that listed it ends by itself (issue 1581). +func (d *Delivery) strandedEnded() bool { + if d.State != Stopped || d.Stranded == "" || d.CoveredBy != "" { + return false + } + missing := d.notMovedBy() + for _, m := range missing { + if !d.cannotBeCarried(m) { + return false + } + } + return len(missing) > 0 +} + +// cannotBeCarried is whether no later merge can deliver a module this delivery's plan moved: it was removed, or the +// plan sends it nowhere. +func (d *Delivery) cannotBeCarried(module string) bool { + if d.Plan == nil { + return false + } + for _, p := range d.Removed { + if strings.HasSuffix(p, "/"+module+"/module.json") { + return true + } + } + return strings.Contains(d.Plan.Summary, ", sent nowhere") && len(d.Plan.receiving(module)) == 0 +} + +// strandedEndedSaid is how an ended stop reads, in `show`. +func (d *Delivery) strandedEndedSaid() string { + return "ended: it waited for a later merge to carry " + strings.Join(d.notMovedBy(), ", ") + + ", and none can: the module was removed from the catalogue or is built for no machine (novox/hq issue 1581)" +} + // coveredStopSaid is how a covered stop or failure reads: on the forge's status, in its view and note, and in `show`. func (d *Delivery) coveredStopSaid() string { ended := "its stop" diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go index b7bee361..7179b032 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go @@ -127,6 +127,8 @@ func (h *Holder) Show(id string) (any, error) { out := map[string]any{"delivery": d, "waits": h.waitsFor(d)} if d.coveredStop() { out["reads"] = d.coveredStopSaid() // stopped, and its change reached the machines (novox/hq issue 480) + } else if d.strandedEnded() { + out["reads"] = d.strandedEndedSaid() // stopped, and nothing can carry what it waited for (issue 1581) } if g := h.groups[d.Group]; g != nil { state, why := GroupState(g, h.membersOf(g))