From 7c5df1d7aa29165069d4057ee6daa33ddd65eba6 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 2 Nov 2026 12:49:38 +0100 Subject: [PATCH] Decide a pin on a pull request head from the forge, not from go's words The group's check has no name lookup, so go failed on the network before it could say it knew no such revision, and the fetch, reading go's words, took the asked head for a download failure. Whether a commit is a pull request's head is now the forge's fact, asked whenever go cannot fetch it; go's words only label a commit on no head (issue 1748, issue 1126 review 2). Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL --- .../mesh-build-go/cmd/mesh-build-go/pins.go | 19 ++++++++++++++++-- .../cmd/mesh-build-go/versions.go | 17 +++++++++------- .../cmd/mesh-build-go/versions_test.go | 20 ++++++++++++------- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/pins.go b/modules/mesh-build-go/cmd/mesh-build-go/pins.go index d8b2f05f..a4452e41 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/pins.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins.go @@ -10,6 +10,7 @@ package main import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -106,10 +107,14 @@ func (m *Module) resolvePins(ctx context.Context, root, dir, failure string) (bo } progress := false for _, p := range forgePins(string(raw)) { - if !unknownRevision(failure) || !strings.Contains(failure, p.short) || strings.Contains(done, "replace "+p.module+" =>") { + if !strings.Contains(failure, p.short) || strings.Contains(done, "replace "+p.module+" =>") { continue } where, err := m.placePin(ctx, root, p, failure) + if errors.Is(err, errNotAHead) { + // On no pull request head: go's failure is the caller's, in go's own words (issue 1748). + continue + } if err != nil { return false, err } @@ -121,8 +126,15 @@ func (m *Module) resolvePins(ctx context.Context, root, dir, failure string) (bo return progress, nil } +// errNotAHead is a pin whose commit no pull request head of the forge carries, when go's failure was not go knowing no +// such revision: the failure is the network's or the forge's, and the caller says it in go's words (issue 1748). +var errNotAHead = errors.New("on no pull request head") + // placePin finds the commit of a pin among the pull request heads of the forge, places the repository at it beside -// the workspace (the way a fetch places a sibling) and answers the directory of the module in it. +// the workspace (the way a fetch places a sibling) and answers the directory of the module in it. **Whether a commit is +// a head is the forge's fact** (issue 1126 review 2), never read from go's words: a group's check may have no name lookup, +// and go then fails on the network before it can say it knows no such revision. go's failure only labels a commit on no +// head: an unknown revision is said so, any other failure is errNotAHead, the caller's to say. func (m *Module) placePin(ctx context.Context, root string, p pin, failure string) (string, error) { repository := "novox/" + p.repo r := m.Run(ctx, []string{"GIT_TERMINAL_PROMPT=0"}, "git", "-c", "credential.helper=", "ls-remote", m.remote(repository), "refs/pull/*/head") @@ -137,6 +149,9 @@ func (m *Module) placePin(ctx context.Context, root string, p pin, failure strin } } if commit == "" { + if !unknownRevision(failure) { + return "", errNotAHead + } return "", fmt.Errorf("the pin of %s at commit %s is on no pull request head of %s on the forge; go said: %s", p.module, p.short, repository, causeLine(failure)) } // One placement per repository and commit, shared by every go.mod directory of the tree that pins it. diff --git a/modules/mesh-build-go/cmd/mesh-build-go/versions.go b/modules/mesh-build-go/cmd/mesh-build-go/versions.go index d580c04f..726319b6 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/versions.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/versions.go @@ -17,6 +17,7 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "maps" "os" @@ -102,16 +103,18 @@ func (m *Module) takeVersion(ctx context.Context, root, dir string, tv *TakenVer _ = json.Unmarshal([]byte(r.Stdout), &info) if r.Status != 0 || r.Err != "" || info.Version == "" || info.Error != "" { // Not a commit Go can fetch: the head of a group's pull request (ADR 0573 ยง4), placed from the forge's heads. + // Whether it is one is the forge's fact (placePin), whatever go said. failure := strings.TrimSpace(info.Error + "\n" + r.Err + "\n" + r.Stderr) - if !unknownRevision(failure) { - // A download that failed for another cause (the forge or the resolver did not answer) says nothing of the pin: it - // fails with go's own words, and not as a kind the builder holds until the module changes (issue 1748). - return fmt.Errorf("downloading the asked version %s of %s failed (a download failure that may be retaken): %s", tv.Asked, tv.Package, causeLine(failure)) - } repo, sub, _ := strings.Cut(strings.TrimPrefix(tv.Package, forgePrefix), "/") p := pin{module: tv.Package, repo: repo, sub: sub, short: tv.Asked[:12]} pins.Lock() where, err := m.placePin(ctx, root, p, failure) + if errors.Is(err, errNotAHead) { + pins.Unlock() + // On no head, and go failed for another cause (the forge or the resolver did not answer): it fails with go's own + // words, and not as a kind the builder holds until the module changes (issue 1748). + return fmt.Errorf("downloading the asked version %s of %s failed (a download failure that may be retaken): %s", tv.Asked, tv.Package, causeLine(failure)) + } if err == nil { err = m.replaceInModfile(dir, tv.Package, where) } @@ -270,8 +273,8 @@ func comparePre(a, b string) int { } // unknownRevision says whether a go failure is go not knowing the revision (cmd/go's "unknown revision" and the -// "invalid version" it wraps it in): the only failure that means a commit no tag or trunk holds, which may be the head of a -// pull request. Every other failure is the network's or the forge's. +// "invalid version" it wraps it in): it labels a commit the forge holds on no pull request head (placePin), and decides +// nothing; every other failure is the network's or the forge's. func unknownRevision(failure string) bool { return strings.Contains(failure, "unknown revision") || strings.Contains(failure, "invalid version") } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/versions_test.go b/modules/mesh-build-go/cmd/mesh-build-go/versions_test.go index 00c82a64..b5288c37 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/versions_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/versions_test.go @@ -20,6 +20,7 @@ import ( func TestAnAskedVersionIsWrittenIntoTheModfileWhateverGoModNames(t *testing.T) { const sdk = "git.novox.be/novox/mesh-sdk/go" trunk := strings.Repeat("a", 40) + offHead := strings.Repeat("b", 40) // a commit no pull request's head carries for _, c := range []struct { name, asked, answer string failing bool @@ -35,11 +36,16 @@ func TestAnAskedVersionIsWrittenIntoTheModfileWhateverGoModNames(t *testing.T) { {name: "a group's head Go cannot fetch", asked: commitSHA, failing: true, answer: `{"Path":"` + sdk + `","Error":"invalid version: unknown revision ` + pinShort + `"}`, built: commitSHA, said: "placed from the pull request's head"}, - // The wording of go's errors: "unknown revision" is in cmd/go/internal/modfetch/coderepo_test.go; the lookup and - // connection texts are those of issue 1748's report and of the dial errors go prints for an unreachable forge. - {name: "a lookup that times out is no pin", asked: commitSHA, failing: true, failsWith: "i/o timeout", - answer: `{"Path":"` + sdk + `","Error":"git.novox.be/novox/mesh-sdk/go@` + commitSHA + `: dial tcp: lookup git.novox.be on 10.10.0.3:53: read udp 10.10.0.3:42292->10.10.0.3:53: i/o timeout"}`}, - {name: "a refused connection is no pin", asked: commitSHA, failing: true, failsWith: "connection refused", + // Whether a commit is a pull request's head is the forge's fact, never read from go's words (issue 1126 review 2): + // a head is placed whatever go said (the group's check has no name lookup, so go fails on the network first), and + // go's words only label a commit on no head. The wording: "unknown revision" is cmd/go's + // (modfetch/coderepo_test.go); the lookup and connection texts are those of issue 1748 and the check of 12:42. + {name: "a head go cannot even look up is placed", asked: commitSHA, failing: true, + answer: `{"Path":"` + sdk + `","Error":"git.novox.be/novox/mesh-sdk/go@` + commitSHA + `: dial tcp: lookup git.novox.be on [::1]:53: connection refused"}`, + built: commitSHA, said: "placed from the pull request's head"}, + {name: "a lookup that times out is no pin", asked: offHead, failing: true, failsWith: "i/o timeout", + answer: `{"Path":"` + sdk + `","Error":"git.novox.be/novox/mesh-sdk/go@` + offHead + `: dial tcp: lookup git.novox.be on 10.10.0.3:53: read udp 10.10.0.3:42292->10.10.0.3:53: i/o timeout"}`}, + {name: "a refused connection is no pin", asked: offHead, failing: true, failsWith: "connection refused", answer: `{"Path":"` + sdk + `","Error":"git ls-remote -q origin in /tmp/gopath: dial tcp 10.10.0.1:443: connect: connection refused"}`}, } { t.Run(c.name, func(t *testing.T) { @@ -78,8 +84,8 @@ func TestAnAskedVersionIsWrittenIntoTheModfileWhateverGoModNames(t *testing.T) { st := states.waitEnd(t) if c.failsWith != "" { if st.State != delivery.StepFailed || !strings.Contains(st.Reason, c.failsWith) || strings.Contains(st.Reason, "no pull request head") || - !strings.Contains(st.Reason, "may be retaken") || delivery.FailedUntilChanged(st.Reason) || h.ran("ls-remote") { - t.Errorf("the download failure ended %s: %q (ls-remote ran: %v)", st.State, st.Reason, h.ran("ls-remote")) + !strings.Contains(st.Reason, "may be retaken") || delivery.FailedUntilChanged(st.Reason) { + t.Errorf("the download failure ended %s: %q", st.State, st.Reason) } return }