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/askrun.go

@@ -170,8 +170,8 @@ func (s AskSteps) check(ctx context.Context, job delivery.CheckJob) (any, error)
 	}
 	defer s.release(f.Workspace)
 	answer := delivery.CheckAnswer{Fetch: r}
-	if len(f.Conflicts) > 0 || job.MergeOnly {
-		return answer, nil // the asker reads the merge or the conflict from the fetch's answer: a verdict, not an error
+	if len(f.Conflicts) > 0 {
+		return answer, nil // the asker reads the conflict from the fetch's answer: a verdict, not an error
 	}
 	if s.Test == nil {
 		return nil, Refuse("this build module tests nothing")

go/buildstep/askrun_test.go

@@ -235,7 +235,7 @@ test {"level":"unit","runner":"workspace","workspace":"/w"}`},
 }
 
 // A Node check tests each module directory, the one whose tests changed first; a conflict ends the ask with the fetch
-// alone, a verdict for the asker; so does a merge-only ask of a head that merges.
+// alone, a verdict for the asker.
 func TestRunAskChecksNodeModulesAndConflicts(t *testing.T) {
 	r := &recorder{fetch: map[string]any{"workspace": "/w", "modules": []string{"a", "b"}, "changed": []string{"b/test/x.test.ts"}},
 		tests: map[string]bool{"a": true, "b": true}}
@@ -252,12 +252,6 @@ func TestRunAskChecksNodeModulesAndConflicts(t *testing.T) {
 	if err != nil || len(r.calls) != 1 || len(out.(delivery.CheckAnswer).Levels) != 0 {
 		t.Fatalf("a conflict: %v %v", err, r.calls)
 	}
-	// A merge-only ask answers the fetch of a head that merges and runs no level.
-	r = &recorder{fetch: map[string]any{"workspace": "/w", "merge": "m1"}, tests: map[string]bool{"a": true}}
-	out, err = RunAsk(context.Background(), r.steps(), ask(delivery.CheckJob{Levels: []string{delivery.LevelUnit}, MergeOnly: true, Fetch: map[string]any{"repository": "novox/x", "commit": runCommit}}))
-	if err != nil || len(r.calls) != 1 || len(out.(delivery.CheckAnswer).Levels) != 0 || !strings.Contains(string(out.(delivery.CheckAnswer).Fetch), `"m1"`) {
-		t.Fatalf("merge only: %v %v %v", err, r.calls, out)
-	}
 }
 
 // Issue 1921: a failure in the packages whose tests changed does not stop the level: the other packages still run, so two

go/buildstep/isolate.go

@@ -7,7 +7,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -34,26 +33,20 @@ const Bubblewrap = "bwrap"
 //     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;
-//   - with a CacheLayer, what one command writes into the LayerCaches 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.
+//   - 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, gone with its workspace (Workspaces.CacheLayer): each command writes
-	// the LayerCaches 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. It must lie outside
-	// the Workspace and every Writable directory, so no command reaches it (a command that could would make the move,
-	// which runs outside the sandbox, write where it points). Empty: what a command writes is thrown away.
+	// 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
-	// LayerCaches are the Caches kept in the CacheLayer: a build cache, whose entries are whole files written by
-	// rename. Every other cache (a module cache, with read-only directories and half-written modules) keeps a
-	// throwaway layer.
-	LayerCaches []string
-	Env         []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.
@@ -117,9 +110,6 @@ func (iso Isolation) command(env []string, name string, args ...string) ([]strin
 			}
 		}
 	}
-	if err := iso.checkLayer(); err != nil {
-		return nil, err
-	}
 	words := append(append([]string{}, iso.Env...), env...)
 	home := false
 	for _, w := range words {
@@ -156,16 +146,16 @@ 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). A cache of
-// LayerCaches, with a CacheLayer, has the ask's kept layer 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.
+// 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 i, c := range iso.Caches {
 		switch {
 		case iso.WriteCaches:
 			argv = append(argv, "--bind", c, c)
-		case iso.layered(c):
+		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:
@@ -175,33 +165,6 @@ func (iso Isolation) cacheArgs() []string {
 	return argv
 }
 
-// layered says whether the cache c is kept in the ask's cache layer.
-func (iso Isolation) layered(c string) bool {
-	return iso.CacheLayer != "" && !iso.WriteCaches && slices.Contains(iso.LayerCaches, c)
-}
-
-// checkLayer refuses a cache layer a command could reach (in or above the workspace or a writable directory, or a
-// cache), and a layered cache that is not one of the caches.
-func (iso Isolation) checkLayer() error {
-	if iso.CacheLayer == "" {
-		return nil
-	}
-	for _, d := range append(append([]string{iso.Workspace}, iso.Writable...), iso.Caches...) {
-		if within(iso.CacheLayer, d) || within(d, iso.CacheLayer) {
-			return fmt.Errorf("buildstep: the cache layer %s is within reach of the step's %s", iso.CacheLayer, d)
-		}
-	}
-	for _, c := range iso.LayerCaches {
-		if !slices.Contains(iso.Caches, c) {
-			return fmt.Errorf("buildstep: the layered cache %s is not one of the caches", c)
-		}
-	}
-	return nil
-}
-
-// within says whether p is dir or lies beneath it.
-func within(p, dir string) bool { return p == dir || strings.HasPrefix(p, dir+"/") }
-
 func layerDirs(layer string) []string {
 	if layer == "" {
 		return nil
@@ -209,9 +172,8 @@ func layerDirs(layer string) []string {
 	return []string{layer}
 }
 
-// keptRel is where, in the cache layer, the ask keeps what its commands wrote into the cache i; keptLayer is its path.
-func keptRel(i int) string                 { return filepath.Join("kept", strconv.Itoa(i)) }
-func keptLayer(layer string, i int) string { return filepath.Join(layer, keptRel(i)) }
+// 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.
@@ -228,12 +190,9 @@ 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.LayerCaches) == 0 {
+	if iso.CacheLayer == "" || iso.WriteCaches || len(iso.Caches) == 0 {
 		return nil
 	}
-	if err := iso.checkLayer(); err != nil {
-		return err
-	}
 	if err := os.MkdirAll(filepath.Join(iso.CacheLayer, "runs"), 0o700); err != nil {
 		return err
 	}
@@ -242,10 +201,7 @@ func (iso *Isolation) prepareLayer() error {
 		return err
 	}
 	iso.run = run
-	for i, c := range iso.Caches {
-		if !iso.layered(c) {
-			continue
-		}
+	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 {
@@ -256,11 +212,10 @@ func (iso *Isolation) prepareLayer() error {
 	return nil
 }
 
-// keepLayer moves what the command wrote into the layered 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), a symlink or any other entry that is not a directory or a regular file is not kept. Every move goes
-// through an os.Root on the cache layer, so no symlink leads it outside the layer. A write that cannot be kept only
-// means a later command builds it again.
+// 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
@@ -269,37 +224,23 @@ func (iso Isolation) keepLayer() {
 	mu, _ := layerLocks.LoadOrStore(iso.CacheLayer, &sync.Mutex{})
 	mu.(*sync.Mutex).Lock()
 	defer mu.(*sync.Mutex).Unlock()
-	root, err := os.OpenRoot(iso.CacheLayer)
-	if err != nil {
-		return
-	}
-	defer root.Close()
-	runRel, err := filepath.Rel(iso.CacheLayer, iso.run)
-	if err != nil {
-		return
-	}
-	for i, c := range iso.Caches {
-		if !iso.layered(c) {
-			continue
-		}
-		upper := filepath.Join(runRel, strconv.Itoa(i), "upper")
-		fs.WalkDir(root.FS(), upper, func(p string, d fs.DirEntry, err error) error {
+	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(keptRel(i), rel)
+			to := filepath.Join(kept, rel)
 			switch {
 			case d.IsDir():
-				if fi, err := root.Lstat(to); err == nil && !fi.IsDir() {
-					root.Remove(to)
+				if fi, err := os.Lstat(to); err == nil && !fi.IsDir() {
+					os.Remove(to)
 				}
-				root.MkdirAll(to, 0o700)
+				os.MkdirAll(to, 0o700)
 			case d.Type().IsRegular():
-				if fi, err := root.Lstat(to); err == nil && !fi.Mode().IsRegular() {
-					return nil // never replace what is not a file the layer kept
-				}
-				root.Rename(p, to)
+				os.Rename(p, to)
 			}
 			return nil
 		})

go/buildstep/isolate_caches_test.go

@@ -32,35 +32,24 @@ func TestACandidatesStepWritesTheCachesOnlyIntoAThrowawayLayer(t *testing.T) {
 	}
 	// 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.
-	// Only the layered caches (the build cache) are; the module cache keeps its throwaway layer.
-	iso = Isolation{Workspace: "/w/ws1", Caches: []string{"/cache/gomod", "/cache/gobuild"}, CacheLayer: "/w/ws1.cachelayer",
-		LayerCaches: []string{"/cache/gobuild"}}
+	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)
 	}
-	if !holds(argv, "--overlay-src", "/cache/gobuild", "--overlay-src", keptLayer(iso.CacheLayer, 1), "--overlay") {
-		t.Errorf("the build cache is not read beneath the ask's kept layer: %v", argv)
-	}
-	if up := overlayOf(argv, "/cache/gobuild"); up == "" || !strings.HasPrefix(up, "/w/ws1.cachelayer/") {
-		t.Errorf("the build cache is written into %q, not a layer of the ask's own: %v", up, argv)
-	}
-	if !holds(argv, "--overlay-src", "/cache/gomod", "--tmp-overlay", "/cache/gomod") || overlayOf(argv, "/cache/gomod") != "" {
-		t.Errorf("the module cache is not under a throwaway layer: %v", argv)
-	}
-	// A layer a command could reach is refused: in the workspace (bound read-write), in a writable directory, in a
-	// cache, or above one of them; so is a relative one, and a layered cache that is no cache.
-	for _, bad := range []Isolation{
-		{Workspace: "/w/ws1", CacheLayer: "/w/ws1/cachelayer"},
-		{Workspace: "/w/ws1", CacheLayer: "/w"},
-		{Workspace: "/w/ws1", Writable: []string{"/out"}, CacheLayer: "/out/layer"},
-		{Workspace: "/w/ws1", Caches: []string{"/cache/gobuild"}, CacheLayer: "/cache/gobuild/layer"},
-		{Workspace: "/w/ws1", CacheLayer: "cachelayer"},
-		{Workspace: "/w/ws1", Caches: []string{"/cache/gobuild"}, CacheLayer: "/w/ws1.cachelayer", LayerCaches: []string{"/cache/other"}},
-	} {
-		if _, err := bad.Command("go"); err == nil {
-			t.Errorf("the cache layer %s of %+v was taken", bad.CacheLayer, bad)
+	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")
 	}
 }
 

go/buildstep/isolate_integration_test.go

@@ -66,9 +66,8 @@ func TestARunStepReachesNothingAndLeavesTheCachesUnchanged(t *testing.T) {
 
 	// 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.
-	layer := filepath.Join(filepath.Dir(workspace), "ws1.cachelayer")
 	asked := Isolated(forgekey.ExecRunnerFor(0), Isolation{Workspace: workspace, Caches: []string{cacheDir},
-		CacheLayer: layer, LayerCaches: []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)
@@ -77,23 +76,10 @@ func TestARunStepReachesNothingAndLeavesTheCachesUnchanged(t *testing.T) {
 	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)
 	}
-	// A command cannot reach the layer (it lies outside every directory the step binds), and a symlink it writes into
-	// the cache is not kept, so nothing it does makes the keeping write outside the layer.
-	r = asked(context.Background(), []string{"PATH=/usr/bin:/bin"}, "/bin/sh", "-c",
-		"ls "+layer+" >/dev/null 2>&1 && echo reached; ln -s "+cacheDir+" "+filepath.Join(cacheDir, "esc")+" && mkdir "+filepath.Join(cacheDir, "cd")+" && echo x > "+filepath.Join(cacheDir, "cd", "f"))
-	if r.Status != 0 || r.Err != "" || strings.Contains(r.Stdout, "reached") {
-		t.Fatalf("the command reached the cache layer or could not write the cache (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 fi, err := os.Lstat(filepath.Join(keptLayer(layer, 0), "esc")); err == nil {
-		t.Fatalf("a symlink the command wrote was kept: %v", fi.Mode())
-	}
-	if b, err := os.ReadFile(filepath.Join(keptLayer(layer, 0), "cd", "f")); err != nil || string(b) != "x\n" {
-		t.Fatalf("the command's file was not kept: %q %v", b, err)
-	}
-	if entries, _ := os.ReadDir(cacheDir); len(entries) != 1 || entries[0].Name() != "warm" {
-		t.Fatalf("the ask's commands wrote the shared cache: %v", entries)
-	}
-	if err := os.RemoveAll(layer); err != nil {
-		t.Fatalf("the ask's cache layer cannot be removed: %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

@@ -124,7 +124,7 @@ func TestIsolatedRunsEachCommandThroughBubblewrap(t *testing.T) {
 	if err := os.MkdirAll(shared, 0o700); err != nil {
 		t.Fatal(err)
 	}
-	layer := filepath.Join(root, "ws1.cachelayer")
+	layer := filepath.Join(root, "ws", "cachelayer")
 	var mu sync.Mutex
 	uppers := map[string]bool{}
 	gate := make(chan struct{})
@@ -141,15 +141,7 @@ func TestIsolatedRunsEachCommandThroughBubblewrap(t *testing.T) {
 		os.WriteFile(filepath.Join(up, "ab", args[len(args)-1]+"-d"), []byte("compiled"), 0o600)
 		return forgekey.Ran{}
 	}
-	// A symlink in the kept layer (none is ever kept; planted here) leads no move outside the layer: the kept directory
-	// replaces it, and the shared cache it points to stays unchanged.
-	if err := os.MkdirAll(keptLayer(layer, 0), 0o700); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.Symlink(shared, filepath.Join(keptLayer(layer, 0), "ab")); err != nil {
-		t.Fatal(err)
-	}
-	run := Isolated(writer, Isolation{Workspace: filepath.Join(root, "ws1"), Caches: []string{shared}, CacheLayer: layer, LayerCaches: []string{shared}})
+	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/buildstep/workspaces.go

@@ -10,7 +10,6 @@ import (
 	"path/filepath"
 	"regexp"
 	"sort"
-	"strings"
 	"sync"
 	"syscall"
 	"time"
@@ -117,14 +116,6 @@ func (w *Workspaces) touch(id string) {
 	_ = os.Chtimes(filepath.Join(w.Dir, id), now, now) // a workspace not made yet has nothing to mark
 }
 
-// cacheLayerSuffix names a workspace's cache layer beside it (Isolation.CacheLayer): never inside it, which every step
-// of the workspace binds read-write.
-const cacheLayerSuffix = ".cachelayer"
-
-// CacheLayer is the directory of the workspace id's cache layer: beside the workspace, out of every step's reach, and
-// removed with it by Release and Sweep.
-func (w *Workspaces) CacheLayer(id string) string { return filepath.Join(w.Dir, id+cacheLayerSuffix) }
-
 // Release removes the workspace at once: its caller's chain of steps has ended. One already gone is no error; one a
 // running step holds is refused and kept; an id that is not a workspace's is refused.
 func (w *Workspaces) Release(id string) error {
@@ -139,9 +130,6 @@ func (w *Workspaces) Release(id string) error {
 	if err := os.RemoveAll(filepath.Join(w.Dir, id)); err != nil {
 		return fmt.Errorf("the workspace %s could not be removed: %w", id, err)
 	}
-	if err := os.RemoveAll(w.CacheLayer(id)); err != nil {
-		return fmt.Errorf("the cache layer of the workspace %s could not be removed: %w", id, err)
-	}
 	return nil
 }
 
@@ -156,13 +144,6 @@ func (w *Workspaces) Sweep(now time.Time, bound time.Duration) []string {
 	w.mu.Lock()
 	defer w.mu.Unlock()
 	for _, e := range entries {
-		if id, ok := strings.CutSuffix(e.Name(), cacheLayerSuffix); ok && workspaceID.MatchString(id) && w.held[id] == 0 {
-			// a cache layer whose workspace is gone (its removal failed or was cut short) goes too
-			if _, err := os.Stat(filepath.Join(w.Dir, id)); os.IsNotExist(err) {
-				os.RemoveAll(filepath.Join(w.Dir, e.Name()))
-			}
-			continue
-		}
 		if !e.IsDir() || !workspaceID.MatchString(e.Name()) || w.held[e.Name()] > 0 {
 			continue
 		}
@@ -171,7 +152,6 @@ func (w *Workspaces) Sweep(now time.Time, bound time.Duration) []string {
 			continue
 		}
 		if os.RemoveAll(filepath.Join(w.Dir, e.Name())) == nil {
-			os.RemoveAll(w.CacheLayer(e.Name()))
 			removed = append(removed, e.Name())
 		}
 	}

go/buildstep/workspaces_test.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 )
@@ -69,15 +68,8 @@ func gone(p string) bool { _, err := os.Stat(p); return os.IsNotExist(err) }
 func TestReleaseRemovesAWorkspaceAtOnce(t *testing.T) {
 	w := &Workspaces{Dir: t.TempDir()}
 	p := makeWorkspace(t, w, "abc1", 0)
-	layer := w.CacheLayer("abc1") // issue 1921/S2: the cache layer lies beside the workspace and goes with it
-	if filepath.Dir(layer) != w.Dir || strings.HasPrefix(layer, p+"/") {
-		t.Fatalf("the cache layer %s does not lie beside the workspace %s", layer, p)
-	}
-	if err := os.MkdirAll(filepath.Join(layer, "kept", "0"), 0o700); err != nil {
-		t.Fatal(err)
-	}
-	if err := w.Release("abc1"); err != nil || !gone(p) || !gone(layer) {
-		t.Fatalf("Release = %v, gone %v, its cache layer gone %v", err, gone(p), gone(layer))
+	if err := w.Release("abc1"); err != nil || !gone(p) {
+		t.Fatalf("Release = %v, gone %v", err, gone(p))
 	}
 	if err := w.Release("abc1"); err != nil {
 		t.Errorf("a second release = %v, want nil", err)
@@ -106,21 +98,11 @@ func TestSweepRemovesUnheldWorkspacesPastTheBound(t *testing.T) {
 	if err := os.WriteFile(filepath.Join(w.Dir, "not-a-workspace.txt"), nil, 0o600); err != nil {
 		t.Fatal(err)
 	}
-	// issue 1921/S2: a swept workspace's cache layer goes with it, and so does a layer whose workspace is gone; a held
-	// workspace's stays
-	for _, id := range []string{"old1", "held1", "orphan1"} {
-		if err := os.MkdirAll(w.CacheLayer(id), 0o700); err != nil {
-			t.Fatal(err)
-		}
-	}
 	done := w.Hold("held1")
 	removed := w.Sweep(time.Now(), 2*time.Hour)
 	if !gone(old) || gone(heldOld) || gone(recent) {
 		t.Errorf("after the sweep: old gone %v, held kept %v, recent kept %v", gone(old), !gone(heldOld), !gone(recent))
 	}
-	if !gone(w.CacheLayer("old1")) || !gone(w.CacheLayer("orphan1")) || gone(w.CacheLayer("held1")) {
-		t.Errorf("after the sweep: old1's layer gone %v, orphan1's gone %v, held1's kept %v", gone(w.CacheLayer("old1")), gone(w.CacheLayer("orphan1")), !gone(w.CacheLayer("held1")))
-	}
 	if len(removed) != 1 || removed[0] != "old1" {
 		t.Errorf("Sweep answered %v, want [old1]", removed)
 	}

go/compose/composed.go

@@ -318,6 +318,15 @@ func Stub(m catalogue.Manifest) catalogue.Manifest {
 		m.OwnsPaths = catalogue.DeclaredPaths(m)
 	}
 	m.Build = nil
+	// Nor is a template its build kept for its own files handed: a stub carries none of them (issue 1126: novox's node
+	// refused "postgres: its build kept a template for consumers, which it does not declare" with postgres held). Its
+	// items stay, and the templates they render through: a held module's contributions to other modules' collections
+	// (seat contributions, shell blocks, seat data) still reach their collectors.
+	if m.Handed != nil {
+		h := *m.Handed
+		h.Templates = nil
+		m.Handed = &h
+	}
 	var dirs []map[string]any
 	for _, r := range m.Resources {
 		if r["type"] == "directory" {

go/compose/frombus/answer.go

@@ -209,21 +209,59 @@ func Answer(ctx context.Context, r Reader, mesh Mesh, s Scope, fps map[string]st
 	return a
 }
 
-// heldAsToday moves each refusal of a module the node holds today (Scope.HeldToday) into Held: the candidate holds it as
-// the node does, which is no change (issue 1126: a seed reproduces today, and today holds it too).
-func heldAsToday(a *delivery.CandidateAnswer, today []string) {
+// heldBehind is why a module is held with a held one it reads (composeEach's dependents).
+func heldBehind(m string) string {
+	return "it reads what " + m + " contributes or listens on, and " + m + " is held"
+}
+
+// behind is the held module a refusal's words say it is held behind, or "".
+func behind(why string) string {
+	rest, ok := strings.CutPrefix(why, "it reads what ")
+	if !ok {
+		return ""
+	}
+	m, _, ok := strings.Cut(rest, " contributes or listens on, and ")
+	if !ok || why != heldBehind(m) {
+		return ""
+	}
+	return m
+}
+
+// heldAsToday moves into Held each refusal of a module the node keeps today as it runs (Scope.HeldToday, with how:
+// held, failed, running a build of the old form), and of each module held only behind one of those: the candidate keeps
+// them as the node does, which is no change (issue 1126: a seed reproduces today, and today keeps them so). Each says
+// how it stands today (ModuleRefused.Today).
+func heldAsToday(a *delivery.CandidateAnswer, today map[string]string) {
 	if len(today) == 0 {
 		return
 	}
-	kept := a.Refusals[:0]
-	for _, r := range a.Refusals {
-		if slices.Contains(today, r.Module) {
-			a.Held = append(a.Held, r)
-		} else {
-			kept = append(kept, r)
+	how := map[string]string{}
+	for m, w := range today {
+		how[m] = w
+	}
+	for moved := true; moved; {
+		moved = false
+		kept := a.Refusals[:0]
+		for _, r := range a.Refusals {
+			w, keep := how[r.Module]
+			if !keep {
+				if m := behind(r.Why); m != "" {
+					if _, behindKept := how[m]; behindKept {
+						w, keep = "held behind "+m, true
+					}
+				}
+			}
+			if keep {
+				r.Today = w
+				how[r.Module] = w
+				a.Held = append(a.Held, r)
+				moved = true
+			} else {
+				kept = append(kept, r)
+			}
 		}
+		a.Refusals = kept
 	}
-	a.Refusals = kept
 	if len(a.Refusals) == 0 {
 		a.Refusals = nil
 	}
@@ -313,7 +351,7 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b
 		// What the others read of its manifest (what it contributes, what it listens on) is its new manifest's, not
 		// necessarily the build's that runs: what reads it is held with it (review of the apply, round 2).
 		for _, r := range dependents(in, node, src[m], failed) {
-			hold(r, "it reads what "+m+" contributes or listens on, and "+m+" is held", false)
+			hold(r, heldBehind(m), false)
 		}
 	}
 	for _, m := range sortedKeys(failed) {

go/compose/frombus/answer_rows_test.go

@@ -93,17 +93,25 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
 	}
 	// Issue 1126: a module the node holds today that the candidate holds too is held as today, no refusal: with every
 	// refused module held today the verdict is no refusal, and each is named under Held.
-	today := Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), HeldToday: []string{"api", "spiegel", "web"}},
+	today := Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), HeldToday: map[string]string{"api": "held", "spiegel": "held", "web": "held"}},
 		map[string]string{"lemurs": fp("e"), "web": fp("1"), "api": fp("2"), "spiegel": fp("3")}, nil, "p-h1", 3)
 	if today.Verdict == delivery.CandidateRefused || len(today.Refusals) != 0 || len(today.Held) != 3 || today.Held[0].Why == "" {
 		t.Errorf("held as today: %+v", today)
 	}
 	// A refused module the node does not hold today stays a refusal: holding only api today leaves spiegel and web refused.
-	part := Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), HeldToday: []string{"api"}},
+	part := Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), HeldToday: map[string]string{"api": "held"}},
 		map[string]string{"lemurs": fp("e"), "web": fp("1"), "api": fp("2"), "spiegel": fp("3")}, nil, "p-h1", 3)
 	if part.Verdict != delivery.CandidateRefused || len(part.Refusals) != 2 || len(part.Held) != 1 || part.Held[0].Module != "api" {
 		t.Errorf("held as today in part: %+v", part)
 	}
+	// A module held only behind one kept today is kept as today too, saying behind which (novox's mailu behind de-spiegel).
+	if a := (delivery.CandidateAnswer{Refusals: []delivery.ModuleRefused{{Module: "de-spiegel", Why: "a value is unset"},
+		{Module: "mailu", Why: heldBehind("de-spiegel")}, {Module: "proxy", Why: heldBehind("mailu")}, {Module: "web", Why: "no build"}}}); true {
+		heldAsToday(&a, map[string]string{"de-spiegel": "runs a build of the old form"})
+		if len(a.Refusals) != 1 || a.Refusals[0].Module != "web" || len(a.Held) != 3 || a.Held[2].Today != "held behind mailu" {
+			t.Errorf("held behind a module kept today: %+v", a)
+		}
+	}
 
 	// The fold: spiegel runs inside the node's own composition at a build of the old form. Its resources are carried as
 	// they run, each after the one it runs after: the node's own is the one it runs, and the node composes itself.
@@ -286,6 +294,31 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
 	if got, want := catalogue.DeclaredPaths(compose.Carried(f1, "f1-circuits", &f1runs)), catalogue.DeclaredPaths(f1); !reflect.DeepEqual(got, want) {
 		t.Errorf("f1-circuits carried owns %v, want what the controller composes from its manifest, %v", got, want)
 	}
+	// Issue 1126: a held module hands no template its build kept for a file it no longer carries (novox's node:
+	// "postgres: its build kept a template for consumers"), but keeps its items and their templates, so what it
+	// contributes to other modules' collections still reaches them.
+	for _, row := range []struct {
+		name   string
+		handed *catalogue.Handed
+		items  []catalogue.Item
+	}{
+		{"templates and items", &catalogue.Handed{Templates: map[string]string{"consumers": "files/consumers.json"},
+			Items: map[string]string{"routes": "files/routes.tmpl"}}, []catalogue.Item{{Collection: "routes", Content: "x"}}},
+		{"items alone", nil, []catalogue.Item{{Collection: "routes", Content: "x"}}},
+	} {
+		kept := f1
+		kept.Handed, kept.Items = row.handed, row.items
+		st := compose.Stub(kept)
+		if st.Handed != nil && st.Handed.Templates != nil {
+			t.Errorf("%s: a stub hands templates %v", row.name, st.Handed.Templates)
+		}
+		if !reflect.DeepEqual(st.Items, row.items) {
+			t.Errorf("%s: a stub collects %+v, want its items %+v", row.name, st.Items, row.items)
+		}
+		if row.handed != nil && (st.Handed == nil || !reflect.DeepEqual(st.Handed.Items, row.handed.Items) || row.handed.Templates == nil) {
+			t.Errorf("%s: a stub hands item templates %+v, want %v and its manifest untouched", row.name, st.Handed, row.handed.Items)
+		}
+	}
 	var k1, k2 any
 	_ = json.Unmarshal(keptComposed, &k1)
 	_ = json.Unmarshal(keptFile, &k2)

go/compose/frombus/frombus.go

@@ -134,10 +134,10 @@ type Scope struct {
 	Placements map[string]*delivery.Placement
 	InOwn      []string
 	Running    []delivery.Composed
-	// HeldToday is, in an answer, the modules the node holds today ("node" for its own composition): their have's phase
-	// is held, or they are held inside its own composition. A candidate that holds one too holds it as today, which is
-	// said under CandidateAnswer.Held and is no refusal (issue 1126).
-	HeldToday []string
+	// HeldToday is, in an answer, the modules the node keeps today as they run ("node" for its own composition), each with
+	// how ("held", "failed", "runs a build of the old form"): a candidate that cannot compose one, or holds one only
+	// behind one of them, keeps it as today, which is said under CandidateAnswer.Held and is no refusal (issue 1126).
+	HeldToday map[string]string
 	// SameValue, in an answer, says whether two sealed values open to the same value (the node opens them with its own
 	// key), so a differing resource shows a value sealed again as such (delivery.CompareComposedWith). Nil shows them
 	// as differing.

go/compose/judge/judge.go

@@ -258,9 +258,13 @@ func Brief(p delivery.PlacementProposal, v delivery.PlacementVerdict) string {
 		if held := v.Answers[n].Held; len(held) > 0 {
 			var names []string
 			for _, h := range held {
-				names = append(names, h.Module)
+				today := h.Today
+				if today == "" {
+					today = "held"
+				}
+				names = append(names, h.Module+" ("+today+")")
 			}
-			fmt.Fprintf(&b, "; stays held as today: %s", strings.Join(names, ", "))
+			fmt.Fprintf(&b, "; stays as today: %s", strings.Join(names, ", "))
 		}
 	}
 	return b.String()

go/compose/judge/judge_test.go

@@ -79,7 +79,7 @@ func TestJudge(t *testing.T) {
 				t.Errorf("the difference is named %q", got)
 			}
 		}
-		if strings.HasPrefix(row.name, "a module held as today") && !strings.Contains(Brief(q, v), "stays held as today: spiegel") {
+		if strings.HasPrefix(row.name, "a module held as today") && !strings.Contains(Brief(q, v), "stays as today: spiegel (held)") {
 			t.Errorf("%s: the brief does not show it: %s", row.name, Brief(q, v))
 		}
 		if row.marked != "" && (!slices.Contains(v.Unintended["anchor"], row.marked) || !strings.Contains(Brief(q, v), "not computed: "+row.marked)) {

go/delivery/placement_candidate.go

@@ -190,10 +190,13 @@ type ModuleAdded struct {
 	Unissued []string `json:"unissued,omitempty"`
 }
 
-// ModuleRefused is a value missing for a module the node runs, in the words the composition stood in for it with.
+// ModuleRefused is a value missing for a module the node runs, in the words the composition stood in for it with. Today,
+// in CandidateAnswer.Held, is how the node keeps it today (held, failed, runs a build of the old form, held behind
+// another), which the candidate keeps.
 type ModuleRefused struct {
 	Module string `json:"module"`
 	Why    string `json:"why"`
+	Today  string `json:"today,omitempty"`
 }
 
 // VerdictOf is the verdict of an answer's lists: refused on any refusal, differs on any difference, addition or

go/delivery/stepasks.go

@@ -345,11 +345,6 @@ type CheckJob struct {
 	// has to list them. The build step hands them to its test tool as the argument NodeNamesArg, and the build module
 	// gives them to the tests in NodeNamesEnv.
 	NodeNames []string `json:"node-names,omitempty"`
-	// MergeOnly says the build module answers the fetch alone, its merge or the files in conflict, and runs no level: the
-	// checker asks it of a delivery group's members after one member's check failed, so one verdict names every member
-	// that does not merge into its trunk instead of the first. Levels stay named, so a build module from before it (which
-	// ignores it) still takes the ask; the checker reads only the fetch of such an answer.
-	MergeOnly bool `json:"merge-only,omitempty"`
 }
 
 // NodeNamesArg is the test tool's argument that carries a check's node names, and NodeNamesEnv the environment