Merge pull request 'A fresh check keeps a build whose key is unchanged instead of building it again (hq issue 703)' (#328) from fix/703-a-fresh-check-keeps-an-unchanged-build into main

This commit was merged in pull request #328.
This commit is contained in:
2026-10-14 08:15:56 +00:00
16 changed files with 623 additions and 54 deletions
+20 -11
View File
@@ -319,7 +319,7 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri
// at build time has no reuse key anyway, so nothing is lost by refusing it here. // at build time has no reuse key anyway, so nothing is lost by refusing it here.
if isRegistry && len(spec.Builds) > 0 { if isRegistry && len(spec.Builds) > 0 {
spec.BuildWith = &builder.CheckBuilding{Publish: publisher, Scratch: registryHere, Held: request.Held, spec.BuildWith = &builder.CheckBuilding{Publish: publisher, Scratch: registryHere, Held: request.Held,
Seats: request.Seats} Seats: request.Seats, Named: namedOf(request.Check.Named)}
} }
var v builder.CheckVerdict var v builder.CheckVerdict
v, err = builder.Check(building, builder.Command, spec, workspace, registry, forgeFrom(), say) v, err = builder.Check(building, builder.Command, spec, workspace, registry, forgeFrom(), say)
@@ -352,15 +352,7 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri
reuse.Pulls = append(reuse.Pulls, builder.PullRef{Owner: p.Owner, Repo: p.Repo, Number: p.Number}) reuse.Pulls = append(reuse.Pulls, builder.PullRef{Owner: p.Owner, Repo: p.Repo, Number: p.Number})
} }
// Only what a verdict names is taken (ADR 0350 §2). // Only what a verdict names is taken (ADR 0350 §2).
for _, b := range request.Reuse.Builds { reuse.Named = namedOf(request.Reuse.Builds)
named := builder.NamedBuild{Pull: builder.PullRef{Owner: b.Pull.Owner, Repo: b.Pull.Repo,
Number: b.Pull.Number}, Check: b.Check, Module: b.Built.Module, Key: b.Built.Key,
Digests: map[string]string{}}
for _, m := range b.Built.Made {
named.Digests[m.Name] = builder.DigestOf(m.Reference)
}
reuse.Named = append(reuse.Named, named)
}
built, err = builder.BuildReusing(building, builder.Labelled(builder.Command, request.ID), publisher, reuse, built, err = builder.BuildReusing(building, builder.Labelled(builder.Command, request.ID), publisher, reuse,
request.Repository, request.Path, request.Ref, workspace, request.Held, npmrc, request.Repository, request.Path, request.Ref, workspace, request.Held, npmrc,
forgeFrom(), say, request.Seats) forgeFrom(), say, request.Seats)
@@ -496,11 +488,28 @@ func triedOf(v builder.CheckVerdict) link.Tried {
return t return t
} }
// namedOf are the builds the seat's verdicts name, as the builder believes a scratch record by them (novox/hq ADR
// 0350 §2, issue 703): each module's key with every artifact's digest.
func namedOf(builds []link.CheckedBuild) []builder.NamedBuild {
var out []builder.NamedBuild
for _, b := range builds {
named := builder.NamedBuild{Pull: builder.PullRef{Owner: b.Pull.Owner, Repo: b.Pull.Repo,
Number: b.Pull.Number}, Check: b.Check, Module: b.Built.Module, Key: b.Built.Key,
Digests: map[string]string{}}
for _, m := range b.Built.Made {
named.Digests[m.Name] = builder.DigestOf(m.Reference)
}
out = append(out, named)
}
return out
}
// checkBuiltOf is a check's builds for its merge as the outcome carries them (novox/hq ADR 0350 §1). // checkBuiltOf is a check's builds for its merge as the outcome carries them (novox/hq ADR 0350 §1).
func checkBuiltOf(built []builder.CheckBuilt) []link.CheckBuilt { func checkBuiltOf(built []builder.CheckBuilt) []link.CheckBuilt {
var out []link.CheckBuilt var out []link.CheckBuilt
for _, b := range built { for _, b := range built {
c := link.CheckBuilt{Module: b.Module, Key: b.Key, NotBuilt: b.NotBuilt, Took: b.Took.Round(time.Second).String()} c := link.CheckBuilt{Module: b.Module, Key: b.Key, NotBuilt: b.NotBuilt, Took: b.Took.Round(time.Second).String(),
Kept: b.Kept}
for _, m := range b.Made { for _, m := range b.Made {
c.Made = append(c.Made, link.MadeArtifact{Name: m.Name, Kind: m.Kind, Reference: m.Reference}) c.Made = append(c.Made, link.MadeArtifact{Name: m.Name, Kind: m.Kind, Reference: m.Reference})
} }
+1 -1
View File
@@ -688,7 +688,7 @@ func spelledOf(m inventory.BatchedMerge) string {
func namedOf(m inventory.BatchedMerge, repository string) inventory.PlanMerge { func namedOf(m inventory.BatchedMerge, repository string) inventory.PlanMerge {
k := announcedOf(m) k := announcedOf(m)
return inventory.PlanMerge{Repository: repository, Commit: m.Commit, Number: k.Number, Title: k.Title, Moves: k.Moves, return inventory.PlanMerge{Repository: repository, Commit: m.Commit, Number: k.Number, Title: k.Title, Moves: k.Moves,
Merged: m.Merged, Heard: m.Heard} Head: k.HeadSHA, Merged: m.Merged, Heard: m.Heard}
} }
// laterMerge says a was merged after b: by the forge's merge time, then by when each was heard. // laterMerge says a was merged after b: by the forge's merge time, then by when each was heard.
+76 -13
View File
@@ -190,27 +190,90 @@ func TestAReusedCheckBuildIsRegisteredByItsDigestAndGatedAsABuiltOne(t *testing.
} }
} }
// A walk's leave names the builds its own pull requests' verdicts made, and none when they cannot be read: the // A walk's leave names the builds its own pull requests' verdicts made at the head each merge merged — never an
// builder takes nothing a verdict does not name (novox/hq ADR 0350 §2). // earlier head's — read once per walk, and none when they cannot be read: the builder takes nothing a verdict does
// not name (novox/hq ADR 0350 §2, issue 703).
func TestAWalksLeaveNamesTheBuildsItsPullRequestsVerdictsMade(t *testing.T) { func TestAWalksLeaveNamesTheBuildsItsPullRequestsVerdictsMade(t *testing.T) {
was := checkedBuildsOf was := checkedBuildsOf
t.Cleanup(func() { checkedBuildsOf = was }) forgetWalkVerdicts()
var asked []link.PullRef t.Cleanup(func() { checkedBuildsOf = was; forgetWalkVerdicts() })
named := []link.CheckedBuild{{Pull: link.PullRef{Owner: "novox", Repo: "shop", Number: 7}, Check: "build-1", var asked [][]link.PullRef
Built: link.CheckBuilt{Module: "app", Key: "r1:1"}}} pull := link.PullRef{Owner: "novox", Repo: "shop", Number: 7}
atHead := link.CheckedBuild{Pull: pull, Check: "build-2", Head: "bbbbbbbbbbbb", Built: link.CheckBuilt{Module: "app", Key: "r1:2"}}
earlier := link.CheckedBuild{Pull: pull, Check: "build-1", Head: "aaaaaaaaaaaa", Built: link.CheckBuilt{Module: "app", Key: "r1:1"}}
checkedBuildsOf = func(_ context.Context, pulls []link.PullRef) ([]link.CheckedBuild, error) { checkedBuildsOf = func(_ context.Context, pulls []link.PullRef) ([]link.CheckedBuild, error) {
asked = pulls asked = append(asked, pulls)
return named, nil return []link.CheckedBuild{atHead, earlier}, nil
} }
pulls := []link.PullRef{{Owner: "novox", Repo: "shop", Number: 7}} pulls := []link.PullRef{pull}
if got := reuseWithVerdicts(t.Context(), &link.ReuseAsk{Pulls: pulls}); !reflect.DeepEqual(got.Builds, named) || heads := map[link.PullRef]string{pull: "bbbbbbbbbbbb0123456789"}
!reflect.DeepEqual(asked, pulls) { for range 3 {
t.Fatalf("the leave names %+v, read for %+v", got.Builds, asked) if got := reuseWithVerdicts(t.Context(), "plan-1", &link.ReuseAsk{Pulls: pulls}, heads); !reflect.DeepEqual(got.Builds,
[]link.CheckedBuild{atHead}) {
t.Fatalf("the leave names %+v, not only the merged head's build", got.Builds)
}
}
if !reflect.DeepEqual(asked, [][]link.PullRef{pulls}) {
t.Fatalf("the verdicts were read %d times for one walk: %v", len(asked), asked)
}
// A merge whose head was not said takes nothing.
if got := reuseWithVerdicts(t.Context(), "plan-1", &link.ReuseAsk{Pulls: pulls}, nil); got.Builds != nil {
t.Fatalf("a merge of no said head names %+v", got.Builds)
} }
checkedBuildsOf = func(context.Context, []link.PullRef) ([]link.CheckedBuild, error) { checkedBuildsOf = func(context.Context, []link.PullRef) ([]link.CheckedBuild, error) {
return nil, errors.New("no bus") return nil, errors.New("no bus")
} }
if got := reuseWithVerdicts(t.Context(), &link.ReuseAsk{Pulls: pulls}); got.Builds != nil { if got := reuseWithVerdicts(t.Context(), "plan-2", &link.ReuseAsk{Pulls: pulls}, heads); got.Builds != nil {
t.Fatalf("verdicts that could not be read named %+v", got.Builds) t.Fatalf("verdicts that could not be read named %+v", got.Builds)
} }
} }
// A check is handed only the builds earlier checks of its own pull request at its own head made: another pull
// request's build, or an earlier head's, is never kept (novox/hq issue 703). None when they cannot be read.
func TestACheckIsHandedOnlyItsOwnHeadsBuilds(t *testing.T) {
was := checkedBuildsOf
t.Cleanup(func() { checkedBuildsOf = was })
pull := link.PullRef{Owner: "novox", Repo: "shop", Number: 7}
mine := link.CheckedBuild{Pull: pull, Check: "build-1", Head: "bbbbbbbbbbbb", Built: link.CheckBuilt{Module: "app", Key: "r1:1"}}
earlier := link.CheckedBuild{Pull: pull, Check: "build-0", Head: "aaaaaaaaaaaa", Built: link.CheckBuilt{Module: "app", Key: "r1:1"}}
another := link.CheckedBuild{Pull: link.PullRef{Owner: "novox", Repo: "shop", Number: 8}, Check: "build-9",
Head: "bbbbbbbbbbbb", Built: link.CheckBuilt{Module: "app", Key: "r1:1"}}
var asked [][]link.PullRef
checkedBuildsOf = func(_ context.Context, pulls []link.PullRef) ([]link.CheckedBuild, error) {
asked = append(asked, pulls)
return []link.CheckedBuild{mine, earlier, another}, nil
}
p := link.PullUpdated{Owner: "novox", Repo: "shop", Number: 7, Commit: "bbbbbbbbbbbb0123456789"}
builds := []link.CheckBuild{{Module: "app"}}
if got := namedForTheCheck(t.Context(), p, builds); !reflect.DeepEqual(got, []link.CheckedBuild{mine}) ||
!reflect.DeepEqual(asked, [][]link.PullRef{{pull}}) {
t.Fatalf("the check is handed %+v, read for %v", got, asked)
}
checkedBuildsOf = func(context.Context, []link.PullRef) ([]link.CheckedBuild, error) {
return nil, errors.New("no bus")
}
if got := namedForTheCheck(t.Context(), p, builds); got != nil {
t.Fatalf("verdicts that could not be read named %+v", got)
}
}
func forgetWalkVerdicts() {
walkVerdicts.Lock()
defer walkVerdicts.Unlock()
walkVerdicts.read = map[string]verdictsRead{}
}
// A walk's merged heads are the forge's word for each merge; a merge announced without its head, or a pull request
// merged at two heads in one walk, has none, and its walk takes no check's build (novox/hq issue 703).
func TestAWalksMergedHeadsAreTheForgesWord(t *testing.T) {
merge := func(number int, head string) inventory.PlanMerge {
return inventory.PlanMerge{Repository: "novox/shop", Number: number, Head: head}
}
p := &inventory.Plan{Delivery: &inventory.PlanDelivery{Merges: []inventory.PlanMerge{
merge(7, "aaaaaaaaaaaa1"), merge(8, ""), merge(9, "bbbbbbbbbbbb"), merge(9, "cccccccccccc")}}}
heads := mergedHeadsOf(p)
shop := func(n int) link.PullRef { return link.PullRef{Owner: "novox", Repo: "shop", Number: n} }
if heads[shop(7)] != "aaaaaaaaaaaa1" || heads[shop(8)] != "" || heads[shop(9)] != "" {
t.Fatalf("merged heads %v", heads)
}
}
+27 -1
View File
@@ -273,6 +273,7 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop
} }
} }
} }
builds := checkBuildsOf(p, scope, entries, clone)
return link.BuildRequest{ return link.BuildRequest{
ID: link.NewBuildID(time.Now()), ID: link.NewBuildID(time.Now()),
Repository: repository, Repository: repository,
@@ -282,10 +283,35 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop
Source: sourceOnSeat(source), Source: sourceOnSeat(source),
Check: &link.CheckRequest{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Base: p.Base, Check: &link.CheckRequest{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Base: p.Base,
Paths: p.Paths, Beside: beside, Modules: scope.Modules, Dependents: scope.Dependents, New: scope.New, Paths: p.Paths, Beside: beside, Modules: scope.Modules, Dependents: scope.Dependents, New: scope.New,
Manifests: scope.Manifests, Judge: scope.Judge, Builds: checkBuildsOf(p, scope, entries, clone)}, Manifests: scope.Manifests, Judge: scope.Judge, Builds: builds, Named: namedForTheCheck(ctx, p, builds)},
}, nil }, nil
} }
// namedForTheCheck are the builds earlier checks of the same pull request at the same head named, read once for the
// check (novox/hq issue 703): asked again because only the trunk moved, the check keeps what they built of a key
// still the same. Another pull request's build, or one of an earlier head, is never kept. None when they cannot be
// read, and the check builds, as before.
func namedForTheCheck(ctx context.Context, p link.PullUpdated, builds []link.CheckBuild) []link.CheckedBuild {
if len(builds) == 0 || p.Commit == "" {
return nil
}
pull := link.PullRef{Owner: p.Owner, Repo: p.Repo, Number: p.Number}
read, err := checkedBuildsOf(ctx, []link.PullRef{pull})
if err != nil {
fmt.Printf(" the earlier checks' builds of %s/%s#%d could not be read, so the check builds: %v\n",
p.Owner, p.Repo, p.Number, err)
// empty-on-error: said above; a check handed no named build builds every module, as before issue 703
return nil
}
var named []link.CheckedBuild
for _, b := range read {
if b.Pull == pull && link.SameHead(b.Head, p.Commit) {
named = append(named, b)
}
}
return named
}
// checkBuildsOf are the modules a merge of the change would build, in the walk's order, as the check builds them // checkBuildsOf are the modules a merge of the change would build, in the walk's order, as the check builds them
// for the walk to take (novox/hq ADR 0350 §1): a module built from the repository checked, at the branch it merges // for the walk to take (novox/hq ADR 0350 §1): a module built from the repository checked, at the branch it merges
// into, from the merge the check makes; any other — a module standing on one — from its own repository at the // into, from the merge the check makes; any other — a module standing on one — from its own repository at the
+64 -4
View File
@@ -9,6 +9,7 @@ import (
"slices" "slices"
"sort" "sort"
"strings" "strings"
"sync"
"time" "time"
"github.com/nats-io/nats.go" "github.com/nats-io/nats.go"
@@ -386,15 +387,73 @@ var checkedBuildsOf = func(ctx context.Context, pulls []link.PullRef) ([]link.Ch
return link.ReadCheckBuilds(ctx, js, buildSeatHeld(ctx), pulls, time.Now().Add(-builder.CheckBuildsKept)) return link.ReadCheckBuilds(ctx, js, buildSeatHeld(ctx), pulls, time.Now().Add(-builder.CheckBuildsKept))
} }
// mergedHeadsOf are the head commits the walk's merges merged, by pull request (novox/hq issue 703): a pull request
// merged twice in one walk, or whose head the announcement did not say, has none, and takes no check's build.
func mergedHeadsOf(p *inventory.Plan) map[link.PullRef]string {
heads := map[link.PullRef]string{}
if p.Delivery == nil {
return heads
}
seen := map[link.PullRef]bool{}
for _, m := range p.Delivery.Merges {
pull, ok := pullOf(m)
if !ok {
continue
}
if seen[pull] && heads[pull] != m.Head {
heads[pull] = ""
continue
}
seen[pull] = true
heads[pull] = m.Head
}
return heads
}
// walkVerdicts are the checks' builds a walk read, by the walk's plan: read once per walk (novox/hq issue 649), not
// once per module it asks for. Kept a day; a walk lasts minutes.
var walkVerdicts = struct {
sync.Mutex
read map[string]verdictsRead
}{read: map[string]verdictsRead{}}
type verdictsRead struct {
at time.Time
builds []link.CheckedBuild
}
// reuseWithVerdicts is a walk's leave with the builds its pull requests' verdicts name: the only builds the builder // reuseWithVerdicts is a walk's leave with the builds its pull requests' verdicts name: the only builds the builder
// may take (ADR 0350 §2). When they cannot be read the leave names none, and the walk builds, saying why. // may take (ADR 0350 §2), and **only those of a check of the head each merge merged** (novox/hq issue 703): a build
func reuseWithVerdicts(ctx context.Context, r *link.ReuseAsk) *link.ReuseAsk { // a check made at an earlier head of the same pull request is never registered. Read once per walk. When they
// cannot be read the leave names none, and the walk builds, saying why.
func reuseWithVerdicts(ctx context.Context, walk string, r *link.ReuseAsk, heads map[link.PullRef]string) *link.ReuseAsk {
walkVerdicts.Lock()
read, known := walkVerdicts.read[walk]
walkVerdicts.Unlock()
if !known || walk == "" {
builds, err := checkedBuildsOf(ctx, r.Pulls) builds, err := checkedBuildsOf(ctx, r.Pulls)
if err != nil { if err != nil {
fmt.Printf(" the checks' builds of %d pull request(s) could not be read, so the walk builds: %v\n", len(r.Pulls), err) fmt.Printf(" the checks' builds of %d pull request(s) could not be read, so the walk builds: %v\n", len(r.Pulls), err)
return r return r
} }
r.Builds = builds read = verdictsRead{at: time.Now(), builds: builds}
if walk != "" {
walkVerdicts.Lock()
for id, w := range walkVerdicts.read {
if time.Since(w.at) > 24*time.Hour {
delete(walkVerdicts.read, id)
}
}
walkVerdicts.read[walk] = read
walkVerdicts.Unlock()
}
}
r.Builds = nil
for _, b := range read.builds {
if link.SameHead(b.Head, heads[b.Pull]) {
r.Builds = append(r.Builds, b)
}
}
return r return r
} }
@@ -437,7 +496,8 @@ func askModule(ctx context.Context, p *inventory.Plan, name string, byName map[s
} }
// **The walk takes the check's build of the same key** (novox/hq ADR 0350 §2): its ask says so, with the pull // **The walk takes the check's build of the same key** (novox/hq ADR 0350 §2): its ask says so, with the pull
// requests it answers, whose checks name the part of a differing key. // requests it answers, whose checks name the part of a differing key.
id, err := askABuild(withReuse(ctx, reuseWithVerdicts(ctx, reuseAskOf(p))), source, e.Source.Path, ref) id, err := askABuild(withReuse(ctx, reuseWithVerdicts(ctx, p.ID, reuseAskOf(p), mergedHeadsOf(p))), source,
e.Source.Path, ref)
if err != nil { if err != nil {
state.State = "failed" state.State = "failed"
state.Why = err.Error() state.Why = err.Error()
+51 -1
View File
@@ -162,6 +162,10 @@ type Reuse struct {
// keyFirst is a check's build (§1): its key is taken before anything is built, and a module with none is not // keyFirst is a check's build (§1): its key is taken before anything is built, and a module with none is not
// built at all — nothing could ever take it. // built at all — nothing could ever take it.
keyFirst bool keyFirst bool
// pull and head are the check's own pull request and the head it judged (novox/hq issue 703): it keeps only a
// build an earlier check of the same pull request at the same head made.
pull PullRef
head string
} }
// errNoKey is a check's build of a module whose source does not pin it: not built (ADR 0350 §1). // errNoKey is a check's build of a module whose source does not pin it: not built (ADR 0350 §1).
@@ -374,7 +378,7 @@ func build(ctx context.Context, run Runner, publish Publisher,
// only when asked — a check, a dry run, `build --ref` and `rebuild` never are. Its key is taken by the same // only when asked — a check, a dry run, `build --ref` and `rebuild` never are. Its key is taken by the same
// steps a build takes, stopped before anything is built; what it takes is copied by digest. // steps a build takes, stopped before anything is built; what it takes is copied by digest.
if reuse != nil && reuse.keyFirst { if reuse != nil && reuse.keyFirst {
_, key, err := keyFor(ctx, run, publish, manifest, within, workspace, commit, credentials, artifacts, args, keyed, key, err := keyFor(ctx, run, publish, manifest, within, workspace, commit, credentials, artifacts, args,
held, npmrcPath, npmrc, seatBases, src, say) held, npmrcPath, npmrc, seatBases, src, say)
if err == nil && key.Key == "" { if err == nil && key.Key == "" {
err = fmt.Errorf("%w: %s", errNoKey, key.Why) err = fmt.Errorf("%w: %s", errNoKey, key.Why)
@@ -383,6 +387,11 @@ func build(ctx context.Context, run Runner, publish Publisher,
say("reuse", "not built for the merge: %v", err) say("reuse", "not built for the merge: %v", err)
return Result{Commit: commit, NoKey: err.Error()}, err return Result{Commit: commit, NoKey: err.Error()}, err
} }
// **A check keeps a build of the same key a verdict names** (novox/hq issue 703): asked again when only
// the trunk moved, it builds nothing the earlier check built of an unchanged source.
if kept := keptCheckBuild(ctx, *reuse, manifest.Module, key.Key, len(artifacts), say); kept != nil {
built, src, reused = kept.Made, keyed, kept
}
} else if reuse != nil && reuse.Scratch != nil { } else if reuse != nil && reuse.Scratch != nil {
taken, keyed, why := takeACheckBuild(ctx, run, publish, *reuse, manifest, within, workspace, commit, taken, keyed, why := takeACheckBuild(ctx, run, publish, *reuse, manifest, within, workspace, commit,
credentials, artifacts, args, held, npmrcPath, npmrc, seatBases, src, onTrunk, say) credentials, artifacts, args, held, npmrcPath, npmrc, seatBases, src, onTrunk, say)
@@ -492,6 +501,47 @@ func takeACheckBuild(ctx context.Context, run Runner, publish Publisher, reuse R
return taken, keyed, "" return taken, keyed, ""
} }
// keptCheckBuild is the build a check keeps instead of building (novox/hq issue 703): the one kept under the
// module's key by an earlier check of the same pull request at the same head — only the trunk moved — when a verdict
// of the build seat names its key and every digest — the record alone is never trusted (ADR 0350 §2) — and it holds
// every artifact the module declares. Another pull request's build, or an earlier head's, is never kept. Nil, and
// the check builds, otherwise.
func keptCheckBuild(ctx context.Context, reuse Reuse, module, key string, artifacts int,
say func(step, format string, args ...any)) *Reused {
if reuse.Scratch == nil || reuse.head == "" {
return nil
}
var same []NamedBuild
for _, n := range reuse.Named {
if n.Pull == reuse.pull {
same = append(same, n)
}
}
if len(same) == 0 {
return nil
}
found, ok, err := reuse.Scratch.Find(ctx, module, key)
switch {
case err != nil:
say("reuse", "the scratch namespace could not be read, so the check builds: %v", err)
return nil
case !ok:
return nil
case found.Pull != reuse.pull || found.Head == "" || twelve(found.Head) != twelve(reuse.head):
say("reuse", "the build kept under %s is %s, not this pull request's at this head: built", twelve(key[3:]),
found.Said())
return nil
case unnamed(same, module, key, found) != "":
say("reuse", "a build is kept under %s and no verdict names it with its digests: built", twelve(key[3:]))
return nil
case len(found.Made) != artifacts:
return nil
}
kept := &Reused{From: found, Made: found.Made}
say("reuse", "kept, not built: %s", strings.Replace(kept.Words(), "reused from", "the build of", 1))
return kept
}
// keyFor takes a build's reuse key by the steps a build takes, each artifact stopped before anything is built or // keyFor takes a build's reuse key by the steps a build takes, each artifact stopped before anything is built or
// published (ADR 0350 §2): its own copy of the source inputs, so the build's are untouched. // published (ADR 0350 §2): its own copy of the source inputs, so the build's are untouched.
func keyFor(ctx context.Context, run Runner, publish Publisher, manifest catalogue.Manifest, func keyFor(ctx context.Context, run Runner, publish Publisher, manifest catalogue.Manifest,
+50 -1
View File
@@ -354,6 +354,22 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
v.Head, v.Trunk, v.Merge, v.Beside, v.Members = judged.Head, judged.Trunk, judged.Merge, judged.Beside, judged.Members v.Head, v.Trunk, v.Merge, v.Beside, v.Members = judged.Head, judged.Trunk, judged.Merge, judged.Beside, judged.Members
return v return v
} }
// **What the merge would move is built from a checkout nothing of the check touches** (novox/hq issue 703): the
// gate and the repository's own check run in containers that mount the workspace read-write, so they can change
// its tree, its objects and its caches. The builds are made from a copy of the merge's objects, cloned before
// anything of the change runs, outside the workspace — so what is kept under a key is what the key describes.
building, buildingTree := "", ""
if len(spec.Builds) > 0 && spec.BuildWith != nil && len(spec.Group) == 0 {
at := judged.Merge
if at == "" {
at = spec.Ref
}
building, buildingTree, err = checkoutForTheBuilds(ctx, run, tree, at, workspace)
if err != nil {
return CheckVerdict{}, err
}
defer os.RemoveAll(building)
}
// **The base branch's check judges the change** (novox/hq issue 310), never the one the change brings. // **The base branch's check judges the change** (novox/hq issue 310), never the one the change brings.
judging, err := TheCheckThatJudges(ctx, tree, spec.Base) judging, err := TheCheckThatJudges(ctx, tree, spec.Base)
if err != nil { if err != nil {
@@ -610,7 +626,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
say("check", "%s", why) say("check", "%s", why)
} else { } else {
began := time.Now() began := time.Now()
v.Built = buildForTheMerge(ctx, run, spec, *spec.BuildWith, workspace, tree, judged.Merge, judged, forge, v.Built = buildForTheMerge(ctx, run, spec, *spec.BuildWith, building, buildingTree, judged.Merge, judged, forge,
func(step, message string) { func(step, message string) {
if log != nil { if log != nil {
log(step, redact.redact(message)) log(step, redact.redact(message))
@@ -623,6 +639,39 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
return named(v), nil return named(v), nil
} }
// checkoutForTheBuilds is a clone of the commit a check builds for its merge, made before anything of the check runs,
// outside the workspace its containers mount (novox/hq issue 703): the directory to remove, and the clone in it.
// Cloned over git's own transport, never by linking the objects: what the check's containers later write into the
// workspace's objects cannot reach it.
func checkoutForTheBuilds(ctx context.Context, run Runner, tree, at, workspace string) (string, string, error) {
dir, err := os.MkdirTemp("", "mesh-check-builds-")
if err != nil {
return "", "", err
}
if withinDir(workspace, dir) {
os.RemoveAll(dir)
return "", "", fmt.Errorf("the temporary directory %s is inside the workspace %s, which a check's containers "+
"mount: the merge is not built from where its tests could change it", dir, workspace)
}
commit, err := run(ctx, tree, "git", "rev-parse", "--verify", at+"^{commit}")
if err != nil {
os.RemoveAll(dir)
return "", "", fmt.Errorf("the commit to build for the merge, %s: %w", at, err)
}
commit = strings.TrimSpace(commit)
if _, err := run(ctx, tree, "git", "update-ref", "refs/heads/mesh-check-builds", commit); err != nil {
os.RemoveAll(dir)
return "", "", err
}
cloned := filepath.Join(dir, "merged")
if _, err := run(ctx, dir, "git", "clone", "--quiet", "--no-local", "--no-checkout", "--branch",
"mesh-check-builds", tree, cloned); err != nil {
os.RemoveAll(dir)
return "", "", fmt.Errorf("cannot copy the merge to build it: %w", err)
}
return dir, cloned, nil
}
// builtCount is how many of a check's builds for its merge were built. // builtCount is how many of a check's builds for its merge were built.
func builtCount(built []CheckBuilt) int { func builtCount(built []CheckBuilt) int {
n := 0 n := 0
+22 -2
View File
@@ -37,6 +37,11 @@ type CheckBuilding struct {
Held map[string]string Held map[string]string
Npmrc Npmrc Npmrc Npmrc
Seats map[string]string Seats map[string]string
// Named are the builds the build seat's verdicts of earlier checks of this pull request at this head name
// (novox/hq issue 703): a build kept under a module's key is kept again, instead of built, only when one of them
// names its key and every digest, and the build is this pull request's at this head. The record alone is never
// enough.
Named []NamedBuild
} }
// CheckBuilt is one module a check built for its merge, as its verdict lists it (§1): its key and its artifacts, or // CheckBuilt is one module a check built for its merge, as its verdict lists it (§1): its key and its artifacts, or
@@ -48,6 +53,9 @@ type CheckBuilt struct {
// NotBuilt says why it was not: no key, a build that failed, or a module after one of those. // NotBuilt says why it was not: no key, a build that failed, or a module after one of those.
NotBuilt string NotBuilt string
Took time.Duration Took time.Duration
// Kept is the check whose build this check kept instead of building, its key unchanged (novox/hq issue 703):
// the verdict names that build, by key and digest, as that check's. Empty when this check built it.
Kept string
} }
// skipsTheBuilds is why a check builds nothing for its merge; empty when it builds. // skipsTheBuilds is why a check builds nothing for its merge; empty when it builds.
@@ -100,8 +108,9 @@ func buildForTheMerge(ctx context.Context, run Runner, spec CheckSpec, with Chec
say("build", "for the merge: %s (tier %d)", b.Module, b.Tier) say("build", "for the merge: %s (tier %d)", b.Module, b.Tier)
var mirrored []string var mirrored []string
result, err := build(ctx, run, scratchPublisher{to: with.Publish}, repository, b.Path, at, result, err := build(ctx, run, scratchPublisher{to: with.Publish}, repository, b.Path, at,
filepath.Join(workspace, "for-the-merge"), held, with.Npmrc, forge, log, &mirrored, &Reuse{keyFirst: true}, filepath.Join(workspace, "for-the-merge"), held, with.Npmrc, forge, log, &mirrored,
with.Seats) &Reuse{keyFirst: true, Scratch: with.Scratch, Named: with.Named,
pull: PullRef{Owner: spec.Owner, Repo: spec.Repo, Number: spec.Number}, head: judged.Head}, with.Seats)
built := CheckBuilt{Module: b.Module, Key: result.ReuseKey, Took: time.Since(began)} built := CheckBuilt{Module: b.Module, Key: result.ReuseKey, Took: time.Since(began)}
switch { switch {
case errors.Is(err, errNoKey): case errors.Is(err, errNoKey):
@@ -123,6 +132,17 @@ func buildForTheMerge(ctx context.Context, run Runner, spec CheckSpec, with Chec
for _, m := range result.Built { for _, m := range result.Built {
held[b.Module+"/"+m.Name] = m.Reference held[b.Module+"/"+m.Name] = m.Reference
} }
if result.Reused != nil {
// **Kept, not built** (novox/hq issue 703): the earlier check's build, named as that check's, so the
// walk takes it as the verdict names it; this check's document names it too, so it stays while this
// check does (ADR 0350 §5).
built.Kept = result.Reused.From.Check
say("build", "%s kept for the merge under %s: %s", b.Module, twelve(result.ReuseKey[3:]),
result.Reused.From.Said())
out = append(out, built)
kept = append(kept, result.Reused.From)
continue
}
say("build", "%s built for the merge under %s (%s)", b.Module, twelve(result.ReuseKey[3:]), built.Took.Round(time.Second)) say("build", "%s built for the merge under %s (%s)", b.Module, twelve(result.ReuseKey[3:]), built.Took.Round(time.Second))
out = append(out, built) out = append(out, built)
if len(result.Built) > 0 { if len(result.Built) > 0 {
+50
View File
@@ -210,3 +210,53 @@ func TestACheckMergeRunsNoHook(t *testing.T) {
t.Fatalf("a hook ran when the check made its merge: %s", said) t.Fatalf("a hook ran when the check made its merge: %s", said)
} }
} }
// What a check's tests change in the workspace never reaches what it builds for the merge (novox/hq issue 703): the
// builds are made from a copy of the merge, outside the workspace, cloned before anything of the change runs.
func TestWhatTheTestsChangeInTheWorkspaceNeverReachesTheBuild(t *testing.T) {
repo, head, _ := aTrunkAndABranch(t, map[string]string{"main.go": "package main // the merge\n"},
map[string]string{"trunk-only": "t"}, map[string]string{"head-only": "h"})
workspace := t.TempDir()
tree := filepath.Join(workspace, "check", "checked")
if out, err := exec.Command("git", "clone", "--quiet", repo, tree).CombinedOutput(); err != nil {
t.Fatalf("%v\n%s", err, out)
}
dir, cloned, err := checkoutForTheBuilds(t.Context(), Command, tree, head, workspace)
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)
if withinDir(workspace, cloned) {
t.Fatalf("the builds' checkout %s is inside the workspace %s the tests mount", cloned, workspace)
}
// The tests rewrite the tree, its branch and every object it holds.
if err := os.WriteFile(filepath.Join(tree, "main.go"), []byte("package main // planted\n"), 0o644); err != nil {
t.Fatal(err)
}
if err := filepath.WalkDir(filepath.Join(tree, ".git", "objects"), func(path string, d os.DirEntry, err error) error {
if err != nil || d.IsDir() {
return err
}
_ = os.Chmod(path, 0o644)
return os.WriteFile(path, []byte("planted"), 0o644)
}); err != nil {
t.Fatal(err)
}
// The build clones the merge from its own checkout, as buildForTheMerge does, and reads what the merge holds.
build := filepath.Join(t.TempDir(), "source")
if out, err := exec.Command("git", "clone", "--quiet", cloned, build).CombinedOutput(); err != nil {
t.Fatalf("%v\n%s", err, out)
}
if out, err := exec.Command("git", "-C", build, "checkout", "--quiet", head).CombinedOutput(); err != nil {
t.Fatalf("%v\n%s", err, out)
}
body, err := os.ReadFile(filepath.Join(build, "main.go"))
if err != nil {
t.Fatal(err)
}
if string(body) != "package main // the merge\n" {
t.Fatalf("the build read %q: what the tests wrote reached it", body)
}
}
+12 -4
View File
@@ -494,11 +494,15 @@ func (r Registry) Discard(ctx context.Context, builds []CheckBuild) error {
func (r Registry) letGo(ctx context.Context, checks, all map[string]checkIndex) (int, error) { func (r Registry) letGo(ctx context.Context, checks, all map[string]checkIndex) (int, error) {
store := r.store() store := r.store()
stays := map[string]bool{} stays := map[string]bool{}
// recorded are the records under a key another check still names (novox/hq issue 703): a later check that kept
// the build names it as the check's that built it, and the walk finds it by that record.
recorded := map[string]bool{}
for tag, c := range all { for tag, c := range all {
if _, going := checks[tag]; going { if _, going := checks[tag]; going {
continue continue
} }
for _, b := range c.Builds { for _, b := range c.Builds {
recorded[b.Module+" "+b.Key+" "+b.Check] = true
for _, m := range b.Made { for _, m := range b.Made {
stays[catalogue.Recorded(m.Reference)] = true stays[catalogue.Recorded(m.Reference)] = true
} }
@@ -523,8 +527,10 @@ func (r Registry) letGo(ctx context.Context, checks, all map[string]checkIndex)
} }
} }
// The record under the key, only while it is still this check's: a later check of the same key // The record under the key, only while it is still this check's: a later check of the same key
// replaced it, and that one is its own check's to let go of. // replaced it, and that one is its own check's to let go of. A build a check kept is that build's
if kept, found, err := r.Find(ctx, b.Module, b.Key); err == nil && found && kept.Check == c.Check { // check's record, and goes with the last check naming it (novox/hq issue 703).
if kept, found, err := r.Find(ctx, b.Module, b.Key); err == nil && found &&
(kept.Check == c.Check || kept.Check == b.Check) && !recorded[b.Module+" "+b.Key+" "+kept.Check] {
if err := store.DeleteTagged(ctx, ScratchPrefix+b.Module, tagOf(b.Key)); err != nil && !errors.Is(err, artifacts.Gone) { if err := store.DeleteTagged(ctx, ScratchPrefix+b.Module, tagOf(b.Key)); err != nil && !errors.Is(err, artifacts.Gone) {
ok = false ok = false
failed = append(failed, fmt.Sprintf("%s's record: %v", b.Module, err)) failed = append(failed, fmt.Sprintf("%s's record: %v", b.Module, err))
@@ -610,6 +616,8 @@ func unnamed(named []NamedBuild, module, key string, found CheckBuild) string {
return "" return ""
} }
} }
return "a build is kept under this key, and no check verdict of the walk's pull requests names it with its " + // A build of another pull request, or of a head the forge did not merge, is in no verdict the walk is handed
"digests: not taken" // (novox/hq issue 703): said with the check that made it, whose head names which head it was.
return "a build is kept under this key, by " + found.Said() + ", and no check verdict of the walk's pull requests " +
"at the head the forge merged names it with its digests: not taken"
} }
+153 -1
View File
@@ -6,6 +6,7 @@ import (
"errors" "errors"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"reflect"
"slices" "slices"
"strconv" "strconv"
"strings" "strings"
@@ -215,7 +216,8 @@ func TestARecordNoVerdictNamesIsRefusedAndBuilt(t *testing.T) {
if got.Reused != nil || len(scratch.took) > 0 || !dockerBuilt(r) { if got.Reused != nil || len(scratch.took) > 0 || !dockerBuilt(r) {
t.Fatalf("a record no verdict names was taken: %+v, copied %v", got.Reused, scratch.took) t.Fatalf("a record no verdict names was taken: %+v, copied %v", got.Reused, scratch.took)
} }
if !strings.Contains(got.Why, "no check verdict of the walk's pull requests names it") { if !strings.Contains(got.Why, "no check verdict of the walk's pull requests at the head the forge merged names it") ||
!strings.Contains(got.Why, "at head abcdefabcdef") {
t.Fatalf("the walk says %q", got.Why) t.Fatalf("the walk says %q", got.Why)
} }
}) })
@@ -625,3 +627,153 @@ func TestABuildTwoChecksShareGoesWithTheLastOfThem(t *testing.T) {
t.Fatal("the build stayed after the last check naming it went") t.Fatal("the build stayed after the last check naming it went")
} }
} }
// aCheckAgain is a later check (build-2) of aPull building the module from a tree, with the builds the build seat's
// verdicts name for it (novox/hq issue 703).
func aCheckAgain(t *testing.T, scratch *aScratch, tree string, leave []NamedBuild) (CheckBuilt, *recorded) {
t.Helper()
return aCheckOf(t, scratch, aPull, "abcdefabcdef", tree, leave)
}
// aCheckOf is a later check (build-2) of a pull request at a head, the trunk moved since build-1.
func aCheckOf(t *testing.T, scratch *aScratch, pull PullRef, head, tree string, leave []NamedBuild) (CheckBuilt, *recorded) {
t.Helper()
r, run, workspace := aModuleAt(t, tree)
spec := CheckSpec{ID: "build-2", Owner: pull.Owner, Repo: pull.Repo, Number: pull.Number,
Builds: []CheckBuildSpec{{Module: "meshboard", Repository: "https://forge.invalid/meshboard.git", Path: "mod",
Ref: "main"}}}
built := buildForTheMerge(t.Context(), run, spec, CheckBuilding{Publish: r, Scratch: scratch, Named: leave},
workspace, "", "", CheckVerdict{Head: head, Trunk: "654321654321"}, GitCredential{}, nil, logging(nil))
if len(built) != 1 || built[0].NotBuilt != "" || built[0].Key == "" {
t.Fatalf("the second check did not answer the module for its merge: %+v", built)
}
return built[0], r
}
// A check asked again when only the trunk moved, whose key is unchanged, builds nothing: it keeps the build an
// earlier verdict names and names it by key and digest, so the walk can take it (novox/hq issue 703).
func TestASecondCheckWhoseKeyIsUnchangedBuildsNothingAndNamesTheEarlierBuild(t *testing.T) {
withVersion(t, "builder-1")
scratch := newScratch()
first := aCheckBuilt(t, scratch, "tree-one")
again, r := aCheckAgain(t, scratch, "tree-one", named(first))
if dockerBuilt(r) || len(r.images) > 0 || len(r.archives) > 0 {
t.Fatalf("the second check built again: ran %v, published %v %v", r.ran, r.images, r.archives)
}
if again.Key != first.Key || again.Kept != "build-1" || !reflect.DeepEqual(again.Made, first.Made) {
t.Fatalf("the second check names %+v, not the first check's build %+v", again, first)
}
// The walk takes it by what the second verdict names.
leave := named(again)
leave[0].Check = again.Kept
w, run, workspace := aModuleAt(t, "tree-one")
got, err := BuildReusing(t.Context(), run, w, Reuse{Scratch: scratch, Pulls: []PullRef{aPull}, Named: leave},
"https://forge.invalid/meshboard.git", "mod", "main", workspace, nil, Npmrc{}, GitCredential{}, nil)
if err != nil || got.Reused == nil || dockerBuilt(w) {
t.Fatalf("the walk did not take the kept build: %v, why %q", err, got.Why)
}
}
// A check whose key changed builds, as before.
func TestASecondCheckWhoseKeyChangedBuilds(t *testing.T) {
withVersion(t, "builder-1")
scratch := newScratch()
first := aCheckBuilt(t, scratch, "tree-one")
again, r := aCheckAgain(t, scratch, "tree-two", named(first))
if !dockerBuilt(r) || again.Kept != "" || again.Key == first.Key {
t.Fatalf("a check whose key changed did not build: %+v", again)
}
}
// A scratch record no verdict names is never kept: the store takes writes from anything on its network, so a
// check builds rather than name a planted record (novox/hq issue 703, ADR 0350 §2).
func TestASecondCheckKeepsNoRecordAVerdictDoesNotName(t *testing.T) {
withVersion(t, "builder-1")
scratch := newScratch()
first := aCheckBuilt(t, scratch, "tree-one")
for name, leave := range map[string][]NamedBuild{
"no verdict": nil,
"another digest": func() []NamedBuild {
n := named(first)
n[0].Digests["server"] = "sha256:" + strings.Repeat("e", 64)
return n
}(),
"another key": func() []NamedBuild { n := named(first); n[0].Key = "r1:" + strings.Repeat("f", 64); return n }(),
} {
t.Run(name, func(t *testing.T) {
again, r := aCheckAgain(t, scratch, "tree-one", leave)
if !dockerBuilt(r) || again.Kept != "" {
t.Fatalf("a record no verdict names was kept: %+v", again)
}
})
}
}
// A build a later check kept stays when the check that built it goes: its artifacts and its record, which the later
// check's verdict names (novox/hq issue 703, ADR 0350 §5).
func TestABuildALaterCheckKeptStaysWhenTheCheckThatBuiltItGoes(t *testing.T) {
m := &aTaggingRegistry{aRegistryOfRepositories: newRegistryOfRepositories(), tags: map[string]map[string]string{}}
server := httptest.NewServer(m.serve())
defer server.Close()
address := strings.TrimPrefix(server.URL, "http://")
registry := Registry{Address: address}
ctx := t.Context()
now := time.Date(2026, 10, 14, 9, 0, 0, 0, time.UTC)
body := `{"mediaType":"` + mediaManifestOCI + `","layers":[]}`
digest := digestOf([]byte(body))
m.manifests["scratch/meshboard/server@"+digest] = []byte(body)
key := "r1:" + strings.Repeat("1", 64)
b := CheckBuild{Key: key, Module: "meshboard", Pull: aPull, Check: "build-1", At: now,
Made: []catalogue.Built{{Name: "server", Kind: "image", Reference: address + "/scratch/meshboard/server@" + digest}}}
if err := registry.Keep(ctx, aPull, "build-1", now, []CheckBuild{b}); err != nil {
t.Fatal(err)
}
// Another pull request's check keeps it: its own document names the build, which stays the first check's.
other := PullRef{Owner: "novox", Repo: "meshboard", Number: 8}
if err := registry.Keep(ctx, other, "build-2", now, []CheckBuild{b}); err != nil {
t.Fatal(err)
}
held := func() bool { _, ok := m.manifests["scratch/meshboard/server@"+digest]; return ok }
found := func() bool { _, ok, _ := registry.Find(ctx, "meshboard", key); return ok }
if n, err := registry.Forget(ctx, aPull); err != nil || n != 1 {
t.Fatalf("forgot %d (%v)", n, err)
}
if !held() || !found() {
t.Fatalf("the build a later check names went with the check that built it: held %v, record %v", held(), found())
}
if n, err := registry.Forget(ctx, other); err != nil || n != 1 {
t.Fatalf("forgot %d (%v)", n, err)
}
if held() || found() {
t.Fatalf("the build stayed after the last check naming it went: held %v, record %v", held(), found())
}
}
// Only an earlier check of the same pull request at the same head is kept from: another head of it, or another pull
// request whose verdict names the same key and digests, builds (novox/hq issue 703).
func TestACheckKeepsNoBuildOfAnotherHeadOrPullRequest(t *testing.T) {
withVersion(t, "builder-1")
other := PullRef{Owner: "novox", Repo: "meshboard", Number: 8}
for name, c := range map[string]struct {
pull PullRef
head string
}{
"another head": {aPull, "999999999999"},
"another pull request": {other, "abcdefabcdef"},
} {
t.Run(name, func(t *testing.T) {
scratch := newScratch()
first := aCheckBuilt(t, scratch, "tree-one")
leave := named(first)
leave[0].Pull = c.pull // as if its own verdict named it
again, r := aCheckOf(t, scratch, c.pull, c.head, "tree-one", leave)
if !dockerBuilt(r) || again.Kept != "" {
t.Fatalf("a build of %s was kept: %+v", name, again)
}
})
}
}
+3
View File
@@ -104,6 +104,9 @@ type PlanMerge struct {
Number int `json:"number,omitempty"` Number int `json:"number,omitempty"`
Title string `json:"title,omitempty"` Title string `json:"title,omitempty"`
Moves []string `json:"moves,omitempty"` Moves []string `json:"moves,omitempty"`
// Head is the pull request's head commit the forge merged (novox/hq issue 703): the walk takes only a build a
// check of that head made. Empty where the announcement said none, and then it takes none.
Head string `json:"head,omitempty"`
// Merged is when the forge made the merge and Heard when the controller heard it (novox/hq ADR 0282): where // Merged is when the forge made the merge and Heard when the controller heard it (novox/hq ADR 0282): where
// its delivery time starts. Zero for a merge named before they were kept. // its delivery time starts. Zero for a merge named before they were kept.
Merged time.Time `json:"merged,omitzero"` Merged time.Time `json:"merged,omitzero"`
+20
View File
@@ -116,9 +116,21 @@ type ReuseAsk struct {
type CheckedBuild struct { type CheckedBuild struct {
Pull PullRef `json:"pull"` Pull PullRef `json:"pull"`
Check string `json:"check"` Check string `json:"check"`
// Head is the pull request's head the check judged, as its verdict says it (twelve characters): a build is
// taken or kept only by a check of the same head (novox/hq issue 703).
Head string `json:"head,omitempty"`
Built CheckBuilt `json:"built"` Built CheckBuilt `json:"built"`
} }
// SameHead says two spellings of a commit name the same one: both said, one the other's beginning, at least
// twelve characters of it (a verdict says twelve).
func SameHead(a, b string) bool {
if len(a) > len(b) {
a, b = b, a
}
return len(a) >= 12 && strings.HasPrefix(b, a)
}
// PullRef names a pull request. // PullRef names a pull request.
type PullRef struct { type PullRef struct {
Owner string `json:"owner"` Owner string `json:"owner"`
@@ -164,6 +176,11 @@ type CheckRequest struct {
// each, from the merge it judged, into the scratch namespace once it has judged and not failed, for the walk to // each, from the merge it judged, into the scratch namespace once it has judged and not failed, for the walk to
// take. A build seat from before it builds nothing, and the walk builds as before. // take. A build seat from before it builds nothing, and the walk builds as before.
Builds []CheckBuild `json:"builds,omitempty"` Builds []CheckBuild `json:"builds,omitempty"`
// Named are the builds the seat's verdicts of earlier checks of this pull request at this head name, within the
// week check builds are kept (novox/hq issue 703): asked again because only the trunk moved, the check keeps a
// build kept under a module's unchanged key, instead of building it again, only when one of these names its key
// and every digest. Never another pull request's, nor an earlier head's. A build seat from before it builds.
Named []CheckedBuild `json:"named,omitempty"`
// Try marks a branch tried before its pull request (novox/hq issue 686): the repository's own check for a // Try marks a branch tried before its pull request (novox/hq issue 686): the repository's own check for a
// pushed branch or commit, asked by a builder through the controller's `try` verb. Its outcome is read from // pushed branch or commit, asked by a builder through the controller's `try` verb. Its outcome is read from
// the build's log (StepTried) and said to nobody: no status on the forge, nothing recorded or registered, // the build's log (StepTried) and said to nobody: no status on the forge, nothing recorded or registered,
@@ -212,6 +229,9 @@ type CheckBuilt struct {
Made []MadeArtifact `json:"made,omitempty"` Made []MadeArtifact `json:"made,omitempty"`
NotBuilt string `json:"not-built,omitempty"` NotBuilt string `json:"not-built,omitempty"`
Took string `json:"took,omitempty"` Took string `json:"took,omitempty"`
// Kept names the check whose build this check kept instead of building, its key unchanged (novox/hq issue 703):
// a walk takes it as that check's build. Empty when this check built it.
Kept string `json:"kept,omitempty"`
} }
// GroupMember is one other head of a delivery group's composed check. // GroupMember is one other head of a delivery group's composed check.
+48
View File
@@ -37,3 +37,51 @@ func TestNatsTheBuildsAWalkMayTakeAreTheChecksOutcomesOfItsPullRequests(t *testi
t.Fatalf("read %+v", got) t.Fatalf("read %+v", got)
} }
} }
// A build is read with the head its check judged, and a kept build is named as the check's that built it (novox/hq
// issue 703).
func TestNatsAKeptBuildIsTheBuildersCheckAndEveryBuildSaysItsHead(t *testing.T) {
js := aBus(t)
since := time.Now().Add(-time.Minute)
say := func(r BuildResult) {
body, _ := json.Marshal(r)
if _, err := js.Context().Publish(BuildOutcomeOf(TheBuildMachine), body); err != nil {
t.Fatal(err)
}
}
mine := &CheckRequest{Owner: "novox", Repo: "shop", Number: 7}
app := CheckBuilt{Module: "app", Key: "r1:1", Made: []MadeArtifact{{Name: "server", Reference: "s/app/server@sha256:a"}}}
say(BuildResult{ID: "build-1", Checked: mine, Check: &CheckOutcome{Verdict: "pass", Head: "aaaaaaaaaaaa",
Built: []CheckBuilt{app}}})
kept := app
kept.Kept = "build-1"
say(BuildResult{ID: "build-2", Checked: mine, Check: &CheckOutcome{Verdict: "pass", Head: "aaaaaaaaaaaa",
Built: []CheckBuilt{kept}}})
got, err := ReadCheckBuilds(t.Context(), js, TheBuildMachine, []PullRef{{Owner: "novox", Repo: "shop", Number: 7}}, since)
if err != nil {
t.Fatal(err)
}
if len(got) != 2 || got[0].Check != "build-1" || got[0].Built.Kept != "build-1" || got[0].Head != "aaaaaaaaaaaa" ||
got[1].Check != "build-1" || got[1].Head != "aaaaaaaaaaaa" {
t.Fatalf("read %+v", got)
}
}
// Two spellings of a head are the same head only when both say at least twelve characters of it.
func TestSameHeadNeedsTwelveCharactersOfBoth(t *testing.T) {
for _, c := range []struct {
a, b string
same bool
}{
{"aaaaaaaaaaaa", "aaaaaaaaaaaabbbb", true},
{"aaaaaaaaaaaabbbb", "aaaaaaaaaaaa", true},
{"aaaaaaaaaaaa", "aaaaaaaaaaab", false},
{"", "", false},
{"aaaa", "aaaaaaaaaaaa", false},
} {
if SameHead(c.a, c.b) != c.same {
t.Errorf("SameHead(%q, %q) is %v", c.a, c.b, !c.same)
}
}
}
+3
View File
@@ -176,6 +176,9 @@ type SourceMoved struct {
Repo string `json:"repo"` Repo string `json:"repo"`
Base string `json:"base"` Base string `json:"base"`
Head string `json:"head"` Head string `json:"head"`
// HeadSHA is the head commit the forge merged (novox/hq issue 703): a walk takes only a build a check of that
// head made. Empty from a forge module from before it, and then the walk takes no check's build.
HeadSHA string `json:"head_sha,omitempty"`
Commit string `json:"merge_commit_sha"` Commit string `json:"merge_commit_sha"`
CloneURL string `json:"clone_url"` CloneURL string `json:"clone_url"`
HTMLURL string `json:"html_url"` HTMLURL string `json:"html_url"`
+13 -5
View File
@@ -375,6 +375,13 @@ func ReadCheckBuilds(ctx context.Context, js *broker.JetStream, seat string, pul
for _, p := range pulls { for _, p := range pulls {
wanted[p] = true wanted[p] = true
} }
return readCheckBuilds(ctx, js, seat, since, func(p PullRef, _ CheckBuilt) bool { return wanted[p] })
}
// readCheckBuilds is every build the seat's checks said they made, or kept, that wanted takes. A kept build is named
// as the check's that built it (novox/hq issue 703): the record under its key is that check's.
func readCheckBuilds(ctx context.Context, js *broker.JetStream, seat string, since time.Time,
wanted func(PullRef, CheckBuilt) bool) ([]CheckedBuild, error) {
subject := BuildOutcomeOf(seat) subject := BuildOutcomeOf(seat)
sub, err := js.Context().PullSubscribe(subject, "", sub, err := js.Context().PullSubscribe(subject, "",
nats.BindStream(broker.EventsStream), nats.StartTime(since), nats.AckNone()) nats.BindStream(broker.EventsStream), nats.StartTime(since), nats.AckNone())
@@ -402,12 +409,13 @@ func ReadCheckBuilds(ctx context.Context, js *broker.JetStream, seat string, pul
continue continue
} }
pull := PullRef{Owner: r.Checked.Owner, Repo: r.Checked.Repo, Number: r.Checked.Number} pull := PullRef{Owner: r.Checked.Owner, Repo: r.Checked.Repo, Number: r.Checked.Number}
if !wanted[pull] {
continue
}
for _, b := range r.Check.Built { for _, b := range r.Check.Built {
if b.NotBuilt == "" && b.Key != "" { if b.NotBuilt == "" && b.Key != "" && wanted(pull, b) {
out = append(out, CheckedBuild{Pull: pull, Check: r.ID, Built: b}) check := r.ID
if b.Kept != "" {
check = b.Kept
}
out = append(out, CheckedBuild{Pull: pull, Check: check, Head: r.Check.Head, Built: b})
} }
} }
} }