From 9194e15072bc15040f0d84fb163688bd2d21ccd5 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 16:19:23 +0200 Subject: [PATCH 1/3] Fetch a Go pin on a pull request's head: place it beside the workspace and hand it to Go by a modfile replace (issue 1148) --- modules/mesh-build-go/README.md | 6 + .../mesh-build-go/cmd/mesh-build-go/build.go | 2 + .../mesh-build-go/cmd/mesh-build-go/fetch.go | 43 ++++- .../mesh-build-go/cmd/mesh-build-go/pins.go | 169 ++++++++++++++++ .../mesh-build-go/pins_integration_test.go | 53 ++++++ .../cmd/mesh-build-go/pins_test.go | 180 ++++++++++++++++++ .../cmd/mesh-build-go/preview.go | 7 +- 7 files changed, 452 insertions(+), 8 deletions(-) create mode 100644 modules/mesh-build-go/cmd/mesh-build-go/pins.go create mode 100644 modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go create mode 100644 modules/mesh-build-go/cmd/mesh-build-go/pins_test.go diff --git a/modules/mesh-build-go/README.md b/modules/mesh-build-go/README.md index 9874d55c..49d164c3 100644 --- a/modules/mesh-build-go/README.md +++ b/modules/mesh-build-go/README.md @@ -40,6 +40,12 @@ with `GOTOOLCHAIN=local`, `GOFLAGS=-mod=mod` and the module's cache directory. E environment built from scratch (`StepRunner`): `PATH`, `HOME` and what the step passes. Workspaces older than a day are removed. +A pin on a pull request's head (issue 1148): when `go mod download` fails on a `git.novox.be/novox/*` pseudo-version, the +fetch looks for that commit among the forge's `refs/pull/*/head`, places the repository at it under the workspace's +`pinned/beside/` as a plain directory, and hands it to Go (the fetch's commands, and the build with `GOPROXY=off`) through a +temporary `-modfile` in the workspace's `pins/` that replaces the module by that directory. A pin Go can fetch is left as +it is, a pin on no ref fails naming the module and the commit, and a directory that vendors is never resolved. + The account's ssh files (`files/forge`, `files/known_hosts`, `files/gitconfig`) are filled on the node by the provisioner's client from the module's entry of `forge-account` (the manifest says `moved`, ADR 0466 §4, ADR 0400 §5). The forge writes that entry only once the module has run and reported its key (ADR 0400 §2), so until then the client diff --git a/modules/mesh-build-go/cmd/mesh-build-go/build.go b/modules/mesh-build-go/cmd/mesh-build-go/build.go index f6839629..0ed375c3 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/build.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/build.go @@ -172,6 +172,8 @@ func (m *Module) buildEnv(dir string) []string { mod := "-mod=mod" if _, err := os.Stat(filepath.Join(dir, "vendor", "modules.txt")); err == nil { mod = "-mod=vendor" + } else if modfile, ok := m.pinModfile(dir); ok { + mod += " -modfile=" + modfile // a pin on a pull request's head the fetch placed beside the workspace (pins.go) } return []string{ "CGO_ENABLED=0", diff --git a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go index a6f1ef7c..c273c230 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go @@ -127,16 +127,47 @@ func (m *Module) fetch(ctx context.Context, args map[string]any) (any, error) { if _, err := os.Stat(filepath.Join(tree, d, "vendor", "modules.txt")); err == nil { continue // a module that vendors resolves nothing } - for _, verb := range []string{"download", "verify"} { - r := m.Run(ctx, m.goEnv(), "go", "-C", filepath.Join(tree, d), "mod", verb) - if r.Status != 0 || r.Err != "" { - return nil, fmt.Errorf("go mod %s in %s: %s", verb, d, causeLine(r.Err+"\n"+r.Stdout+"\n"+r.Stderr)) - } + if err := m.downloadModules(ctx, root, filepath.Join(tree, d), d); err != nil { + return nil, err } } return res, nil } +// downloadModules downloads and verifies the Go modules of the directory dir (d in the tree). A pin of a forge module +// that Go cannot fetch is looked for among the forge's pull request heads and handed to Go by a modfile (pins.go); a +// failure that names no such pin is the first failure. +func (m *Module) downloadModules(ctx context.Context, root, dir, d string) error { + for { + failure := m.downloadAndVerify(ctx, dir, d) + if failure == nil { + return nil + } + progress, err := m.resolvePins(ctx, root, dir, failure.Error()) + if err != nil { + return err + } + if !progress { + return failure + } + } +} + +func (m *Module) downloadAndVerify(ctx context.Context, dir, d string) error { + for _, verb := range []string{"download", "verify"} { + cmd, env := []string{"-C", dir, "mod", verb}, m.goEnvFor(dir) + if modfile, ok := m.pinModfile(dir); ok && verb == "verify" { + // go mod verify takes no -modfile: it is run on the modfile itself, which names the placed repositories. + cmd, env = []string{"-C", filepath.Dir(modfile), "mod", verb}, m.goEnv() + } + r := m.Run(ctx, env, "go", cmd...) + if r.Status != 0 || r.Err != "" { + return fmt.Errorf("go mod %s in %s: %s", verb, d, causeLine(r.Err+"\n"+r.Stdout+"\n"+r.Stderr)) + } + } + return nil +} + // goModDirs are the directories of the tree that hold a go.mod, relative to it, in order. func (m *Module) goModDirs(ctx context.Context, tree string) ([]string, error) { r := m.Run(ctx, nil, "git", "-C", tree, "ls-files", "--", "go.mod", "*/go.mod") @@ -186,6 +217,8 @@ func (m *Module) goEnvFor(dir string) []string { env := m.goEnv() if _, err := os.Stat(filepath.Join(dir, "vendor", "modules.txt")); err == nil { env = append(env, "GOFLAGS=-mod=vendor") // the last value of a name is the one used + } else if modfile, ok := m.pinModfile(dir); ok { + env = append(env, "GOFLAGS=-mod=mod -modfile="+modfile) // a pin on a pull request's head (pins.go) } return env } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/pins.go b/modules/mesh-build-go/cmd/mesh-build-go/pins.go new file mode 100644 index 00000000..80a3dfb8 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins.go @@ -0,0 +1,169 @@ +package main + +// Pins on a pull request's head (novox/hq ADR 0482 §2, ADR 0500 §2; issue 1148): a group's Go members pin the head of +// the group's mesh-sdk pull request, a commit that only refs/pull//head of the forge carries until the merge. Go +// cannot fetch it by its pseudo-version, so a fetch that meets "unknown revision" for such a pin finds the commit among +// the forge's pull request heads, places that repository beside the workspace as a plain directory, and hands it to +// Go through a temporary -modfile, a copy of the directory's go.mod that replaces the module by that directory. The +// fetch's go commands and the build use the same modfile, and the build still runs with GOPROXY=off. A pin Go can +// fetch is not touched, and a directory that vendors is never resolved. + +import ( + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" +) + +const forgePrefix = "git.novox.be/novox/" + +var pseudoVersion = regexp.MustCompile(`[-.][0-9]{14}-([0-9a-f]{12})$`) + +// pin is a requirement of a go.mod on a commit of a repository of the forge, by a pseudo-version. +type pin struct { + module string // git.novox.be/novox/[/] + repo string // + sub string // the module's directory in the repository, "" for its root + short string // the 12 digits of the commit the pseudo-version ends in +} + +// forgePins are the pseudo-version requirements of a go.mod on the forge's own modules. +func forgePins(gomod string) []pin { + var pins []pin + for _, line := range strings.Split(gomod, "\n") { + if i := strings.Index(line, "//"); i >= 0 { + line = line[:i] + } + f := strings.Fields(line) + if len(f) > 0 && f[0] == "require" { + f = f[1:] + } + if len(f) != 2 || !strings.HasPrefix(f[0], forgePrefix) { + continue + } + v := pseudoVersion.FindStringSubmatch(f[1]) + if v == nil { + continue + } + parts := strings.SplitN(strings.TrimPrefix(f[0], forgePrefix), "/", 2) + p := pin{module: f[0], repo: parts[0], short: v[1]} + if len(parts) == 2 { + p.sub = parts[1] + } + pins = append(pins, p) + } + return pins +} + +// pinsDir is where the temporary modfile of a module directory of a workspace lies: the workspace's pins directory, +// by the directory's path in the tree. dir is outside any workspace of the work directory, and the answer empty, when +// it does not lie in //tree. +func (m *Module) pinsDir(dir string) string { + rel, err := filepath.Rel(m.Work, dir) + if err != nil { + return "" + } + parts := strings.Split(filepath.ToSlash(rel), "/") + if len(parts) < 2 || parts[0] == ".." || parts[1] != "tree" { + return "" + } + name := strings.Join(parts[2:], "_") + if name == "" { + name = "root" + } + return filepath.Join(m.Work, parts[0], "pins", name) +} + +// pinModfile is the modfile handed to Go for dir, and whether there is one: only a directory a fetch resolved a pin +// for has it. +func (m *Module) pinModfile(dir string) (string, bool) { + p := m.pinsDir(dir) + if p == "" { + return "", false + } + p = filepath.Join(p, "go.mod") + _, err := os.Stat(p) + return p, err == nil +} + +// resolvePins resolves the pins of dir that a go command failed on: those whose commit the failure names. It answers +// false when there is none, and the failure is then the caller's. +func (m *Module) resolvePins(ctx context.Context, root, dir, failure string) (bool, error) { + raw, err := os.ReadFile(filepath.Join(dir, "go.mod")) + if err != nil { + return false, nil + } + modfile, _ := m.pinModfile(dir) + done := "" + if modfile != "" { + if b, err := os.ReadFile(modfile); err == nil { + done = string(b) + } + } + progress := false + for _, p := range forgePins(string(raw)) { + if !strings.Contains(failure, p.short) || strings.Contains(done, "replace "+p.module+" =>") { + continue + } + where, err := m.placePin(ctx, root, p) + if err != nil { + return false, err + } + if err := m.replaceInModfile(dir, p.module, where); err != nil { + return false, err + } + progress = true + } + return progress, nil +} + +// 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. +func (m *Module) placePin(ctx context.Context, root string, p pin) (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") + if r.Status != 0 || r.Err != "" { + return "", fmt.Errorf("the pin of %s at commit %s: listing the pull requests of %s: %s", p.module, p.short, repository, firstLine(r.Err+" "+r.Stderr)) + } + commit, ref := "", "" + for _, l := range strings.Split(r.Stdout, "\n") { + if sha, name, ok := strings.Cut(strings.TrimSpace(l), "\t"); ok && commitOK.MatchString(sha) && strings.HasPrefix(sha, p.short) && strings.HasSuffix(name, "/head") { + commit, ref = sha, name + break + } + } + if commit == "" { + return "", fmt.Errorf("the pin of %s at commit %s is on no branch, tag or pull request head of %s on the forge", p.module, p.short, repository) + } + if _, err := m.placeSiblings(ctx, filepath.Join(root, "pinned"), []siblingArg{{repository: repository, commit: commit, ref: ref}}); err != nil { + return "", fmt.Errorf("the pin of %s at commit %s: %w", p.module, p.short, err) + } + return filepath.Join(root, "pinned", "beside", p.repo, filepath.FromSlash(p.sub)), nil +} + +// replaceInModfile adds `replace module => where` to the temporary modfile of dir, made from the directory's go.mod and +// go.sum the first time; Go reads the sums beside the modfile. +func (m *Module) replaceInModfile(dir, module, where string) error { + pins := m.pinsDir(dir) + if pins == "" { + return fmt.Errorf("the directory %s lies outside a workspace", dir) + } + if err := os.MkdirAll(pins, 0o700); err != nil { + return err + } + modfile := filepath.Join(pins, "go.mod") + body, err := os.ReadFile(modfile) + if err != nil { + if body, err = os.ReadFile(filepath.Join(dir, "go.mod")); err != nil { + return err + } + sum, _ := os.ReadFile(filepath.Join(dir, "go.sum")) + if err := os.WriteFile(filepath.Join(pins, "go.sum"), sum, 0o600); err != nil { + return err + } + } + body = append(body, []byte("\nreplace "+module+" => "+where+"\n")...) + return os.WriteFile(modfile, body, 0o600) +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go b/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go new file mode 100644 index 00000000..96941d3d --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go @@ -0,0 +1,53 @@ +//go:build integration + +package main + +// A pin on a pull request's head, on real git repositories and the real go (issue 1148); pins_test.go proves the +// commands with a fake runner. + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// 1148/AC2: given a forge with a mesh-sdk-like pull request whose branch is deleted (only refs/pull/7/head carries its +// commit) and a Go member pinning that head by a pseudo-version, when build_go_fetch then build_go_build run on the +// member, then both pass, the build with GOPROXY=off. +func TestRealFetchAndBuildOfAMemberPinningAPullHead(t *testing.T) { + needTools(t, "the pin", "go", "git") + f := newForge(t) + work, fork := f.repo("novox/dep", map[string]string{"README": "dep\n"}) + head := f.commit("novox/dep", work, fork, map[string]string{ + "go/go.mod": "module git.novox.be/novox/dep/go\n\ngo 1.25.0\n", + "go/dep.go": "package dep\n\n// Name is what the member prints.\nconst Name = \"from the pull request\"\n", + }, "refs/pull/7/head") + git(t, work, "checkout", "-q", "main") // no branch of the repository carries the head + member := map[string]string{ + "go.mod": "module fixture/member\n\ngo 1.25.0\n\nrequire git.novox.be/novox/dep/go v0.0.0-20261022120000-" + head[:12] + "\n", + "cmd/prog/main.go": "package main\n\nimport (\n\t\"fmt\"\n\n\tdep \"git.novox.be/novox/dep/go\"\n)\n\n" + + "func main() { fmt.Println(dep.Name) }\n", + "module.json": `{"module":"fixture-member","version":"1","build":{"artifacts":[{"name":"tools","kind":"bundle","language":"go","system":"arch","from":"cmd/prog","binary":"prog-bin","loads":["prog-bin"]}]}}`, + } + _, memberHead := f.repo("novox/member", member) + m, states := f.module(t) + + res, st := fetched(t, m, states, map[string]any{"repository": "novox/member", "commit": memberHead}) + if st.State != delivery.StepPassed { + t.Fatalf("the fetch ended %s: %s", st.State, st.Reason) + } + if _, err := os.Stat(filepath.Join(m.Work, res.Workspace, "pinned", "beside", "dep", "go", "dep.go")); err != nil { + t.Errorf("the pull request's repository was not placed beside the workspace: %v", err) + } + built, err := m.build(context.Background(), map[string]any{"workspace": res.Workspace, "module": ".", "artifact": "tools"}) + if err != nil { + t.Fatal(fmt.Errorf("the build: %w", err)) + } + if b := built.(BuildResult); b.Digest == "" || b.Binary != "prog-bin" { + t.Errorf("the build answered %+v", b) + } +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/pins_test.go b/modules/mesh-build-go/cmd/mesh-build-go/pins_test.go new file mode 100644 index 00000000..e48d3e83 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins_test.go @@ -0,0 +1,180 @@ +package main + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +// pinShort is the 12 digits of commitSHA a pseudo-version ends in. +const pinShort = "0123456789ab" + +const pinnedGoMod = "module fixture/member\n\ngo 1.26\n\nrequire (\n\tgit.novox.be/novox/mesh-sdk/go v0.1.15-0.20261022120000-" + pinShort + "\n)\n" + +// pinHarness runs a fetch of a tree whose only go.mod is gomod (a vendoring one when vendor is set) against a runner +// that stands for the forge and for go: a pull request's head is listed by ls-remote when pulls says so, and go mod +// download of a pseudo-version that no ref carries fails with "unknown revision" unless the command was handed a +// -modfile. +type pinHarness struct { + t *testing.T + m *Module + f *fakeStep + pulls string // what git ls-remote answers + onTrunk bool // the pin is fetchable as it is: go mod download never says unknown revision + cmds []cmdRecord +} + +func newPinHarness(t *testing.T, gomod string, vendor bool, pulls string) (*pinHarness, *keptStates) { + f := &fakeStep{gomods: "go.mod\n"} + m, states := fetchModule(t, f) + h := &pinHarness{t: t, m: m, f: f, pulls: pulls} + m.Run = func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + line := name + " " + strings.Join(args, " ") + h.cmds = append(h.cmds, cmdRecord{env: env, line: line}) + switch { + case name == "git" && strings.Contains(line, " checkout ") && !strings.Contains(line, "pinned"): + for i, a := range args { + if a == "-C" { + os.MkdirAll(args[i+1], 0o755) + os.WriteFile(filepath.Join(args[i+1], "go.mod"), []byte(gomod), 0o644) + os.WriteFile(filepath.Join(args[i+1], "go.sum"), []byte("sum\n"), 0o644) + if vendor { + os.MkdirAll(filepath.Join(args[i+1], "vendor"), 0o755) + os.WriteFile(filepath.Join(args[i+1], "vendor", "modules.txt"), nil, 0o644) + } + } + } + case name == "git" && strings.Contains(line, "ls-remote"): + return forgekey.Ran{Stdout: h.pulls} + case name == "go" && strings.Contains(line, " mod download"): + if !h.onTrunk && !strings.Contains(strings.Join(env, " "), "-modfile=") { + return forgekey.Ran{Status: 1, Stderr: "go: git.novox.be/novox/mesh-sdk/go@v0.1.15-0.20261022120000-" + pinShort + ": invalid version: unknown revision " + pinShort + "\n"} + } + } + return f.run(ctx, env, name, args...) + } + return h, states +} + +func (h *pinHarness) fetch(states *keptStates) (FetchResult, delivery.Step) { + h.t.Helper() + if _, err := fetchTool(h.t, h.m)(fetchArgs); err != nil { + h.t.Fatal(err) + } + st := states.waitEnd(h.t) + var res FetchResult + if st.State == delivery.StepPassed { + json.Unmarshal(st.Result, &res) + } + return res, st +} + +func (h *pinHarness) ran(sub string) bool { + for _, c := range h.cmds { + if strings.Contains(c.line, sub) { + return true + } + } + return false +} + +const pullList = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\trefs/pull/6/head\n" + commitSHA + "\trefs/pull/7/head\n" + +// 1148/AC1: a go.mod requiring mesh-sdk at a pseudo-version only a pull request's head carries is fetched: the +// repository is placed beside the workspace at that commit, go mod download and verify succeed through a temporary +// -modfile replacing the module by that directory, and the build is handed the same -modfile with GOPROXY=off. +func TestAPinOnAPullHeadIsPlacedBesideAndHandedToGoByAModfile(t *testing.T) { + h, states := newPinHarness(t, pinnedGoMod, false, pullList) + res, st := h.fetch(states) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + placed := filepath.Join(h.m.Work, res.Workspace, "pinned", "beside", "mesh-sdk") + if !h.ran("-C " + placed + " checkout -q --detach " + commitSHA) { + t.Errorf("mesh-sdk was not placed at %s at the commit:\n%s", placed, strings.Join(h.f.lines(), "\n")) + } + if !h.ran("refs/pull/*/head") { + t.Errorf("the forge's pull request heads were not listed") + } + dir := filepath.Join(h.m.Work, res.Workspace, "tree") + flags := "" + for _, e := range h.m.goEnvFor(dir) { + if strings.HasPrefix(e, "GOFLAGS=") { + flags = e + } + } + _, modfile, ok := strings.Cut(flags, "-modfile=") + if !ok { + t.Fatalf("go is not handed a -modfile: %v", h.m.goEnvFor(dir)) + } + raw, err := os.ReadFile(strings.Fields(modfile)[0]) + if err != nil { + t.Fatal(err) + } + if want := "replace git.novox.be/novox/mesh-sdk/go => " + filepath.Join(placed, "go"); !strings.Contains(string(raw), want) || !strings.Contains(string(raw), "module fixture/member") { + t.Errorf("the modfile lacks %q:\n%s", want, raw) + } + build := strings.Join(h.m.buildEnv(dir), " ") + if !strings.Contains(build, "-modfile="+strings.Fields(modfile)[0]) || !strings.Contains(build, "GOPROXY=off") { + t.Errorf("the build is not handed the modfile with GOPROXY=off: %s", build) + } + if !h.ran(" mod verify") { + t.Errorf("go mod verify did not run") + } +} + +// 1148/AC3: a pin go downloads as it is (on the trunk or a tag) behaves as today: no listing of pull requests, no +// repository placed, no -modfile in the fetch or the build. +func TestAPinGoCanDownloadIsLeftAsToday(t *testing.T) { + h, states := newPinHarness(t, pinnedGoMod, false, pullList) + h.onTrunk = true + res, st := h.fetch(states) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + if h.ran("ls-remote") || h.ran("pinned") { + t.Errorf("a pin on the trunk made the fetch look for a pull request or place a repository:\n%s", strings.Join(h.f.lines(), "\n")) + } + dir := filepath.Join(h.m.Work, res.Workspace, "tree") + for _, e := range append(h.m.goEnvFor(dir), h.m.buildEnv(dir)...) { + if strings.Contains(e, "-modfile") { + t.Errorf("a -modfile is handed to go: %s", e) + } + } + if !h.ran(" mod download") || !h.ran(" mod verify") { + t.Errorf("go mod download and verify did not run as before") + } +} + +// 1148/AC4: a pin that no ref of the forge carries ends the step failed, naming the module and the commit. +func TestAPinOnNoRefFailsNamingTheModuleAndTheCommit(t *testing.T) { + h, states := newPinHarness(t, pinnedGoMod, false, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\trefs/pull/6/head\n") + _, st := h.fetch(states) + if st.State != delivery.StepFailed { + t.Fatalf("the step ended %s, want failed", st.State) + } + if !strings.Contains(st.Reason, "git.novox.be/novox/mesh-sdk/go") || !strings.Contains(st.Reason, pinShort) { + t.Errorf("the reason does not name the module and the commit: %q", st.Reason) + } + if !h.ran("ls-remote") { + t.Errorf("the forge's pull request heads were never looked at") + } +} + +// 1148/AC5: a directory that vendors resolves nothing: no go command, no listing, no repository placed. +func TestAVendoringDirectoryResolvesNothing(t *testing.T) { + h, states := newPinHarness(t, pinnedGoMod, true, pullList) + _, st := h.fetch(states) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + if h.ran("go -C") || h.ran("ls-remote") || h.ran("pinned") { + t.Errorf("a vendoring directory was resolved:\n%s", strings.Join(h.f.lines(), "\n")) + } +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/preview.go b/modules/mesh-build-go/cmd/mesh-build-go/preview.go index 14ab2b81..32fbd3af 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/preview.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/preview.go @@ -47,7 +47,8 @@ func (m *Module) merge(ctx context.Context, res *FetchResult, tree, remote, base return err } -type siblingArg struct{ repository, commit string } +// ref is the pull request head carrying the commit, when only it does (a pin on a pull request, issue 1148). +type siblingArg struct{ repository, commit, ref string } func siblingsArg(v any) ([]siblingArg, error) { if v == nil { @@ -74,7 +75,7 @@ func siblingsArg(v any) ([]siblingArg, error) { return nil, fmt.Errorf("beside: two repositories are named %s", name) } seen[name] = true - out = append(out, siblingArg{repository, commit}) + out = append(out, siblingArg{repository: repository, commit: commit}) } return out, nil } @@ -94,7 +95,7 @@ func (m *Module) placeSiblings(ctx context.Context, root string, siblings []sibl if err := os.MkdirAll(dir, 0o700); err != nil { return nil, err } - if err := buildstep.FetchCommit(ctx, m.Run, dir, m.remote(s.repository), s.commit, "", m.FetchOptions...); err != nil { + if err := buildstep.FetchCommit(ctx, m.Run, dir, m.remote(s.repository), s.commit, s.ref, m.FetchOptions...); err != nil { return nil, fmt.Errorf("beside %s: %w", s.repository, err) } if link := firstSymlink(dir); link != "" { From 2d8c8f6d755ddbad5c595cac81d5675aac05d3e2 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 16:24:18 +0200 Subject: [PATCH 2/3] Share one placement per repository and commit for pins on pull heads; say a pin is on no pull request head and what go said (issue 1148) --- .../mesh-build-go/cmd/mesh-build-go/pins.go | 16 ++-- .../mesh-build-go/pins_integration_test.go | 8 +- .../cmd/mesh-build-go/pins_test.go | 74 ++++++++++++++++++- 3 files changed, 88 insertions(+), 10 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 80a3dfb8..4247d447 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/pins.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins.go @@ -107,7 +107,7 @@ func (m *Module) resolvePins(ctx context.Context, root, dir, failure string) (bo if !strings.Contains(failure, p.short) || strings.Contains(done, "replace "+p.module+" =>") { continue } - where, err := m.placePin(ctx, root, p) + where, err := m.placePin(ctx, root, p, failure) if err != nil { return false, err } @@ -121,7 +121,7 @@ func (m *Module) resolvePins(ctx context.Context, root, dir, failure string) (bo // 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. -func (m *Module) placePin(ctx context.Context, root string, p pin) (string, error) { +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") if r.Status != 0 || r.Err != "" { @@ -135,12 +135,18 @@ func (m *Module) placePin(ctx context.Context, root string, p pin) (string, erro } } if commit == "" { - return "", fmt.Errorf("the pin of %s at commit %s is on no branch, tag or pull request head of %s on the forge", p.module, p.short, repository) + 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)) } - if _, err := m.placeSiblings(ctx, filepath.Join(root, "pinned"), []siblingArg{{repository: repository, commit: commit, ref: ref}}); err != nil { + // One placement per repository and commit, shared by every go.mod directory of the tree that pins it. + at := filepath.Join(root, "pinned", p.repo+"@"+commit) + where := filepath.Join(at, "beside", p.repo, filepath.FromSlash(p.sub)) + if _, err := os.Stat(filepath.Join(at, "beside", p.repo)); err == nil { + return where, nil + } + if _, err := m.placeSiblings(ctx, at, []siblingArg{{repository: repository, commit: commit, ref: ref}}); err != nil { return "", fmt.Errorf("the pin of %s at commit %s: %w", p.module, p.short, err) } - return filepath.Join(root, "pinned", "beside", p.repo, filepath.FromSlash(p.sub)), nil + return where, nil } // replaceInModfile adds `replace module => where` to the temporary modfile of dir, made from the directory's go.mod and diff --git a/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go b/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go index 96941d3d..f61519e5 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "os" + "os/exec" "path/filepath" "testing" @@ -19,7 +20,12 @@ import ( // commit) and a Go member pinning that head by a pseudo-version, when build_go_fetch then build_go_build run on the // member, then both pass, the build with GOPROXY=off. func TestRealFetchAndBuildOfAMemberPinningAPullHead(t *testing.T) { - needTools(t, "the pin", "go", "git") + // Not skipped when a tool is missing: a pass of the check must mean this test ran (the check prints no test log). + for _, tool := range []string{"go", "git"} { + if _, err := exec.LookPath(tool); err != nil { + t.Fatalf("no %s where this check runs: the pin on a pull request's head cannot be proven", tool) + } + } f := newForge(t) work, fork := f.repo("novox/dep", map[string]string{"README": "dep\n"}) head := f.commit("novox/dep", work, fork, map[string]string{ diff --git a/modules/mesh-build-go/cmd/mesh-build-go/pins_test.go b/modules/mesh-build-go/cmd/mesh-build-go/pins_test.go index e48d3e83..bbdc0ff7 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/pins_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins_test.go @@ -42,8 +42,12 @@ func newPinHarness(t *testing.T, gomod string, vendor bool, pulls string) (*pinH for i, a := range args { if a == "-C" { os.MkdirAll(args[i+1], 0o755) - os.WriteFile(filepath.Join(args[i+1], "go.mod"), []byte(gomod), 0o644) - os.WriteFile(filepath.Join(args[i+1], "go.sum"), []byte("sum\n"), 0o644) + for _, g := range strings.Fields(f.gomods) { + d := filepath.Join(args[i+1], filepath.Dir(g)) + os.MkdirAll(d, 0o755) + os.WriteFile(filepath.Join(d, "go.mod"), []byte(gomod), 0o644) + os.WriteFile(filepath.Join(d, "go.sum"), []byte("sum\n"), 0o644) + } if vendor { os.MkdirAll(filepath.Join(args[i+1], "vendor"), 0o755) os.WriteFile(filepath.Join(args[i+1], "vendor", "modules.txt"), nil, 0o644) @@ -95,7 +99,7 @@ func TestAPinOnAPullHeadIsPlacedBesideAndHandedToGoByAModfile(t *testing.T) { if st.State != delivery.StepPassed { t.Fatalf("the step ended %s: %s", st.State, st.Reason) } - placed := filepath.Join(h.m.Work, res.Workspace, "pinned", "beside", "mesh-sdk") + placed := filepath.Join(h.m.Work, res.Workspace, "pinned", "mesh-sdk@"+commitSHA, "beside", "mesh-sdk") if !h.ran("-C " + placed + " checkout -q --detach " + commitSHA) { t.Errorf("mesh-sdk was not placed at %s at the commit:\n%s", placed, strings.Join(h.f.lines(), "\n")) } @@ -159,7 +163,8 @@ func TestAPinOnNoRefFailsNamingTheModuleAndTheCommit(t *testing.T) { if st.State != delivery.StepFailed { t.Fatalf("the step ended %s, want failed", st.State) } - if !strings.Contains(st.Reason, "git.novox.be/novox/mesh-sdk/go") || !strings.Contains(st.Reason, pinShort) { + if !strings.Contains(st.Reason, "no pull request head") || !strings.Contains(st.Reason, "unknown revision") || + !strings.Contains(st.Reason, "git.novox.be/novox/mesh-sdk/go") || !strings.Contains(st.Reason, pinShort) { t.Errorf("the reason does not name the module and the commit: %q", st.Reason) } if !h.ran("ls-remote") { @@ -178,3 +183,64 @@ func TestAVendoringDirectoryResolvesNothing(t *testing.T) { t.Errorf("a vendoring directory was resolved:\n%s", strings.Join(h.f.lines(), "\n")) } } + +// 1148/AC1 (a repository with several go.mod directories, as mesh-catalog has): two directories pinning the same pull +// head share one placement and both are handed a modfile; the fetch does not fail placing it twice. +func TestTwoDirectoriesPinningOneHeadShareOnePlacement(t *testing.T) { + h, states := newPinHarness(t, pinnedGoMod, false, pullList) + h.f.gomods = "go.mod\nsub/go.mod\n" + res, st := h.fetch(states) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + checkouts := 0 + for _, c := range h.cmds { + if strings.Contains(c.line, "pinned") && strings.Contains(c.line, " checkout ") { + checkouts++ + } + } + if checkouts != 1 { + t.Errorf("the repository was placed %d times, want once:\n%s", checkouts, strings.Join(h.f.lines(), "\n")) + } + for _, d := range []string{"tree", "tree/sub"} { + if _, ok := h.m.pinModfile(filepath.Join(h.m.Work, res.Workspace, d)); !ok { + t.Errorf("%s was not handed a modfile", d) + } + } +} + +// 1148/AC1: two directories pinning two different commits of one repository get two placements, each its own. +func TestTwoCommitsOfOneRepositoryGetTwoPlacements(t *testing.T) { + other := "fedcba9876543210fedcba9876543210fedcba98" + h, states := newPinHarness(t, pinnedGoMod, false, commitSHA+"\trefs/pull/7/head\n"+other+"\trefs/pull/8/head\n") + h.f.gomods = "go.mod\nsub/go.mod\n" + base := h.m.Run + h.m.Run = func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + if name == "git" && strings.Contains(strings.Join(args, " "), "pinned/mesh-sdk@"+other+"/") && strings.Contains(strings.Join(args, " "), " rev-parse HEAD") { + return forgekey.Ran{Stdout: other + "\n"} + } + // the second directory pins the other commit + if name == "git" && strings.Contains(strings.Join(args, " "), " checkout ") && !strings.Contains(strings.Join(args, " "), "pinned") { + r := base(ctx, env, name, args...) + for i, a := range args { + if a == "-C" { + os.WriteFile(filepath.Join(args[i+1], "sub", "go.mod"), []byte(strings.ReplaceAll(pinnedGoMod, pinShort, other[:12])), 0o644) + } + } + return r + } + if name == "go" && strings.Contains(strings.Join(args, " "), "/sub mod download") && !strings.Contains(strings.Join(env, " "), "-modfile=") { + return forgekey.Ran{Status: 1, Stderr: "go: git.novox.be/novox/mesh-sdk/go@v0.1.15-0.20261022120000-" + other[:12] + ": invalid version: unknown revision " + other[:12] + "\n"} + } + return base(ctx, env, name, args...) + } + _, st := h.fetch(states) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + for _, c := range []string{commitSHA, other} { + if !h.ran("pinned/mesh-sdk@" + c) { + t.Errorf("no placement of mesh-sdk at %s", c) + } + } +} From 555ed4963a8a44f44c549e9b917841438c85b2f7 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 16:29:37 +0200 Subject: [PATCH 3/3] Read the pinned placement at its new path in the pin integration test (issue 1148) --- .../mesh-build-go/cmd/mesh-build-go/pins_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go b/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go index f61519e5..d32cfe2b 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go @@ -46,7 +46,7 @@ func TestRealFetchAndBuildOfAMemberPinningAPullHead(t *testing.T) { if st.State != delivery.StepPassed { t.Fatalf("the fetch ended %s: %s", st.State, st.Reason) } - if _, err := os.Stat(filepath.Join(m.Work, res.Workspace, "pinned", "beside", "dep", "go", "dep.go")); err != nil { + if _, err := os.Stat(filepath.Join(m.Work, res.Workspace, "pinned", "dep@"+head, "beside", "dep", "go", "dep.go")); err != nil { t.Errorf("the pull request's repository was not placed beside the workspace: %v", err) } built, err := m.build(context.Background(), map[string]any{"workspace": res.Workspace, "module": ".", "artifact": "tools"})