Merge pull request 'The checker fails any repository's change that adds a binary, in any commit (issue 1577)' (#77) from fix/1577-checker-refuses-binaries into main
This commit was merged in pull request #77.
This commit is contained in:
@@ -0,0 +1,42 @@
|
||||
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))
|
||||
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
|
||||
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 {
|
||||
return failed(t.node, fmt.Sprintf("%s: %s", key, strings.Join(found, "; ")))
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,63 @@
|
||||
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 {
|
||||
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
|
||||
}
|
||||
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},
|
||||
{"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)
|
||||
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())
|
||||
}
|
||||
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)
|
||||
}
|
||||
continue
|
||||
}
|
||||
if v.State != delivery.VerdictFail {
|
||||
t.Errorf("%s: verdict = %+v, want a failure", c.name, v)
|
||||
continue
|
||||
}
|
||||
for _, p := range c.fail {
|
||||
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)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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()
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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: `<owner/repo>#<number>`.
|
||||
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:
|
||||
|
||||
@@ -44,6 +44,17 @@ 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"`
|
||||
// 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.
|
||||
|
||||
@@ -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
|
||||
|
||||
+1
-1
@@ -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"}}}}
|
||||
|
||||
+1
-1
@@ -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"}}}
|
||||
|
||||
Reference in New Issue
Block a user