Compare commits

...
Author SHA1 Message Date
jschoubben 979f2cd718 buildstep: keep a check ask's cache layer out of every step's reach, move through an os.Root, and layer the build cache alone
Review of 62a50eb: the layer lay in the workspace, which every step binds read-write,
so a change's command could swap a kept directory for a symlink and the move, run
outside the sandbox, would write into the shared cache (ADR 0541 §2). The layer now
lies beside the workspace (Workspaces.CacheLayer, removed by Release and Sweep),
Command refuses a layer within reach of the workspace, a writable directory or a
cache, and every move goes through an os.Root on the layer and keeps no symlink. Only
LayerCaches (the build cache) are layered; a module cache keeps its throwaway layer
(issue 1921).
2026-10-29 01:04:20 +01:00
jschoubben 62a50ebcf8 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).
2026-10-29 00:53:20 +01:00
jschoubben 261f41aec1 A check ask may ask the merge alone, so the checker can name every group member that does not merge in one verdict
A group's check named only its first member in conflict; each hidden one cost a check round (issue 1921's kind; no open issue covers it).
2026-10-29 00:07:18 +01:00
9 changed files with 367 additions and 15 deletions
+2 -2
View File
@@ -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 {
return answer, nil // the asker reads the conflict from the fetch's answer: a verdict, not an error
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 s.Test == nil {
return nil, Refuse("this build module tests nothing")
+7 -1
View File
@@ -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.
// alone, a verdict for the asker; so does a merge-only ask of a head that merges.
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,6 +252,12 @@ 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
+172 -9
View File
@@ -3,10 +3,14 @@ package buildstep
import (
"context"
"fmt"
"io/fs"
"os"
"path"
"path/filepath"
"slices"
"strconv"
"strings"
"sync"
"git.novox.be/novox/mesh-sdk/go/forgekey"
)
@@ -29,16 +33,31 @@ 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 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.
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 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
// 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,13 +110,16 @@ 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
}
}
}
if err := iso.checkLayer(); err != nil {
return nil, err
}
words := append(append([]string{}, iso.Env...), env...)
home := false
for _, w := range words {
@@ -113,7 +135,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 +156,156 @@ 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). 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.
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.layered(c):
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
}
// 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
}
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)) }
// 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.LayerCaches) == 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
}
run, err := os.MkdirTemp(filepath.Join(iso.CacheLayer, "runs"), "run-")
if err != nil {
return err
}
iso.run = run
for i, c := range iso.Caches {
if !iso.layered(c) {
continue
}
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 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.
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()
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 {
if err != nil || p == upper {
return nil
}
rel, _ := filepath.Rel(upper, p)
to := filepath.Join(keptRel(i), rel)
switch {
case d.IsDir():
if fi, err := root.Lstat(to); err == nil && !fi.IsDir() {
root.Remove(to)
}
root.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)
}
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 +325,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()}
+46 -1
View File
@@ -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,46 @@ 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.
// 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"}}
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)
}
}
}
// 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 ""
}
+33
View File
@@ -63,4 +63,37 @@ 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.
layer := filepath.Join(filepath.Dir(workspace), "ws1.cachelayer")
asked := Isolated(forgekey.ExecRunnerFor(0), Isolation{Workspace: workspace, Caches: []string{cacheDir},
CacheLayer: layer, LayerCaches: []string{cacheDir}})
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)
}
// 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 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)
}
}
+62
View File
@@ -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,63 @@ 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, "ws1.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{}
}
// 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}})
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)
}
}
+20
View File
@@ -10,6 +10,7 @@ import (
"path/filepath"
"regexp"
"sort"
"strings"
"sync"
"syscall"
"time"
@@ -116,6 +117,14 @@ 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 {
@@ -130,6 +139,9 @@ 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
}
@@ -144,6 +156,13 @@ 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
}
@@ -152,6 +171,7 @@ 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())
}
}
+20 -2
View File
@@ -4,6 +4,7 @@ import (
"errors"
"os"
"path/filepath"
"strings"
"testing"
"time"
)
@@ -68,8 +69,15 @@ 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)
if err := w.Release("abc1"); err != nil || !gone(p) {
t.Fatalf("Release = %v, gone %v", err, gone(p))
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 {
t.Errorf("a second release = %v, want nil", err)
@@ -98,11 +106,21 @@ 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)
}
+5
View File
@@ -345,6 +345,11 @@ 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