Keep a check build whose key is unchanged instead of building it again
mesh/merge-gate head 1666747a2246 merged into trunk 04730b54a8f2 — pass: builds build-agent, mesh-controller → ace, g14, novox, shanks; no bus step; every …
mesh/repo-check head 1666747a2246 merged into trunk 04730b54a8f2 — pass: its merge-check.sh passed
mesh/delivery superseded: a newer head of the same pull request
mesh/merge-gate head 1666747a2246 merged into trunk 04730b54a8f2 — pass: builds build-agent, mesh-controller → ace, g14, novox, shanks; no bus step; every …
mesh/repo-check head 1666747a2246 merged into trunk 04730b54a8f2 — pass: its merge-check.sh passed
mesh/delivery superseded: a newer head of the same pull request
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).
This commit is contained in:
+20
-11
@@ -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})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -214,3 +214,28 @@ func TestAWalksLeaveNamesTheBuildsItsPullRequestsVerdictsMade(t *testing.T) {
|
|||||||
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 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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -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,33 @@ 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, builds)},
|
||||||
}, nil
|
}, 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
|
// 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
|
||||||
|
|||||||
@@ -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))
|
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
|
// 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). 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 {
|
func reuseWithVerdicts(ctx context.Context, r *link.ReuseAsk) *link.ReuseAsk {
|
||||||
|
|||||||
@@ -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
|
// 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 +383,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 +497,32 @@ 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, 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
|
// 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,
|
||||||
|
|||||||
@@ -37,6 +37,10 @@ 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 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
|
// 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 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 +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)
|
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}, 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 +130,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 {
|
||||||
|
|||||||
@@ -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))
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
|
"reflect"
|
||||||
"slices"
|
"slices"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -625,3 +626,122 @@ 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()
|
||||||
|
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())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -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
|
// 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 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
|
// 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 +216,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.
|
||||||
|
|||||||
@@ -37,3 +37,33 @@ func TestNatsTheBuildsAWalkMayTakeAreTheChecksOutcomesOfItsPullRequests(t *testi
|
|||||||
t.Fatalf("read %+v", got)
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
+26
-5
@@ -375,6 +375,26 @@ 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] })
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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)
|
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 +422,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, Built: b})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user