From 1a4c788ed712ac576d8fdc0b2431818e3b7aea59 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 19:01:40 +0100 Subject: [PATCH 1/4] Checker fails a change whose fetch reports a compiled program, and warns when a build module reports none: build output kept getting committed (issue 1577) --- .../internal/check/binaries.go | 36 +++++++++++++ .../internal/check/binaries_test.go | 53 +++++++++++++++++++ .../internal/check/examiner_test.go | 1 + .../internal/check/judge.go | 3 ++ .../internal/check/results.go | 9 ++++ 5 files changed, 102 insertions(+) create mode 100644 modules/mesh-delivery-checker/internal/check/binaries.go create mode 100644 modules/mesh-delivery-checker/internal/check/binaries_test.go diff --git a/modules/mesh-delivery-checker/internal/check/binaries.go b/modules/mesh-delivery-checker/internal/check/binaries.go new file mode 100644 index 0000000..a125367 --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/binaries.go @@ -0,0 +1,36 @@ +package check + +import ( + "fmt" + "strings" +) + +// A change that adds or modifies a compiled program fails (issue 1577): it is build output, `go build` in a module's folder +// writes one beside go.mod, and `git add -A` takes it. The build module's fetch step, which holds the merged clone, reads the +// first bytes of each changed file and reports the programs (mesh-sdk buildstep.MergeInto); the checker judges the answer. + +// NoProgramsReported is the warning of a build module whose fetch answered no programs field: an older module, which cannot +// tell, so the check does not refuse a program it did not look for. +const NoProgramsReported = "the build module did not report programs: a compiled program in the change is not refused" + +// checkBinariesOf fails a pull request whose change adds or modifies a compiled program, naming each file. It judges the +// first fetch answer of the pull request, whichever build module gave it; a build module that reports no programs leaves a +// warning on the check and passes it. +func (st *state) checkBinariesOf(t *target) verdictPtr { + key := fmt.Sprintf("%s#%d", t.pull.Repository, t.pull.Number) + if t.fetched.Merge == "" { + return nil // no merge was made: nothing was changed to judge + } + if t.fetched.Programs == nil { + st.c.note(fmt.Sprintf("%s: %s (%s)", key, NoProgramsReported, t.module)) + return nil + } + var found []string + for _, p := range *t.fetched.Programs { + found = append(found, p.Path+": a compiled program is build output: remove it and add it to .gitignore (issue 1577)") + } + if len(found) > 0 { + return failed(t.node, fmt.Sprintf("%s: %s", key, strings.Join(found, "; "))) + } + return nil +} diff --git a/modules/mesh-delivery-checker/internal/check/binaries_test.go b/modules/mesh-delivery-checker/internal/check/binaries_test.go new file mode 100644 index 0000000..84b63ce --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/binaries_test.go @@ -0,0 +1,53 @@ +package check + +import ( + "slices" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +func TestAChangeWhoseFetchReportsProgramsFailsNamingEachFile(t *testing.T) { // 1577 + prog := func(p ...string) *[]ProgramFile { + out := []ProgramFile{} + for _, f := range p { + out = append(out, ProgramFile{Path: f, Kind: "ELF"}) + } + return &out + } + for _, c := range []struct { + name string + programs *[]ProgramFile + fail []string // paths named; none passes + warned bool // the build module reported no programs field + }{ + {"none found", prog(), nil, false}, + {"one program", prog("modules/m/m"), []string{"modules/m/m"}, false}, + {"two programs", prog("a/x", "b/y"), []string{"a/x", "b/y"}, false}, + {"an older build module reports nothing", nil, nil, true}, + } { + b := newBench(t) + b.fetch.Merge, b.fetch.Programs = "mmmm", c.programs + cs := caseOf(t, pullOf(repoA, 7, head1)) + v := b.judge(t, cs).Verdict + if got := slices.ContainsFunc(cs.Notes(), func(n string) bool { return strings.Contains(n, NoProgramsReported) }); got != c.warned { + t.Errorf("%s: warned = %v, want %v (%v)", c.name, got, c.warned, cs.Notes()) + } + if len(c.fail) == 0 { + if v.State != delivery.VerdictPass { + t.Errorf("%s: verdict = %+v", c.name, v) + } + continue + } + if v.State != delivery.VerdictFail { + t.Errorf("%s: verdict = %+v, want a failure", c.name, v) + continue + } + for _, p := range c.fail { + if !strings.Contains(v.Failed.Words, p+": a compiled program is build output: remove it and add it to .gitignore (issue 1577)") { + t.Errorf("%s: words %q do not name %s", c.name, v.Failed.Words, p) + } + } + } +} diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index c8f5b13..e708dbb 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -180,6 +180,7 @@ func newBench(t testing.TB) *bench { b := &bench{steps: &fakeSteps{}, kept: newMemKV(), prev: newMemKV()} b.fetch = captured[Fetched](t, "go-fetch-clean") b.fetch.Modules, b.fetch.Changed, b.fetch.Trees = []string{"."}, []string{"README.md"}, nil + b.fetch.Programs = &[]ProgramFile{} // the build module reports it (issue 1577) one := captured[Digested](t, "go-digest") b.digest = map[string]Digested{ delivery.LevelUnit: {Level: "unit", Packages: map[string]string{"fixture/slow": digA}}, diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index f493289..43a0b6b 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -489,6 +489,9 @@ func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, l if v := st.checkPinsOf(ctx, t); v != nil { return v } + if v := st.checkBinariesOf(t); v != nil { + return v + } } ran := 0.0 for _, la := range answer.Levels { diff --git a/modules/mesh-delivery-checker/internal/check/results.go b/modules/mesh-delivery-checker/internal/check/results.go index bd61b15..bc5e523 100644 --- a/modules/mesh-delivery-checker/internal/check/results.go +++ b/modules/mesh-delivery-checker/internal/check/results.go @@ -44,6 +44,15 @@ type Fetched struct { Conflicts []string `json:"conflicts,omitempty"` Changed []string `json:"changed,omitempty"` Trees map[string]MergeTree `json:"trees,omitempty"` + // Programs are the changed files whose first bytes are an executable binary, read by the build module in its workspace + // (issue 1577); nil for a build module that does not report it, a pointer to an empty list for one that found none. + Programs *[]ProgramFile `json:"programs,omitempty"` +} + +// ProgramFile is a changed file that is a compiled program, and its format (ELF, Mach-O or PE). +type ProgramFile struct { + Path string `json:"path"` + Kind string `json:"kind"` } // GoPackage is one package of a Go test result. From dc9d65524d59c67d66712353ff873e725ecb1590 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 19:08:00 +0100 Subject: [PATCH 2/4] A check judged by a build module that reports no programs says so in its status line, still passing: the refusal did not run (issue 1577) --- .../internal/check/binaries.go | 2 ++ .../internal/check/binaries_test.go | 4 ++++ .../internal/check/case.go | 21 ++++++++++++++++++- .../internal/check/line.go | 9 ++++++-- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/binaries.go b/modules/mesh-delivery-checker/internal/check/binaries.go index a125367..c81dd29 100644 --- a/modules/mesh-delivery-checker/internal/check/binaries.go +++ b/modules/mesh-delivery-checker/internal/check/binaries.go @@ -23,6 +23,8 @@ func (st *state) checkBinariesOf(t *target) verdictPtr { } if t.fetched.Programs == nil { st.c.note(fmt.Sprintf("%s: %s (%s)", key, NoProgramsReported, t.module)) + st.c.warn(key, fmt.Sprintf("a compiled program is not refused: %s reported no programs", t.module)) + st.j.logf("%s: %s (%s)", key, NoProgramsReported, t.module) return nil } var found []string diff --git a/modules/mesh-delivery-checker/internal/check/binaries_test.go b/modules/mesh-delivery-checker/internal/check/binaries_test.go index 84b63ce..6b9c3b4 100644 --- a/modules/mesh-delivery-checker/internal/check/binaries_test.go +++ b/modules/mesh-delivery-checker/internal/check/binaries_test.go @@ -34,6 +34,10 @@ func TestAChangeWhoseFetchReportsProgramsFailsNamingEachFile(t *testing.T) { // if got := slices.ContainsFunc(cs.Notes(), func(n string) bool { return strings.Contains(n, NoProgramsReported) }); got != c.warned { t.Errorf("%s: warned = %v, want %v (%v)", c.name, got, c.warned, cs.Notes()) } + line := Line(v, cs, cs.Pulls[0], nil, nil, nil) + if got := strings.Contains(line, "a compiled program is not refused: mesh-build-go reported no programs"); got != c.warned { + t.Errorf("%s: the status line holds the warning = %v, want %v: %s", c.name, got, c.warned, line) + } if len(c.fail) == 0 { if v.State != delivery.VerdictPass { t.Errorf("%s: verdict = %+v", c.name, v) diff --git a/modules/mesh-delivery-checker/internal/check/case.go b/modules/mesh-delivery-checker/internal/check/case.go index 169d55e..7215adc 100644 --- a/modules/mesh-delivery-checker/internal/check/case.go +++ b/modules/mesh-delivery-checker/internal/check/case.go @@ -43,7 +43,9 @@ type Case struct { Try *TryOptions notesMu sync.Mutex - notes []string + // warnings are, by pull request, what part of its check did not run (Case.warn). + warnings map[string]string + notes []string // asks are the ids of the asks the check put on the build seats' streams, in order: a try names them (Try.Asks). asks []string // Progress says what the check is doing, so a check that ran past its bound names the step it was stuck at. @@ -186,6 +188,23 @@ func (c *Case) note(s string) { } } +// warn says of a pull request (pullKey) that a part of its check did not run, in words for its status line (issue 1577). +func (c *Case) warn(key, words string) { + c.notesMu.Lock() + defer c.notesMu.Unlock() + if c.warnings == nil { + c.warnings = map[string]string{} + } + c.warnings[key] = words +} + +// Warning is what warn said of a pull request, or "". +func (c *Case) Warning(key string) string { + c.notesMu.Lock() + defer c.notesMu.Unlock() + return c.warnings[key] +} + // Notes are the words noted on the case, in the order they were noted. func (c *Case) Notes() []string { c.notesMu.Lock() diff --git a/modules/mesh-delivery-checker/internal/check/line.go b/modules/mesh-delivery-checker/internal/check/line.go index f09b839..59d2da8 100644 --- a/modules/mesh-delivery-checker/internal/check/line.go +++ b/modules/mesh-delivery-checker/internal/check/line.go @@ -87,18 +87,20 @@ type pieces struct { judged string // carried says, of a carried verdict, the commits it was carried from (ADR 0548 ยง8); never dropped. carried string + // warning says that a part of the check did not run, for instance that an older build module could not refuse a compiled program; never dropped. + warning string } // pullKey names a pull request of a case: `#`. func pullKey(p Pull) string { return fmt.Sprintf("%s#%d", p.Repository, p.Number) } func (p pieces) join(drop int) string { - parts := []string{p.head, p.commits, p.carried, p.counts, p.judged} + parts := []string{p.head, p.commits, p.carried, p.warning, p.counts, p.judged} switch { case drop < 3 && p.times != "": parts = append(parts, p.times) case drop == 3 && p.shortTimes != "": - parts = []string{p.head, p.commits, p.carried, p.shortCounts, p.judged, p.shortTimes} + parts = []string{p.head, p.commits, p.carried, p.warning, p.shortCounts, p.judged, p.shortTimes} } if p.over != "" { parts = append(parts, p.over) @@ -210,6 +212,9 @@ func carriedWords(from map[string]string, v delivery.Verdict, cs *Case, p Pull) func parts(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []string, judged map[string]string) pieces { var pc pieces pc.judged = judged[pullKey(p)] + if cs != nil { + pc.warning = cs.Warning(pullKey(p)) + } commits := short12(p.Head) + " on " + short12(p.Trunk) switch v.State { case delivery.VerdictRunning: From 820c458f4cf15d560661c90e884d3943b84d6078 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 19:16:25 +0100 Subject: [PATCH 3/4] The captured clean fetches carry the programs a current build module answers, so lines asserted whole do not warn (issue 1577) --- modules/mesh-delivery-checker/internal/check/examiner_test.go | 1 - .../internal/check/testdata/captured/README | 2 +- .../internal/check/testdata/captured/go-fetch-clean.json | 2 +- .../internal/check/testdata/captured/node-fetch-clean.json | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index e708dbb..c8f5b13 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -180,7 +180,6 @@ func newBench(t testing.TB) *bench { b := &bench{steps: &fakeSteps{}, kept: newMemKV(), prev: newMemKV()} b.fetch = captured[Fetched](t, "go-fetch-clean") b.fetch.Modules, b.fetch.Changed, b.fetch.Trees = []string{"."}, []string{"README.md"}, nil - b.fetch.Programs = &[]ProgramFile{} // the build module reports it (issue 1577) one := captured[Digested](t, "go-digest") b.digest = map[string]Digested{ delivery.LevelUnit: {Level: "unit", Packages: map[string]string{"fixture/slow": digA}}, diff --git a/modules/mesh-delivery-checker/internal/check/testdata/captured/README b/modules/mesh-delivery-checker/internal/check/testdata/captured/README index 58a30ed..253863c 100644 --- a/modules/mesh-delivery-checker/internal/check/testdata/captured/README +++ b/modules/mesh-delivery-checker/internal/check/testdata/captured/README @@ -10,7 +10,7 @@ TestFetchWithABaseMergesAndAnswersThePreview and TestFetchWithAConflictEndsPasse into a throwaway test that wrote them out, at mesh-catalog feat/r4c5-checker d5804927 (the git commands are the test's fake, so the shas are its constants; the shape and the field names are the module's own marshalling). node-fetch-clean.json is the FetchResult json.Marshal of mesh-build-node's fetch (fetch.go, preview.go) in its test TestFetchWithABaseMergesAndAnswersThePreview -(cmd/mesh-build-node/preview_test.go) at the same commit. Captured on 2026-10-21 for issue 1679. +(cmd/mesh-build-node/preview_test.go) at the same commit. Captured on 2026-10-21 for issue 1679. Both clean fetches carry `"programs":[]`, which a merging fetch of the build modules answers since issue 1577 (mesh-sdk buildstep.MergeInto's Programs), added by hand to the captured bytes: a fixture without it is an older build module's, and its line warns that no compiled program is refused. forge-get-file-go-mod.json is the answer of the forge's gitea_get_file (mesh-catalog modules/gitea, tools/index.ts) for modules/mesh-delivery-builder/go.mod of novox/mesh-delivery at main, and forge-compare-trunk.json the answer of gitea_compare for novox/mesh-sdk base main, head cb54756c3865 (a commit on the diff --git a/modules/mesh-delivery-checker/internal/check/testdata/captured/go-fetch-clean.json b/modules/mesh-delivery-checker/internal/check/testdata/captured/go-fetch-clean.json index ce2e947..a87c222 100644 --- a/modules/mesh-delivery-checker/internal/check/testdata/captured/go-fetch-clean.json +++ b/modules/mesh-delivery-checker/internal/check/testdata/captured/go-fetch-clean.json @@ -1 +1 @@ -{"workspace":"mvii1iwu5414e4","commit":"0123456789abcdef0123456789abcdef01234567","modules":["."],"base":"abcdef0123456789abcdef0123456789abcdef01","merge":"fedcba9876543210fedcba9876543210fedcba98","changed":["a.go","modules/m/main.go"],"trees":{"modules/m":{"tree":"1111111111111111111111111111111111111111","manifest":"ca3d163bab055381827226140568f3bef7eaac187cebd76878e0b63e9e442356","locks":{"go.sum":"c5fc83c01e92404452b986527d239140ccf9a48b88e0c268fbf38c2e1429e9c9"}}}} +{"workspace":"mvii1iwu5414e4","commit":"0123456789abcdef0123456789abcdef01234567","modules":["."],"base":"abcdef0123456789abcdef0123456789abcdef01","merge":"fedcba9876543210fedcba9876543210fedcba98","changed":["a.go","modules/m/main.go"],"programs":[],"trees":{"modules/m":{"tree":"1111111111111111111111111111111111111111","manifest":"ca3d163bab055381827226140568f3bef7eaac187cebd76878e0b63e9e442356","locks":{"go.sum":"c5fc83c01e92404452b986527d239140ccf9a48b88e0c268fbf38c2e1429e9c9"}}}} diff --git a/modules/mesh-delivery-checker/internal/check/testdata/captured/node-fetch-clean.json b/modules/mesh-delivery-checker/internal/check/testdata/captured/node-fetch-clean.json index 4c8eedb..55ecfec 100644 --- a/modules/mesh-delivery-checker/internal/check/testdata/captured/node-fetch-clean.json +++ b/modules/mesh-delivery-checker/internal/check/testdata/captured/node-fetch-clean.json @@ -1 +1 @@ -{"workspace":"mvii1sxt979f67","commit":"0123456789abcdef0123456789abcdef01234567","modules":["modules/m"],"installed":[],"refused":[],"base":"abcdef0123456789abcdef0123456789abcdef01","merge":"0123456789abcdef0123456789abcdef01234567","changed":["modules/m/package.json"],"trees":{"modules/m":{"tree":"0123456789abcdef0123456789abcdef01234567","manifest":"0581b699d9d05517e69b21f5cedbeb2eee8aba2b205280b561be731594d7feef"}}} +{"workspace":"mvii1sxt979f67","commit":"0123456789abcdef0123456789abcdef01234567","modules":["modules/m"],"installed":[],"refused":[],"base":"abcdef0123456789abcdef0123456789abcdef01","merge":"0123456789abcdef0123456789abcdef01234567","changed":["modules/m/package.json"],"programs":[],"trees":{"modules/m":{"tree":"0123456789abcdef0123456789abcdef01234567","manifest":"0581b699d9d05517e69b21f5cedbeb2eee8aba2b205280b561be731594d7feef"}}} From d74e0821552ce67eca7fc41887f81d2c2b10cf2c Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 19:47:53 +0100 Subject: [PATCH 4/4] Checker refuses a program a commit of the branch added and a later one removed: it stays in history (issue 1577) --- .../mesh-delivery-checker/internal/check/binaries.go | 4 ++++ .../internal/check/binaries_test.go | 10 ++++++++-- .../mesh-delivery-checker/internal/check/results.go | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/binaries.go b/modules/mesh-delivery-checker/internal/check/binaries.go index c81dd29..5329c61 100644 --- a/modules/mesh-delivery-checker/internal/check/binaries.go +++ b/modules/mesh-delivery-checker/internal/check/binaries.go @@ -29,6 +29,10 @@ func (st *state) checkBinariesOf(t *target) verdictPtr { } var found []string for _, p := range *t.fetched.Programs { + if p.Commit != "" { + found = append(found, fmt.Sprintf("%s: a compiled program added in %s and removed later: it stays in history; squash the branch (issue 1577)", p.Path, short12(p.Commit))) + continue + } found = append(found, p.Path+": a compiled program is build output: remove it and add it to .gitignore (issue 1577)") } if len(found) > 0 { diff --git a/modules/mesh-delivery-checker/internal/check/binaries_test.go b/modules/mesh-delivery-checker/internal/check/binaries_test.go index 6b9c3b4..2a3ab5e 100644 --- a/modules/mesh-delivery-checker/internal/check/binaries_test.go +++ b/modules/mesh-delivery-checker/internal/check/binaries_test.go @@ -12,7 +12,8 @@ func TestAChangeWhoseFetchReportsProgramsFailsNamingEachFile(t *testing.T) { // prog := func(p ...string) *[]ProgramFile { out := []ProgramFile{} for _, f := range p { - out = append(out, ProgramFile{Path: f, Kind: "ELF"}) + path, commit, _ := strings.Cut(f, "@") // `path@commit`: added by a commit of the branch and removed later + out = append(out, ProgramFile{Path: path, Kind: "ELF", Commit: commit}) } return &out } @@ -25,6 +26,7 @@ func TestAChangeWhoseFetchReportsProgramsFailsNamingEachFile(t *testing.T) { // {"none found", prog(), nil, false}, {"one program", prog("modules/m/m"), []string{"modules/m/m"}, false}, {"two programs", prog("a/x", "b/y"), []string{"a/x", "b/y"}, false}, + {"added and removed in the branch", prog("gone/prog@0123456789abcdef0123"), []string{"gone/prog: a compiled program added in 0123456789ab and removed later: it stays in history; squash the branch (issue 1577)"}, false}, {"an older build module reports nothing", nil, nil, true}, } { b := newBench(t) @@ -49,7 +51,11 @@ func TestAChangeWhoseFetchReportsProgramsFailsNamingEachFile(t *testing.T) { // continue } for _, p := range c.fail { - if !strings.Contains(v.Failed.Words, p+": a compiled program is build output: remove it and add it to .gitignore (issue 1577)") { + want := p + if !strings.Contains(p, " added in ") { + want = p + ": a compiled program is build output: remove it and add it to .gitignore (issue 1577)" + } + if !strings.Contains(v.Failed.Words, want) { t.Errorf("%s: words %q do not name %s", c.name, v.Failed.Words, p) } } diff --git a/modules/mesh-delivery-checker/internal/check/results.go b/modules/mesh-delivery-checker/internal/check/results.go index bc5e523..12597a7 100644 --- a/modules/mesh-delivery-checker/internal/check/results.go +++ b/modules/mesh-delivery-checker/internal/check/results.go @@ -53,6 +53,8 @@ type Fetched struct { type ProgramFile struct { Path string `json:"path"` Kind string `json:"kind"` + // Commit is set for a program a commit of the branch added and a later one removed: it stays in history once merged. + Commit string `json:"commit,omitempty"` } // GoPackage is one package of a Go test result.