Check a delivery group's member beside its siblings' merges, judged by the group's controller (issue 867, ADR 0382)
mesh/merge-gate head 37fd73f3781e merged into trunk 4446027108fd — pass: builds build-agent, mesh-controller → ace, g14, novox, shanks; no bus step; every …
mesh/repo-check head 37fd73f3781e merged into trunk 4446027108fd — pass: its merge-check.sh passed
mesh/delivery delivered
mesh/delivery-group group fix/867-group-siblings delivered: every member is delivered
mesh/merge-gate head 37fd73f3781e merged into trunk 4446027108fd — pass: builds build-agent, mesh-controller → ace, g14, novox, shanks; no bus step; every …
mesh/repo-check head 37fd73f3781e merged into trunk 4446027108fd — pass: its merge-check.sh passed
mesh/delivery delivered
mesh/delivery-group group fix/867-group-siblings delivered: every member is delivered
A mesh-host change and a mesh-controller change that need each other could not both pass their own checks: each read the other at its base. The check now asks mesh-delivery for the group's open siblings into a base of the same name, reads each at its head merged onto its base, builds the judge from a controller sibling's merge in the isolated judge environment keyed on its inputs, and names the siblings in the verdict. With no answer from mesh-delivery the verdict says it could not learn them; a pull request with no sibling is checked as before. TestReplay867 replays the failure. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
+187
-5
@@ -4,6 +4,8 @@ import (
|
||||
"bufio"
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -122,6 +124,42 @@ type CheckSpec struct {
|
||||
// request (novox/hq issue 686): handed to the script as EnvTests and EnvStoreOnly, which it may read.
|
||||
Tests string
|
||||
StoreOnly bool
|
||||
// Siblings are the other open pull requests of this one's delivery group, each read beside the check at its head
|
||||
// merged onto its base head in place of the commit the mesh runs (novox/hq ADR 0382 §2). SiblingsUnknown says the
|
||||
// controller could not learn them, and the verdict says so.
|
||||
Siblings []Sibling
|
||||
SiblingsUnknown string
|
||||
}
|
||||
|
||||
// Sibling is one other open pull request of a delivery group (novox/hq ADR 0382): the repository it is in as
|
||||
// owner/repo, its number, its whole head, the branch it merges into, and Dir, the directory the check finds that
|
||||
// repository under beside it (an entry of Beside).
|
||||
type Sibling struct {
|
||||
Repository string
|
||||
Number int
|
||||
Head string
|
||||
Base string
|
||||
Dir string
|
||||
}
|
||||
|
||||
// JudgedSibling is a sibling as the check read it (ADR 0382 §4): its head, and BaseCommit, the base's head the head was
|
||||
// merged onto — both whole.
|
||||
type JudgedSibling struct {
|
||||
Repository string
|
||||
Number int
|
||||
Head string
|
||||
Base string
|
||||
BaseCommit string
|
||||
}
|
||||
|
||||
// siblingIn is the sibling read under a directory, nil when none is.
|
||||
func siblingIn(spec CheckSpec, dir string) *Sibling {
|
||||
for i := range spec.Siblings {
|
||||
if spec.Siblings[i].Dir == dir {
|
||||
return &spec.Siblings[i]
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// selection is what a check hands the repository's script to narrow what it runs (novox/hq issue 686): nothing
|
||||
@@ -206,6 +244,12 @@ type CheckVerdict struct {
|
||||
// mesh-host's of the release branch with its commit, the one the release branch's controller vendors, the
|
||||
// change's own, or the one the change's own controller vendors. Empty for a check of any other base.
|
||||
Validator string
|
||||
// Siblings are the pull requests of the delivery group the check read beside it, each merged onto its base commit
|
||||
// (novox/hq ADR 0382 §4); SiblingsUnknown is why it could not learn them; JudgeFrom says where the judge came from
|
||||
// when the group made it (§3), the validator's source being in Validator.
|
||||
Siblings []JudgedSibling
|
||||
SiblingsUnknown string
|
||||
JudgeFrom string
|
||||
}
|
||||
|
||||
// Layer is one layer of a check, judged.
|
||||
@@ -412,6 +456,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
|
||||
named := func(v CheckVerdict) CheckVerdict {
|
||||
v.Head, v.Trunk, v.Merge, v.Beside, v.Members = judged.Head, judged.Trunk, judged.Merge, judged.Beside, judged.Members
|
||||
v.Release, v.JudgedBy, v.BesideAt, v.Validator = spec.Release, judged.JudgedBy, judged.BesideAt, judged.Validator
|
||||
v.Siblings, v.SiblingsUnknown, v.JudgeFrom = judged.Siblings, spec.SiblingsUnknown, judged.JudgeFrom
|
||||
return v
|
||||
}
|
||||
// **What the merge would move is built from a checkout nothing of the check touches** (novox/hq issue 703): the
|
||||
@@ -467,7 +512,27 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
|
||||
}
|
||||
b.Else = ""
|
||||
}
|
||||
if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil {
|
||||
if sibling := siblingIn(spec, dir); sibling != nil && dir != "mesh-lab" {
|
||||
// **A sibling's repository is read at the sibling's merge** (novox/hq ADR 0382 §2): its head merged onto its
|
||||
// base's head, as its own check merges it — in place of the commit the mesh runs.
|
||||
if err := cloneAt(b.Repository, "", "", dir); err != nil {
|
||||
return CheckVerdict{}, fmt.Errorf("beside it, %w", err)
|
||||
}
|
||||
m, err := mergeIntoTrunk(ctx, run, filepath.Join(root, dir), sibling.Base, sibling.Head)
|
||||
if err != nil {
|
||||
return CheckVerdict{}, fmt.Errorf("beside it, the sibling %s#%d: %w", sibling.Repository, sibling.Number, err)
|
||||
}
|
||||
judged.Siblings = append(judged.Siblings, JudgedSibling{Repository: sibling.Repository, Number: sibling.Number,
|
||||
Head: m.Head, Base: sibling.Base, BaseCommit: m.Trunk})
|
||||
if len(m.Conflicts) > 0 {
|
||||
said := fmt.Sprintf("the sibling %s#%d (head %s) does not merge cleanly into its base %s %s: in conflict %s",
|
||||
sibling.Repository, sibling.Number, twelve(m.Head), sibling.Base, twelve(m.Trunk), strings.Join(m.Conflicts, ", "))
|
||||
say("check", "%s", said)
|
||||
return named(CheckVerdict{Verdict: "fail", Summary: said, Report: said, Took: time.Since(began),
|
||||
Gate: &Layer{Verdict: "fail", Summary: said}, Repo: &Layer{Verdict: "fail", Summary: said}}), nil
|
||||
}
|
||||
readAt[dir] = sibling.Base
|
||||
} else if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil {
|
||||
return CheckVerdict{}, fmt.Errorf("beside it, %w", err)
|
||||
}
|
||||
if dir == "mesh-lab" {
|
||||
@@ -491,7 +556,12 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
|
||||
}
|
||||
judged.Beside[dir] = at
|
||||
}
|
||||
say("check", "beside it %s at %s", dir, refWords(readAt[dir]))
|
||||
if sibling := siblingIn(spec, dir); sibling != nil && dir != "mesh-lab" {
|
||||
say("check", "beside it %s at the merge of %s#%d (%s) onto %s", dir, sibling.Repository, sibling.Number,
|
||||
short(sibling.Head), sibling.Base)
|
||||
} else {
|
||||
say("check", "beside it %s at %s", dir, refWords(readAt[dir]))
|
||||
}
|
||||
}
|
||||
|
||||
// A delivery group's other heads (novox/hq ADR 0239), each at its head, for the gate to compose with this.
|
||||
@@ -658,7 +728,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
|
||||
}
|
||||
defer os.RemoveAll(judgeHome)
|
||||
inJudgeBuild := func(dir string, _ []string, command ...string) []string {
|
||||
args := in(spec.Toolchain, dir, JudgeBuildEnv(judgeHome, judgeGoCache), command...)
|
||||
args := in(spec.Toolchain, dir, judgeBuildEnv(judgeHome, judgeGoCache), command...)
|
||||
if spec.JudgeCache == "" {
|
||||
return args // under the check's root, in the workspace mounted already
|
||||
}
|
||||
@@ -685,10 +755,15 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
|
||||
|
||||
// The judge. Its failing to build is the change's fault when the change is the judge or its validator,
|
||||
// and the check's when it is the controller the mesh runs.
|
||||
gate, judgeFault, err := judgeFor(ctx, labelled, run, spec, root, tree, judged.BesideAt, inJudgeBuild, say)
|
||||
var siblingJudge siblingJudgement
|
||||
gate, judgeFault, err := judgeFor(ctx, labelled, run, spec, root, tree, judged.BesideAt, judged, &siblingJudge, inJudgeBuild, say)
|
||||
if err != nil {
|
||||
return CheckVerdict{}, err
|
||||
}
|
||||
judged.JudgeFrom = siblingJudge.From
|
||||
if siblingJudge.Validator != "" {
|
||||
judged.Validator = siblingJudge.Validator
|
||||
}
|
||||
if spec.Release != "" {
|
||||
// **A release-branch judgement says so** (novox/hq ADR 0358 §4): the branch, the controller's commit that
|
||||
// judged, and each repository read beside it at its branch and commit.
|
||||
@@ -697,7 +772,9 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
|
||||
judgedBy = tree
|
||||
}
|
||||
judged.JudgedBy = headAt(ctx, run, judgedBy)
|
||||
judged.Validator = releaseValidator(spec, judged.BesideAt, judged.Beside)
|
||||
if siblingJudge.Validator == "" {
|
||||
judged.Validator = releaseValidator(spec, judged.BesideAt, judged.Beside)
|
||||
}
|
||||
said := releaseWords(spec.Release, judged.JudgedBy, judged.Validator, judged.BesideAt, judged.Beside)
|
||||
fmt.Fprintf(&out, "--- %s\n", said)
|
||||
say("check", "%s", said)
|
||||
@@ -1160,8 +1237,12 @@ func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFil
|
||||
// has that branch too, or with the change's own validator for a change to the node-engine — never the controller the
|
||||
// mesh runs. besideAt is the branch each repository beside it was read at.
|
||||
func judgeFor(ctx context.Context, labelled, run Runner, spec CheckSpec, root, tree string, besideAt map[string]string,
|
||||
judged CheckVerdict, siblings *siblingJudgement,
|
||||
inToolchain func(string, []string, ...string) []string, say func(step, format string, args ...any)) (string, string, error) {
|
||||
gated := spec.Gated()
|
||||
if bin, fault, done, err := judgeFromTheGroup(ctx, labelled, run, spec, root, tree, judged, siblings, inToolchain, say); done {
|
||||
return bin, fault, err
|
||||
}
|
||||
switch spec.Judge {
|
||||
case "self":
|
||||
bin := filepath.Join(root, "bin", "judge-of-itself")
|
||||
@@ -1263,6 +1344,10 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s
|
||||
return bin, dir, nil
|
||||
}
|
||||
|
||||
// judgeBuildEnv is JudgeBuildEnv, a seam only a test replaces: to show what the plain environment would let a planted
|
||||
// workspace do to a judge's build.
|
||||
var judgeBuildEnv = JudgeBuildEnv
|
||||
|
||||
// JudgeBuildEnv is the environment every judge is built in (novox/hq ADR 0358 §6, issue 482): home, made fresh for the
|
||||
// build, as HOME and configuration; goCache, the judge cache's Go cache that no check's container mounts; no global
|
||||
// or system git configuration; no Go environment file; and no version stamped from git, so no git runs at all.
|
||||
@@ -1859,3 +1944,100 @@ func lastLine(s string) string {
|
||||
lines := strings.Split(strings.TrimSpace(s), "\n")
|
||||
return strings.TrimSpace(lines[len(lines)-1])
|
||||
}
|
||||
|
||||
// siblingJudgement says, besides the judge's path, where a judge the group made came from (novox/hq ADR 0382 §3): From
|
||||
// the controller it was built from and Validator the node-engine validator in it. Both empty for any other judge.
|
||||
type siblingJudgement struct {
|
||||
From, Validator string
|
||||
}
|
||||
|
||||
// judgeFromTheGroup builds the judge of a member that has siblings (novox/hq ADR 0382 §3); done says it did (or failed
|
||||
// to), and false leaves the judge to the rules before it.
|
||||
//
|
||||
// - A member with a mesh-controller sibling is judged by the controller built from that sibling's merge (read beside
|
||||
// the check), with the node-engine's validator from the member's own change when it is mesh-host's, else from a
|
||||
// mesh-host sibling's merge, else the one that controller vendors.
|
||||
// - A mesh-controller member is judged by itself, with a mesh-host sibling's validator in place of the one it vendors.
|
||||
//
|
||||
// **The judge is unreviewed code** and is built where nothing the change's script does can reach it: from a copy of the
|
||||
// merge made apart from the workspace's tree, with the judge-build environment (JudgeBuildEnv) — a home and Go cache of
|
||||
// its own, no global or system git configuration, no version stamping from git — and kept in the judge cache, which no
|
||||
// check's container mounts, only for the same inputs: the controller merge and the base commits it was merged onto, the
|
||||
// validator's source and commit.
|
||||
func judgeFromTheGroup(ctx context.Context, labelled, run Runner, spec CheckSpec, root, tree string, judged CheckVerdict,
|
||||
said *siblingJudgement, inToolchain func(string, []string, ...string) []string,
|
||||
say func(step, format string, args ...any)) (string, string, bool, error) {
|
||||
controller, host := siblingIn(spec, "mesh-controller"), siblingIn(spec, "mesh-host")
|
||||
var source, validatorDir, from, validator string
|
||||
var inputs []string
|
||||
merge := func(dir string, s *Sibling) string {
|
||||
return fmt.Sprintf("sibling %s#%d merge %s", s.Repository, s.Number, judged.Beside[dir])
|
||||
}
|
||||
switch {
|
||||
case spec.Judge == "self" && host != nil:
|
||||
source, validatorDir = tree, filepath.Join(root, "mesh-host")
|
||||
from = "this change's own merge " + judged.Head
|
||||
validator = merge("mesh-host", host) + "'s validator"
|
||||
case spec.Judge != "self" && controller != nil:
|
||||
source = filepath.Join(root, "mesh-controller")
|
||||
from = merge("mesh-controller", controller)
|
||||
switch {
|
||||
case spec.Judge == "validator":
|
||||
validatorDir, validator = tree, "the change's own validator"
|
||||
case host != nil:
|
||||
validatorDir, validator = filepath.Join(root, "mesh-host"), merge("mesh-host", host)+"'s validator"
|
||||
default:
|
||||
validator = "the validator " + merge("mesh-controller", controller) + " vendors"
|
||||
}
|
||||
default:
|
||||
return "", "", false, nil
|
||||
}
|
||||
for _, sib := range judged.Siblings {
|
||||
inputs = append(inputs, "base "+sib.Repository+" "+sib.BaseCommit+" head "+sib.Head)
|
||||
}
|
||||
// A merge made for a check is a new commit each time (its time is in it); what it holds is the same for the same
|
||||
// heads and bases, so the key names those and the trees the judge is built from.
|
||||
inputs = append(inputs, "member "+judged.Head+" onto "+judged.Trunk, "controller tree "+treeAt(ctx, run, source))
|
||||
if validatorDir != "" {
|
||||
inputs = append(inputs, "validator tree "+treeAt(ctx, run, validatorDir))
|
||||
}
|
||||
sum := sha256.Sum256([]byte(strings.Join(inputs, "\n")))
|
||||
key := "siblings-" + hex.EncodeToString(sum[:16])
|
||||
bin := filepath.Join(root, "bin", "judge-of-the-group")
|
||||
said.From, said.Validator = from, validator
|
||||
if judgeKept(keptJudges(spec.JudgeCache), key, bin) {
|
||||
say("check", "judged by %s, with %s, built for an earlier check of the same inputs", from, validator)
|
||||
return bin, "", true, nil
|
||||
}
|
||||
src := filepath.Join(root, "judge-src")
|
||||
defer os.RemoveAll(src)
|
||||
if err := copyCheckout(ctx, run, source, src); err != nil {
|
||||
return "", "", true, fmt.Errorf("the controller that judges (%s) could not be copied: %w", from, err)
|
||||
}
|
||||
if validatorDir != "" {
|
||||
vendored := filepath.Join(src, "vendor", "github.com", "novox", "mesh-host")
|
||||
for _, pkg := range []string{"validate", filepath.Join("internal", "declaration")} {
|
||||
if err := replaceGoFiles(filepath.Join(validatorDir, pkg), filepath.Join(vendored, pkg)); err != nil {
|
||||
return "", "the validator (" + validator + ") could not be put in the judge: " + err.Error(), true, nil
|
||||
}
|
||||
}
|
||||
}
|
||||
if _, err := labelled(ctx, root, "docker", inToolchain(src, nil,
|
||||
"go", "build", "-o", bin, "./cmd/mesh-controller")...); err != nil {
|
||||
return "", "the controller that judges (" + from + ") does not build with " + validator + ": " + firstLine(err.Error()),
|
||||
true, nil
|
||||
}
|
||||
keepJudge(keptJudges(spec.JudgeCache), key, bin, say)
|
||||
say("check", "judged by %s, with %s", from, validator)
|
||||
return bin, "", true, nil
|
||||
}
|
||||
|
||||
// treeAt is the tree object a checkout's commit holds: the content a merge made for a check has, whatever time it was
|
||||
// made at.
|
||||
func treeAt(ctx context.Context, run Runner, dir string) string {
|
||||
out, err := run(ctx, dir, "git", "rev-parse", "--verify", "--quiet", "HEAD^{tree}")
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
return strings.TrimSpace(out)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user