feat(dirty-set): stay incremental across package boundary changes

README.md

@@ -88,7 +88,7 @@ For example, a repository rule may materialize files from a baseline revision se
 
 This affects target selection only. If the consumer is selected for another reason, Bazel still uses and hashes the real repository contents when executing it. Callers must ensure that changes which should select the consumer—such as its current source inputs or comparison policy—remain normal, non-opaque inputs.
 
-Incremental hashing is an optimization rather than a weaker correctness mode. If `hash-persister` cannot prove that reuse is safe, it logs a bounded fallback code and performs a normal full computation. This includes incompatible or malformed seeds and changes that can affect Bazel loading or package boundaries without appearing in the persisted target graph, such as changes to Starlark, workspace or module metadata, Bazel configuration files, or BUILD-file boundaries. The resulting output is still a complete artifact for the requested revision. It can seed a later incremental run only when `--seedable-output` was requested.
+Incremental hashing is an optimization rather than a weaker correctness mode. If `hash-persister` cannot prove that reuse is safe, it logs a bounded fallback code and performs a normal full computation. This includes incompatible or malformed seeds and changes that can affect Bazel loading or package boundaries without appearing in the persisted target graph, such as changes to Starlark, workspace or module metadata, or Bazel configuration files. Adding or deleting a BUILD file does not force a fallback: the new or removed package and its nearest enclosing package are rehashed incrementally. The resulting output is still a complete artifact for the requested revision. It can seed a later incremental run only when `--seedable-output` was requested.
 
 By default, `hash-persister` also falls back when incremental hashing would recompute at least 70% of the targets in the seed, since the fixed costs of loading and merging the seed are unlikely to pay off. Use `--incremental-recomputation-fallback-percent` to tune that threshold from 1 to 100 for a particular repository.
 

hash-persister/hash-persister.go

@@ -13,6 +13,7 @@ import (
 	"log"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"strings"
 	"time"
@@ -258,7 +259,10 @@ func runSeeded(cfg *config) (seededOutcome, error) {
 
 	phaseStart = time.Now()
 	allLabels := pkg.CollectAllLabels(seedData.TargetEdges, seedData.TargetHashes)
-	dirtyResult := pkg.ComputeDirtySet(changedFiles, seedData.TargetEdges, allLabels, fingerprintFiles, cfg.FallbackTriggerPatterns)
+	packagesExistAtTarget := func(pkgs []string) (map[string]bool, error) {
+		return gitPackagesExist(cfg.Context.WorkspacePath, cfg.CommitSha, pkgs)
+	}
+	dirtyResult := pkg.ComputeDirtySet(changedFiles, seedData.TargetEdges, allLabels, fingerprintFiles, cfg.FallbackTriggerPatterns, packagesExistAtTarget)
 	outcome.DirtyPackageCount = len(dirtyResult.DirtyPackages)
 	outcome.DirtyTargetCount = len(dirtyResult.DirtyStarLabels)
 	log.Printf("Dirty set computed in %v: %d dirty, %d dirty*, %d dirty packages (fallback=%v)",
@@ -341,24 +345,7 @@ func runSeeded(cfg *config) (seededOutcome, error) {
 	// The scoped universe re-lists every dirty package with a wildcard (to
 	// pick up added and deleted targets) and names the rdeps-propagated
 	// targets in unchanged packages explicitly.
-	carriedLabels := make([]string, 0, len(dirtyResult.DirtyStarLabels))
-	dirtyPackageSet := make(map[string]bool, len(dirtyResult.DirtyPackages))
-	for _, p := range dirtyResult.DirtyPackages {
-		dirtyPackageSet[p] = true
-	}
-	for label := range dirtyResult.DirtyStarLabels {
-		// Only carry labels that were matching targets in the seed: labels
-		// that only appear as dependency edges (source files, external or
-		// manual targets) are not part of the persisted target set.
-		if _, ok := seedData.TargetHashes[label]; !ok {
-			continue
-		}
-		if dirtyPackageSet[pkg.LabelPackage(label)] {
-			continue
-		}
-		carriedLabels = append(carriedLabels, label)
-	}
-
+	carriedLabels := carriedScopedLabels(dirtyResult, seedData.TargetHashes)
 	universe := pkg.BuildScopedUniverse(dirtyResult.DirtyPackages, carriedLabels)
 	scopedPattern, err := pkg.ScopeTargetsPattern(cfg.Targets.String(), universe)
 	if err != nil {
@@ -424,6 +411,34 @@ func runSeeded(cfg *config) (seededOutcome, error) {
 	return outcome, nil
 }
 
+// carriedScopedLabels returns the dirty* labels that must be named explicitly
+// in the scoped universe: seeded matching targets outside the dirty packages,
+// which are already re-listed by wildcard, and outside removed packages,
+// whose targets no longer exist.
+func carriedScopedLabels(dirtyResult *pkg.DirtySetResult, seedTargetHashes map[string]map[string]string) []string {
+	skipPackages := make(map[string]bool, len(dirtyResult.DirtyPackages)+len(dirtyResult.RemovedPackages))
+	for _, p := range dirtyResult.DirtyPackages {
+		skipPackages[p] = true
+	}
+	for _, p := range dirtyResult.RemovedPackages {
+		skipPackages[p] = true
+	}
+	carried := make([]string, 0, len(dirtyResult.DirtyStarLabels))
+	for label := range dirtyResult.DirtyStarLabels {
+		// Only carry labels that were matching targets in the seed: labels
+		// that only appear as dependency edges (source files, external or
+		// manual targets) are not part of the persisted target set.
+		if _, ok := seedTargetHashes[label]; !ok {
+			continue
+		}
+		if skipPackages[pkg.LabelPackage(label)] {
+			continue
+		}
+		carried = append(carried, label)
+	}
+	return carried
+}
+
 // probePruneDirtySet runs a small probe query on just the dirty packages,
 // hashes those targets with seed hashes for their dependencies, and returns
 // a pruned DirtySetResult where rdeps are only propagated from targets
@@ -880,6 +895,41 @@ func gitDiffNameStatus(workingDirectory, fromSha, toSha string) (map[string]stri
 	return parseGitNameStatus(stdout.Bytes())
 }
 
+// gitPackagesExist reports which of the given packages ("//a/b", or "//" for
+// the root) have a BUILD or BUILD.bazel file at sha. All packages are looked
+// up in a single path-limited, non-recursive git ls-tree.
+func gitPackagesExist(workingDirectory, sha string, pkgs []string) (map[string]bool, error) {
+	args := []string{"ls-tree", "-z", "--name-only", sha, "--"}
+	for _, p := range pkgs {
+		dir := strings.TrimPrefix(p, "//")
+		for _, name := range []string{"BUILD", "BUILD.bazel"} {
+			args = append(args, path.Join(dir, name))
+		}
+	}
+	gitCmd := exec.Command("git", args...)
+	gitCmd.Dir = workingDirectory
+	var stdout, stderr bytes.Buffer
+	gitCmd.Stdout = &stdout
+	gitCmd.Stderr = &stderr
+	if err := gitCmd.Run(); err != nil {
+		return nil, fmt.Errorf("git ls-tree %s failed: %w. Stderr: %s", sha, err, stderr.String())
+	}
+
+	existing := make(map[string]bool, len(pkgs))
+	for _, entry := range bytes.Split(stdout.Bytes(), []byte{0}) {
+		if len(entry) == 0 {
+			continue
+		}
+		dir := path.Dir(string(entry))
+		if dir == "." {
+			existing["//"] = true
+		} else {
+			existing["//"+dir] = true
+		}
+	}
+	return existing, nil
+}
+
 func parseGitNameStatus(output []byte) (map[string]string, error) {
 	fields := bytes.Split(output, []byte{0})
 	if len(fields) > 0 && len(fields[len(fields)-1]) == 0 {

hash-persister/hash-persister_test.go

@@ -3,6 +3,8 @@ package main
 import (
 	"encoding/json"
 	"os"
+	"os/exec"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
@@ -300,3 +302,68 @@ func TestBuildProbePattern(t *testing.T) {
 		})
 	}
 }
+
+func TestCarriedScopedLabelsSkipsDirtyAndRemovedPackages(t *testing.T) {
+	dirty := &pkg.DirtySetResult{
+		DirtyStarLabels: map[string]bool{
+			"//dirty:a":   true,
+			"//gone:b":    true,
+			"//carried:c": true,
+			"//carried:d": true,
+		},
+		DirtyPackages:   []string{"//dirty"},
+		RemovedPackages: []string{"//gone"},
+	}
+	seedTargetHashes := map[string]map[string]string{
+		"//dirty:a":   {"": "00"},
+		"//gone:b":    {"": "00"},
+		"//carried:c": {"": "00"},
+	}
+
+	got := carriedScopedLabels(dirty, seedTargetHashes)
+
+	if !reflect.DeepEqual(got, []string{"//carried:c"}) {
+		t.Errorf("carriedScopedLabels = %v, want [//carried:c]", got)
+	}
+}
+
+func TestGitPackagesExist(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not available")
+	}
+	dir := t.TempDir()
+	for _, f := range []string{"BUILD.bazel", "a/b/BUILD.bazel", "c/BUILD", "d/src.java"} {
+		if err := os.MkdirAll(filepath.Join(dir, filepath.Dir(f)), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(dir, f), nil, 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	git := func(args ...string) string {
+		cmd := exec.Command("git", append([]string{"-c", "user.name=t", "-c", "user.email=t@t"}, args...)...)
+		cmd.Dir = dir
+		out, err := cmd.CombinedOutput()
+		if err != nil {
+			t.Fatalf("git %v: %v: %s", args, err, out)
+		}
+		return strings.TrimSpace(string(out))
+	}
+	git("init", "-q")
+	git("add", ".")
+	git("commit", "-qm", "init")
+	sha := git("rev-parse", "HEAD")
+
+	got, err := gitPackagesExist(dir, sha, []string{"//", "//a/b", "//c", "//d", "//missing"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := map[string]bool{"//": true, "//a/b": true, "//c": true}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("gitPackagesExist = %v, want %v", got, want)
+	}
+
+	if _, err := gitPackagesExist(dir, "0000000000000000000000000000000000000000", []string{"//a/b"}); err == nil {
+		t.Error("expected an error for an unknown commit")
+	}
+}