diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/try_candidates.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/try_candidates.go index 1e8de96e..d2feeece 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/try_candidates.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/try_candidates.go @@ -9,6 +9,8 @@ import ( "git.novox.be/novox/mesh-delivery/contracts/go/clients/meshcatalogclient" "git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog" + + "mesh-delivery-checker/internal/check" ) // tryCandidates answers the indexer's candidate of a try's group (novox/hq ADR 0593 §7, issue 1921): the candidate the @@ -59,7 +61,7 @@ func (t tryCandidates) TryCandidate(ctx context.Context, group, heads string, bo func waitsOnPreview(c meshcatalog.CandidatesByGroupValue) bool { for _, p := range c.Pulls { for _, m := range p.Fps { - if m.None2 == meshcatalogclient.NoneNoPreview { + if check.WaitsOnPreview(m) { return true } } diff --git a/modules/mesh-delivery-checker/internal/check/builds.go b/modules/mesh-delivery-checker/internal/check/builds.go index f26ae985..ef5bd073 100644 --- a/modules/mesh-delivery-checker/internal/check/builds.go +++ b/modules/mesh-delivery-checker/internal/check/builds.go @@ -124,21 +124,22 @@ func (j *Examiner) buildWait() time.Duration { // awaitBuilds waits for the candidate's builds before the check composes (ADR 0603 §10): the check builds what the builder // builds of the candidate and composes with it. A failed build fails the check, naming the module and the builder's words; -// a build not ended within the bound is named, and the check composes without it. It answers the words of the composition's -// warnings (what the check composes without waiting on its build, and why) and the failures of the builds that failed, for -// the rules to take in (rules.JudgeComposition); failed says the check has failed already. -func (st *state) awaitBuilds(ctx context.Context, failed bool) ([]string, []FailedFirst) { +// a build not ended within the bound is named in late, and the check does not compose: an engine composes a module only at +// a done build, so its answer would be the builder's lateness read as the change's refusal (issue 1921). It answers the +// words of the composition's warnings (what the check composes without waiting on its build, and why) and the failures of +// the builds that failed, for the rules to take in (rules.JudgeComposition); failed says the check has failed already. +func (st *state) awaitBuilds(ctx context.Context, failed bool) (words []string, failedBuilds []FailedFirst, late string) { wait, words := st.buildsOfTheCheck(ctx) if len(wait) == 0 { - return words, nil + return words, nil, "" } modules := slices.Sorted(maps.Keys(wait)) if failed { // The check has failed already: its red status is never held back by a build (the lead, 2026-11-03). - return append(words, fmt.Sprintf("the check failed before its builds were waited on, so it composes at once, without waiting on the builds of %s (ADR 0603 §10)", strings.Join(modules, ", "))), nil + return append(words, fmt.Sprintf("the check failed before its builds were waited on, so it composes at once, without waiting on the builds of %s (ADR 0603 §10)", strings.Join(modules, ", "))), nil, "" } if st.j.BuildEnds == nil { - return append(words, fmt.Sprintf("the builds of %s cannot be heard, so the check composes without waiting on them (ADR 0603 §10)", strings.Join(modules, ", "))), nil + return append(words, fmt.Sprintf("the builds of %s cannot be heard, so the check composes without waiting on them (ADR 0603 §10)", strings.Join(modules, ", "))), nil, "" } bound := st.j.buildWait() waits := fmt.Sprintf("waits on the builder's builds of %s before it composes, at most %s (ADR 0603 §10)", strings.Join(modules, ", "), bound) @@ -149,21 +150,20 @@ func (st *state) awaitBuilds(ctx context.Context, failed bool) ([]string, []Fail cancel() if err != nil { st.j.logf("%s: the builds of %s cannot be heard: %v", st.c.Key, strings.Join(modules, ", "), err) - return append(words, fmt.Sprintf("the builds of %s cannot be heard (%v), so the check composes without waiting on them (ADR 0603 §10)", strings.Join(modules, ", "), err)), nil + return append(words, fmt.Sprintf("the builds of %s cannot be heard (%v), so the check composes without waiting on them (ADR 0603 §10)", strings.Join(modules, ", "), err)), nil, "" } - var late []string - var failedBuilds []FailedFirst + var notEnded []string for _, module := range modules { b, done := ended[module] switch { case !done: - late = append(late, module) + notEnded = append(notEnded, module) case b.State == builds.BuildFailed: failedBuilds = append(failedBuilds, FailedFirst{Test: BuildFailedTest + module, Words: fmt.Sprintf("the candidate's build at %s failed (rebuild with builder_build_again; the check asks itself again when the build is done): %s", wait[module], b.Error)}) } } - if len(late) > 0 && ctx.Err() == nil { - words = append(words, fmt.Sprintf("the builds of %s did not end within the build-wait-bound of %s, so the check composes without them (ADR 0603 §10)", strings.Join(late, ", "), bound)) + if len(notEnded) > 0 && ctx.Err() == nil { + late = fmt.Sprintf("the builder's builds of %s did not end within the build-wait-bound of %s, so the change is not composed: an engine composes a module only at a done build (ADR 0603 §10)", strings.Join(notEnded, ", "), bound) } - return words, failedBuilds + return words, failedBuilds, late } diff --git a/modules/mesh-delivery-checker/internal/check/composition_test.go b/modules/mesh-delivery-checker/internal/check/composition_test.go index 28b435d3..46437e5c 100644 --- a/modules/mesh-delivery-checker/internal/check/composition_test.go +++ b/modules/mesh-delivery-checker/internal/check/composition_test.go @@ -13,6 +13,7 @@ import ( nec "git.novox.be/novox/mesh-contracts/go/clients/nodeengineclient" ne "git.novox.be/novox/mesh-contracts/go/seats/nodeengine" + "git.novox.be/novox/mesh-delivery/contracts/go/clients/meshcatalogclient" "git.novox.be/novox/mesh-delivery/contracts/go/clients/meshdeliverycheckerclient" "git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog" "git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdeliverychecker" @@ -223,6 +224,8 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { failed string // the failure's test, when the check fails on a build tests bool // the tests fail: the check fails before it would wait served []string // what the running builds serve (ADR 0614 §7); nil reads nothing of what runs + preview bool // the check began on the candidate the indexer wrote before its merge preview (issue 1921) + late bool // a build did not end within the bound: the change is not composed (issue 1921) }{ {name: "the Go build module invokes a verb no running build serves", manifest: map[string]catalogue.Manifest{"mesh-build-go": {Module: "mesh-build-go", Build: goBuild, @@ -254,8 +257,10 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { builds: map[string]buildsrec.BuilderBuild{"web": {State: buildsrec.BuildBuilding}}, tests: true, want: []string{"composes at once, without waiting on the builds of web"}}, {name: "a build does not end within the bound", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, - builds: map[string]buildsrec.BuilderBuild{"web": {State: buildsrec.BuildBuilding}}, - want: []string{"the builds of web did not end within the build-wait-bound"}, waited: "web"}, + builds: map[string]buildsrec.BuilderBuild{"web": {State: buildsrec.BuildBuilding}}, late: true, + want: []string{"the builder's builds of web did not end within the build-wait-bound", "the change is not composed"}, waited: "web"}, + {name: "the check began before the merge preview", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, + builds: map[string]buildsrec.BuilderBuild{"web": done}, preview: true, waited: "web"}, {name: "a module the trunk does not hold", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}, "new": {Module: "new", Build: goBuild}}, builds: map[string]buildsrec.BuilderBuild{"web": done}, runs: knownRuns{"web"}, want: []string{"new: the trunk holds no record of it yet"}, waited: "web"}, @@ -294,8 +299,20 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { b.ex.Read = read p := pullOf(catalogueRepository, 9, head1) p.Fps, p.Modules = fps, slices.Sorted(maps.Keys(fps)) + if row.preview { + previewed := p + b.ex.Candidates = &fakeTryCandidates{cand: meshcatalog.CandidatesByGroupValue{Fpset: "fps", Pulls: []meshcatalog.CandidatesByGroupValuePullsItem{previewed}}} + p.Fps = map[string]meshcatalog.CandidatesByGroupValuePullsItemFpsValue{} + for module, m := range fps { + p.Fps[module] = meshcatalog.CandidatesByGroupValuePullsItemFpsValue{Manifest: m.Manifest, None2: meshcatalogclient.NoneNoPreview} + } + } v := b.judge(t, caseOf(t, p)).Verdict - if v.Composition == nil || len(comps.asked) != 1 { + asks := 1 + if row.late { + asks = 0 // never asked: an engine would read the builder's lateness as the change's refusal + } + if v.Composition == nil || len(comps.asked) != asks || row.late && v.Composition.State != meshdeliverycheckerclient.CompositionNotRun { t.Fatalf("%s: composed %d times: %+v", row.name, len(comps.asked), v) } if ends != nil { diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 8afd1362..bc6adc0a 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "git.novox.be/novox/mesh-delivery/contracts/go/clients/meshcatalogclient" "git.novox.be/novox/mesh-delivery/contracts/go/clients/meshdeliverycheckerclient" "git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog" "git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdeliverychecker" @@ -161,8 +162,9 @@ type Examiner struct { Names(ctx context.Context) ([]string, error) } // Candidates answers the indexer's candidate of a try's group once it wrote it from the try's previews, the manifests - // and fingerprints a group try's manifest validator and composition judge (issue 1921); nil leaves a whole try with no - // manifest or composition judged, said. + // and fingerprints a group try's manifest validator and composition judge (issue 1921), and of a pull request's check + // that began before the indexer read its merge preview; nil leaves such a case with no manifest or composition judged, + // said. Candidates TryCandidates // Inputs keeps the checking program's inputs (the state InputsStateName, served on InputVerb); BeginBound is how long a run ask may wait for a // build node to begin it (DefaultBeginBound when zero). @@ -289,23 +291,52 @@ type state struct { tags groupVersions // outOfStep are, by pull request (pullKey), the generated files a level's compare step found out of step (design 26). outOfStep map[string][]string - // noCandidate says why a whole try has no candidate of its group from the indexer, so its manifests were not judged and - // it was not composed; empty when it has one, and for a pull request's check. + // noCandidate says why the case has no previewed candidate of its heads from the indexer within the bound: for a whole + // try its manifests were not judged and it was not composed; for a pull request's check that began on the indexer's + // first write, its manifests were judged from that write and it was not composed. Empty when it has one, and for a pull + // request's check whose case already holds its fingerprints. noCandidate string } // narrowTry says the case is a try narrowed to a level or to some tests: it judges no manifest, graph or composition. func (st *state) narrowTry() bool { return st.c.Try != nil && !st.c.Try.Whole() } -// takeTryCandidate gives a whole try's pulls the fingerprints and manifests the indexer's candidate of its group holds -// (the group's pull-request check has them from its candidate, CaseOf), once the indexer wrote it from the try's previews. -// A try with no such candidate is said so, in a note and in its composition, and is no pass (Tryer.run). +// WaitsOnPreview says a candidate's module has no fingerprint yet because the indexer wrote it from a pull message alone, +// before it read the head's merge preview (meshcatalogclient.NoneNoPreview): its fingerprint comes with the preview, on the +// same heads, so a candidate that holds one is not yet the candidate of its heads previewed. +func WaitsOnPreview(m meshcatalog.CandidatesByGroupValuePullsItemFpsValue) bool { + return m.None2 == meshcatalogclient.NoneNoPreview +} + +// previewPending says a module of the case's pulls waits on its merge preview (WaitsOnPreview). +func (st *state) previewPending() bool { + for _, p := range st.c.Pulls { + for _, m := range p.Fps { + if WaitsOnPreview(m) { + return true + } + } + } + return false +} + +// takeTryCandidate gives a whole try's pulls the fingerprints and manifests the indexer's candidate of its group holds, +// once the indexer wrote it from the try's previews. A pull request's check has them from its candidate (CaseOf), but the +// indexer writes a pull request's candidate first from its pull message alone and its fingerprints only with the merge +// preview, on the same heads: a check that began on that first write takes the previewed candidate too, before it judges +// the manifests, waits on the builds and composes (ADR 0603 §10; issue 1921: a check otherwise waited on no build and +// the engines, reading the previewed candidate, refused its modules as not yet built). A case with no such candidate is +// said so, in a note and in its composition, and is no pass (Tryer.run). func (st *state) takeTryCandidate(ctx context.Context) { - if st.c.Try == nil || !st.c.Try.Whole() { + if st.c.Try != nil && !st.c.Try.Whole() || st.c.Try == nil && !st.previewPending() { return } no := func(why string) { - st.noCandidate = "the group's manifests and composition were not judged: " + why + what := "the group's manifests and composition were not judged: " + if st.c.Try == nil { + what = "the composition was not judged (the manifests were judged from the indexer's first write): " + } + st.noCandidate = what + why st.c.note(st.noCandidate) } if st.j.Candidates == nil { @@ -400,9 +431,13 @@ func (st *state) compose(ctx context.Context) { // ADR 0603 §10: the check composes with the candidate's builds; whether it failed already is the running rules' word // on the facts so far, an asking choice only (the verdict is the rules' of every fact). failed := rules.Judge(st.factsOf()).State == meshdeliverycheckerclient.VerdictFail - words, builds := st.awaitBuilds(ctx, failed) + words, builds, late := st.awaitBuilds(ctx, failed) c := &rules.Composition{Waits: words, BuildsFailed: builds, FPSet: st.c.FPSet} st.composition = c + if late != "" { + c.NotRun = late // the builder's lateness, said by name, never the change's refusal (issue 1921) + return + } // A composer built at the node-engine member's head is built against the group's tagged versions the check heard, as // its other Go builds are (design 51 *Tagged versions*); it composes against the heads' seat list (seatList). st.c.Tagged = st.tags.goPackages