novox/mesh-delivery
19
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request 'Read a go.mod the head does not hold at the trunk, so a branch older than a new Go module passes the pin check (issue 1845)' (#21) from fix/1845-pin-read-at-trunk into main

Browse Source
This commit was merged in pull request #21.
This commit is contained in:
mesh-tools
2026-10-22 13:44:08 +00:00
parent 2b635e2138 4a0c771059
commit 0e372c87f1
4 changed files with 93 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge.go
+11
View File
@@ -4,7 +4,10 @@ import (
"context"
"encoding/json"
"fmt"
"regexp"
"strings"
"mesh-delivery-checker/internal/check"
)
// The forge's tools the checker reads with (mesh-catalog modules/gitea): gitea_get_file answers {file: {content}}, the text
@@ -16,6 +19,9 @@ const (
commitsTool = "gitea.gitea_commits"
)
// notFoundStatus is the 404 of the forge's error, `Gitea API <path>: <status> <body>`.
var notFoundStatus = regexp.MustCompile(`: 404( |$)`)
// forgeReads reads the forge through call (mesh-sdk process.Conn.Call as the checker's own account).
type forgeReads struct {
call func(key string, body any) (json.RawMessage, error)
@@ -31,6 +37,11 @@ func (f forgeReads) File(_ context.Context, repository, path, ref string) (strin
owner, repo := ownerRepo(repository)
raw, err := f.call(getFileTool, map[string]string{"owner": owner, "repo": repo, "path": path, "ref": ref})
if err != nil {
if notFoundStatus.MatchString(err.Error()) {
// The forge answers 404 for a file the ref does not hold (issue 1845). The status is matched in its own place,
// after the path's colon: the path carries a sha that may hold the digits 404.
return "", fmt.Errorf("%w: %v", check.ErrNotFound, err)
}
return "", err
}
var a struct {
modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge_test.go
+18
View File
@@ -3,10 +3,13 @@ package main
import (
"context"
"encoding/json"
"errors"
"os"
"path/filepath"
"strings"
"testing"
"mesh-delivery-checker/internal/check"
)
// capturedAnswer is what the forge's tool answered (internal/check/testdata/captured/README).
@@ -68,3 +71,18 @@ func TestTheForgeNamesTheWholeShaOfAShortCommit(t *testing.T) { // review D2
t.Error("a commit that does not start with the pin was taken")
}
}
func TestAFileTheRefDoesNotHoldIsNotFoundAndOtherFailuresAreNot(t *testing.T) { // 1845/AC1, AC4
f := forgeReads{call: func(string, any) (json.RawMessage, error) {
return nil, errors.New("Gitea API contents/checks/particulars/go.mod?ref=98ca97fc: 404 object does not exist")
}}
if _, err := f.File(context.Background(), "novox/mesh-catalog", "checks/particulars/go.mod", "98ca97fc"); !errors.Is(err, check.ErrNotFound) {
t.Errorf("a 404 is %v, want ErrNotFound", err)
}
f = forgeReads{call: func(string, any) (json.RawMessage, error) {
return nil, errors.New("Gitea API contents/go.mod?ref=1111111111111111111111111111111404abcde: 500 internal error")
}}
if _, err := f.File(context.Background(), "novox/mesh-catalog", "go.mod", "main"); err == nil || errors.Is(err, check.ErrNotFound) {
t.Errorf("a 500 is %v, want a failure that is not ErrNotFound", err)
}
}
modules/mesh-delivery-checker/internal/check/pins.go
+12 -2
View File
@@ -2,6 +2,7 @@ package check
import (
"context"
"errors"
"fmt"
"path"
"regexp"
@@ -22,6 +23,10 @@ const (
// Issue1148 is why a pin that only a head carries fails the check.
const Issue1148 = "issue 1148"
// ErrNotFound is what Forge.File wraps when the ref holds no such file (the forge answered 404): a file the ref does not hold,
// told from a forge that could not be read (issue 1845).
var ErrNotFound = errors.New("the ref holds no such file")
// Forge reads a file of a repository at a ref, and says whether a commit is on a repository's trunk.
type Forge interface {
// File is the text of a file at a ref (a branch, a tag or a commit).
@@ -51,8 +56,9 @@ func SDKPin(goMod string) (commit string, ok bool) {
}
// checkPins refuses a Go member whose go.mod names a mesh-sdk commit that is neither on mesh-sdk's trunk nor the head of the
// group's mesh-sdk pull request. It reads each go.mod the fetch named at the member's head from the forge. It answers nil
// when every pin is the group's.
// group's mesh-sdk pull request. It reads each go.mod the fetch named from the forge: the one the check builds, which is the
// head's when the head holds the file and the trunk's when it does not, since a module added on the trunk after the branch was
// cut is in the merge and not at the head (issue 1845). It answers nil when every pin is the group's.
func (st *state) checkPins(ctx context.Context) verdictPtr {
if st.j.Forge == nil {
return nil
@@ -66,6 +72,10 @@ func (st *state) checkPins(ctx context.Context) verdictPtr {
file := path.Join(dir, "go.mod")
st.c.Progress.Set(fmt.Sprintf("read %s of %s#%d", file, t.pull.Repository, t.pull.Number), t.node)
text, err := st.j.Forge.File(ctx, t.pull.Repository, file, t.pull.Head)
if errors.Is(err, ErrNotFound) && t.pull.Trunk != "" {
// Only a file the head does not hold is read at the trunk; any other failure fails the check as it did.
text, err = st.j.Forge.File(ctx, t.pull.Repository, file, t.pull.Trunk)
}
if err != nil {
return failed(forgeMachine(err, t.node), fmt.Sprintf("%s#%d: %s cannot be read to check its mesh-sdk pin: %v", t.pull.Repository, t.pull.Number, file, err))
}
modules/mesh-delivery-checker/internal/check/pins_test.go
+52 -1
View File
@@ -17,16 +17,20 @@ type fakeForge struct {
goMods map[string]string // `<repository> <path> <ref>` -> text
onTrunk map[string]bool // by commit prefix
err error
errAt map[string]error // by ref: a read at that ref fails with it
}
func (f *fakeForge) File(_ context.Context, repository, path, ref string) (string, error) {
if f.err != nil {
return "", f.err
}
if e := f.errAt[ref]; e != nil {
return "", e
}
if s, ok := f.goMods[repository+" "+path+" "+ref]; ok {
return s, nil
}
return "", fmt.Errorf("no file %s of %s at %s", path, repository, ref)
return "", fmt.Errorf("no file %s of %s at %s: %w", path, repository, ref, ErrNotFound)
}
func (f *fakeForge) FullSHA(_ context.Context, _ string, commit string) (string, error) {
@@ -153,6 +157,53 @@ func TestAPinThatCannotBeCheckedFailsAndNeverPasses(t *testing.T) { // ADR 0453
}
}
// trunkOnlyBench is a group of mesh-delivery#7 whose fetch holds two Go modules: modules/m, which the head holds, and
// modules/new, which only the trunk holds (added on the trunk after the branch was cut). The SDK pin of each go.mod is given.
func trunkOnlyBench(t *testing.T, headPin, trunkPin string) *bench {
b := newBench(t)
b.fetch.Modules = []string{"modules/m", "modules/new"}
forge := &fakeForge{goMods: map[string]string{
repoA + " modules/m/go.mod " + head1: capturedGoMod(t, headPin),
repoA + " modules/m/go.mod " + trunkA: capturedGoMod(t, headPin),
repoA + " modules/new/go.mod " + trunkA: capturedGoMod(t, trunkPin),
}, onTrunk: map[string]bool{"cb54756c3865": true}}
b.ex.Forge = forge
return b
}
func TestAModuleOnlyTheTrunkHoldsIsReadAtTheTrunkAndPasses(t *testing.T) { // 1845/AC1
b := trunkOnlyBench(t, "cb54756c3865", "cb54756c3865")
if v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict; v.State != delivery.VerdictPass {
t.Fatalf("verdict = %+v", v)
}
}
func TestAStalePinOfAModuleOnlyTheTrunkHoldsStillFailsNamingTheFileAndTheCommit(t *testing.T) { // 1845/AC2
b := trunkOnlyBench(t, "cb54756c3865", "9aaa5c80ac5c")
v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict
if v.State != delivery.VerdictFail || !strings.Contains(v.Failed.Words, repoA+"#7") || !strings.Contains(v.Failed.Words, "modules/new/go.mod") || !strings.Contains(v.Failed.Words, "9aaa5c80ac5c") {
t.Fatalf("verdict = %+v", v)
}
}
func TestAModuleTheHeadHoldsIsJudgedByTheHeadsPin(t *testing.T) { // 1845/AC3
b := trunkOnlyBench(t, "9aaa5c80ac5c", "cb54756c3865") // the head's pin is stale, the trunk's go.mod of the same module is fine
b.ex.Forge.(*fakeForge).goMods[repoA+" modules/m/go.mod "+trunkA] = capturedGoMod(t, "cb54756c3865")
v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict
if v.State != delivery.VerdictFail || !strings.Contains(v.Failed.Words, "modules/m/go.mod") || !strings.Contains(v.Failed.Words, "9aaa5c80ac5c") {
t.Fatalf("verdict = %+v", v)
}
}
func TestAReadThatFailsForAnotherReasonStillFailsAndIsNotTriedAtTheTrunk(t *testing.T) { // 1845/AC4
b := trunkOnlyBench(t, "cb54756c3865", "cb54756c3865")
b.ex.Forge.(*fakeForge).errAt = map[string]error{head1: fmt.Errorf("the forge answered 500")}
v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict
if v.State != delivery.VerdictFail || !strings.Contains(v.Failed.Words, "modules/m/go.mod") || !strings.Contains(v.Failed.Words, "the forge answered 500") {
t.Fatalf("verdict = %+v", v)
}
}
type runsAt map[string]string
func (r runsAt) CommitOf(module string) string { return r[module] }