From aeab5b80b411fe47f7555d5bac58110709020c7a Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 19:59:43 +0200 Subject: [PATCH 1/3] mesh-delivery: a stop ends only its own change on a shared walk (hq issue 459, ADR 0299) A group stop ended the stopped member's walk, and with it every other delivery the batch put on that walk. It now asks delivery-stop to leave the stopped merges out, so the others are walked again; a person's stop on a shared walk first says what ending the walk would take down and acts only on a way. --- .../cmd/mesh-delivery/fakes_test.go | 63 +++++ .../mesh-delivery/cmd/mesh-delivery/holder.go | 110 +++++++- .../cmd/mesh-delivery/issue459_test.go | 265 ++++++++++++++++++ .../mesh-delivery/cmd/mesh-delivery/main.go | 10 +- .../mesh-delivery/cmd/mesh-delivery/model.go | 80 +++++- .../mesh-delivery/cmd/mesh-delivery/ports.go | 28 ++ .../mesh-delivery/cmd/mesh-delivery/table.go | 40 ++- .../mesh-delivery/cmd/mesh-delivery/verbs.go | 98 ++++++- 8 files changed, 659 insertions(+), 35 deletions(-) create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index b140619a..a491df78 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -94,6 +94,8 @@ type fakeController struct { asked int delivers []string stops []string + // withouts are the stops that left merges out (novox/hq ADR 0299): " without ,…". + withouts []string plans int down bool // onCheck runs as a check is asked, before it is answered: what the forge held at that moment. @@ -188,6 +190,67 @@ func (c *fakeController) Stop(walk, why, by string) error { } return nil } + +// StopWithout ends a walk without the merges named, as the controller does (novox/hq ADR 0299): each merge named, +// and each merge of its repository whose commit the walk carried for it, is left out; the others are walked again +// by a walk of their own, "-again", waiting for its word. +func (c *fakeController) StopWithout(walk, why, by string, without []string) (StopAnswer, error) { + c.mu.Lock() + defer c.mu.Unlock() + if c.down { + return StopAnswer{}, errors.New("down") + } + c.withouts = append(c.withouts, walk+" without "+strings.Join(without, ",")) + w, known := c.walks[walk] + if !known || w.State == walkFailed || w.State == walkDone { + return StopAnswer{}, fmt.Errorf("%s is no open walk", walk) + } + wd := WalkDelivery{} + if w.Delivery != nil { + wd = *w.Delivery + } + named := func(m WalkMerge) bool { return containsString(without, m.Repository+"@"+m.Commit) } + a := StopAnswer{Stopped: walk} + var again []WalkMerge + for _, m := range wd.Merges { + if named(m) { + a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit}) + } + } + for _, m := range wd.Merges { + if named(m) { + continue + } + contains := "" + for _, s := range wd.Merges { + if named(s) && s.Repository == m.Repository && s.Carried == m.Commit { + contains = s.Commit + } + } + if contains != "" { + a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: contains}) + continue + } + m.Carried = "" + again = append(again, m) + a.WalkedAgain = append(a.WalkedAgain, m.Repository+"@"+m.Commit) + } + if len(again) > 0 { + id := walk + "-again" + c.walks[id] = Walk{ID: id, Repository: again[0].Repository, Branch: "main", Commit: again[0].Commit, + Created: w.Created.Add(time.Minute), State: walkBuilding, Tiers: [][]string{{"app"}}, + Modules: map[string]*WalkModule{"app": {}}, Revision: 1, + Delivery: &WalkDelivery{Awaits: "mesh-delivery", Merges: again}} + wd.WalkedAgainBy, a.WalkedAgainBy = id, id + } + wd.Stopped, wd.StoppedWhy, wd.LeftOut = by, why, a.LeftOut + w.Delivery = &wd + w.State = walkFailed + w.Note = "stopped by " + by + ": " + why + w.Revision++ + c.walks[walk] = w + return a, nil +} func (c *fakeController) Walks(walk string) (bool, []Walk, error) { c.mu.Lock() defer c.mu.Unlock() diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 3130671b..d66cdac6 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -57,6 +57,8 @@ type Holder struct { turning map[string]*turnWant // asking are the walks whose word is being asked right now, outside the lock: not asked twice meanwhile. asking map[string]bool + // leaving are, by walk, the merges a stop gathered to leave out of it, asked once (novox/hq ADR 0299). + leaving map[string][]string } // unmatched is a walk no delivery is known for yet: its merge not heard, or merged before this owner held. @@ -642,6 +644,15 @@ func (h *Holder) linkWalk(d *Delivery, w Walk, carried string) { d.LetGo, d.LetGoWhy = time.Time{}, "" // a new walk waits for a word of its own } d.Walk = w.Seen() + // Stopped without some of its merges (novox/hq ADR 0299): this delivery's merge was left out, maybe for + // containing a stopped one, or is walked again by the batch the record names. + if w.Delivery != nil && (w.Delivery.WalkedAgainBy != "" || len(w.Delivery.LeftOut) > 0) { + if o, out := w.leftOut(d.Repository, d.MergedAs); out { + d.Walk.Contains = o.Contains + } else { + d.Walk.WalkedAgainBy = w.Delivery.WalkedAgainBy + } + } if carried != "" { d.AnsweredBy = carried } @@ -1807,7 +1818,13 @@ func mayCarry(c, d *Delivery, module string) bool { c.Plan != nil && slices.Contains(c.Plan.Moved, module) } -// stopInGroup stops a member after one that failed, and its walk if it has one open. +// stopInGroup stops a member after one that failed, and leaves its merge out of its walk if it has one open. +// +// **Only its own change** (novox/hq issue 459, ADR 0299): one walk carries every merge of its batch, so ending the +// member's walk ended every other delivery on it, which nobody stopped. Its merge is left out instead: the +// controller ends the walk and walks the other merges again without it, and without each later merge of its +// repository, which contains it. Every member a pass stops on one walk is gathered first, and the walk is asked +// once, by the first member's ask, after the pass: a member is never walked again because its stop came second. func (h *Holder) stopInGroup(d *Delivery, why string) func() { if d == nil || d.State.Final() { return nil @@ -1822,11 +1839,98 @@ func (h *Holder) stopInGroup(d *Delivery, why string) func() { return nil } walk := d.Walk.ID + if !h.leave(walk, d) { + return nil // the first member left out of this walk asks for every one + } return func() { - if err := h.Controller.Stop(walk, why, byOwner); err != nil { - h.Logf("[mesh-delivery] the walk %s could not be stopped: %v", walk, err) + without, shared := h.leaving459(walk) + if !shared { + if err := h.Controller.Stop(walk, why, byOwner); err != nil { + h.Logf("[mesh-delivery] the walk %s could not be stopped: %v", walk, err) + } + return + } + a, err := h.Controller.StopWithout(walk, why, byOwner, without) + if err != nil { + h.Logf("[mesh-delivery] the walk %s could not be stopped without %s: %v", walk, strings.Join(without, ", "), err) + return + } + h.Logf("[mesh-delivery] the walk %s is stopped without %s; %s", walk, strings.Join(without, ", "), againWords(a)) + } +} + +// leave gathers a delivery's merge to leave out of a walk (ADR 0299), for a caller holding the lock; true for the +// first of that walk, whose ask asks for all. +func (h *Holder) leave(walk string, d *Delivery) bool { + if h.leaving == nil { + h.leaving = map[string][]string{} + } + first := len(h.leaving[walk]) == 0 + if m := d.Repository + "@" + d.MergedAs; d.MergedAs != "" && !containsString(h.leaving[walk], m) { + h.leaving[walk] = append(h.leaving[walk], m) + } + return first +} + +// leaving459 takes the merges gathered to leave out of a walk, and whether the walk carries anything else: another +// merge it names, or a delivery not ended that follows it. Taken under the lock, outside any. +func (h *Holder) leaving459(walk string) ([]string, bool) { + h.mu.Lock() + defer h.mu.Unlock() + without := h.leaving[walk] + delete(h.leaving, walk) + return without, len(h.othersOn(walk, without)) > 0 +} + +// othersOn are what a walk carries besides the merges named: each delivery not ended that follows it, by id, and +// each merge it names that no such delivery holds, as `@`. +func (h *Holder) othersOn(walk string, without []string) []string { + out := map[string]bool{} + held := map[string]bool{} + for _, d := range h.deliveries { + if d.Walk == nil || d.Walk.ID != walk || d.MergedAs == "" { + continue + } + m := d.Repository + "@" + d.MergedAs + held[strings.ToLower(m)] = true + if !d.State.Final() && !containsString(without, m) { + out[d.ID] = true } } + if s, known := h.walks[walk]; known && s.walk.Delivery != nil { + for _, m := range s.walk.Delivery.Merges { + key := m.Repository + "@" + m.Commit + if !held[strings.ToLower(key)] && !slices.ContainsFunc(without, func(w string) bool { return strings.EqualFold(w, key) }) { + out[key] = true + } + } + } + keys := make([]string, 0, len(out)) + for k := range out { + keys = append(keys, k) + } + slices.Sort(keys) + return keys +} + +// againWords is what a stop that left merges out did, in a person's words. +func againWords(a StopAnswer) string { + var left []string + for _, o := range a.LeftOut { + s := o.Repository + "@" + shortOf(o.Commit, 12) + if o.Contains != "" { + s += " (contains " + shortOf(o.Contains, 12) + ")" + } + left = append(left, s) + } + switch { + case a.WalkedAgainBy != "": + return fmt.Sprintf("%s walks its other merges again: %s; left out: %s", a.WalkedAgainBy, + strings.Join(a.WalkedAgain, ", "), orNone(strings.Join(left, ", "))) + case len(left) > 0: + return "left out: " + strings.Join(left, ", ") + "; nothing else to walk" + } + return "the controller ended it whole: it named no walk that walks its other merges again" } // GroupState is a group's state, derived from its members and its composed check — never set apart (novox/hq diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go new file mode 100644 index 00000000..98dc5e62 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go @@ -0,0 +1,265 @@ +package main + +import ( + "fmt" + "strings" + "testing" + "time" +) + +// novox/hq issue 459, ADR 0299: since the merge window (ADR 0276) one walk carries every merge of its batch. A group +// stop asked the controller to end the stopped member's walk, and with it every other delivery on it, which no one +// stopped. Now a group stop asks `delivery-stop` with the stopped members' merges to leave out: the walk ends, and +// the controller walks its other merges again in a fresh batch; a later merge of a stopped merge's repository +// contains it and ends with it, named. A person's stop of a delivery on a shared walk answers first with what +// ending the walk would take down, and acts only on a way given: `again` or `walk`. + +// shape459 is a world of a delivery group feat/g (its members in the repositories group names, in that order) and +// lone deliveries (the repositories alone names), each checked and merged. The group's first member is on a walk of +// its own, plan-a; every other merge is carried by one walk, plan-2, waiting for its word. +type shape459 struct { + *world + ids, heads, merges map[string]string // by repository +} + +func open459(t *testing.T, group, alone []string) *shape459 { + t.Helper() + w := newWorld(t) + s := &shape459{world: w, ids: map[string]string{}, heads: map[string]string{}, merges: map[string]string{}} + n := 0 + add := func(repo, branch string) { + n++ + head := fmt.Sprintf("%02d%s", n, strings.Repeat("h", 14)) + s.ids[repo], s.heads[repo] = IDOf(repo, head), head + s.merges[repo] = fmt.Sprintf("%02d%s", n, strings.Repeat("m", 10)) + w.h.PullUpdated(pr(repo, n, head, branch, "x")) + w.h.Checked(verdict(repo, n, head, "pass")) + } + for _, r := range group { + add(r, "feat/g") + } + for _, r := range alone { + add(r, "fix/"+r[strings.LastIndex(r, "/")+1:]) + } + w.settleAll() + w.h.Checked(CheckedEvent{Group: "feat/g", ID: "check-1", Verdict: "pass", Summary: "compose"}) + w.settleAll() + for i, r := range append(append([]string{}, group...), alone...) { + w.h.PullMerged(merged(r, i+1, s.heads[r], s.merges[r])) + } + first := aWalk("plan-a", group[0], s.merges[group[0]], true) + first.Delivery.Merges = []WalkMerge{{Repository: group[0], Commit: s.merges[group[0]]}} + w.h.WalkMoved(w.ctl.put(first)) + two := aWalk("plan-2", alone[0], s.merges[alone[0]], true) + two.Created = two.Created.Add(time.Minute) + for _, r := range append(append([]string{}, group[1:]...), alone...) { + two.Delivery.Merges = append(two.Delivery.Merges, WalkMerge{Repository: r, Commit: s.merges[r]}) + } + w.h.WalkMoved(w.ctl.put(two)) + w.settleAll() + return s +} + +// failFirst fails the group's first member's walk: the members after it are stopped by their group. +func (s *shape459) failFirst() { + a := s.ctl.walk("plan-a") + a.State, a.Note = walkFailed, "the first member failed its gate on anchor" + s.h.WalkMoved(s.ctl.put(a)) + s.settleAll() +} + +// heard is the controller's records of the stopped walk and the walk that walks its merges again, as plan-moved says +// them, and then a walk done. +func (s *shape459) heard(walks ...string) { + for _, id := range walks { + s.h.WalkMoved(s.ctl.walk(id)) + } + s.settleAll() +} + +func (s *shape459) done(walk string) { + r := s.ctl.walk(walk) + sent := s.now + for m := range r.Modules { + r.Modules[m] = &WalkModule{State: "built", SentAt: &sent} + } + r.State = walkDone + s.h.WalkMoved(s.ctl.put(r)) + s.settleAll() +} + +func lastWhy(d *Delivery) string { + if len(d.Transitions) == 0 { + return "" + } + return d.Transitions[len(d.Transitions)-1].Why +} + +// A group member stopped on a walk it shares with a lone delivery: the walk is stopped without the member's merge, +// never whole, and the lone delivery follows the walk that walks its merge again and is delivered by it. +func TestAGroupStopWalksTheOtherDeliveriesAgainWithoutIt(t *testing.T) { + s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z"}) + if containsString(s.ctl.delivers, "plan-2") { + t.Fatalf("plan-2 was let go before b's turn: %v", s.ctl.delivers) + } + s.failFirst() + if b := s.delivery(s.ids["novox/b"]); b.State != Stopped || !strings.Contains(lastWhy(b), s.ids["novox/a"]) { + t.Fatalf("b is %s (%q); want it stopped by its group, naming a", b.State, lastWhy(b)) + } + if containsString(s.ctl.stops, "plan-2") { + t.Fatalf("plan-2 was ended whole %v, and with it z, which is in no group and was not stopped", s.ctl.stops) + } + if want := "plan-2 without novox/b@" + s.merges["novox/b"]; !containsString(s.ctl.withouts, want) { + t.Fatalf("the controller was asked %v; want %q", s.ctl.withouts, want) + } + s.heard("plan-2", "plan-2-again") + z := s.delivery(s.ids["novox/z"]) + if z.State.Final() || z.Walk == nil || z.Walk.ID != "plan-2-again" || !containsString(s.ctl.delivers, "plan-2-again") { + t.Fatalf("z is %s on %+v, let go %v; want it following plan-2-again, let go", z.State, z.Walk, s.ctl.delivers) + } + s.done("plan-2-again") + if z, b := s.state(s.ids["novox/z"]), s.state(s.ids["novox/b"]); z != Delivered || b != Stopped { + t.Fatalf("once plan-2-again is done, z is %s and b %s; want z delivered and b stopped", z, b) + } +} + +// Two members after the failed one, both on the shared walk: one ask leaves both out, so neither is walked again +// because its own stop came second. +func TestAGroupStopLeavesOutEveryMemberItStopsOnOneWalk(t *testing.T) { + s := open459(t, []string{"novox/a", "novox/b", "novox/c"}, []string{"novox/z"}) + s.failFirst() + if len(s.ctl.withouts) != 1 || !strings.Contains(s.ctl.withouts[0], "novox/b@"+s.merges["novox/b"]) || + !strings.Contains(s.ctl.withouts[0], "novox/c@"+s.merges["novox/c"]) { + t.Fatalf("the controller was asked %v; want one stop of plan-2 leaving out b and c", s.ctl.withouts) + } + s.heard("plan-2", "plan-2-again") + again := s.ctl.walk("plan-2-again") + if len(again.Delivery.Merges) != 1 || again.Delivery.Merges[0].Repository != "novox/z" { + t.Fatalf("plan-2-again walks %+v; want z's merge alone", again.Delivery.Merges) + } + for _, r := range []string{"novox/b", "novox/c"} { + if st := s.state(s.ids[r]); st != Stopped { + t.Fatalf("%s is %s", r, st) + } + } +} + +// A lone delivery whose merge is a later merge of the stopped member's repository contains the member's merge: it +// is left out and ends stopped, naming the merge it contains; the other lone delivery is walked again. +func TestAGroupStopEndsAMergeThatContainsTheStoppedOne(t *testing.T) { + s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z", "novox/b2"}) + // b2 is a later merge of b's repository: the walk carries it for b. + s.retarget("novox/b2", "novox/b") + s.failFirst() + s.heard("plan-2", "plan-2-again") + b2 := s.delivery(s.ids["novox/b2"]) + if b2.State != Stopped || !strings.Contains(lastWhy(b2), shortOf(s.merges["novox/b"], 8)) || + !strings.Contains(lastWhy(b2), "contains") { + t.Fatalf("b2 is %s (%q); want it stopped, naming b's merge it contains", b2.State, lastWhy(b2)) + } + if z := s.delivery(s.ids["novox/z"]); z.Walk == nil || z.Walk.ID != "plan-2-again" || z.State.Final() { + t.Fatalf("z is %s on %+v; want it following plan-2-again", z.State, z.Walk) + } +} + +// retarget makes the delivery of repo `later` a later merge of repository `of`: same repository, carried by plan-2 +// for the merge of `of`. +func (s *shape459) retarget(later, of string) { + s.t.Helper() + s.h.mu.Lock() + d := s.h.deliveries[s.ids[later]] + d.Repository, d.MergedAt = of, d.MergedAt.Add(time.Minute) + s.h.mu.Unlock() + two := s.ctl.walk("plan-2") + merges := []WalkMerge{} + for _, m := range two.Delivery.Merges { + switch m.Repository { + case of: + m.Carried = s.merges[later] + case later: + m.Repository = of + } + merges = append(merges, m) + } + wd := *two.Delivery + wd.Merges = merges + two.Delivery = &wd + s.h.WalkMoved(s.ctl.put(two)) + s.settleAll() +} + +// shared is two lone deliveries, y and z, let go together on one walk, plan-2. +func shared459(t *testing.T) *shape459 { + t.Helper() + s := open459(t, []string{"novox/a"}, []string{"novox/y", "novox/z"}) + for _, r := range []string{"novox/y", "novox/z"} { + if st := s.state(s.ids[r]); st != Delivering { + t.Fatalf("%s is %s, let go %v; want both delivering on plan-2", r, st, s.ctl.delivers) + } + } + return s +} + +// A person's stop of y, which shares its walk with z, does nothing at first: it names z and the two ways. +func TestAPersonsStopOfASharedWalkAsksFirst(t *testing.T) { + s := shared459(t) + said, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "") + if err == nil { + t.Fatalf("the stop acted at once: %q", said) + } + for _, want := range []string{s.ids["novox/z"], "plan-2", "again", "walk", "not stopped"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("the stop said %q; want it to name %q", err, want) + } + } + if st := s.state(s.ids["novox/y"]); st != Delivering || len(s.ctl.stops)+len(s.ctl.withouts) != 0 { + t.Fatalf("y is %s, the controller was asked %v %v; want nothing done", st, s.ctl.stops, s.ctl.withouts) + } + if _, err := s.h.StopWay(s.ids["novox/y"], "x", "jochen", "sideways"); err == nil { + t.Fatal("a way that is neither again nor walk was taken") + } +} + +// With `again`, y is stopped, the walk is stopped without y's merge, and z goes back to published and follows the +// walk that walks it again. +func TestAPersonsStopAgainWalksTheOthersAgain(t *testing.T) { + s := shared459(t) + said, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(said, "plan-2-again") || !strings.Contains(said, "novox/z@") { + t.Fatalf("the stop said %q; want it to name the walk that walks z again", said) + } + if want := "plan-2 without novox/y@" + s.merges["novox/y"]; !containsString(s.ctl.withouts, want) || + containsString(s.ctl.stops, "plan-2") { + t.Fatalf("the controller was asked %v, ended whole %v; want %q", s.ctl.withouts, s.ctl.stops, want) + } + s.heard("plan-2") + if z := s.delivery(s.ids["novox/z"]); z.State != Published { + t.Fatalf("z is %s once plan-2 was stopped and walked again; want published, waiting for that walk", z.State) + } + s.heard("plan-2-again") + if z := s.delivery(s.ids["novox/z"]); z.Walk == nil || z.Walk.ID != "plan-2-again" || z.State != Delivering { + t.Fatalf("z is %s on %+v; want it delivering on plan-2-again", z.State, z.Walk) + } + if st := s.state(s.ids["novox/y"]); st != Stopped { + t.Fatalf("y is %s", st) + } +} + +// With `walk`, the whole walk is stopped, and z ends stopped, naming y's stop. +func TestAPersonsStopOfTheWalkStopsEveryDelivery(t *testing.T) { + s := shared459(t) + if _, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "walk"); err != nil { + t.Fatal(err) + } + if !containsString(s.ctl.stops, "plan-2") || len(s.ctl.withouts) != 0 { + t.Fatalf("the controller was asked %v %v; want plan-2 ended whole", s.ctl.stops, s.ctl.withouts) + } + s.heard("plan-2") + z := s.delivery(s.ids["novox/z"]) + if z.State != Stopped || !strings.Contains(lastWhy(z), s.ids["novox/y"]) { + t.Fatalf("z is %s (%q); want it stopped, naming y's stop", z.State, lastWhy(z)) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/main.go b/modules/mesh-delivery/cmd/mesh-delivery/main.go index 6fe80a48..92be1bb8 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/main.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/main.go @@ -293,14 +293,18 @@ func tools(h *Holder, l *listening) []stdio.Tool { }}, {Name: seat + "stop", Description: "Stop a delivery that is not final, with why: its walk is ended through the controller, and " + - "in a delivering group every member after it is stopped too, naming it.", + "in a delivering group every member after it is stopped too, naming it. When its walk carries other " + + "deliveries, it first answers what ending the walk would take down, and acts only on a way: `again` " + + "ends the walk and walks the others again without this merge, `walk` stops the whole walk (novox/hq ADR 0299).", Input: map[string]any{"type": "object", "properties": map[string]any{"id": str("the delivery's id"), - "why": str("why, kept with the transition")}, "required": []string{"id", "why"}}, + "why": str("why, kept with the transition"), + "way": str("on a walk shared with other deliveries: again (the others walked again without it) or walk " + + "(the whole walk stopped)")}, "required": []string{"id", "why"}}, Run: func(a map[string]any) (any, error) { if err := need(a, "id", "why"); err != nil { return nil, err } - return h.Stop(strArg(a, "id"), strArg(a, "why"), aPerson) + return h.StopWay(strArg(a, "id"), strArg(a, "why"), aPerson, strArg(a, "way")) }}, {Name: seat + "close", Description: "Healer H2's verb: for a delivery held past its bound, read its walk again and take the " + diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index 5c9617b9..0fb09377 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -383,6 +383,11 @@ type WalkDelivery struct { // TakenOverBy is, on a walk folded before it started, the walk that took it over and answers every merge // it answered. TakenOverBy string `json:"taken_over_by,omitempty"` + // WalkedAgainBy is, on a walk stopped without some of its merges, the batch that walks its other merges again, + // and LeftOut the merges it left out (novox/hq ADR 0299): a delivery of a merge left out ends stopped with it, + // one of a merge walked again follows that batch's walk. + WalkedAgainBy string `json:"walked_again_by,omitempty"` + LeftOut []WalkLeftOut `json:"left_out,omitempty"` // Batch is set while the record is a batch, not yet a walk: its state `assembling` or `queued`. A batch, once // 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. @@ -398,6 +403,27 @@ type WalkMerge struct { Carried string `json:"carried,omitempty"` } +// WalkLeftOut is a merge a stop left out (novox/hq ADR 0299): a merge stopped, or a later merge of its +// repository's branch that contains it, Contains naming the stopped merge. +type WalkLeftOut struct { + Repository string `json:"repository"` + Commit string `json:"commit"` + Contains string `json:"contains,omitempty"` +} + +// leftOut is the merge of a repository a stop left out, when it did. +func (w Walk) leftOut(repository, merge string) (WalkLeftOut, bool) { + if w.Delivery == nil || merge == "" { + return WalkLeftOut{}, false + } + for _, o := range w.Delivery.LeftOut { + if strings.EqualFold(o.Repository, repository) && sameCommit(o.Commit, merge) { + return o, true + } + } + return WalkLeftOut{}, false +} + // WalkBatch is a batch's window: when it closes unless another merge arrives, when it closes at the latest, // and, once closed, the walk it waits behind. type WalkBatch struct { @@ -454,18 +480,24 @@ func (w Walk) carriedFor(repository, merge string) (string, bool) { // WalkSeen is what a delivery keeps of its walk. type WalkSeen struct { - ID string `json:"id"` - Commit string `json:"commit"` - State string `json:"state"` - Note string `json:"note,omitempty"` - Revision int64 `json:"revision"` - Waits bool `json:"waits,omitempty"` - WaitedFor string `json:"waited_for,omitempty"` - GoBy string `json:"go_by,omitempty"` - StoppedBy string `json:"stopped_by,omitempty"` - Tier int `json:"tier"` - Tiers int `json:"tiers"` - Updated time.Time `json:"updated"` + ID string `json:"id"` + Commit string `json:"commit"` + State string `json:"state"` + Note string `json:"note,omitempty"` + Revision int64 `json:"revision"` + Waits bool `json:"waits,omitempty"` + WaitedFor string `json:"waited_for,omitempty"` + GoBy string `json:"go_by,omitempty"` + StoppedBy string `json:"stopped_by,omitempty"` + // StoppedWhy is why it was stopped; WalkedAgainBy, on a walk stopped without some merges, the batch that walks + // this delivery's merge again; Contains, when this delivery's merge was left out for containing a stopped + // merge, that merge (novox/hq ADR 0299). + StoppedWhy string `json:"stopped_why,omitempty"` + WalkedAgainBy string `json:"walked_again_by,omitempty"` + Contains string `json:"contains,omitempty"` + Tier int `json:"tier"` + Tiers int `json:"tiers"` + Updated time.Time `json:"updated"` // Branch is the branch the walk's commit was merged into; empty in a record kept before it was read. Branch string `json:"branch,omitempty"` // Created is when the controller made the walk: a delivery follows the newest walk that answers it. @@ -487,7 +519,7 @@ func (w Walk) Seen() *WalkSeen { s.WaitedFor = d.Awaits s.Waits = d.Awaits != "" && d.Go == nil s.GoBy = d.By - s.StoppedBy = d.Stopped + s.StoppedBy, s.StoppedWhy = d.Stopped, d.StoppedWhy } s.Branch = w.Branch s.Created = w.Created @@ -511,6 +543,28 @@ func (d *Delivery) endedUnstarted() bool { return d.State == Published && d.Walk != nil && d.Walk.State == walkDone && !d.Walk.Started() } +// walkStopped is whether the walk it follows was stopped through this owner, and walkedAgain whether its merge is +// walked again by another walk since (novox/hq ADR 0299). +func (d *Delivery) walkStopped() bool { + return d.Walk != nil && d.Walk.State == walkFailed && d.Walk.StoppedBy != "" +} + +func (d *Delivery) walkedAgain() bool { return d.walkStopped() && d.Walk.WalkedAgainBy != "" } + +// stoppedWords is why a delivery ends with the walk it follows stopped: the stop, and the stopped merge its own +// contains when it was left out for that. +func (d *Delivery) stoppedWords() string { + s := fmt.Sprintf("its walk %s was stopped by %s", d.Walk.ID, d.Walk.StoppedBy) + if d.Walk.StoppedWhy != "" { + s += ": " + d.Walk.StoppedWhy + } + if d.Walk.Contains != "" { + s += fmt.Sprintf("; its merge contains the stopped merge %s, so it cannot be walked without delivering it, "+ + "and was left out with it (novox/hq ADR 0299)", shortOf(d.Walk.Contains, 12)) + } + return s +} + // walkMovedItsModules is whether the walk it follows built every module its plan moves, and whether that is known: // 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. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ports.go b/modules/mesh-delivery/cmd/mesh-delivery/ports.go index a9819fc7..072220ce 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ports.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ports.go @@ -61,6 +61,10 @@ type Controller interface { Deliver(walk, why string) error // Stop ends a walk, said as stopped by whom. Stop(walk, why, by string) error + // StopWithout ends a walk and walks its other merges again without the merges named, each + // `@`, and without each later merge of their repository, which contains one (novox/hq + // ADR 0299). + StopWithout(walk, why, by string, without []string) (StopAnswer, error) // Walks are the walks the controller keeps — one, given its id — and whether this seat has a holder on record. Walks(walk string) (bool, []Walk, error) } @@ -269,6 +273,30 @@ func (c seatController) Stop(walk, why, by string) error { return err } +// StopAnswer is what the controller's `delivery-stop` with merges to leave out answers (novox/hq ADR 0299): the walk +// stopped, the batch that walks its other merges again with those merges, and every merge left out. +type StopAnswer struct { + Stopped string `json:"stopped"` + WalkedAgainBy string `json:"walked_again_by,omitempty"` + WalkedAgain []string `json:"walked_again,omitempty"` + LeftOut []WalkLeftOut `json:"left_out,omitempty"` +} + +func (c seatController) StopWithout(walk, why, by string, without []string) (StopAnswer, error) { + raw, err := c.verb("delivery-stop", map[string]any{"plan": walk, "why": why, "by": by, + "without": strings.Join(without, ",")}) + if err != nil { + return StopAnswer{}, err + } + var a StopAnswer + if json.Unmarshal(raw, &a) != nil || a.Stopped == "" { + // A controller from before ADR 0299 ends the walk whole and says it as text: its record names no walk + // that walks the others again, and every delivery on it ends stopped, as it reads. + return StopAnswer{Stopped: walk}, nil + } + return a, nil +} + func (c seatController) Walks(walk string) (bool, []Walk, error) { args := map[string]any{} if walk != "" { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table.go b/modules/mesh-delivery/cmd/mesh-delivery/table.go index 76005611..707bd563 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -84,7 +84,7 @@ const ( EvRetire Event = "retire" // a person's word that a held delivery is history, never to be walked EvCovered Event = "covered" // a later merge into the same trunk delivered every module it moves EvLifted Event = "lifted" // what held it for a person is no longer so: its group's composed check passed, or a walk now answers its merge - EvRewalk Event = "rewalk" // the walk that carried a later commit for it failed: it waits for a walk of its own commit + EvRewalk Event = "rewalk" // the walk that carried a later commit for it failed, or was stopped for another merge (ADR 0299): it waits for the walk that walks it again ) // Facts are what a guard reads beyond the delivery itself: the moment, and who asks, for an act. @@ -259,6 +259,9 @@ var Table = []Row{ if d.Walk != nil && d.Walk.State == walkFailed && d.carriedLater() { return errors.New("its walk, which carried a later commit for it, failed: it waits for a walk of its own commit") } + if d.walkStopped() { + return errors.New("its walk was stopped: it waits for the walk that walks its merge again, or ends stopped") + } return unless(d.Walk != nil && d.Walk.Started(), "its walk waits") }}, // **A walk that ended before its word settles its delivery** (novox/hq issue 474): a batch's walk the @@ -284,11 +287,21 @@ var Table = []Row{ }}, {From: []State{Published}, Event: EvStop, To: Stopped, Guard: "its walk ended done before its word without building every module it moves, and no later merge that " + - "contains it delivered them: no start can come, and nothing delivered it (novox/hq issue 474)", + "contains it delivered them: no start can come, and nothing delivered it (novox/hq issue 474); or the walk " + + "it waits on was stopped through this owner and walks its merge again nowhere — left out, or the walk " + + "ended whole (novox/hq ADR 0299)", Holds: func(d *Delivery, f Facts) error { + if d.walkStopped() && !d.walkedAgain() { + return nil + } return unless(d.endedUnstarted() && d.Stranded != "", "its walk has not ended unstarted, or something may cover it") }, - Says: func(d *Delivery) string { return d.Stranded }}, + Says: func(d *Delivery) string { + if d.walkStopped() { + return d.stoppedWords() + } + return d.Stranded + }}, {From: []State{Held}, Event: EvGo, To: Delivering, Guard: "its walk started on a word that was not this owner's: the controller's own path, which waits for " + "nobody, or a person's `plans go`", @@ -332,10 +345,15 @@ var Table = []Row{ return "covered: a later merge that contains it was delivered: " + d.Carrier }}, {From: []State{Delivering}, Event: EvStop, To: Stopped, - Guard: "its walk was stopped through this owner", + Guard: "its walk was stopped through this owner, and walks its merge again nowhere: left out, or the walk ended " + + "whole (novox/hq ADR 0299)", Holds: func(d *Delivery, f Facts) error { - return unless(d.Walk != nil && d.Walk.State == walkFailed && d.Walk.StoppedBy != "", "its walk was not stopped") - }}, + if d.walkedAgain() { + return fmt.Errorf("its walk was stopped without it, and %s walks its merge again", d.Walk.WalkedAgainBy) + } + return unless(d.walkStopped(), "its walk was not stopped") + }, + Says: func(d *Delivery) string { return d.stoppedWords() }}, {From: []State{Delivering}, Event: EvFailed, To: Failed, Guard: "its walk failed, carrying its own commit: a gate on a first machine (what it carried put back), a build, a machine", Holds: func(d *Delivery, f Facts) error { @@ -344,12 +362,20 @@ var Table = []Row{ }}, {From: []State{Delivering}, Event: EvRewalk, To: Published, Guard: "its walk failed carrying a later merge of its branch for it: a failed walk marks no merge delivered, and this " + - "merge waits, published, for a walk of its own commit (novox/hq ADR 0276 decision 3)", + "merge waits, published, for a walk of its own commit (novox/hq ADR 0276 decision 3); or its walk was stopped " + + "for another merge, and its merge is walked again without that one (novox/hq ADR 0299)", Holds: func(d *Delivery, f Facts) error { + if d.walkedAgain() { + return nil + } return unless(d.Walk != nil && d.Walk.State == walkFailed && d.Walk.StoppedBy == "" && d.carriedLater(), "its walk did not fail, or carried its own commit") }, Says: func(d *Delivery) string { + if d.walkedAgain() { + return fmt.Sprintf("its walk %s was stopped for another merge (%s): %s walks its merge again without that one", + d.Walk.ID, d.Walk.StoppedWhy, d.Walk.WalkedAgainBy) + } return fmt.Sprintf("its walk %s, which carried %s for it, failed: it waits for a walk of its own commit", d.Walk.ID, shortOf(d.AnsweredBy, 12)) }}, diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index fbee6df1..14fe3209 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -346,24 +346,56 @@ func (h *Holder) Release(id, why, by string) (string, error) { return fmt.Sprintf("%s is delivering on %s's word; its walk %s is let go", id, by, walk), nil } -// Stop is the `stop` verb: a delivery stopped, its walk ended through the controller, and in a group every +// Stop is the `stop` verb with no way given: see StopWay. +func (h *Holder) Stop(id, why, by string) (string, error) { return h.StopWay(id, why, by, "") } + +// The ways a person's stop of a delivery on a shared walk takes (novox/hq ADR 0299). +const ( + wayAgain = "again" // the walk ended, its other merges walked again without this one + wayWalk = "walk" // the whole walk stopped, every delivery on it with it +) + +// StopWay is the `stop` verb: a delivery stopped, its walk ended through the controller, and in a group every // member after it stopped too. -func (h *Holder) Stop(id, why, by string) (string, error) { +// +// **A stop never takes other deliveries down quietly** (novox/hq issue 459, ADR 0299): one walk carries every merge +// of its batch. When the delivery's open walk carries anything else, the stop does nothing at first: it says what +// ending the walk would take down — each other delivery, and which of them contain this merge and end either way — +// and the two ways, given with `way`: `again` ends the walk and walks the others again without this merge and the +// merges that contain it; `walk` stops the whole walk, and every delivery on it ends stopped, naming this stop. +func (h *Holder) StopWay(id, why, by, way string) (string, error) { + if way != "" && way != wayAgain && way != wayWalk { + return "", fmt.Errorf("way %q: %q or %q", way, wayAgain, wayWalk) + } h.mu.Lock() d := h.deliveries[id] if d == nil { h.mu.Unlock() return "", fmt.Errorf("no delivery %q", id) } + var walk string + if d.Walk != nil && (d.Walk.State == walkBuilding || d.Walk.State == walkRolling) { + walk = d.Walk.ID + } + var others []string + if walk != "" && !d.State.Final() { + others = h.othersOn(walk, []string{d.Repository + "@" + d.MergedAs}) + } + if len(others) > 0 && way == "" { + said := h.takenDown(d, walk, others) + h.mu.Unlock() + return "", errors.New(said) + } t, err := Apply(d, EvStop, true, Facts{Now: h.Now(), Why: why, By: by}) if err != nil { h.mu.Unlock() return "", err } h.moved(d, []Transition{t}) - var walk string - if d.Walk != nil && (d.Walk.State == walkBuilding || d.Walk.State == walkRolling) { - walk = d.Walk.ID + if walk != "" { + // Gathered before the group's turns, so a member after it on the same walk adds to this ask, never asks one + // of its own that would walk this merge again. + h.leave(walk, d) } var asks []func() if g := h.groups[d.Group]; g != nil && g.Closed { @@ -378,11 +410,59 @@ func (h *Holder) Stop(id, why, by string) (string, error) { if walk == "" { return fmt.Sprintf("%s is stopped, on %s's word", id, by), nil } - if err := h.Controller.Stop(walk, why, by); err != nil { - return "", fmt.Errorf("%s is stopped, and its walk %s could not be ended yet: %v — `plans stop %s --why …` "+ - "ends it by hand", id, walk, err, walk) + without, shared := h.leaving459(walk) + if !shared || way == wayWalk { + whole := why + if shared { + whole = fmt.Sprintf("%s was stopped, and its whole walk with it: %s", id, why) + } + if err := h.Controller.Stop(walk, whole, by); err != nil { + return "", fmt.Errorf("%s is stopped, and its walk %s could not be ended yet: %v — `plans stop %s --why …` "+ + "ends it by hand", id, walk, err, walk) + } + if shared { + return fmt.Sprintf("%s is stopped, on %s's word; its walk %s is ended whole, and every delivery on it with "+ + "it: %s", id, by, walk, strings.Join(others, ", ")), nil + } + return fmt.Sprintf("%s is stopped, on %s's word; its walk %s is ended", id, by, walk), nil } - return fmt.Sprintf("%s is stopped, on %s's word; its walk %s is ended", id, by, walk), nil + a, err := h.Controller.StopWithout(walk, why, by, without) + if err != nil { + return "", fmt.Errorf("%s is stopped, and its walk %s could not be ended without it yet: %v", id, walk, err) + } + return fmt.Sprintf("%s is stopped, on %s's word; its walk %s is ended without it, and %s", id, by, walk, + againWords(a)), nil +} + +// takenDown is what ending a delivery's shared walk would take down, and the two ways: for a caller holding the +// lock (ADR 0299). +func (h *Holder) takenDown(d *Delivery, walk string, others []string) string { + var ride, contain []string + for _, o := range others { + od := h.deliveries[o] + if od != nil && containsMerge(od, d) { + contain = append(contain, o) + continue + } + ride = append(ride, o) + } + s := fmt.Sprintf("%s is not stopped: its walk %s carries other changes, and ending the walk would take them down: %s.", + d.ID, walk, orNone(strings.Join(ride, ", "))) + if len(contain) > 0 { + s += fmt.Sprintf(" These contain its merge and end with it either way: %s.", strings.Join(contain, ", ")) + } + return s + fmt.Sprintf(" Choose a way: `way: %s` ends the walk and walks the others again without this merge; "+ + "`way: %s` stops the whole walk and every delivery on it.", wayAgain, wayWalk) +} + +// containsMerge is whether o is a later merge of d's repository and branch, which contains d's merge on a linear +// trunk: merged after it, or the merge the walk carries for it. +func containsMerge(o, d *Delivery) bool { + if o.ID == d.ID || !strings.EqualFold(o.Repository, d.Repository) || !strings.EqualFold(o.Base, d.Base) || o.MergedAs == "" { + return false + } + return o.MergedAt.After(d.MergedAt) || d.AnsweredBy != "" && sameCommit(d.AnsweredBy, o.MergedAs) && + !sameCommit(d.AnsweredBy, d.MergedAs) } // Close is healer H2's verb: for a delivery past its bound, the walk's record is read again and the From 8d7f60c9e2e70cc1e4ec6333ab9a0f5669d8d244 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 20:16:37 +0200 Subject: [PATCH 2/3] mesh-delivery: never ship a stopped merge walked again, and stop only once the walk is (review of #221, hq issue 459) A merge walked again into a batch and then stopped by its group stayed in that batch, which was later let go. Any walk or batch that names a stopped delivery's merge is now asked to leave it out and is never let go; a refused leave-out stops the whole walk. A person's stop marks the delivery stopped only after its walk is, and the answer says what stays on the machines and what will still carry the change. --- .../cmd/mesh-delivery/answers.go | 7 + .../cmd/mesh-delivery/fakes_test.go | 102 ++++++++---- .../mesh-delivery/cmd/mesh-delivery/holder.go | 156 ++++++++++++++++-- .../cmd/mesh-delivery/issue459_test.go | 118 +++++++++++++ .../mesh-delivery/cmd/mesh-delivery/ports.go | 17 +- .../mesh-delivery/cmd/mesh-delivery/verbs.go | 88 ++++++---- 6 files changed, 409 insertions(+), 79 deletions(-) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/answers.go b/modules/mesh-delivery/cmd/mesh-delivery/answers.go index 13467d97..2dd8e594 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/answers.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/answers.go @@ -382,6 +382,13 @@ func (h *Holder) words() []func() { continue } ids, why := append([]string(nil), want.ids...), strings.Join(want.whys, "; ") + // **Never the word for a walk that carries a stopped change** (ADR 0299, review of #221): it is asked to + // leave the merge out first (leaveStopped). + if stopped, _ := h.carriesStopped(walk); len(stopped) > 0 { + h.Logf("[mesh-delivery] the walk %s is not let go: it carries %s, whose delivery was stopped", walk, + strings.Join(stopped, ", ")) + continue + } if h.asking[walk] { continue // its word is being asked } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index a491df78..cc42cb60 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -96,8 +96,12 @@ type fakeController struct { stops []string // withouts are the stops that left merges out (novox/hq ADR 0299): " without ,…". withouts []string - plans int - down bool + // refuseWithout refuses every stop with merges to leave out, as a controller before ADR 0299 does; mergedAt are + // the merges' times, by commit, for what a later merge contains. + refuseWithout bool + mergedAt map[string]time.Time + plans int + down bool // onCheck runs as a check is asked, before it is answered: what the forge held at that moment. onCheck func() // planned is what Plan answers instead of "builds app", when set. @@ -105,7 +109,7 @@ type fakeController struct { } func newFakeController() *fakeController { - return &fakeController{walks: map[string]Walk{}, held: true} + return &fakeController{walks: map[string]Walk{}, held: true, mergedAt: map[string]time.Time{}} } func (c *fakeController) Plan(repository, base, head string, paths, dirs, removed []string) (*DeliveryPlan, error) { @@ -179,7 +183,10 @@ func (c *fakeController) Stop(walk, why, by string) error { return errors.New("down") } c.stops = append(c.stops, walk) - if w, known := c.walks[walk]; known { + if w, known := c.walks[walk]; known && !w.Open() { + // As the controller: only a walk that is building or rolling is stopped (`delivery-stop`). + return fmt.Errorf("%s is already %s", walk, w.State) + } else if known { w.State = walkFailed if w.Delivery == nil { w.Delivery = &WalkDelivery{} @@ -192,8 +199,10 @@ func (c *fakeController) Stop(walk, why, by string) error { } // StopWithout ends a walk without the merges named, as the controller does (novox/hq ADR 0299): each merge named, -// and each merge of its repository whose commit the walk carried for it, is left out; the others are walked again -// by a walk of their own, "-again", waiting for its word. +// and each later merge of its repository by merge time (mergedAt), is left out; the others are put in a batch, +// "-again", assembling until a test cuts it. Given a batch, the merges left out go to a stopped record, +// "-left", and the batch goes on with the others. refuseWithout refuses every such stop, as an older +// controller does. func (c *fakeController) StopWithout(walk, why, by string, without []string) (StopAnswer, error) { c.mu.Lock() defer c.mu.Unlock() @@ -201,46 +210,64 @@ func (c *fakeController) StopWithout(walk, why, by string, without []string) (St return StopAnswer{}, errors.New("down") } c.withouts = append(c.withouts, walk+" without "+strings.Join(without, ",")) + if c.refuseWithout { + return StopAnswer{}, errors.New(`delivery-stop takes no argument "without"`) + } w, known := c.walks[walk] - if !known || w.State == walkFailed || w.State == walkDone { - return StopAnswer{}, fmt.Errorf("%s is no open walk", walk) + if !known || !(w.Open() || w.IsBatch()) { + return StopAnswer{}, fmt.Errorf("%s is no open walk or batch", walk) } wd := WalkDelivery{} if w.Delivery != nil { wd = *w.Delivery } named := func(m WalkMerge) bool { return containsString(without, m.Repository+"@"+m.Commit) } - a := StopAnswer{Stopped: walk} - var again []WalkMerge - for _, m := range wd.Merges { - if named(m) { - a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit}) - } - } - for _, m := range wd.Merges { - if named(m) { - continue - } - contains := "" + contains := func(m WalkMerge) string { for _, s := range wd.Merges { - if named(s) && s.Repository == m.Repository && s.Carried == m.Commit { - contains = s.Commit + if named(s) && s.Repository == m.Repository && c.mergedAt[m.Commit].After(c.mergedAt[s.Commit]) { + return s.Commit } } - if contains != "" { - a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: contains}) - continue + return "" + } + a := StopAnswer{Stopped: walk} + var again, left []WalkMerge + for _, m := range wd.Merges { + switch { + case named(m): + a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit}) + left = append(left, m) + case contains(m) != "": + a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: contains(m)}) + left = append(left, m) + default: + m.Carried = "" + again = append(again, m) + a.WalkedAgain = append(a.WalkedAgain, m.Repository+"@"+m.Commit) } - m.Carried = "" - again = append(again, m) - a.WalkedAgain = append(a.WalkedAgain, m.Repository+"@"+m.Commit) + } + if len(a.LeftOut) == 0 { + return StopAnswer{}, fmt.Errorf("%s answers no merge %s", walk, strings.Join(without, ", ")) + } + if w.IsBatch() { + rid := walk + "-left" + c.walks[rid] = Walk{ID: rid, Repository: left[0].Repository, Branch: "main", Commit: left[0].Commit, + Created: w.Created, State: walkFailed, Revision: 1, Modules: map[string]*WalkModule{}, + Delivery: &WalkDelivery{Stopped: by, StoppedWhy: why, Merges: left, LeftOut: a.LeftOut, WalkedAgainBy: walk}} + wd.Merges = again + w.Delivery = &wd + w.Repository, w.Commit = again[0].Repository, again[0].Commit + w.Revision++ + c.walks[walk] = w + a.Stopped, a.WalkedAgainBy = rid, walk + return a, nil } if len(again) > 0 { id := walk + "-again" + closes := w.Created.Add(time.Minute) c.walks[id] = Walk{ID: id, Repository: again[0].Repository, Branch: "main", Commit: again[0].Commit, - Created: w.Created.Add(time.Minute), State: walkBuilding, Tiers: [][]string{{"app"}}, - Modules: map[string]*WalkModule{"app": {}}, Revision: 1, - Delivery: &WalkDelivery{Awaits: "mesh-delivery", Merges: again}} + Created: w.Created.Add(time.Minute), State: walkAssembling, Revision: 1, + Modules: map[string]*WalkModule{}, Delivery: &WalkDelivery{Merges: again, Batch: &WalkBatch{ClosesAt: &closes}}} wd.WalkedAgainBy, a.WalkedAgainBy = id, id } wd.Stopped, wd.StoppedWhy, wd.LeftOut = by, why, a.LeftOut @@ -251,6 +278,19 @@ func (c *fakeController) StopWithout(walk, why, by string, without []string) (St c.walks[walk] = w return a, nil } + +// cut makes a batch the walk under the same id, waiting for its word, as the controller cuts it. +func (c *fakeController) cut(id string) Walk { + c.mu.Lock() + w := c.walks[id] + c.mu.Unlock() + wd := *w.Delivery + wd.Batch, wd.Awaits = nil, "mesh-delivery" + w.Delivery = &wd + w.State, w.Tiers, w.Modules = walkBuilding, [][]string{{"app"}}, map[string]*WalkModule{"app": {}} + return c.put(w) +} + func (c *fakeController) Walks(walk string) (bool, []Walk, error) { c.mu.Lock() defer c.mu.Unlock() diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index d66cdac6..841a5c10 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -59,6 +59,8 @@ type Holder struct { asking map[string]bool // leaving are, by walk, the merges a stop gathered to leave out of it, asked once (novox/hq ADR 0299). leaving map[string][]string + // leaveAsked is when a walk was last asked to leave out the stopped merges it carries (leaveStopped). + leaveAsked map[string]leaveAsk } // unmatched is a walk no delivery is known for yet: its merge not heard, or merged before this owner held. @@ -832,6 +834,9 @@ func (h *Holder) Tick() { for _, ask := range h.plan() { ask() } + for _, ask := range h.leaveStopped() { + ask() + } } // plan works out, under the lock, what to ask; each ask applies its own answer. @@ -1835,10 +1840,19 @@ func (h *Holder) stopInGroup(d *Delivery, why string) func() { return nil } h.moved(d, []Transition{t}) - if d.Walk == nil || d.Walk.State == walkDone || d.Walk.State == walkFailed || d.Walk.State == walkSuperseded { + // Its open walk; or, its walk stopped for another merge, the batch or walk that walks its merge again (review + // of #221): a merge walked again and then stopped by its group is left out there too. + var walk string + switch { + case d.Walk == nil: + return nil + case d.Walk.State == walkBuilding || d.Walk.State == walkRolling: + walk = d.Walk.ID + case d.walkedAgain(): + walk = d.Walk.WalkedAgainBy + default: return nil } - walk := d.Walk.ID if !h.leave(walk, d) { return nil // the first member left out of this walk asks for every one } @@ -1850,15 +1864,114 @@ func (h *Holder) stopInGroup(d *Delivery, why string) func() { } return } - a, err := h.Controller.StopWithout(walk, why, byOwner, without) - if err != nil { - h.Logf("[mesh-delivery] the walk %s could not be stopped without %s: %v", walk, strings.Join(without, ", "), err) - return - } - h.Logf("[mesh-delivery] the walk %s is stopped without %s; %s", walk, strings.Join(without, ", "), againWords(a)) + h.Logf("[mesh-delivery] %s", h.stopLeaving(walk, why, byOwner, without)) } } +// stopLeaving asks the controller to stop a walk or batch without merges (ADR 0299) and says what came of it. A +// refusal — an older controller takes no `without`, or the leave-out could not be made — falls back to stopping the +// whole walk: the stopped change is held back, and every delivery on it ends stopped with the reason (review of +// #221). Outside the lock. +func (h *Holder) stopLeaving(walk, why, by string, without []string) string { + h.mu.Lock() + h.askedToLeave(walk) + h.mu.Unlock() + a, err := h.Controller.StopWithout(walk, why, by, without) + if err == nil { + return fmt.Sprintf("the walk %s is stopped without %s; %s", walk, strings.Join(without, ", "), againWords(a)) + } + refused := err + if err := h.Controller.Stop(walk, fmt.Sprintf("%s, and it could not be left out alone (%v)", why, refused), by); err != nil { + return fmt.Sprintf("the walk %s could not be stopped without %s (%v), nor whole (%v): `plans stop %s --why …` "+ + "ends it by hand", walk, strings.Join(without, ", "), refused, err, walk) + } + return fmt.Sprintf("the walk %s could not be stopped without %s (%v), so it was stopped whole: every delivery on it "+ + "ends stopped", walk, strings.Join(without, ", "), refused) +} + +// carriesStopped are the merges a walk or batch carries whose deliveries were stopped, as `@`, +// and why the first was: a record that names a stopped delivery's merge and has not left it out would deliver a +// stopped change (review of #221). Under the lock. +func (h *Holder) carriesStopped(walk string) ([]string, string) { + s, known := h.walks[walk] + if !known || !(s.walk.Open() || s.walk.IsBatch()) || s.walk.Delivery == nil || len(s.walk.Delivery.Merges) == 0 { + return nil, "" + } + var out []string + why := "" + ids := make([]string, 0, len(h.deliveries)) + for id := range h.deliveries { + ids = append(ids, id) + } + slices.Sort(ids) + for _, id := range ids { + d := h.deliveries[id] + if d.State != Stopped || d.MergedAs == "" { + continue + } + // By the merges it names alone: its own commit may be another's merge it no longer answers. + if !slices.ContainsFunc(s.walk.Delivery.Merges, func(m WalkMerge) bool { + return strings.EqualFold(m.Repository, d.Repository) && sameCommit(m.Commit, d.MergedAs) + }) { + continue + } + if _, out := s.walk.leftOut(d.Repository, d.MergedAs); out { + continue + } + out = append(out, d.Repository+"@"+d.MergedAs) + if why == "" { + why = fmt.Sprintf("%s was stopped: %s", d.ID, lastWhyOf(d)) + } + } + return out, why +} + +// lastWhyOf is the reason of a delivery's last transition. +func lastWhyOf(d *Delivery) string { + if len(d.Transitions) == 0 { + return "" + } + return d.Transitions[len(d.Transitions)-1].Why +} + +// leaveAsk is when a walk was last asked to leave merges out, and at which revision of its record. +type leaveAsk struct { + at time.Time + revision int64 +} + +// askedToLeave notes an ask to leave merges out of a walk, at its record's revision: under the lock. +func (h *Holder) askedToLeave(walk string) { + if h.leaveAsked == nil { + h.leaveAsked = map[string]leaveAsk{} + } + h.leaveAsked[walk] = leaveAsk{at: h.Now(), revision: h.walks[walk].walk.Revision} +} + +// leaveStopped asks, for every open walk or batch that carries a stopped delivery's merge, that the merge be left +// out (ADR 0299, review of #221): a merge walked again and then stopped, or stopped while its batch assembled, never +// ships. Asked once per record of it while it is carried; such a walk is never given its word (words). +func (h *Holder) leaveStopped() []func() { + h.mu.Lock() + defer h.mu.Unlock() + now := h.Now() + var asks []func() + for id, seen := range h.walks { + without, why := h.carriesStopped(id) + if len(without) == 0 { + continue + } + // Once per record of it: the record that answers an ask is newer. Asked again after five minutes, should + // the ask have gone nowhere. + if a, asked := h.leaveAsked[id]; asked && seen.walk.Revision <= a.revision && now.Sub(a.at) < 5*time.Minute { + continue + } + walk := id + asks = append(asks, func() { h.Logf("[mesh-delivery] %s", h.stopLeaving(walk, why, byOwner, without)) }) + } + return asks +} + // leave gathers a delivery's merge to leave out of a walk (ADR 0299), for a caller holding the lock; true for the // first of that walk, whose ask asks for all. func (h *Holder) leave(walk string, d *Delivery) bool { @@ -1913,7 +2026,9 @@ func (h *Holder) othersOn(walk string, without []string) []string { return keys } -// againWords is what a stop that left merges out did, in a person's words. +// againWords is what a stop that left merges out did, in a person's words: the walk that walks the others again, +// what it left out (the controller's word on what contains a stopped merge), what stays on the machines, and what +// will still deliver a stopped change (ADR 0299). func againWords(a StopAnswer) string { var left []string for _, o := range a.LeftOut { @@ -1923,14 +2038,31 @@ func againWords(a StopAnswer) string { } left = append(left, s) } + var out string switch { case a.WalkedAgainBy != "": - return fmt.Sprintf("%s walks its other merges again: %s; left out: %s", a.WalkedAgainBy, + out = fmt.Sprintf("%s walks its other merges again: %s; left out: %s", a.WalkedAgainBy, strings.Join(a.WalkedAgain, ", "), orNone(strings.Join(left, ", "))) case len(left) > 0: - return "left out: " + strings.Join(left, ", ") + "; nothing else to walk" + out = "left out: " + strings.Join(left, ", ") + "; nothing else to walk" + default: + out = "nothing else to walk" } - return "the controller ended it whole: it named no walk that walks its other merges again" + if len(a.Withheld) > 0 { + out += "; its walk withholds " + strings.Join(a.Withheld, ", ") + } + if len(a.Kept) > 0 { + var kept []string + for _, k := range a.Kept { + kept = append(kept, k.Module+" on "+strings.Join(k.Machines, ", ")) + } + out += "; what the stopped walk already sent stays on the machines: " + strings.Join(kept, "; ") + } + if len(a.StillCarriedBy) > 0 { + out += "; still to deliver the stopped change when walked, a revert's to hold back: " + + strings.Join(a.StillCarriedBy, ", ") + } + return out } // GroupState is a group's state, derived from its members and its composed check — never set apart (novox/hq diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go index 98dc5e62..e70b3671 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go @@ -77,6 +77,12 @@ func (s *shape459) heard(walks ...string) { s.settleAll() } +// cutAgain is the controller cutting the batch that walks a stopped walk's merges again, as plan-moved says it. +func (s *shape459) cutAgain(id string) { + s.h.WalkMoved(s.ctl.cut(id)) + s.settleAll() +} + func (s *shape459) done(walk string) { r := s.ctl.walk(walk) sent := s.now @@ -113,6 +119,10 @@ func TestAGroupStopWalksTheOtherDeliveriesAgainWithoutIt(t *testing.T) { t.Fatalf("the controller was asked %v; want %q", s.ctl.withouts, want) } s.heard("plan-2", "plan-2-again") + if containsString(s.ctl.delivers, "plan-2-again") { + t.Fatal("a batch not yet cut was given its word") + } + s.cutAgain("plan-2-again") z := s.delivery(s.ids["novox/z"]) if z.State.Final() || z.Walk == nil || z.Walk.ID != "plan-2-again" || !containsString(s.ctl.delivers, "plan-2-again") { t.Fatalf("z is %s on %+v, let go %v; want it following plan-2-again, let go", z.State, z.Walk, s.ctl.delivers) @@ -152,6 +162,7 @@ func TestAGroupStopEndsAMergeThatContainsTheStoppedOne(t *testing.T) { s.retarget("novox/b2", "novox/b") s.failFirst() s.heard("plan-2", "plan-2-again") + s.cutAgain("plan-2-again") b2 := s.delivery(s.ids["novox/b2"]) if b2.State != Stopped || !strings.Contains(lastWhy(b2), shortOf(s.merges["novox/b"], 8)) || !strings.Contains(lastWhy(b2), "contains") { @@ -170,6 +181,9 @@ func (s *shape459) retarget(later, of string) { d := s.h.deliveries[s.ids[later]] d.Repository, d.MergedAt = of, d.MergedAt.Add(time.Minute) s.h.mu.Unlock() + s.ctl.mu.Lock() + s.ctl.mergedAt[s.merges[of]], s.ctl.mergedAt[s.merges[later]] = d.MergedAt.Add(-time.Minute), d.MergedAt + s.ctl.mu.Unlock() two := s.ctl.walk("plan-2") merges := []WalkMerge{} for _, m := range two.Delivery.Merges { @@ -240,6 +254,7 @@ func TestAPersonsStopAgainWalksTheOthersAgain(t *testing.T) { t.Fatalf("z is %s once plan-2 was stopped and walked again; want published, waiting for that walk", z.State) } s.heard("plan-2-again") + s.cutAgain("plan-2-again") if z := s.delivery(s.ids["novox/z"]); z.Walk == nil || z.Walk.ID != "plan-2-again" || z.State != Delivering { t.Fatalf("z is %s on %+v; want it delivering on plan-2-again", z.State, z.Walk) } @@ -263,3 +278,106 @@ func TestAPersonsStopOfTheWalkStopsEveryDelivery(t *testing.T) { t.Fatalf("z is %s (%q); want it stopped, naming y's stop", z.State, lastWhy(z)) } } + +// The reviewer's case (review of #221): group [a, b]; y and z ride plan-2 with b. y is stopped `way: again`, and the +// batch that walks plan-2's others again carries b and z. Then a fails: b is stopped by its group, and its merge is +// left out of that batch — never shipped by it — and z alone is walked and delivered. +func TestAMergeWalkedAgainAndThenStoppedByItsGroupNeverShips(t *testing.T) { + s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/y", "novox/z"}) + if _, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again"); err != nil { + t.Fatal(err) + } + s.heard("plan-2", "plan-2-again") + s.failFirst() + if b := s.state(s.ids["novox/b"]); b != Stopped { + t.Fatalf("b is %s", b) + } + if want := "plan-2-again without novox/b@" + s.merges["novox/b"]; !containsString(s.ctl.withouts, want) { + t.Fatalf("the controller was asked %v; want %q", s.ctl.withouts, want) + } + s.heard("plan-2-again", "plan-2-again-left") + s.cutAgain("plan-2-again") + again := s.ctl.walk("plan-2-again") + for _, m := range again.Delivery.Merges { + if m.Repository == "novox/b" { + t.Fatalf("the walk %s still carries b's stopped merge: %+v", again.ID, again.Delivery.Merges) + } + } + if !containsString(s.ctl.delivers, "plan-2-again") { + t.Fatalf("z's walk was not let go: %v", s.ctl.delivers) + } + s.done("plan-2-again") + if z := s.state(s.ids["novox/z"]); z != Delivered { + t.Fatalf("z is %s", z) + } +} + +// Whatever brought it there, a walk that carries a stopped delivery's merge is asked to leave it out, and is never +// given its word while it carries it. +func TestAWalkCarryingAStoppedMergeIsAskedToLeaveItOutAndNeverLetGo(t *testing.T) { + s := shared459(t) + if _, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again"); err != nil { + t.Fatal(err) + } + // A walk the controller made that carries y's merge anyway, waiting for its word, with z on it. + x := aWalk("plan-x", "novox/z", s.merges["novox/z"], true) + x.Created = x.Created.Add(10 * time.Minute) + x.Delivery.Merges = []WalkMerge{{Repository: "novox/y", Commit: s.merges["novox/y"]}, + {Repository: "novox/z", Commit: s.merges["novox/z"]}} + s.ctl.refuseWithout = true // it cannot be left out: the walk is stopped whole + s.h.WalkMoved(s.ctl.put(x)) + s.settleAll() + if want := "plan-x without novox/y@" + s.merges["novox/y"]; !containsString(s.ctl.withouts, want) { + t.Fatalf("the controller was asked %v; want %q", s.ctl.withouts, want) + } + if containsString(s.ctl.delivers, "plan-x") { + t.Fatalf("a walk carrying a stopped merge was let go: %v", s.ctl.delivers) + } + if !containsString(s.ctl.stops, "plan-x") { + t.Fatalf("refused the leave-out, the walk was not stopped whole: %v", s.ctl.stops) + } +} + +// A group stop the controller refuses to make without the merge (an older controller) stops the whole walk: the +// stopped change is held back, and the delivery riding with it ends stopped, saying why. +func TestARefusedLeaveOutStopsTheWholeWalk(t *testing.T) { + s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z"}) + s.ctl.refuseWithout = true + s.failFirst() + if len(s.ctl.withouts) != 1 || !containsString(s.ctl.stops, "plan-2") { + t.Fatalf("asked %v, ended whole %v; want the leave-out tried, then plan-2 ended whole", s.ctl.withouts, s.ctl.stops) + } + s.heard("plan-2") + if z := s.delivery(s.ids["novox/z"]); z.State != Stopped || !strings.Contains(lastWhy(z), "could not be left out") { + t.Fatalf("z is %s (%q); want it stopped, saying the leave-out was refused", z.State, lastWhy(z)) + } +} + +// A person's stop the controller cannot make leaves the delivery as it was, and says how to go on. +func TestAPersonsStopThatCannotBeMadeStopsNothing(t *testing.T) { + s := shared459(t) + s.ctl.refuseWithout = true + _, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again") + if err == nil || !strings.Contains(err.Error(), "is not stopped") || !strings.Contains(err.Error(), "plans stop") { + t.Fatalf("the refused stop said %v", err) + } + if st := s.state(s.ids["novox/y"]); st != Delivering { + t.Fatalf("y is %s; want it delivering, its stop not made", st) + } +} + +// What a stop that left merges out says: the walk that walks the others, what it left out and what each contains, +// as the controller says it, what stays on which machines, and what will still deliver the stopped change. +func TestAStopSaysWhatStaysAndWhatStillCarriesIt(t *testing.T) { + said := againWords(StopAnswer{Stopped: "plan-2", WalkedAgainBy: "plan-3", WalkedAgain: []string{"novox/z@zz"}, + LeftOut: []WalkLeftOut{{Repository: "novox/b", Commit: "b1b1b1b1b1b1"}, + {Repository: "novox/b", Commit: "b2b2b2b2b2b2", Contains: "b1b1b1b1b1b1"}}, + Kept: []WalkKept{{Module: "lab", Machines: []string{"anchor"}}}, + StillCarriedBy: []string{"novox/b@b3b3 (in plan-4)"}}) + for _, want := range []string{"plan-3", "novox/z@zz", "b2b2b2b2b2b2 (contains b1b1b1b1b1b1)", "lab on anchor", + "stays on the machines", "novox/b@b3b3 (in plan-4)", "revert"} { + if !strings.Contains(said, want) { + t.Fatalf("the stop said %q; want %q in it", said, want) + } + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ports.go b/modules/mesh-delivery/cmd/mesh-delivery/ports.go index 072220ce..8675075e 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ports.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ports.go @@ -280,6 +280,18 @@ type StopAnswer struct { WalkedAgainBy string `json:"walked_again_by,omitempty"` WalkedAgain []string `json:"walked_again,omitempty"` LeftOut []WalkLeftOut `json:"left_out,omitempty"` + // Kept are the modules the stopped walk sent that nothing walks again, on the machines named; Withheld the + // modules the walk that walks the others withholds; StillCarriedBy the merges elsewhere that will deliver a + // stopped change when walked — a later merge of its repository, or one moving its modules (ADR 0299). + Kept []WalkKept `json:"kept,omitempty"` + Withheld []string `json:"withheld,omitempty"` + StillCarriedBy []string `json:"still_carried_by,omitempty"` +} + +// WalkKept is a module a stopped walk left running its build, and where. +type WalkKept struct { + Module string `json:"module"` + Machines []string `json:"machines"` } func (c seatController) StopWithout(walk, why, by string, without []string) (StopAnswer, error) { @@ -290,9 +302,8 @@ func (c seatController) StopWithout(walk, why, by string, without []string) (Sto } var a StopAnswer if json.Unmarshal(raw, &a) != nil || a.Stopped == "" { - // A controller from before ADR 0299 ends the walk whole and says it as text: its record names no walk - // that walks the others again, and every delivery on it ends stopped, as it reads. - return StopAnswer{Stopped: walk}, nil + return StopAnswer{}, fmt.Errorf("the controller's delivery-stop did not say what it left out: %s", + lastLine(string(raw))) } return a, nil } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index 14fe3209..cb389a14 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -326,10 +326,19 @@ func (h *Holder) Release(id, why, by string) (string, error) { if d.Walk != nil && d.Walk.Waits { walk = d.Walk.ID } + var stopped []string + if walk != "" { + stopped, _ = h.carriesStopped(walk) + } h.mu.Unlock() if walk == "" { return fmt.Sprintf("%s is delivering, on %s's word", id, by), nil } + if len(stopped) > 0 { + // Never the word for a walk that carries a stopped change (ADR 0299): asked to leave it out first. + return "", fmt.Errorf("%s is delivering on %s's word, and its walk %s is not let go yet: it carries %s, whose "+ + "delivery was stopped, and is asked to leave it out first", id, by, walk, strings.Join(stopped, ", ")) + } if err := h.Controller.Deliver(walk, "released by "+by+": "+why); err != nil { return "", fmt.Errorf("%s is delivering on %s's word, and its walk %s could not be let go yet (%v): the next "+ "tick asks again", id, by, walk, err) @@ -373,30 +382,66 @@ func (h *Holder) StopWay(id, why, by, way string) (string, error) { h.mu.Unlock() return "", fmt.Errorf("no delivery %q", id) } + // Its guard is read on a copy: the delivery is stopped only once its walk is (review of #221). + trial := *d + if _, err := Apply(&trial, EvStop, true, Facts{Now: h.Now(), Why: why, By: by}); err != nil { + h.mu.Unlock() + return "", err + } var walk string if d.Walk != nil && (d.Walk.State == walkBuilding || d.Walk.State == walkRolling) { walk = d.Walk.ID } + merge := d.Repository + "@" + d.MergedAs var others []string - if walk != "" && !d.State.Final() { - others = h.othersOn(walk, []string{d.Repository + "@" + d.MergedAs}) + if walk != "" { + others = h.othersOn(walk, []string{merge}) } if len(others) > 0 && way == "" { said := h.takenDown(d, walk, others) h.mu.Unlock() return "", errors.New(said) } + h.mu.Unlock() + + var did string + switch { + case walk == "": + case len(others) == 0 || way == wayWalk: + whole := why + if len(others) > 0 { + whole = fmt.Sprintf("%s was stopped, and its whole walk with it: %s", id, why) + } + if err := h.Controller.Stop(walk, whole, by); err != nil { + return "", fmt.Errorf("%s is not stopped: its walk %s could not be ended yet: %v — `plans stop %s --why …` "+ + "ends it by hand", id, walk, err, walk) + } + did = fmt.Sprintf("; its walk %s is ended", walk) + if len(others) > 0 { + did = fmt.Sprintf("; its walk %s is ended whole, and every delivery on it with it: %s", walk, + strings.Join(others, ", ")) + } + default: + a, err := h.Controller.StopWithout(walk, why, by, []string{merge}) + if err != nil { + return "", fmt.Errorf("%s is not stopped: its walk %s could not be ended without it: %v — `way: %s` stops "+ + "the whole walk, and `plans stop %s --why …` ends it by hand", id, walk, err, wayWalk, walk) + } + did = fmt.Sprintf("; its walk %s is ended without it, and %s", walk, againWords(a)) + } + + h.mu.Lock() + d = h.deliveries[id] + if d == nil || d.State.Final() { + h.mu.Unlock() + return fmt.Sprintf("%s ended meanwhile%s", id, did), nil + } t, err := Apply(d, EvStop, true, Facts{Now: h.Now(), Why: why, By: by}) if err != nil { h.mu.Unlock() return "", err } h.moved(d, []Transition{t}) - if walk != "" { - // Gathered before the group's turns, so a member after it on the same walk adds to this ask, never asks one - // of its own that would walk this merge again. - h.leave(walk, d) - } var asks []func() if g := h.groups[d.Group]; g != nil && g.Closed { asks = h.groupTurns(g, h.membersOf(g)) @@ -407,31 +452,7 @@ func (h *Holder) StopWay(id, why, by, way string) (string, error) { for _, a := range asks { a() } - if walk == "" { - return fmt.Sprintf("%s is stopped, on %s's word", id, by), nil - } - without, shared := h.leaving459(walk) - if !shared || way == wayWalk { - whole := why - if shared { - whole = fmt.Sprintf("%s was stopped, and its whole walk with it: %s", id, why) - } - if err := h.Controller.Stop(walk, whole, by); err != nil { - return "", fmt.Errorf("%s is stopped, and its walk %s could not be ended yet: %v — `plans stop %s --why …` "+ - "ends it by hand", id, walk, err, walk) - } - if shared { - return fmt.Sprintf("%s is stopped, on %s's word; its walk %s is ended whole, and every delivery on it with "+ - "it: %s", id, by, walk, strings.Join(others, ", ")), nil - } - return fmt.Sprintf("%s is stopped, on %s's word; its walk %s is ended", id, by, walk), nil - } - a, err := h.Controller.StopWithout(walk, why, by, without) - if err != nil { - return "", fmt.Errorf("%s is stopped, and its walk %s could not be ended without it yet: %v", id, walk, err) - } - return fmt.Sprintf("%s is stopped, on %s's word; its walk %s is ended without it, and %s", id, by, walk, - againWords(a)), nil + return fmt.Sprintf("%s is stopped, on %s's word%s", id, by, did), nil } // takenDown is what ending a delivery's shared walk would take down, and the two ways: for a caller holding the @@ -451,7 +472,8 @@ func (h *Holder) takenDown(d *Delivery, walk string, others []string) string { if len(contain) > 0 { s += fmt.Sprintf(" These contain its merge and end with it either way: %s.", strings.Join(contain, ", ")) } - return s + fmt.Sprintf(" Choose a way: `way: %s` ends the walk and walks the others again without this merge; "+ + return s + fmt.Sprintf(" What the walk already sent stays where it was sent; the stop's answer names it."+ + " Choose a way: `way: %s` ends the walk and walks the others again without this merge; "+ "`way: %s` stops the whole walk and every delivery on it.", wayAgain, wayWalk) } From 3d7f9a3e669f42d9883d7c72c93d60b9a18e13bf Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 20:27:55 +0200 Subject: [PATCH 3/3] mesh-delivery: say a shared-module leave-out truly, hold back only what was stopped by an act, and say a held walk where it is read (review of #221) A merge left out for sharing a module read as containing the stopped merge; the safety net also held back merges stranded by their walk, which nobody stopped; a held walk said why only in the log; and a stop of a merge in a batch did not ask the batch. --- .../cmd/mesh-delivery/fakes_test.go | 42 ++++++++- .../mesh-delivery/cmd/mesh-delivery/holder.go | 46 +++++++++- .../cmd/mesh-delivery/issue459_test.go | 92 +++++++++++++++++++ .../mesh-delivery/cmd/mesh-delivery/model.go | 30 ++++-- .../mesh-delivery/cmd/mesh-delivery/table.go | 6 ++ .../mesh-delivery/cmd/mesh-delivery/verbs.go | 42 +++++++++ 6 files changed, 244 insertions(+), 14 deletions(-) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index cc42cb60..4c1b703d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -100,8 +100,12 @@ type fakeController struct { // the merges' times, by commit, for what a later merge contains. refuseWithout bool mergedAt map[string]time.Time - plans int - down bool + // refuseStop refuses every whole stop too; moves are the modules each merge moves, by commit, for what a left-out + // merge shares (ADR 0299). + refuseStop bool + moves map[string][]string + plans int + down bool // onCheck runs as a check is asked, before it is answered: what the forge held at that moment. onCheck func() // planned is what Plan answers instead of "builds app", when set. @@ -109,7 +113,8 @@ type fakeController struct { } func newFakeController() *fakeController { - return &fakeController{walks: map[string]Walk{}, held: true, mergedAt: map[string]time.Time{}} + return &fakeController{walks: map[string]Walk{}, held: true, mergedAt: map[string]time.Time{}, + moves: map[string][]string{}} } func (c *fakeController) Plan(repository, base, head string, paths, dirs, removed []string) (*DeliveryPlan, error) { @@ -183,6 +188,9 @@ func (c *fakeController) Stop(walk, why, by string) error { return errors.New("down") } c.stops = append(c.stops, walk) + if c.refuseStop { + return errors.New("the controller refuses") + } if w, known := c.walks[walk]; known && !w.Open() { // As the controller: only a walk that is building or rolling is stopped (`delivery-stop`). return fmt.Errorf("%s is already %s", walk, w.State) @@ -230,13 +238,33 @@ func (c *fakeController) StopWithout(walk, why, by string, without []string) (St } return "" } + shares := func(m WalkMerge) (string, []string) { + for _, s := range wd.Merges { + if named(s) { + var both []string + for _, x := range c.moves[m.Commit] { + if containsString(c.moves[s.Commit], x) { + both = append(both, x) + } + } + if len(both) > 0 { + return s.Commit, both + } + } + } + return "", nil + } a := StopAnswer{Stopped: walk} var again, left []WalkMerge for _, m := range wd.Merges { + s, both := shares(m) switch { case named(m): a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit}) left = append(left, m) + case len(both) > 0: + a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: s, Shares: both}) + left = append(left, m) case contains(m) != "": a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: contains(m)}) left = append(left, m) @@ -249,6 +277,14 @@ func (c *fakeController) StopWithout(walk, why, by string, without []string) (St if len(a.LeftOut) == 0 { return StopAnswer{}, fmt.Errorf("%s answers no merge %s", walk, strings.Join(without, ", ")) } + if w.IsBatch() && len(again) == 0 { + // Every merge left out: the batch itself is stopped, as the controller does. + wd.Stopped, wd.StoppedWhy, wd.LeftOut, wd.Batch = by, why, a.LeftOut, nil + w.Delivery, w.State = &wd, walkFailed + w.Revision++ + c.walks[walk] = w + return a, nil + } if w.IsBatch() { rid := walk + "-left" c.walks[rid] = Walk{ID: rid, Repository: left[0].Repository, Branch: "main", Commit: left[0].Commit, diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 841a5c10..23d1185d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -61,6 +61,9 @@ type Holder struct { leaving map[string][]string // leaveAsked is when a walk was last asked to leave out the stopped merges it carries (leaveStopped). leaveAsked map[string]leaveAsk + // leaveRefused is, by walk, what the controller answered when it refused both to leave a stopped merge out and + // to stop the walk whole. + leaveRefused map[string]string } // unmatched is a walk no delivery is known for yet: its merge not heard, or merged before this owner held. @@ -650,7 +653,7 @@ func (h *Holder) linkWalk(d *Delivery, w Walk, carried string) { // containing a stopped one, or is walked again by the batch the record names. if w.Delivery != nil && (w.Delivery.WalkedAgainBy != "" || len(w.Delivery.LeftOut) > 0) { if o, out := w.leftOut(d.Repository, d.MergedAs); out { - d.Walk.Contains = o.Contains + d.Walk.Contains, d.Walk.Shares = o.Contains, o.Shares } else { d.Walk.WalkedAgainBy = w.Delivery.WalkedAgainBy } @@ -1882,6 +1885,12 @@ func (h *Holder) stopLeaving(walk, why, by string, without []string) string { } refused := err if err := h.Controller.Stop(walk, fmt.Sprintf("%s, and it could not be left out alone (%v)", why, refused), by); err != nil { + h.mu.Lock() + if h.leaveRefused == nil { + h.leaveRefused = map[string]string{} + } + h.leaveRefused[walk] = fmt.Sprintf("leaving it out (%v), and stopping the walk whole (%v)", refused, err) + h.mu.Unlock() return fmt.Sprintf("the walk %s could not be stopped without %s (%v), nor whole (%v): `plans stop %s --why …` "+ "ends it by hand", walk, strings.Join(without, ", "), refused, err, walk) } @@ -1906,7 +1915,12 @@ func (h *Holder) carriesStopped(walk string) ([]string, string) { slices.Sort(ids) for _, id := range ids { d := h.deliveries[id] - if d.State != Stopped || d.MergedAs == "" { + // Stopped by a person or by its group, the acts that hold a change back: never one ended by what its walk + // did (the observed rows, which name nobody), such as a merge stranded after its walk (issue 474). + if d.State != Stopped || d.MergedAs == "" || len(d.Transitions) == 0 { + continue + } + if last := d.Transitions[len(d.Transitions)-1]; last.Event != EvStop || last.By == "" { continue } // By the merges it names alone: its own commit may be another's merge it no longer answers. @@ -1956,9 +1970,32 @@ func (h *Holder) leaveStopped() []func() { defer h.mu.Unlock() now := h.Now() var asks []func() + held := map[string]string{} + for id := range h.walks { + if without, _ := h.carriesStopped(id); len(without) > 0 { + said := fmt.Sprintf("its walk %s is not let go: it carries %s, whose delivery was stopped, and is asked to "+ + "leave it out (novox/hq ADR 0299)", id, strings.Join(without, ", ")) + if r := h.leaveRefused[id]; r != "" { + said += "; the controller refused: " + r + } + held[id] = said + } + } + // What the waiting deliveries say: `waits`, and `stalled` past their bound. + for _, d := range h.deliveries { + said := "" + if d.Walk != nil && !d.State.Final() { + said = held[d.Walk.ID] + } + if d.HeldBack != said { + d.HeldBack = said + h.keep(d) + } + } for id, seen := range h.walks { without, why := h.carriesStopped(id) if len(without) == 0 { + delete(h.leaveRefused, id) continue } // Once per record of it: the record that answers an ask is newer. Asked again after five minutes, should @@ -2033,7 +2070,10 @@ func againWords(a StopAnswer) string { var left []string for _, o := range a.LeftOut { s := o.Repository + "@" + shortOf(o.Commit, 12) - if o.Contains != "" { + switch { + case len(o.Shares) > 0: + s += " (moves " + strings.Join(o.Shares, ", ") + ", as " + shortOf(o.Contains, 12) + " does)" + case o.Contains != "": s += " (contains " + shortOf(o.Contains, 12) + ")" } left = append(left, s) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go index e70b3671..20cdfe89 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go @@ -381,3 +381,95 @@ func TestAStopSaysWhatStaysAndWhatStillCarriesIt(t *testing.T) { } } } + +// A merge left out for moving a module the stopped merge moves, from another repository, ends stopped saying so — +// never that it contains the stopped merge (review of #221). +func TestALeftOutMergeSharingAModuleSaysSoTruly(t *testing.T) { + s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z"}) + s.ctl.mu.Lock() + s.ctl.moves[s.merges["novox/b"]], s.ctl.moves[s.merges["novox/z"]] = []string{"lab"}, []string{"lab"} + s.ctl.mu.Unlock() + s.failFirst() + s.heard("plan-2") + z := s.delivery(s.ids["novox/z"]) + why := lastWhy(z) + if z.State != Stopped || !strings.Contains(why, "moves lab, as the stopped merge "+shortOf(s.merges["novox/b"], 12)) || + strings.Contains(why, "contains the stopped merge") { + t.Fatalf("z is %s (%q); want it stopped, saying it moves lab as b's merge does", z.State, why) + } +} + +// A person's first answer names the deliveries that would be left out for moving a module this merge moves, and says +// the controller may leave out more. +func TestAPersonsFirstAnswerNamesWhatSharesItsModules(t *testing.T) { + s := shared459(t) + _, err := s.h.StopWay(s.ids["novox/y"], "x", "jochen", "") + if err == nil || !strings.Contains(err.Error(), "for moving a module this merge moves: "+s.ids["novox/z"]+" (moves app)") || + !strings.Contains(err.Error(), "may leave out more") { + t.Fatalf("the first answer said %v", err) + } +} + +// The safety net holds back only what a person or a group stopped: a delivery ended by what its walk did (the +// observed rows, which name nobody) is not asked out of a walk that carries its merge. +func TestTheSafetyNetLeavesAStrandedStopAlone(t *testing.T) { + s := shared459(t) + s.h.mu.Lock() + y := s.h.deliveries[s.ids["novox/y"]] + y.State = Stopped + y.Transitions = append(y.Transitions, Transition{At: s.now, From: Published, To: Stopped, Event: EvStop, + Why: "its walk ended done before its word"}) + s.h.mu.Unlock() + s.settleAll() + if len(s.ctl.withouts) != 0 { + t.Fatalf("a stop nobody asked for was held back: %v", s.ctl.withouts) + } +} + +// A walk the safety net holds — its leave-out and its whole stop both refused — says why on the deliveries waiting on +// it, in `waits`, and in `stalled` past their bound. +func TestAHeldWalkIsSaidOnItsDeliveriesAndStalls(t *testing.T) { + s := shared459(t) + if _, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again"); err != nil { + t.Fatal(err) + } + s.ctl.refuseWithout, s.ctl.refuseStop = true, true + x := aWalk("plan-x", "novox/z", s.merges["novox/z"], true) + x.Created = x.Created.Add(10 * time.Minute) + x.Delivery.Merges = []WalkMerge{{Repository: "novox/y", Commit: s.merges["novox/y"]}, + {Repository: "novox/z", Commit: s.merges["novox/z"]}} + s.h.WalkMoved(s.ctl.put(x)) + s.settleAll() + z := s.delivery(s.ids["novox/z"]) + if !strings.Contains(z.HeldBack, "plan-x") || !strings.Contains(z.HeldBack, "refused") { + t.Fatalf("z says it is held back by %q; want plan-x and the refusal", z.HeldBack) + } + if w := s.h.waitsFor(z); w != z.HeldBack { + t.Fatalf("z waits for %q", w) + } + s.later(Bounds[Published].For + time.Minute) + s.settleAll() + for _, l := range s.h.Stalled() { + if l.ID == z.ID && strings.Contains(l.Says, "plan-x") { + return + } + } + t.Fatalf("z is not listed stalled naming plan-x: %+v", s.h.Stalled()) +} + +// A person's stop of a delivery whose merge waits in a batch asks that batch now to leave it out, and says so. +func TestAPersonsStopOfAMergeInABatchAsksTheBatch(t *testing.T) { + s := shared459(t) + if _, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again"); err != nil { + t.Fatal(err) + } + s.heard("plan-2", "plan-2-again") + said, err := s.h.StopWay(s.ids["novox/z"], "this one too", "jochen", "") + if err != nil { + t.Fatal(err) + } + if want := "plan-2-again without novox/z@" + s.merges["novox/z"]; !containsString(s.ctl.withouts, want) || + !strings.Contains(said, "batch plan-2-again") { + t.Fatalf("said %q, asked %v; want the batch asked now, and said", said, s.ctl.withouts) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index 0fb09377..74315c8b 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -71,7 +71,10 @@ type Delivery struct { // TurnAwaits says, for a group member whose turn waits for a later merge to carry a superseded member before it, // which merges and until when; TurnAwaitsUntil is that moment, which bounds the wait (ADR 0276 decision 9, // novox/hq issue 445). `show` shows both, and `stalled` says them past that moment. - TurnAwaits string `json:"turn_awaits,omitempty"` + TurnAwaits string `json:"turn_awaits,omitempty"` + // HeldBack is why the walk it waits on is not let go: it carries a merge whose delivery was stopped, and is + // asked to leave it out (novox/hq ADR 0299). Said by `waits`, and by `stalled` past its bound. + HeldBack string `json:"held_back,omitempty"` TurnAwaitsUntil *time.Time `json:"turn_awaits_until,omitempty"` // ContainedIn is the forge's answer, by a later merge commit, to whether that commit contains this one's // merge (gitea_contains): asked once per pair, and kept (ADR 0270). @@ -409,6 +412,9 @@ type WalkLeftOut struct { Repository string `json:"repository"` Commit string `json:"commit"` Contains string `json:"contains,omitempty"` + // Shares are the modules it moves that a stopped merge moves too, when that is why it was left out: Contains + // then names that stopped merge, which it does not contain. + Shares []string `json:"shares,omitempty"` } // leftOut is the merge of a repository a stop left out, when it did. @@ -492,12 +498,15 @@ type WalkSeen struct { // StoppedWhy is why it was stopped; WalkedAgainBy, on a walk stopped without some merges, the batch that walks // this delivery's merge again; Contains, when this delivery's merge was left out for containing a stopped // merge, that merge (novox/hq ADR 0299). - StoppedWhy string `json:"stopped_why,omitempty"` - WalkedAgainBy string `json:"walked_again_by,omitempty"` - Contains string `json:"contains,omitempty"` - Tier int `json:"tier"` - Tiers int `json:"tiers"` - Updated time.Time `json:"updated"` + StoppedWhy string `json:"stopped_why,omitempty"` + WalkedAgainBy string `json:"walked_again_by,omitempty"` + Contains string `json:"contains,omitempty"` + // Shares are, when this delivery's merge was left out for moving modules a stopped merge moves, those modules: + // built at the branch, which holds the stopped merge, they would deliver it (ADR 0299). + Shares []string `json:"shares,omitempty"` + Tier int `json:"tier"` + Tiers int `json:"tiers"` + Updated time.Time `json:"updated"` // Branch is the branch the walk's commit was merged into; empty in a record kept before it was read. Branch string `json:"branch,omitempty"` // Created is when the controller made the walk: a delivery follows the newest walk that answers it. @@ -558,7 +567,12 @@ func (d *Delivery) stoppedWords() string { if d.Walk.StoppedWhy != "" { s += ": " + d.Walk.StoppedWhy } - if d.Walk.Contains != "" { + switch { + case len(d.Walk.Shares) > 0: + s += fmt.Sprintf("; its merge moves %s, as the stopped merge %s does: built at the branch, which holds that "+ + "merge, it would deliver it, so it was left out with it (novox/hq ADR 0299)", strings.Join(d.Walk.Shares, ", "), + shortOf(d.Walk.Contains, 12)) + case d.Walk.Contains != "": s += fmt.Sprintf("; its merge contains the stopped merge %s, so it cannot be walked without delivering it, "+ "and was left out with it (novox/hq ADR 0299)", shortOf(d.Walk.Contains, 12)) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table.go b/modules/mesh-delivery/cmd/mesh-delivery/table.go index 707bd563..5203ee17 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -568,6 +568,12 @@ func boundOf(d *Delivery) (Bound, bool) { if d.waitsForItsOwnWalk() { return rewalkBound, true } + if (d.State == Published || d.State == Delivering) && d.HeldBack != "" { + // Its walk carries a stopped merge and is not let go (ADR 0299): said with why, past the bound of its state. + b := Bounds[d.State] + b.For, b.Says = Bounds[Published].For, d.HeldBack // a walk not let go waits for a word, whatever its state + return b, true + } if d.State == Published && d.TurnAwaits != "" && d.TurnAwaitsUntil != nil { // Its turn waits for a later merge to carry a member before it: bounded by that carrier's own bound, and // said with it, never the generic "waits for its turn or its word" (novox/hq issue 445). diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index cb389a14..9771e281 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -29,6 +29,9 @@ type Line struct { // waitsFor is what a delivery waits for, in words. func (h *Holder) waitsFor(d *Delivery) string { + if d.HeldBack != "" && !d.State.Final() { + return d.HeldBack + } switch d.State { case Proposed: return "its check" @@ -393,6 +396,11 @@ func (h *Holder) StopWay(id, why, by, way string) (string, error) { walk = d.Walk.ID } merge := d.Repository + "@" + d.MergedAs + // A merge that waits in a batch not yet cut: the batch is asked to leave it out once it is stopped (ADR 0299). + var batch string + if b, batched := h.batchOf(d); batched && walk == "" { + batch = b.ID + } var others []string if walk != "" { others = h.othersOn(walk, []string{merge}) @@ -452,6 +460,11 @@ func (h *Holder) StopWay(id, why, by, way string) (string, error) { for _, a := range asks { a() } + if batch != "" { + // Asked now, not at the next tick; and the batch's walk is never let go while it carries the merge. + did = fmt.Sprintf("; its merge waits in the batch %s, which was asked now to leave it out and is never let go "+ + "while it carries it: %s", batch, h.stopLeaving(batch, why, by, []string{merge})) + } return fmt.Sprintf("%s is stopped, on %s's word%s", id, by, did), nil } @@ -472,11 +485,40 @@ func (h *Holder) takenDown(d *Delivery, walk string, others []string) string { if len(contain) > 0 { s += fmt.Sprintf(" These contain its merge and end with it either way: %s.", strings.Join(contain, ", ")) } + // A module is built at its branch, which holds this merge: a delivery moving a module this one moves would + // deliver it, and the controller leaves it out too (ADR 0299). Its plan is what is known here; the controller + // decides, and its answer names each. + var share []string + for _, o := range ride { + if od := h.deliveries[o]; od != nil && od.Plan != nil && d.Plan != nil { + if both := sharedModules(od.Plan.Moved, d.Plan.Moved); len(both) > 0 { + share = append(share, fmt.Sprintf("%s (moves %s)", o, strings.Join(both, ", "))) + } + } + } + if len(share) > 0 { + s += fmt.Sprintf(" With `way: %s` these are left out too, for moving a module this merge moves: %s.", wayAgain, + strings.Join(share, ", ")) + } + s += fmt.Sprintf(" With `way: %s` the controller may leave out more for sharing a module, and its answer names each.", + wayAgain) return s + fmt.Sprintf(" What the walk already sent stays where it was sent; the stop's answer names it."+ " Choose a way: `way: %s` ends the walk and walks the others again without this merge; "+ "`way: %s` stops the whole walk and every delivery on it.", wayAgain, wayWalk) } +// sharedModules are the modules in both lists, sorted. +func sharedModules(a, b []string) []string { + var out []string + for _, m := range a { + if containsString(b, m) && !containsString(out, m) { + out = append(out, m) + } + } + sort.Strings(out) + return out +} + // containsMerge is whether o is a later merge of d's repository and branch, which contains d's merge on a linear // trunk: merged after it, or the merge the walk carries for it. func containsMerge(o, d *Delivery) bool {