From 5fed42982770fe3df3082c7b2b080742d969f2d9 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:33:35 +0100 Subject: [PATCH] =?UTF-8?q?checker:=20a=20verdict=20names=20the=20siblings?= =?UTF-8?q?=20it=20read=20at=20their=20trunk,=20and=20a=20sibling's=20move?= =?UTF-8?q?=20is=20carried=20as=20a=20trunk=20move=20is=20(ADR=200548=20?= =?UTF-8?q?=C2=A76;=20issue=20568=20D4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../internal/check/adr548_test.go | 34 +++++++++++++++++++ .../internal/check/beside.go | 18 ++++++++-- .../internal/check/checker.go | 34 +++++++++++++++---- .../internal/check/judge.go | 9 ++++- .../internal/check/pins_test.go | 6 +++- .../internal/check/read.go | 2 +- 6 files changed, 92 insertions(+), 11 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/adr548_test.go b/modules/mesh-delivery-checker/internal/check/adr548_test.go index b4db1a8..99e2d95 100644 --- a/modules/mesh-delivery-checker/internal/check/adr548_test.go +++ b/modules/mesh-delivery-checker/internal/check/adr548_test.go @@ -177,6 +177,40 @@ func TestOneCheckPerDelivery(t *testing.T) { t.Errorf("the composition was kept in the checks %v; want composed, kept on the carry, composed again once a fingerprint changed", kept) } }, + // §6: a sibling's move puts the verdict out of date, its status pending saying it is carried, and the carry's verdict + // names the sibling's old commit; the status follows the carry's verdict. Which packages run is the digest's (only + // those that name MESH_CHECK_BESIDE change with a sibling), the build module's part. + "a sibling's move is carried and the status follows the carry": func(t *testing.T) { + var r *rig + var from map[string]string + j := &judgeFunc{f: func(_ context.Context, c *Case, call int) (Outcome, error) { + if call == 2 { + from = c.CarriedFrom + if s := owedState(t, r, repoA, 7, head1); s != "pending" { + t.Errorf("the status during the sibling's carry is %q, want pending", s) + } + } + return Outcome{Verdict: passing(), Beside: map[string]string{catalogueRepository: r.trunks.Heads()[catalogueRepository]}}, nil + }} + r = newRig(t, j) + r.setTrunks(t, map[string]string{repoA: trunkA, repoSDK: trunkS, catalogueRepository: trunkS}) + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) + r.ch.Candidate(ctx, "feat%2Fx", cand) + r.ch.Wait() + if b := r.verdict(t, keyOf(t, "feat/x", cand)).Beside; b[catalogueRepository] != trunkS { + t.Fatalf("the verdict names the siblings %v, want the catalogue at %s", b, trunkS[:12]) + } + r.setTrunks(t, map[string]string{repoA: trunkA, repoSDK: trunkS, catalogueRepository: trunkS2}) + r.ch.TrunkMoved(ctx) + r.ch.Wait() + v := r.verdict(t, keyOf(t, "feat/x", cand)) + if j.count() != 2 || from[catalogueRepository] != trunkS || v.CarriedFrom[catalogueRepository] != trunkS || v.Beside[catalogueRepository] != trunkS2 { + t.Errorf("judged %d times, carried from %v, verdict beside %v carried from %v; want one carry over the catalogue's move", j.count(), from, v.Beside, v.CarriedFrom) + } + if s := owedState(t, r, repoA, 7, head1); s != "success" { + t.Errorf("the status after the carry is %q, want success", s) + } + }, // §5: a carry that fails names the move it failed on. "a carry that fails names the move": func(t *testing.T) { j := &judgeFunc{f: func(_ context.Context, c *Case, call int) (Outcome, error) { diff --git a/modules/mesh-delivery-checker/internal/check/beside.go b/modules/mesh-delivery-checker/internal/check/beside.go index ebe0df5..042d178 100644 --- a/modules/mesh-delivery-checker/internal/check/beside.go +++ b/modules/mesh-delivery-checker/internal/check/beside.go @@ -12,6 +12,10 @@ type BesideRef struct { Repository string `json:"repository"` Commit string `json:"commit"` Ref string `json:"ref,omitempty"` + // atTrunk says the sibling was read at its trunk's head, so a move of that trunk puts the verdict out of date and is + // carried (novox/hq ADR 0548 §6). Never sent: the node-engine at the commit the mesh runs, mesh-sdk at a pin and a + // group's member at its head do not move with a trunk. + atTrunk bool } const ( @@ -51,7 +55,7 @@ func (j *Examiner) besideFor(ctx context.Context, c *Case, p Pull) ([]BesideRef, switch { case repo == catalogueRepository: if head := heads[repo]; head != "" { - out = append(out, BesideRef{Repository: repo, Commit: head}) + out = append(out, BesideRef{Repository: repo, Commit: head, atTrunk: c.member(repo) == nil}) } case repo == nodeEngineRepo: commit := "" @@ -82,7 +86,7 @@ func (j *Examiner) besideFor(ctx context.Context, c *Case, p Pull) ([]BesideRef, if member := c.member(repo); member != nil { out = append(out, BesideRef{Repository: repo, Commit: member.Head, Ref: refOf(member.Number)}) } else if head := heads[repo]; head != "" { - out = append(out, BesideRef{Repository: repo, Commit: head}) + out = append(out, BesideRef{Repository: repo, Commit: head, atTrunk: true}) } else if repo == controllerRepo && p.Repository == nodeEngineRepo { // The node-engine's step-nine test judges the controller's own manifest and fails without it (issue 1295): a // checker that cannot place it says so, where placing none left that test to skip. @@ -108,3 +112,13 @@ func refOf(number int) string { } return "refs/pull/" + strconv.Itoa(number) + "/head" } + +// besideAtTrunk are the siblings of refs read at their trunk's head that the case does not merge, by repository: what a +// verdict names as Beside (ADR 0548 §6). +func besideAtTrunk(c *Case, refs []BesideRef, into map[string]string) { + for _, r := range refs { + if _, merged := c.Trunk[r.Repository]; r.atTrunk && !merged { + into[r.Repository] = r.Commit + } + } +} diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index af8cd96..8b838b7 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -43,6 +43,9 @@ type Outcome struct { // Judged says, by pull request (pullKey), which program judged a change to a build module (ADR 0535 §5); a pull request // that changes none is not named. Judged map[string]string + // Beside are the siblings the check read at their trunk's head, by repository (ADR 0548 §6): the verdict names them, and + // a move of one is carried. + Beside map[string]string } // BuildNote is the state of a candidate's module build, by module: built, failed or waiting (ADR 0484 clause 7). @@ -255,9 +258,20 @@ func (ch *Checker) prepare(ctx context.Context, cs *Case) { } } -// stale says whether a verdict was judged on commits that are no longer the case's: a trunk it merged moved. +// stale says whether a verdict was judged on commits that are no longer the case's: a trunk it merged moved, or a sibling it +// read at its trunk's head (ADR 0548 §6). func (ch *Checker) stale(v delivery.Verdict, cs *Case) bool { - return !sameTrunk(v.Trunk, cs.Trunk) + return !sameTrunk(v.Trunk, cs.Trunk) || besideMoved(v.Beside, ch.heads()) +} + +// besideMoved says whether the trunk of a sibling read at these commits moved: its head now is known and another. +func besideMoved(beside, heads map[string]string) bool { + for r, c := range beside { + if h := heads[r]; h != "" && h != c { + return true + } + } + return false } // judgedOn are the trunk and sibling commits a verdict was judged on, by repository. @@ -361,7 +375,7 @@ func (ch *Checker) claim(ctx context.Context, cs *Case, first bool, epoch uint64 return &claimed{rev}, nil } return nil, nil - case !sameTrunk(cur.Trunk, cs.Trunk): + case ch.stale(cur, cs): retake = true case cur.State == delivery.VerdictFail && IsNodesFailure(cur): ch.mu.Lock() @@ -456,7 +470,15 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { return } v := out.Verdict - v.Group, v.Heads, v.Trunk = cs.Group, cs.Heads, cs.Trunk + v.Group, v.Heads, v.Trunk, v.Beside = cs.Group, cs.Heads, cs.Trunk, nil + for r, c := range out.Beside { + if _, merged := cs.Trunk[r]; !merged && c != "" { + if v.Beside == nil { + v.Beside = map[string]string{} + } + v.Beside[r] = c + } + } if cs.CarriedFrom != nil { // A carry (ADR 0548 §5): a pass names the commits it was carried from; a failure names the move it failed on. switch v.State { @@ -489,7 +511,7 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { ch.nodesFail[cs.Key] = epoch ch.mu.Unlock() } - if ch.movedSince(cs) { + if ch.movedSince(cs) || besideMoved(v.Beside, ch.heads()) { // Judged on a trunk that moved while it ran: the verdict is kept as judged, and the status stays pending until the // check of the new trunk, which Reconsider starts, ends. ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, Outcome{}) @@ -633,7 +655,7 @@ func (ch *Checker) TrunkMoved(ctx context.Context) { if err != nil || !found { continue // never judged: no status to put out of date } - if v, err := delivery.DecodeVerdict(value); err == nil && sameTrunk(v.Trunk, cs.Trunk) { + if v, err := delivery.DecodeVerdict(value); err == nil && !ch.stale(v, cs) { continue } ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, Outcome{}) diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 0687a57..38355b0 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -203,12 +203,15 @@ type state struct { nodes map[string]bool clone time.Duration slow []string + // beside are the siblings read at their trunk's head, by repository (ADR 0548 §6). + beside map[string]string } // Judge runs the case (it is the Judge of a Checker). A failure of the change, of a merge or of a step is a verdict; an error is only a case that cannot // be judged at all. func (j *Examiner) Judge(ctx context.Context, c *Case) (Outcome, error) { - st := &state{j: j, c: c, levels: map[string]*acc{delivery.LevelUnit: {}, delivery.LevelIntegration: {}}, nodes: map[string]bool{}} + st := &state{j: j, c: c, levels: map[string]*acc{delivery.LevelUnit: {}, delivery.LevelIntegration: {}}, nodes: map[string]bool{}, + beside: map[string]string{}} if v := st.judgeAll(ctx); v != nil { return st.finish(ctx, *v), nil } @@ -292,6 +295,9 @@ func (st *state) outcome(v delivery.Verdict) Outcome { } } out := Outcome{Verdict: v, Slowest: slow} + if len(st.beside) > 0 { + out.Beside = st.beside + } for _, t := range st.targets { if t.judged != "" { if out.Judged == nil { @@ -353,6 +359,7 @@ func (st *state) judgeAll(ctx context.Context) *delivery.Verdict { if len(beside) > 0 { args["beside"] = beside t.beside = besideLines(beside) + besideAtTrunk(st.c, beside, st.beside) } if p.Repository == catalogueRepository { t.runner = RunnerWorkspace // used by the build module when the merge changes it (ADR 0535 §1) diff --git a/modules/mesh-delivery-checker/internal/check/pins_test.go b/modules/mesh-delivery-checker/internal/check/pins_test.go index 0c4dba0..a2ddebc 100644 --- a/modules/mesh-delivery-checker/internal/check/pins_test.go +++ b/modules/mesh-delivery-checker/internal/check/pins_test.go @@ -324,7 +324,11 @@ func TestTheChangesOwnRepositoryIsBesideItAtItsTrunk(t *testing.T) { func TestACatalogueChangeHasTheCatalogueBesideItAtItsTrunk(t *testing.T) { b := newBench(t) b.ex.Trunks = trunksOf(t, map[string]string{catalogueRepository: trunkA, SDKRepository: trunkS, controllerRepo: trunkA2}) - b.judge(t, caseOf(t, pullOf(catalogueRepository, 12, head1))) + out := b.judge(t, caseOf(t, pullOf(catalogueRepository, 12, head1))) + // ADR 0548 §6: the outcome names the siblings read at their trunk's head, never the repository it merges. + if out.Beside[SDKRepository] != trunkS || out.Beside[controllerRepo] != trunkA2 || out.Beside[catalogueRepository] != "" { + t.Errorf("the siblings named for the verdict = %v, want mesh-sdk and mesh-controller at their trunks and not the catalogue", out.Beside) + } got := besideOf(t, b, catalogueRepository) if c := got[catalogueRepository]; c.Commit != trunkA || c.Ref != "" { t.Fatalf("the catalogue beside its own change = %+v, want its trunk %s", c, trunkA) diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index 6bfa883..11be7c4 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -489,7 +489,7 @@ func (r Reader) Status() (Status, error) { case IsNodesFailure(v): st.Waiting[KindNodes] = append(st.Waiting[KindNodes], c.Group) case heads != nil: - if cs := CaseOf(c, heads); cs != nil && !sameTrunk(v.Trunk, cs.Trunk) { + if cs := CaseOf(c, heads); cs != nil && (!sameTrunk(v.Trunk, cs.Trunk) || besideMoved(v.Beside, heads)) { st.Waiting[KindTrunk] = append(st.Waiting[KindTrunk], c.Group) if why := heldBy(turns, c, heads); why != "" { st.HeldByTurn[c.Group] = why