siso: ignore tap in TMPDIR TMPDIR is shared by many build steps, so it may see other step's intermediates, which would cause cache miss due to inputs mismatch. Ignore tap data in TMPDIR Bug: b/534982229 Change-Id: Ie6bcbc0f3905205780f61057a9767088bf75da1e Reviewed-on: https://chromium-review.googlesource.com/c/build/+/8130201 Reviewed-by: Yuke Liao <liaoyuke@google.com> Auto-Submit: Fumitoshi Ukai <ukai@google.com> Commit-Queue: Fumitoshi Ukai <ukai@google.com>
diff --git a/siso/build/tap.go b/siso/build/tap.go index 2a1c914..17564ef 100644 --- a/siso/build/tap.go +++ b/siso/build/tap.go
@@ -8,7 +8,11 @@ "context" "errors" "io/fs" + "os" "path/filepath" + "strings" + + log "github.com/golang/glog" "go.chromium.org/build/siso/execute" "go.chromium.org/build/siso/execute/localexec" @@ -26,12 +30,43 @@ // ignore out of workspace root // TODO: use with input root absolute path? // TODO: just use detected inputs? - seen := make(map[string]bool) + seen := make(map[path.Path]bool) for _, input := range cmd.AllInputs() { - seen[string(input)] = true + seen[input] = true ninputs++ } + // os.TempDir() is $TMPDIR or /tmp. + ignores := map[path.Path]struct{}{path.New(os.TempDir()): {}} + ignored := 0 + if len(cmd.Env) > 0 { + for _, env := range cmd.Env { + tmpdir, ok := strings.CutPrefix(env, "TMPDIR=") + if ok { + if !filepath.IsAbs(tmpdir) { + clog.Warningf(ctx, "ignore TMPDIR: not absolute path: %q", tmpdir) + continue + } + ignores[path.New(tmpdir)] = struct{}{} + } + } + } + shouldIgnore := func(op string, p path.Path) bool { + for ignore := range ignores { + if p.HasPrefix(ignore) { + if log.V(1) { + clog.Infof(ctx, "ignore %s %q in %q", op, p, ignore) + } + ignored++ + return true + } + } + return false + } + for _, input := range tapData.Reads { + if shouldIgnore("reads", path.New(input)) { + continue + } rel, err := filepath.Rel(b.path.WorkspaceRoot, input) if err != nil { clog.Warningf(ctx, "reads relpath %q: %v", input, err) @@ -40,25 +75,29 @@ if !filepath.IsLocal(rel) { continue } - _, err = b.hashFS.Stat(ctx, b.path.WorkspaceRoot, path.New(rel)) + relPath := path.New(rel) + _, err = b.hashFS.Stat(ctx, b.path.WorkspaceRoot, relPath) if errors.Is(err, fs.ErrNotExist) { continue } - if seen[rel] { + if seen[relPath] { continue } - seen[rel] = true - cmd.Inputs = append(cmd.Inputs, path.New(rel)) + seen[relPath] = true + cmd.Inputs = append(cmd.Inputs, relPath) } clear(seen) // need to use both original outputs and detected outputs. // it might not detect output for restat action. // it might detect unspecified outputs. for _, output := range cmd.AllOutputs() { - seen[string(output)] = true + seen[output] = true noutputs++ } for _, output := range tapData.Writes { + if shouldIgnore("writes", path.New(output)) { + continue + } rel, err := filepath.Rel(b.path.WorkspaceRoot, output) if err != nil { clog.Warningf(ctx, "writes relpath %q: %v", output, err) @@ -67,7 +106,8 @@ if !filepath.IsLocal(rel) { continue } - fi, err := b.hashFS.Stat(ctx, b.path.WorkspaceRoot, path.New(rel)) + relPath := path.New(rel) + fi, err := b.hashFS.Stat(ctx, b.path.WorkspaceRoot, relPath) if err != nil { continue } @@ -77,11 +117,11 @@ // by hashfs Update. continue } - if seen[rel] { + if seen[relPath] { continue } - seen[rel] = true - cmd.Outputs = append(cmd.Outputs, path.New(rel)) + seen[relPath] = true + cmd.Outputs = append(cmd.Outputs, relPath) } for _, del := range tapData.Deletes { rel, err := filepath.Rel(b.path.WorkspaceRoot, del) @@ -98,12 +138,12 @@ continue } b.hashFS.Forget(ctx, b.path.WorkspaceRoot, []path.Path{relPath}) - clog.Infof(ctx, "delete %q", rel) + clog.Infof(ctx, "delete %q", relPath) } // now tap results are applied to cmd, so we can consider // this cmd is pure, thus cacheable. cmd.Pure = true - clog.Infof(ctx, "tap canonicalized inputs=%d->%d outputs=%d->%d", ninputs, len(cmd.Inputs), noutputs, len(cmd.Outputs)) + clog.Infof(ctx, "tap canonicalized inputs=%d->%d outputs=%d->%d ignored=%d", ninputs, len(cmd.Inputs), noutputs, len(cmd.Outputs), ignored) return nil }
diff --git a/siso/build/tap_test.go b/siso/build/tap_test.go new file mode 100644 index 0000000..b87135b --- /dev/null +++ b/siso/build/tap_test.go
@@ -0,0 +1,349 @@ +// Copyright 2026 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package build + +import ( + "errors" + "io/fs" + "os" + "path/filepath" + "slices" + "testing" + + "github.com/golang/glog" + "google.golang.org/protobuf/types/known/anypb" + + rpb "go.chromium.org/build/remote-apis/build/bazel/remote/execution/v2" + + "go.chromium.org/build/siso/execute" + pb "go.chromium.org/build/siso/execute/proto" + "go.chromium.org/build/siso/hashfs" + "go.chromium.org/build/siso/path" +) + +func init() { + glog.Infof("create log file before setting TMPDIR") +} + +func setTapResult(cmd *execute.Cmd, reads, writes, deletes []string) error { + tapData := &pb.TapResult{ + Reads: reads, + Writes: writes, + Deletes: deletes, + } + any, err := anypb.New(tapData) + if err != nil { + return err + } + res := &rpb.ActionResult{ + ExecutionMetadata: &rpb.ExecutedActionMetadata{ + AuxiliaryMetadata: []*anypb.Any{any}, + }, + } + cmd.SetActionResult(res, false) + return nil +} + +func setupDirForTapTest(t *testing.T) (string, string) { + tmpdir := t.TempDir() + dir := t.TempDir() + dir, err := filepath.EvalSymlinks(dir) + if err != nil { + t.Fatal(err) + } + t.Setenv("TMPDIR", tmpdir) + t.Setenv("TMP", tmpdir) + if path.New(dir).HasPrefix(path.New(tmpdir)) { + t.Fatalf("dir=%q under TMPDIR=%q", dir, tmpdir) + } + wsDir := filepath.Join(dir, "workspace") + if err := os.MkdirAll(wsDir, 0755); err != nil { + t.Fatal(err) + } + customTmpDir := filepath.Join(dir, "workspace/out/soong/.temp") + if err := os.MkdirAll(customTmpDir, 0755); err != nil { + t.Fatal(err) + } + return dir, tmpdir +} + +func TestTapCanonicalizeCmd_NoTapData(t *testing.T) { + ctx := t.Context() + dir, _ := setupDirForTapTest(t) + wsDir := filepath.Join(dir, "workspace") + + hfs, err := hashfs.New(ctx, hashfs.Option{}) + if err != nil { + t.Fatal(err) + } + defer hfs.Close(ctx) + err = hfs.WaitReady(ctx) + if err != nil { + t.Fatal(err) + } + + b := &Builder{ + hashFS: hfs, + path: NewPath(wsDir, "out/siso"), + } + + cmd := &execute.Cmd{ + WorkspaceRoot: wsDir, + } + + err = b.tapCanonicalizeCmd(ctx, cmd) + if err == nil { + t.Errorf("tapCanonicalizeCmd(cmd) = nil; want error") + } +} + +func TestTapCanonicalizeCmd(t *testing.T) { + ctx := t.Context() + dir, tmpDir := setupDirForTapTest(t) + wsDir := filepath.Join(dir, "workspace") + + hfs, err := hashfs.New(ctx, hashfs.Option{}) + if err != nil { + t.Fatal(err) + } + defer hfs.Close(ctx) + if err := hfs.WaitReady(ctx); err != nil { + t.Fatal(err) + } + + setupFile := func(root, rel string) string { + t.Helper() + abs := filepath.Join(root, rel) + if err := os.MkdirAll(filepath.Dir(abs), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(abs, []byte("content"), 0644); err != nil { + t.Fatal(err) + } + return abs + } + setupDir := func(root, rel string) string { + t.Helper() + abs := filepath.Join(root, rel) + if err := os.MkdirAll(abs, 0755); err != nil { + t.Fatal(err) + } + return abs + } + + b := &Builder{ + hashFS: hfs, + path: NewPath(wsDir, "out/siso"), + } + + existingInput := setupFile(wsDir, "input_existing.h") + newInput := setupFile(wsDir, "input_new.h") + nonExistentInput := filepath.Join(wsDir, "input_nonexistent.h") + outsideInput := setupFile(tmpDir, "outside_input.h") + + existingOutput := setupFile(wsDir, "out/siso/output_existing.o") + newOutput := setupFile(wsDir, "out/siso/output_new.o") + dirOutput := setupDir(wsDir, "out/siso/output_dir") + + toDeleteFile := setupFile(wsDir, "to_delete.txt") + + // Cache toDeleteFile in hashFS first + _, err = hfs.Stat(ctx, wsDir, path.New("to_delete.txt")) + if err != nil { + t.Fatalf("hfs.Stat to_delete.txt failed: %v", err) + } + + // Remove toDeleteFile from disk to simulate action deletion + if err := os.Remove(toDeleteFile); err != nil { + t.Fatal(err) + } + + cmd := &execute.Cmd{ + WorkspaceRoot: wsDir, + Inputs: []path.Path{path.New("input_existing.h")}, + Outputs: []path.Path{path.New("out/siso/output_existing.o")}, + } + + err = setTapResult(cmd, + []string{existingInput, newInput, nonExistentInput, outsideInput}, + []string{existingOutput, newOutput, dirOutput}, + []string{toDeleteFile}, + ) + if err != nil { + t.Fatal(err) + } + + err = b.tapCanonicalizeCmd(ctx, cmd) + if err != nil { + t.Fatalf("tapCanonicalizeCmd returned error: %v", err) + } + + if !cmd.Pure { + t.Errorf("cmd.Pure = false; want true") + } + + wantInputs := []path.Path{path.New("input_existing.h"), path.New("input_new.h")} + if !slices.Equal(cmd.Inputs, wantInputs) { + t.Errorf("cmd.Inputs = %v; want %v", cmd.Inputs, wantInputs) + } + + wantOutputs := []path.Path{path.New("out/siso/output_existing.o"), path.New("out/siso/output_new.o")} + if !slices.Equal(cmd.Outputs, wantOutputs) { + t.Errorf("cmd.Outputs = %v; want %v", cmd.Outputs, wantOutputs) + } + + // Verify to_delete.txt is forgotten from hashFS and returns ErrNotExist + _, err = hfs.Stat(ctx, wsDir, path.New("to_delete.txt")) + if !errors.Is(err, fs.ErrNotExist) { + t.Errorf("hfs.Stat(to_delete.txt) = %v; want %v after forget", err, fs.ErrNotExist) + } +} + +func TestTapCanonicalizeCmd_IgnoreOSTMPDIR(t *testing.T) { + ctx := t.Context() + dir, _ := setupDirForTapTest(t) + wsDir := filepath.Join(dir, "workspace") + customTmpDir := filepath.Join(dir, "workspace/out/soong/.temp") + t.Setenv("TMPDIR", customTmpDir) + t.Setenv("TMP", customTmpDir) + + hfs, err := hashfs.New(ctx, hashfs.Option{}) + if err != nil { + t.Fatal(err) + } + defer hfs.Close(ctx) + err = hfs.WaitReady(ctx) + if err != nil { + t.Fatal(err) + } + + setupFile := func(abs string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(abs), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(abs, []byte("content"), 0644); err != nil { + t.Fatal(err) + } + } + + wsInput := filepath.Join(wsDir, "input.h") + setupFile(wsInput) + + tmpRead := filepath.Join(os.TempDir(), "tmp_read.h") + setupFile(tmpRead) + defer os.Remove(tmpRead) + + customTmpRead := filepath.Join(customTmpDir, "custom_tmp_read.h") + setupFile(customTmpRead) + + customTmpWrite := filepath.Join(customTmpDir, "custom_tmp_write.o") + setupFile(customTmpWrite) + + b := &Builder{ + hashFS: hfs, + path: NewPath(wsDir, "out/siso"), + } + + cmd := &execute.Cmd{ + WorkspaceRoot: wsDir, + } + + err = setTapResult(cmd, + []string{wsInput, tmpRead, customTmpRead}, + []string{customTmpWrite}, + nil, + ) + if err != nil { + t.Fatal(err) + } + + err = b.tapCanonicalizeCmd(ctx, cmd) + if err != nil { + t.Fatalf("tapCanonicalizeCmd returned error: %v", err) + } + + wantInputs := []path.Path{path.New("input.h")} + if !slices.Equal(cmd.Inputs, wantInputs) { + t.Errorf("cmd.Inputs = %v; want %v (tmp reads should be ignored)", cmd.Inputs, wantInputs) + } + + if len(cmd.Outputs) != 0 { + t.Errorf("cmd.Outputs = %v; want empty (tmp writes should be ignored)", cmd.Outputs) + } +} + +func TestTapCanonicalizeCmd_IgnoreCmdEnvTMPDIR(t *testing.T) { + ctx := t.Context() + dir, _ := setupDirForTapTest(t) + wsDir := filepath.Join(dir, "workspace") + customTmpDir := filepath.Join(dir, "workspace/out/soong/.temp") + + hfs, err := hashfs.New(ctx, hashfs.Option{}) + if err != nil { + t.Fatal(err) + } + defer hfs.Close(ctx) + err = hfs.WaitReady(ctx) + if err != nil { + t.Fatal(err) + } + + setupFile := func(abs string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(abs), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(abs, []byte("content"), 0644); err != nil { + t.Fatal(err) + } + } + + wsInput := filepath.Join(wsDir, "input.h") + setupFile(wsInput) + + tmpRead := filepath.Join(os.TempDir(), "tmp_read.h") + setupFile(tmpRead) + defer os.Remove(tmpRead) + + customTmpRead := filepath.Join(customTmpDir, "custom_tmp_read.h") + setupFile(customTmpRead) + + customTmpWrite := filepath.Join(customTmpDir, "custom_tmp_write.o") + setupFile(customTmpWrite) + + b := &Builder{ + hashFS: hfs, + path: NewPath(wsDir, "out/siso"), + } + + cmd := &execute.Cmd{ + WorkspaceRoot: wsDir, + Env: []string{"TMPDIR=" + customTmpDir}, + } + + err = setTapResult(cmd, + []string{wsInput, tmpRead, customTmpRead}, + []string{customTmpWrite}, + nil, + ) + if err != nil { + t.Fatal(err) + } + + err = b.tapCanonicalizeCmd(ctx, cmd) + if err != nil { + t.Fatalf("tapCanonicalizeCmd returned error: %v", err) + } + + wantInputs := []path.Path{path.New("input.h")} + if !slices.Equal(cmd.Inputs, wantInputs) { + t.Errorf("cmd.Inputs = %v; want %v (tmp reads should be ignored)", cmd.Inputs, wantInputs) + } + + if len(cmd.Outputs) != 0 { + t.Errorf("cmd.Outputs = %v; want empty (tmp writes should be ignored)", cmd.Outputs) + } +}