buildstep: keep what one command of an ask writes into the caches for its later commands

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

@@ -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
+	// 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).
+// nothing it wrote outlives it (bubblewrap's --tmp-overlay, an overlay mount in the step's user namespace). With a
-func cacheArgs(caches []string, write bool) []string {
+// 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 {
+	for i, c := range iso.Caches {
-		if write {
+		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)
 		}
+	}
 	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()}

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 ""
 }

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)
+	}
 }

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)
+	}
 }