From 62a50ebcf82256b9c314f0faf1a7aba36a96a274 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 29 Oct 2026 00:51:26 +0100 Subject: [PATCH] buildstep: keep what one command of an ask writes into the caches for its later commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every command of a check ran over the shared Go build cache beneath a throwaway tmpfs layer of its own, so the digests, the vet, the unit tests and the integration compile of one ask each compiled every package again. With Isolation.CacheLayer a command writes into a layer on disk of its own and Isolated moves it into the ask's kept layer when it ends; later commands read the kept layer above the shared cache, which stays unchanged (ADR 0541 §2, ADR 0448 §3). Empty CacheLayer keeps today's throwaway layer. Ported from feat/try-speed S2 (issue 1921). --- go/buildstep/isolate.go | 124 +++++++++++++++++++++-- go/buildstep/isolate_caches_test.go | 36 ++++++- go/buildstep/isolate_integration_test.go | 19 ++++ go/buildstep/isolate_test.go | 54 ++++++++++ 4 files changed, 222 insertions(+), 11 deletions(-) diff --git a/go/buildstep/isolate.go b/go/buildstep/isolate.go index 2efa739..497f4bb 100644 --- a/go/buildstep/isolate.go +++ b/go/buildstep/isolate.go @@ -3,10 +3,13 @@ package buildstep import ( "context" "fmt" + "io/fs" "os" "path" "path/filepath" + "strconv" "strings" + "sync" "git.novox.be/novox/mesh-sdk/go/forgekey" ) @@ -29,16 +32,25 @@ const Bubblewrap = "bwrap" // - the Workspace and Writable directories read-write, the ReadOnly ones (the toolchain, the modules the fetch step // downloaded) read-only; // - the shared warm Caches read-only under a throwaway writable layer (S18), or read-write for a trunk build alone -// (WriteCaches), so a candidate's code never changes what a later build reads. +// (WriteCaches), so a candidate's code never changes what a later build reads; +// - with a CacheLayer, what one command writes into the caches kept for the later commands of the same ask (issue 1921, +// the try speed-up): the vet, the unit tests and the integration compile of one ask compile each package once. type Isolation struct { Workspace string Writable []string ReadOnly []string Caches []string WriteCaches bool - Env []string + // CacheLayer is a directory of the ask's own, in its workspace and gone with it: each command writes the caches into + // a layer of its own beneath it (overlayfs refuses an upper layer two mounts share, and an ask's commands run side by + // side), which Isolated moves into the ask's kept layer when the command ends; every later command reads the kept + // layer above the shared cache. The shared cache stays unchanged. Empty: what a command writes is thrown away. + CacheLayer string + Env []string // Hosts is the file bound as /etc/hosts; Isolated writes one naming only the loopback. Hosts string + // run is the directory of this command's own writable layers under CacheLayer; Isolated makes one per command. + run string } // etcNeeded is what of /etc a program in the step may read: who the users are, where the libraries are, the @@ -91,7 +103,7 @@ func (iso Isolation) command(env []string, name string, args ...string) ([]strin for _, group := range []struct { what string dirs []string - }{{"writable directory", iso.Writable}, {"read-only directory", iso.ReadOnly}, {"cache", iso.Caches}} { + }{{"writable directory", iso.Writable}, {"read-only directory", iso.ReadOnly}, {"cache", iso.Caches}, {"cache layer", layerDirs(iso.CacheLayer)}} { for _, d := range group.dirs { if err := checkDir(group.what, d); err != nil { return nil, err @@ -113,7 +125,7 @@ func (iso Isolation) command(env []string, name string, args ...string) ([]strin for _, d := range iso.ReadOnly { argv = append(argv, "--ro-bind", d, d) } - argv = append(argv, cacheArgs(iso.Caches, iso.WriteCaches)...) + argv = append(argv, iso.cacheArgs()...) argv = append(argv, "--bind", iso.Workspace, iso.Workspace) for _, d := range iso.Writable { argv = append(argv, "--bind", d, d) @@ -134,19 +146,107 @@ func (iso Isolation) command(env []string, name string, args ...string) ([]strin // cacheArgs binds the shared warm caches (S18): for a trunk build read-write, so the next build is warm; for any other // read-only beneath an overlay whose writes go to a tmpfs of the step's own, so the step writes as a build does and -// nothing it wrote outlives it (bubblewrap's --tmp-overlay, an overlay mount in the step's user namespace). -func cacheArgs(caches []string, write bool) []string { +// nothing it wrote outlives it (bubblewrap's --tmp-overlay, an overlay mount in the step's user namespace). With a +// CacheLayer the ask's kept layer lies above the shared cache and the command's writes go to its own layer on disk +// (--overlay), which Isolated keeps for the ask when the command ends. +func (iso Isolation) cacheArgs() []string { var argv []string - for _, c := range caches { - if write { + for i, c := range iso.Caches { + switch { + case iso.WriteCaches: argv = append(argv, "--bind", c, c) - continue + case iso.CacheLayer != "": + upper, work := iso.runLayer(i) + argv = append(argv, "--overlay-src", c, "--overlay-src", keptLayer(iso.CacheLayer, i), "--overlay", upper, work, c) + default: + argv = append(argv, "--overlay-src", c, "--tmp-overlay", c) } - argv = append(argv, "--overlay-src", c, "--tmp-overlay", c) } return argv } +func layerDirs(layer string) []string { + if layer == "" { + return nil + } + return []string{layer} +} + +// keptLayer is where the ask keeps what its commands wrote into the cache i. +func keptLayer(layer string, i int) string { return filepath.Join(layer, "kept", strconv.Itoa(i)) } + +// runLayer is the upper and work directory of this command's writes into the cache i. Command, which makes no +// directory, names the layer "run"; Isolated gives each command one of its own. +func (iso Isolation) runLayer(i int) (upper, work string) { + run := iso.run + if run == "" { + run = filepath.Join(iso.CacheLayer, "run") + } + return filepath.Join(run, strconv.Itoa(i), "upper"), filepath.Join(run, strconv.Itoa(i), "work") +} + +// layerLocks hold one mutex per cache layer, so two commands ending at once keep their writes one after the other. +var layerLocks sync.Map + +// prepareLayer makes the kept layers and a run directory of the command's own under the cache layer. +func (iso *Isolation) prepareLayer() error { + if iso.CacheLayer == "" || iso.WriteCaches || len(iso.Caches) == 0 { + return nil + } + if err := os.MkdirAll(filepath.Join(iso.CacheLayer, "runs"), 0o700); err != nil { + return err + } + run, err := os.MkdirTemp(filepath.Join(iso.CacheLayer, "runs"), "run-") + if err != nil { + return err + } + iso.run = run + for i := range iso.Caches { + upper, work := iso.runLayer(i) + for _, d := range []string{keptLayer(iso.CacheLayer, i), upper, work} { + if err := os.MkdirAll(d, 0o700); err != nil { + return err + } + } + } + return nil +} + +// keepLayer moves what the command wrote into the caches into the ask's kept layers, and removes its run directory. +// A file the command wrote replaces the kept one (a cache's newest write wins); a whiteout (a file the command +// deleted) is not kept, since a cache entry deleted by one command is no fault of a later one. A write that cannot be +// kept only means a later command builds it again. +func (iso Isolation) keepLayer() { + if iso.run == "" { + return + } + defer os.RemoveAll(iso.run) + mu, _ := layerLocks.LoadOrStore(iso.CacheLayer, &sync.Mutex{}) + mu.(*sync.Mutex).Lock() + defer mu.(*sync.Mutex).Unlock() + for i := range iso.Caches { + upper, _ := iso.runLayer(i) + kept := keptLayer(iso.CacheLayer, i) + filepath.WalkDir(upper, func(p string, d fs.DirEntry, err error) error { + if err != nil || p == upper { + return nil + } + rel, _ := filepath.Rel(upper, p) + to := filepath.Join(kept, rel) + switch { + case d.IsDir(): + if fi, err := os.Lstat(to); err == nil && !fi.IsDir() { + os.Remove(to) + } + os.MkdirAll(to, 0o700) + case d.Type().IsRegular(): + os.Rename(p, to) + } + return nil + }) + } +} + // loopbackHosts is the hosts file of a step: the loopback and nothing else. const loopbackHosts = "127.0.0.1 localhost\n::1 localhost\n" @@ -166,6 +266,10 @@ func Isolated(run forgekey.Runner, iso Isolation) forgekey.Runner { } iso := iso iso.Hosts = hosts + if err := iso.prepareLayer(); err != nil { + return forgekey.Ran{Status: 1, Err: "the step's cache layer: " + err.Error()} + } + defer iso.keepLayer() argv, err := iso.command(env, name, args...) if err != nil { return forgekey.Ran{Status: 1, Err: err.Error()} diff --git a/go/buildstep/isolate_caches_test.go b/go/buildstep/isolate_caches_test.go index 46afae5..de2d816 100644 --- a/go/buildstep/isolate_caches_test.go +++ b/go/buildstep/isolate_caches_test.go @@ -1,6 +1,9 @@ package buildstep -import "testing" +import ( + "strings" + "testing" +) // The shared warm caches are read under a throwaway writable layer (novox/hq ADR 0448; issue 1309): a candidate's step // writes into them as a build does, and nothing it writes outlives it or reaches a later build; only a trunk build @@ -27,4 +30,35 @@ func TestACandidatesStepWritesTheCachesOnlyIntoAThrowawayLayer(t *testing.T) { t.Errorf("a trunk build does not write the cache %s: %v", c, argv) } } + // 1921/S2: with a cache layer, a command reads the ask's kept layer above the shared cache and writes into a layer + // on disk of the ask's own, never into the shared cache and never into a throwaway tmpfs. + iso = Isolation{Workspace: "/w", Caches: []string{"/cache/gomod", "/cache/gobuild"}, CacheLayer: "/w/cachelayer"} + argv, err = iso.Command("go", "vet") + if err != nil { + t.Fatal(err) + } + for i, c := range iso.Caches { + if !holds(argv, "--overlay-src", c, "--overlay-src", keptLayer(iso.CacheLayer, i), "--overlay") { + t.Errorf("the cache %s is not read beneath the ask's kept layer: %v", c, argv) + } + if up := overlayOf(argv, c); up == "" || !strings.HasPrefix(up, "/w/cachelayer/") { + t.Errorf("the cache %s is written into %q, not a layer of the ask's own: %v", c, up, argv) + } + if holds(argv, "--tmp-overlay", c) || holds(argv, "--bind", c, c) { + t.Errorf("the cache %s is thrown away or written: %v", c, argv) + } + } + if _, err := (Isolation{Workspace: "/w", CacheLayer: "cachelayer"}).Command("go"); err == nil { + t.Error("a relative cache layer was taken") + } +} + +// overlayOf answers the upper directory a command line gives the cache c, "" when it gives none. +func overlayOf(argv []string, c string) string { + for i := 0; i+3 < len(argv); i++ { + if argv[i] == "--overlay" && argv[i+3] == c { + return argv[i+1] + } + } + return "" } diff --git a/go/buildstep/isolate_integration_test.go b/go/buildstep/isolate_integration_test.go index 6f7aa16..75c0c9b 100644 --- a/go/buildstep/isolate_integration_test.go +++ b/go/buildstep/isolate_integration_test.go @@ -63,4 +63,23 @@ func TestARunStepReachesNothingAndLeavesTheCachesUnchanged(t *testing.T) { if b, err := os.ReadFile(filepath.Join(workspace, "out")); r.Status != 0 || err != nil || string(b) != "built\n" { t.Fatalf("the step's write to its workspace did not stay (status %d, %s%s): %q %v", r.Status, r.Stderr, r.Err, b, err) } + + // 1921/S2: with the ask's cache layer, what one command writes into the cache the next command of the ask reads, + // above what the shared cache holds, and the shared cache stays unchanged. + asked := Isolated(forgekey.ExecRunnerFor(0), Isolation{Workspace: workspace, Caches: []string{cacheDir}, + CacheLayer: filepath.Join(workspace, "cachelayer")}) + r = asked(context.Background(), []string{"PATH=/usr/bin:/bin"}, "/bin/sh", "-c", "cat "+filepath.Join(cacheDir, "warm")+" && mkdir -p "+filepath.Join(cacheDir, "ab")+" && echo compiled > "+filepath.Join(cacheDir, "ab", "one-d")) + if r.Status != 0 || r.Err != "" || strings.TrimSpace(r.Stdout) != "warm" { + t.Fatalf("the first command of the ask did not read the shared cache and write its own (status %d, %s): %s%s", r.Status, r.Err, r.Stdout, r.Stderr) + } + r = asked(context.Background(), []string{"PATH=/usr/bin:/bin"}, "/bin/cat", filepath.Join(cacheDir, "ab", "one-d"), filepath.Join(cacheDir, "warm")) + if r.Status != 0 || r.Err != "" || r.Stdout != "compiled\nwarm\n" { + t.Fatalf("the next command of the ask did not find what the first wrote (status %d, %s): %q %s", r.Status, r.Err, r.Stdout, r.Stderr) + } + if _, err := os.Stat(filepath.Join(cacheDir, "ab")); !os.IsNotExist(err) { + t.Fatalf("the ask's commands wrote the shared cache: %v", err) + } + if err := os.RemoveAll(filepath.Join(workspace, "cachelayer")); err != nil { + t.Fatalf("the ask's cache layer cannot be removed with its workspace: %v", err) + } } diff --git a/go/buildstep/isolate_test.go b/go/buildstep/isolate_test.go index 861aafc..7a1eda3 100644 --- a/go/buildstep/isolate_test.go +++ b/go/buildstep/isolate_test.go @@ -2,7 +2,10 @@ package buildstep import ( "context" + "os" + "path/filepath" "strings" + "sync" "testing" "git.novox.be/novox/mesh-sdk/go/forgekey" @@ -112,4 +115,55 @@ func TestIsolatedRunsEachCommandThroughBubblewrap(t *testing.T) { if hosts == "" { t.Fatalf("no hosts file was bound: %v", ran) } + + // 1921/S2: with a cache layer, what one command wrote into the caches is kept for the ask's later commands, the + // shared cache stays unchanged, two commands at once write into layers of their own (overlayfs refuses an upper + // layer two mounts share), and a command's own layer is gone when it ends. + root := t.TempDir() + shared := filepath.Join(root, "gobuild") + if err := os.MkdirAll(shared, 0o700); err != nil { + t.Fatal(err) + } + layer := filepath.Join(root, "ws", "cachelayer") + var mu sync.Mutex + uppers := map[string]bool{} + gate := make(chan struct{}) + var started sync.WaitGroup + started.Add(2) + writer := func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + up := overlayOf(args, shared) + mu.Lock() + uppers[up] = true + mu.Unlock() + started.Done() + <-gate // both commands are running before either ends + os.MkdirAll(filepath.Join(up, "ab"), 0o700) + os.WriteFile(filepath.Join(up, "ab", args[len(args)-1]+"-d"), []byte("compiled"), 0o600) + return forgekey.Ran{} + } + run := Isolated(writer, Isolation{Workspace: filepath.Join(root, "ws"), Caches: []string{shared}, CacheLayer: layer}) + var wg sync.WaitGroup + for _, pkg := range []string{"one", "two"} { + wg.Add(1) + go func() { defer wg.Done(); run(context.Background(), nil, "go", "build", pkg) }() + } + started.Wait() + close(gate) + wg.Wait() + if len(uppers) != 2 { + t.Fatalf("two commands at once wrote into %d layers, want 2 of their own: %v", len(uppers), uppers) + } + for _, pkg := range []string{"one", "two"} { + if b, err := os.ReadFile(filepath.Join(keptLayer(layer, 0), "ab", pkg+"-d")); err != nil || string(b) != "compiled" { + t.Errorf("what the build of %s compiled is not kept for the ask: %q %v", pkg, b, err) + } + } + for up := range uppers { + if _, err := os.Stat(up); !os.IsNotExist(err) { + t.Errorf("the command's own layer %s outlived it: %v", up, err) + } + } + if entries, _ := os.ReadDir(shared); len(entries) != 0 { + t.Errorf("the shared cache was written: %v", entries) + } }