From 1666747a2246ace84b873867b9999f7e863ea23b Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 14 Oct 2026 09:41:39 +0200 Subject: [PATCH] Keep a check build whose key is unchanged instead of building it again A fresh check asked when only the trunk moved rebuilt every image, though the reuse key said nothing it is built from changed: a controller check took about 12 minutes instead of 5.5. The check now keeps a build kept under its key when a verdict of the build seat names that key and every digest, names it as the build's own check so a walk takes it, and the cleanup keeps it while any check names it (novox/hq issue 703). --- cmd/mesh-builder/main.go | 31 +++--- cmd/mesh-controller/check_builds_test.go | 25 +++++ cmd/mesh-controller/checks.go | 26 ++++- cmd/mesh-controller/release_plan.go | 15 +++ internal/builder/builder.go | 33 ++++++- internal/builder/check_builds.go | 22 ++++- internal/builder/reuse.go | 10 +- internal/builder/reuse_test.go | 120 +++++++++++++++++++++++ internal/link/build.go | 7 ++ internal/link/check_builds_test.go | 30 ++++++ internal/link/queue.go | 31 +++++- 11 files changed, 328 insertions(+), 22 deletions(-) diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index be469d09..da49ce3b 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -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. if isRegistry && len(spec.Builds) > 0 { 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 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}) } // Only what a verdict names is taken (ADR 0350 §2). - for _, b := range 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) - } + reuse.Named = namedOf(request.Reuse.Builds) built, err = builder.BuildReusing(building, builder.Labelled(builder.Command, request.ID), publisher, reuse, request.Repository, request.Path, request.Ref, workspace, request.Held, npmrc, forgeFrom(), say, request.Seats) @@ -496,11 +488,28 @@ func triedOf(v builder.CheckVerdict) link.Tried { 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). func checkBuiltOf(built []builder.CheckBuilt) []link.CheckBuilt { var out []link.CheckBuilt 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 { c.Made = append(c.Made, link.MadeArtifact{Name: m.Name, Kind: m.Kind, Reference: m.Reference}) } diff --git a/cmd/mesh-controller/check_builds_test.go b/cmd/mesh-controller/check_builds_test.go index d0f41d6a..c5dc716c 100644 --- a/cmd/mesh-controller/check_builds_test.go +++ b/cmd/mesh-controller/check_builds_test.go @@ -214,3 +214,28 @@ func TestAWalksLeaveNamesTheBuildsItsPullRequestsVerdictsMade(t *testing.T) { t.Fatalf("verdicts that could not be read named %+v", got.Builds) } } + +// A check is handed the builds the seat's verdicts name for the modules it builds, read once, and none when they +// cannot be read (novox/hq issue 703). +func TestACheckIsHandedTheBuildsTheVerdictsNameForItsModules(t *testing.T) { + was := checkedBuildsOfModules + t.Cleanup(func() { checkedBuildsOfModules = was }) + var asked [][]string + named := []link.CheckedBuild{{Pull: link.PullRef{Owner: "novox", Repo: "shop", Number: 7}, Check: "build-1", + Built: link.CheckBuilt{Module: "app", Key: "r1:1"}}} + checkedBuildsOfModules = func(_ context.Context, modules []string) ([]link.CheckedBuild, error) { + asked = append(asked, modules) + return named, nil + } + builds := []link.CheckBuild{{Module: "app"}, {Module: "web", Skip: "the merge deletes it"}} + if got := namedForTheCheck(t.Context(), builds); !reflect.DeepEqual(got, named) || + !reflect.DeepEqual(asked, [][]string{{"app"}}) { + t.Fatalf("the check is handed %+v, read %v", got, asked) + } + checkedBuildsOfModules = func(context.Context, []string) ([]link.CheckedBuild, error) { + return nil, errors.New("no bus") + } + if got := namedForTheCheck(t.Context(), builds); got != nil { + t.Fatalf("verdicts that could not be read named %+v", got) + } +} diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index b6243751..c43ed0e8 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -273,6 +273,7 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop } } } + builds := checkBuildsOf(p, scope, entries, clone) return link.BuildRequest{ ID: link.NewBuildID(time.Now()), Repository: repository, @@ -282,10 +283,33 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop Source: sourceOnSeat(source), 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, - Manifests: scope.Manifests, Judge: scope.Judge, Builds: checkBuildsOf(p, scope, entries, clone)}, + Manifests: scope.Manifests, Judge: scope.Judge, Builds: builds, Named: namedForTheCheck(ctx, builds)}, }, nil } +// namedForTheCheck are the builds the seat's verdicts name for the modules a check builds, read once for the check +// (novox/hq issue 703): what it may keep instead of building again. None when they cannot be read, and the check +// builds, as before. +func namedForTheCheck(ctx context.Context, builds []link.CheckBuild) []link.CheckedBuild { + var modules []string + for _, b := range builds { + if b.Skip == "" { + modules = append(modules, b.Module) + } + } + if len(modules) == 0 { + return nil + } + named, err := checkedBuildsOfModules(ctx, modules) + if err != nil { + fmt.Printf(" the checks' builds of %d module(s) could not be read, so the check builds them: %v\n", + len(modules), err) + // empty-on-error: said above; a check handed no named build builds every module, as before issue 703 + return nil + } + return named +} + // 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 // into, from the merge the check makes; any other — a module standing on one — from its own repository at the diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index f28516fa..6a26dd2c 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -386,6 +386,21 @@ var checkedBuildsOf = func(ctx context.Context, pulls []link.PullRef) ([]link.Ch return link.ReadCheckBuilds(ctx, js, buildSeatHeld(ctx), pulls, time.Now().Add(-builder.CheckBuildsKept)) } +// checkedBuildsOfModules are the builds the build seat's checks of any pull request said they made, or kept, of +// those modules within the week check builds are kept (novox/hq issue 703): what a check may keep instead of +// building. A variable so a test needs no bus. +var checkedBuildsOfModules = func(ctx context.Context, modules []string) ([]link.CheckedBuild, error) { + if len(modules) == 0 { + return nil, nil + } + js, err := aBus() + if err != nil { + return nil, err + } + defer js.Close() + return link.ReadCheckBuildsOf(ctx, js, buildSeatHeld(ctx), modules, time.Now().Add(-builder.CheckBuildsKept)) +} + // 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. func reuseWithVerdicts(ctx context.Context, r *link.ReuseAsk) *link.ReuseAsk { diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 5d203e09..b46118aa 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -374,7 +374,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 // steps a build takes, stopped before anything is built; what it takes is copied by digest. 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) if err == nil && key.Key == "" { err = fmt.Errorf("%w: %s", errNoKey, key.Why) @@ -383,6 +383,11 @@ func build(ctx context.Context, run Runner, publish Publisher, say("reuse", "not built for the merge: %v", 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 { taken, keyed, why := takeACheckBuild(ctx, run, publish, *reuse, manifest, within, workspace, commit, credentials, artifacts, args, held, npmrcPath, npmrc, seatBases, src, onTrunk, say) @@ -492,6 +497,32 @@ func takeACheckBuild(ctx context.Context, run Runner, publish Publisher, reuse R 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, 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. 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 || len(reuse.Named) == 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 unnamed(reuse.Named, 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 // 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, diff --git a/internal/builder/check_builds.go b/internal/builder/check_builds.go index 6c9cd37d..5c8e3e50 100644 --- a/internal/builder/check_builds.go +++ b/internal/builder/check_builds.go @@ -37,6 +37,10 @@ type CheckBuilding struct { Held map[string]string Npmrc Npmrc Seats map[string]string + // Named are the builds the build seat's verdicts name for the modules this check builds, of any pull request, + // within the week check builds are kept (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. 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 @@ -48,6 +52,9 @@ type CheckBuilt struct { // NotBuilt says why it was not: no key, a build that failed, or a module after one of those. NotBuilt string 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. @@ -100,8 +107,8 @@ func buildForTheMerge(ctx context.Context, run Runner, spec CheckSpec, with Chec say("build", "for the merge: %s (tier %d)", b.Module, b.Tier) var mirrored []string 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}, - with.Seats) + filepath.Join(workspace, "for-the-merge"), held, with.Npmrc, forge, log, &mirrored, + &Reuse{keyFirst: true, Scratch: with.Scratch, Named: with.Named}, with.Seats) built := CheckBuilt{Module: b.Module, Key: result.ReuseKey, Took: time.Since(began)} switch { case errors.Is(err, errNoKey): @@ -123,6 +130,17 @@ func buildForTheMerge(ctx context.Context, run Runner, spec CheckSpec, with Chec for _, m := range result.Built { 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)) out = append(out, built) if len(result.Built) > 0 { diff --git a/internal/builder/reuse.go b/internal/builder/reuse.go index 2def775d..1f8e01c1 100644 --- a/internal/builder/reuse.go +++ b/internal/builder/reuse.go @@ -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) { store := r.store() 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 { if _, going := checks[tag]; going { continue } for _, b := range c.Builds { + recorded[b.Module+" "+b.Key+" "+b.Check] = true for _, m := range b.Made { 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 - // replaced it, and that one is its own check's to let go of. - if kept, found, err := r.Find(ctx, b.Module, b.Key); err == nil && found && kept.Check == c.Check { + // replaced it, and that one is its own check's to let go of. A build a check kept is that build's + // 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) { ok = false failed = append(failed, fmt.Sprintf("%s's record: %v", b.Module, err)) diff --git a/internal/builder/reuse_test.go b/internal/builder/reuse_test.go index e3e4ec24..9f6ac590 100644 --- a/internal/builder/reuse_test.go +++ b/internal/builder/reuse_test.go @@ -6,6 +6,7 @@ import ( "errors" "net/http" "net/http/httptest" + "reflect" "slices" "strconv" "strings" @@ -625,3 +626,122 @@ func TestABuildTwoChecksShareGoesWithTheLastOfThem(t *testing.T) { 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() + r, run, workspace := aModuleAt(t, tree) + spec := CheckSpec{ID: "build-2", Owner: aPull.Owner, Repo: aPull.Repo, Number: aPull.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: "fedcbafedcba", 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()) + } +} diff --git a/internal/link/build.go b/internal/link/build.go index 73384e9f..cf2a6898 100644 --- a/internal/link/build.go +++ b/internal/link/build.go @@ -164,6 +164,10 @@ type CheckRequest struct { // 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. Builds []CheckBuild `json:"builds,omitempty"` + // Named are the builds the seat's verdicts name for those modules, of any pull request, within the week check + // builds are kept (novox/hq issue 703): 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. 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 // 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, @@ -212,6 +216,9 @@ type CheckBuilt struct { Made []MadeArtifact `json:"made,omitempty"` NotBuilt string `json:"not-built,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. diff --git a/internal/link/check_builds_test.go b/internal/link/check_builds_test.go index 96ccbbf4..13f1100a 100644 --- a/internal/link/check_builds_test.go +++ b/internal/link/check_builds_test.go @@ -37,3 +37,33 @@ func TestNatsTheBuildsAWalkMayTakeAreTheChecksOutcomesOfItsPullRequests(t *testi t.Fatalf("read %+v", got) } } + +// A check reads the builds every pull request's checks made of the modules it builds, and a kept build is named as +// the check's that built it (novox/hq issue 703). +func TestNatsACheckReadsTheBuildsOfItsModulesAndAKeptBuildIsTheBuildersCheck(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) + } + } + app := CheckBuilt{Module: "app", Key: "r1:1", Made: []MadeArtifact{{Name: "server", Reference: "s/app/server@sha256:a"}}} + web := CheckBuilt{Module: "web", Key: "r1:2", Made: []MadeArtifact{{Name: "server", Reference: "s/web/server@sha256:b"}}} + say(BuildResult{ID: "build-1", Checked: &CheckRequest{Owner: "novox", Repo: "shop", Number: 7}, + Check: &CheckOutcome{Verdict: "pass", Built: []CheckBuilt{app, web}}}) + kept := app + kept.Kept = "build-1" + say(BuildResult{ID: "build-2", Checked: &CheckRequest{Owner: "novox", Repo: "till", Number: 3}, + Check: &CheckOutcome{Verdict: "pass", Built: []CheckBuilt{kept}}}) + + got, err := ReadCheckBuildsOf(t.Context(), js, TheBuildMachine, []string{"app"}, since) + if err != nil { + t.Fatal(err) + } + if len(got) != 2 || got[0].Check != "build-1" || got[0].Pull.Repo != "till" || got[1].Check != "build-1" || + got[1].Built.Module != "app" { + t.Fatalf("read %+v", got) + } +} diff --git a/internal/link/queue.go b/internal/link/queue.go index 3bbbb8a2..b54e87d5 100644 --- a/internal/link/queue.go +++ b/internal/link/queue.go @@ -375,6 +375,26 @@ func ReadCheckBuilds(ctx context.Context, js *broker.JetStream, seat string, pul for _, p := range pulls { wanted[p] = true } + return readCheckBuilds(ctx, js, seat, since, func(p PullRef, _ CheckBuilt) bool { return wanted[p] }) +} + +// ReadCheckBuildsOf is every build the seat's checks of any pull request said they made, or kept, of those modules +// since a moment (novox/hq issue 703): what a check may keep instead of building. Newest first. +func ReadCheckBuildsOf(ctx context.Context, js *broker.JetStream, seat string, modules []string, since time.Time) ([]CheckedBuild, error) { + if len(modules) == 0 { + return nil, nil + } + wanted := map[string]bool{} + for _, m := range modules { + wanted[m] = true + } + return readCheckBuilds(ctx, js, seat, since, func(_ PullRef, b CheckBuilt) bool { return wanted[b.Module] }) +} + +// 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) sub, err := js.Context().PullSubscribe(subject, "", nats.BindStream(broker.EventsStream), nats.StartTime(since), nats.AckNone()) @@ -402,12 +422,13 @@ func ReadCheckBuilds(ctx context.Context, js *broker.JetStream, seat string, pul continue } pull := PullRef{Owner: r.Checked.Owner, Repo: r.Checked.Repo, Number: r.Checked.Number} - if !wanted[pull] { - continue - } for _, b := range r.Check.Built { - if b.NotBuilt == "" && b.Key != "" { - out = append(out, CheckedBuild{Pull: pull, Check: r.ID, Built: b}) + if b.NotBuilt == "" && b.Key != "" && wanted(pull, b) { + check := r.ID + if b.Kept != "" { + check = b.Kept + } + out = append(out, CheckedBuild{Pull: pull, Check: check, Built: b}) } } }