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)

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
+}

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)
+			}
+		}
+	}
+}

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}},

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 {

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.