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 b8392a58..37cb8b85 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go @@ -6,12 +6,15 @@ package main import ( "context" + "crypto/sha256" + "encoding/hex" "fmt" "os" "path/filepath" "regexp" "sort" "strings" + "sync" "time" "git.novox.be/novox/mesh-sdk/go/buildstep" @@ -143,11 +146,27 @@ func (m *Module) fetchInto(ctx context.Context, req buildstep.FetchRequest, id s if base != "" && res.Merge == "" { return res, nil // a conflict: the tree is at the trunk, nothing of the head is built or tested } - for _, d := range dirs { + // The directories download side by side, up to the bound (ADR 0536 §2); the first failure in directory order is the + // step's. Resolving a pin places a repository beside the workspace, which two directories may share: one at a time. + var pins sync.Mutex + errs := make([]error, len(dirs)) + slots := make(chan struct{}, m.parallel()) + var wg sync.WaitGroup + for i, d := range dirs { if _, err := os.Stat(filepath.Join(tree, d, "vendor", "modules.txt")); err == nil { continue // a module that vendors resolves nothing } - if err := m.downloadModules(ctx, root, filepath.Join(tree, d), d); err != nil { + wg.Add(1) + slots <- struct{}{} + go func() { + defer wg.Done() + defer func() { <-slots }() + errs[i] = m.downloadModules(ctx, root, filepath.Join(tree, d), d, &pins) + }() + } + wg.Wait() + for _, err := range errs { + if err != nil { return nil, err } } @@ -157,13 +176,15 @@ func (m *Module) fetchInto(ctx context.Context, req buildstep.FetchRequest, id s // 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 { +func (m *Module) downloadModules(ctx context.Context, root, dir, d string, pins *sync.Mutex) error { for { failure := m.downloadAndVerify(ctx, dir, d) if failure == nil { return nil } + pins.Lock() progress, err := m.resolvePins(ctx, root, dir, failure.Error()) + pins.Unlock() if err != nil { return err } @@ -173,8 +194,41 @@ func (m *Module) downloadModules(ctx context.Context, root, dir, d string) error } } +// verifyLocks hold one mutex per digest of a go.mod and go.sum, so two directories with the same pair verify once. +var verifyLocks sync.Map + +// verifiedMark is the file naming a pass of go mod verify for a go.mod and go.sum pair, "" when a pass is not kept: +// the module cache is only read by steps that are shut off with it read-only (ADR 0448 §3), so the pass holds only then, +// and a directory with a pin modfile verifies another file. +func (m *Module) verifiedMark(dir string) string { + if m.Cache == "" || m.Bubblewrap == nil { + return "" + } + if _, ok := m.pinModfile(dir); ok { + return "" + } + h := sha256.New() + for _, name := range []string{"go.mod", "go.sum"} { + raw, _ := os.ReadFile(filepath.Join(dir, name)) + fmt.Fprintf(h, "%s %d\n", name, len(raw)) + h.Write(raw) + } + return filepath.Join(m.Cache, "verified", hex.EncodeToString(h.Sum(nil))) +} + func (m *Module) downloadAndVerify(ctx context.Context, dir, d string) error { for _, verb := range []string{"download", "verify"} { + mark := "" + if verb == "verify" { + if mark = m.verifiedMark(dir); mark != "" { + mu, _ := verifyLocks.LoadOrStore(mark, &sync.Mutex{}) + mu.(*sync.Mutex).Lock() + defer mu.(*sync.Mutex).Unlock() + if _, err := os.Stat(mark); err == nil { + continue // this go.mod and go.sum passed before + } + } + } 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. @@ -184,6 +238,12 @@ func (m *Module) downloadAndVerify(ctx context.Context, dir, d string) error { 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)) } + if mark != "" { + // Said after the pass; a mark that cannot be written only means the next fetch verifies again. + if os.MkdirAll(filepath.Dir(mark), 0o700) == nil { + os.WriteFile(mark, nil, 0o600) + } + } } return nil } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go b/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go index d9d092c5..04874659 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go @@ -30,6 +30,7 @@ type fakeStep struct { gomods string gate chan struct{} // when set, the first git command waits for it goRule func(line string) forgekey.Ran + files map[string]string // files (relative to the tree) written at checkout: go.mod and go.sum of the fixture vendors []string // directories (relative to the tree) that get a vendor/modules.txt at checkout rule func(line string) (forgekey.Ran, bool) // answers a command before the defaults do } @@ -55,6 +56,10 @@ func (f *fakeStep) run(ctx context.Context, env []string, name string, args ...s case name == "git" && strings.Contains(line, " checkout "): for i, a := range args { if a == "-C" { + for name, text := range f.files { + os.MkdirAll(filepath.Dir(filepath.Join(args[i+1], name)), 0o755) + os.WriteFile(filepath.Join(args[i+1], name), []byte(text), 0o644) + } for _, v := range f.vendors { os.MkdirAll(filepath.Join(args[i+1], v, "vendor"), 0o755) os.WriteFile(filepath.Join(args[i+1], v, "vendor", "modules.txt"), nil, 0o644) 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 bbdc0ff7..a29e4fac 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 @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "git.novox.be/novox/mesh-sdk/go/delivery" @@ -28,6 +29,7 @@ type pinHarness struct { 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 + mu sync.Mutex } func newPinHarness(t *testing.T, gomod string, vendor bool, pulls string) (*pinHarness, *keptStates) { @@ -36,7 +38,9 @@ func newPinHarness(t *testing.T, gomod string, vendor bool, pulls string) (*pinH 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.mu.Lock() h.cmds = append(h.cmds, cmdRecord{env: env, line: line}) + h.mu.Unlock() switch { case name == "git" && strings.Contains(line, " checkout ") && !strings.Contains(line, "pinned"): for i, a := range args { diff --git a/modules/mesh-build-go/cmd/mesh-build-go/verify_once_test.go b/modules/mesh-build-go/cmd/mesh-build-go/verify_once_test.go new file mode 100644 index 00000000..6d9776b4 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/verify_once_test.go @@ -0,0 +1,112 @@ +package main + +import ( + "context" + "strings" + "sync" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +func shutOffModule(t *testing.T, f *fakeStep) (*Module, *keptStates) { + t.Helper() + m, states := fetchModule(t, f) + // A module with bubblewrap runs its steps shut off, with the module cache read-only: a pass of go mod verify holds. + m.Bubblewrap = func(context.Context) (string, error) { return "bubblewrap 0.13.0", nil } + return m, states +} + +func countVerifies(f *fakeStep) int { + n := 0 + for _, l := range f.lines() { + if strings.HasPrefix(l, "go -C ") && strings.HasSuffix(l, " mod verify") { + n++ + } + } + return n +} + +// Issue 890/AC5 (ADR 0536 §2): two directories with the same go.mod and go.sum verify once, and a later fetch with the +// same pair verifies no time; a different go.mod verifies again. +func TestGoModVerifyRunsOncePerGoModAndGoSumDigest(t *testing.T) { + same := map[string]string{"a/go.mod": "module a\n", "a/go.sum": "x\n", "b/go.mod": "module a\n", "b/go.sum": "x\n"} + f := &fakeStep{gomods: "a/go.mod\nb/go.mod\n", files: same} + m, states := shutOffModule(t, f) + fetchOnce(t, m, states, fetchArgs) + if n := countVerifies(f); n != 1 { + t.Errorf("two directories with one go.mod and go.sum verified %d times, want 1", n) + } + fetchOnce(t, m, states, fetchArgs) + if n := countVerifies(f); n != 1 { + t.Errorf("a later fetch with the same pair verified again (%d in all)", n) + } + + g := &fakeStep{gomods: "a/go.mod\nb/go.mod\n", files: map[string]string{"a/go.mod": "module a\n", "a/go.sum": "x\n", "b/go.mod": "module b\n", "b/go.sum": "x\n"}} + m, states = shutOffModule(t, g) + fetchOnce(t, m, states, fetchArgs) + if n := countVerifies(g); n != 2 { + t.Errorf("two different go.mod verified %d times, want 2", n) + } +} + +// Without bubblewrap the module cache is not read-only to a step: a pass is never reused. +func TestAPassOfVerifyIsNotReusedWhenStepsAreNotShutOff(t *testing.T) { + f := &fakeStep{gomods: "a/go.mod\nb/go.mod\n", files: map[string]string{"a/go.mod": "m\n", "b/go.mod": "m\n"}} + m, states := fetchModule(t, f) // no Bubblewrap + fetchOnce(t, m, states, fetchArgs) + fetchOnce(t, m, states, fetchArgs) + if n := countVerifies(f); n != 4 { + t.Errorf("verified %d times, want every directory every fetch (4)", n) + } +} + +// A verify that fails leaves no mark: the next fetch verifies again. +func TestAFailedVerifyLeavesNoMark(t *testing.T) { + fail := true + f := &fakeStep{gomods: "go.mod\n", files: map[string]string{"go.mod": "m\n"}} + f.goRule = func(line string) forgekey.Ran { + if strings.HasSuffix(line, " mod verify") && fail { + return forgekey.Ran{Status: 1, Stdout: "x v1: dir has been modified\n"} + } + return forgekey.Ran{} + } + m, states := shutOffModule(t, f) + fetchTool(t, m)(fetchArgs) + states.waitEnd(t) + fail = false + fetchOnce(t, m, states, fetchArgs) + if n := countVerifies(f); n != 2 { + t.Errorf("verified %d times, want 2 (the failed one and the retry)", n) + } +} + +// Issue 890 (ADR 0536 §2): the downloads of the directories run side by side, never more at once than the bound. +func TestDownloadsRunSideBySideUpToTheBound(t *testing.T) { + var mu sync.Mutex + running, most := 0, 0 + f := &fakeStep{gomods: "a/go.mod\nb/go.mod\nc/go.mod\nd/go.mod\n", files: map[string]string{ + "a/go.mod": "a\n", "b/go.mod": "b\n", "c/go.mod": "c\n", "d/go.mod": "d\n"}} + f.goRule = func(line string) forgekey.Ran { + if strings.HasSuffix(line, " mod download") { + mu.Lock() + running++ + if running > most { + most = running + } + mu.Unlock() + time.Sleep(40 * time.Millisecond) + mu.Lock() + running-- + mu.Unlock() + } + return forgekey.Ran{} + } + m, states := fetchModule(t, f) + m.Parallel = 2 + fetchOnce(t, m, states, fetchArgs) + if most != 2 { + t.Errorf("at most %d downloads ran at once, want 2", most) + } +}