explain: Distinguish transitive invalidation through tools (#8359)

If a change to a library causes a large number of transitive
invalidations, this can be because it affects the output of a tool.
Making this explicit in the invalidation summary helps explain the
situation.

Author: fmeum

cli/explain/compactgraph/compactgraph.go

@@ -267,6 +267,9 @@ func Diff(old, new *CompactGraph) (*spawn_diff.DiffResult, error) {
 		}
 		resultEntry, _ := diffResults.Load(output)
 		result := resultEntry.(*diffResult)
+		if len(result.spawnDiff.GetModified().Diffs) == 0 {
+			continue
+		}
 		spawn := new.spawns[output]
 		foundTransitiveCause := false
 		// Get the deduplicated primary outputs for those spawns referenced via invalidatedBy.
@@ -286,13 +289,13 @@ func Diff(old, new *CompactGraph) (*spawn_diff.DiffResult, error) {
 				invalidatingResult.invalidates = append(invalidatingResult.invalidates, result.invalidates, spawn)
 			}
 		}
-		if len(result.spawnDiff.GetModified().Diffs) > 0 && (result.localChange || !foundTransitiveCause) {
+		if result.localChange || !foundTransitiveCause {
 			if len(result.invalidates) > 0 {
 				// result.invalidates isn't modified after this point as the spawns are visited in topological order.
 				diffWG.Add(1)
 				go func() {
 					defer diffWG.Done()
-					result.spawnDiff.GetModified().TransitivelyInvalidated = flattenInvalidates(result.invalidates)
+					result.spawnDiff.GetModified().TransitivelyInvalidated = flattenInvalidates(result.invalidates, isExecOutputPath(output))
 				}()
 			}
 			spawnDiffs = append(spawnDiffs, result.spawnDiff)
@@ -309,7 +312,10 @@ func Diff(old, new *CompactGraph) (*spawn_diff.DiffResult, error) {
 
 // flattenInvalidates flattens a tree of Spawn nodes into a deduplicated map of mnemonic to count of transitively
 // invalidated spawns.
-func flattenInvalidates(invalidates []any) map[string]uint32 {
+// Mnemonics of spawns are suffixed with " (as tool)" if the invalidating spawn is a tool and the invalidated spawn is
+// not, that is, if the dependency path crosses an edge with an "exec" transition (ignoring "exec" transitions on
+// targets that are already in the "exec" configuration).
+func flattenInvalidates(invalidates []any, isTool bool) map[string]uint32 {
 	transitivelyInvalidated := make(map[string]uint32)
 	spawnsSeen := make(map[*Spawn]struct{})
 	toVisit := invalidates
@@ -320,7 +326,11 @@ func flattenInvalidates(invalidates []any) map[string]uint32 {
 		case *Spawn:
 			if _, seen := spawnsSeen[n]; !seen {
 				spawnsSeen[n] = struct{}{}
-				transitivelyInvalidated[n.Mnemonic]++
+				suffix := ""
+				if isTool && !isExecOutputPath(n.PrimaryOutputPath()) {
+					suffix = " (as tool)"
+				}
+				transitivelyInvalidated[n.Mnemonic+suffix]++
 			}
 		default:
 			// If n is not a Spawn, it must be a slice of Spawns or slices.

cli/explain/compactgraph/compactgraph_test.go

@@ -315,6 +315,33 @@ func TestTransitiveInvalidation(t *testing.T) {
 	}
 }
 
+func TestTransitiveInvalidationForTool(t *testing.T) {
+	spawnDiffs := diffLogs(t, "transitive_invalidation_for_tool", "8.0.0")
+	require.Len(t, spawnDiffs, 2)
+
+	{
+		sd := spawnDiffs[0]
+		assert.Regexp(t, "^bazel-out/[^/]+-fastbuild/bin/pkg/lib.out$", sd.PrimaryOutput)
+		assert.Equal(t, "//pkg:library", sd.TargetLabel)
+		assert.Equal(t, "Genrule", sd.Mnemonic)
+		assert.Equal(t, map[string]uint32{"Genrule": 1}, sd.GetModified().GetTransitivelyInvalidated())
+		require.Len(t, sd.GetModified().GetDiffs(), 1)
+		assert.Len(t, sd.GetModified().GetDiffs()[0].GetInputContents().GetFileDiffs(), 1)
+	}
+	{
+		sd := spawnDiffs[1]
+		assert.Regexp(t, "^bazel-out/[^/]+-exec-[^/]+/bin/pkg/lib.out$", sd.PrimaryOutput)
+		assert.Equal(t, "//pkg:library", sd.TargetLabel)
+		assert.Equal(t, "Genrule", sd.Mnemonic)
+		assert.Equal(t, map[string]uint32{
+			"Genrule":           1,
+			"Genrule (as tool)": 3,
+		}, sd.GetModified().GetTransitivelyInvalidated())
+		require.Len(t, sd.GetModified().GetDiffs(), 1)
+		assert.Len(t, sd.GetModified().GetDiffs()[0].GetInputContents().GetFileDiffs(), 1)
+	}
+}
+
 func TestNonHermetic(t *testing.T) {
 	spawnDiffs := diffLogs(t, "non_hermetic", "7.3.1")
 	require.Len(t, spawnDiffs, 1)
@@ -512,7 +539,7 @@ func TestToolRunfilesPaths(t *testing.T) {
 	assert.Regexp(t, "^bazel-out/[^/]+/bin/pkg/tool.runfiles", sd.PrimaryOutput)
 	assert.Equal(t, "//tools:tool_sh", sd.TargetLabel)
 	assert.Equal(t, "Runfiles directory", sd.Mnemonic)
-	assert.Equal(t, map[string]uint32{"Genrule": 2}, sd.GetModified().GetTransitivelyInvalidated())
+	assert.Equal(t, map[string]uint32{"Genrule (as tool)": 2}, sd.GetModified().GetTransitivelyInvalidated())
 	require.Len(t, sd.GetModified().GetDiffs(), 1)
 	d := sd.GetModified().Diffs[0]
 	require.IsType(t, &spawn_diff.Diff_InputPaths{}, d.Diff)
@@ -528,7 +555,7 @@ func TestToolRunfilesContents(t *testing.T) {
 	assert.Regexp(t, "^bazel-out/[^/]+/bin/pkg/tool.runfiles", sd.PrimaryOutput)
 	assert.Equal(t, "//tools:tool_sh", sd.TargetLabel)
 	assert.Equal(t, "Runfiles directory", sd.Mnemonic)
-	assert.Equal(t, map[string]uint32{"Genrule": 2}, sd.GetModified().GetTransitivelyInvalidated())
+	assert.Equal(t, map[string]uint32{"Genrule (as tool)": 2}, sd.GetModified().GetTransitivelyInvalidated())
 	require.Len(t, sd.GetModified().GetDiffs(), 1)
 	d := sd.GetModified().Diffs[0]
 	require.IsType(t, &spawn_diff.Diff_InputContents{}, d.Diff)
@@ -549,7 +576,7 @@ func TestToolRunfilesContentsTransitive(t *testing.T) {
 	assert.Regexp(t, "^bazel-out/[^/]+/bin/tools/tool.sh", sd.PrimaryOutput)
 	assert.Equal(t, "//tools:tool_sh", sd.TargetLabel)
 	assert.Equal(t, "Genrule", sd.Mnemonic)
-	assert.Equal(t, map[string]uint32{"Genrule": 2, "Runfiles directory": 1}, sd.GetModified().GetTransitivelyInvalidated())
+	assert.Equal(t, map[string]uint32{"Genrule (as tool)": 2, "Runfiles directory": 1}, sd.GetModified().GetTransitivelyInvalidated())
 	require.Len(t, sd.GetModified().GetDiffs(), 1)
 	d := sd.GetModified().Diffs[0]
 	require.IsType(t, &spawn_diff.Diff_Args{}, d.Diff)
@@ -571,7 +598,7 @@ func TestToolRunfilesSymlinksPaths(t *testing.T) {
 	assert.Regexp(t, "^bazel-out/[^/]+/bin/tools/tool.runfiles$", sd.PrimaryOutput)
 	assert.Empty(t, sd.TargetLabel)
 	assert.Equal(t, "Runfiles directory", sd.Mnemonic)
-	assert.Equal(t, map[string]uint32{"Genrule": 1}, sd.GetModified().GetTransitivelyInvalidated())
+	assert.Equal(t, map[string]uint32{"Genrule (as tool)": 1}, sd.GetModified().GetTransitivelyInvalidated())
 	require.Len(t, sd.GetModified().GetDiffs(), 1)
 	d := sd.GetModified().Diffs[0]
 	require.IsType(t, &spawn_diff.Diff_InputPaths{}, d.Diff)
@@ -588,7 +615,7 @@ func TestToolRunfilesSymlinksContents(t *testing.T) {
 	assert.Regexp(t, "^bazel-out/[^/]+/bin/tools/tool.runfiles$", sd.PrimaryOutput)
 	assert.Empty(t, sd.TargetLabel)
 	assert.Equal(t, "Runfiles directory", sd.Mnemonic)
-	assert.Equal(t, map[string]uint32{"Genrule": 1}, sd.GetModified().GetTransitivelyInvalidated())
+	assert.Equal(t, map[string]uint32{"Genrule (as tool)": 1}, sd.GetModified().GetTransitivelyInvalidated())
 	require.Len(t, sd.GetModified().GetDiffs(), 1)
 	d := sd.GetModified().Diffs[0]
 	require.IsType(t, &spawn_diff.Diff_InputContents{}, d.Diff)
@@ -620,7 +647,7 @@ func TestToolRunfilesSymlinksContentsTransitive(t *testing.T) {
 	assert.Equal(t, "//gen:gen", sd.TargetLabel)
 	assert.Equal(t, "Genrule", sd.Mnemonic)
 	assert.Equal(t, map[string]uint32{
-		"Genrule":            1,
+		"Genrule (as tool)":  1,
 		"Runfiles directory": 1,
 	}, sd.GetModified().GetTransitivelyInvalidated())
 	require.Len(t, sd.GetModified().GetDiffs(), 1)

cli/explain/compactgraph/input.go

@@ -678,6 +678,16 @@ func isSourcePath(path string) bool {
 	return !strings.HasPrefix(path, "bazel-out/")
 }
 
+// Exec-configured output paths look like:
+// bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/pkg/foo
+// bazel-out/my_platform-opt-exec/bin/pkg/foo
+var execOutputRegexp = regexp.MustCompile("bazel-out/[^/]+-exec[-/].*")
+
+// Whether the given path is an output path of an artifact built in the exec configuration.
+func isExecOutputPath(path string) bool {
+	return execOutputRegexp.MatchString(path)
+}
+
 func computeRunfilesPath(input Input) string {
 	p := input.Path()
 	// For a path such as "bazel-out/k8-fastbuild/bin/pkg/foo", trim to "pkg/foo".

cli/explain/compactgraph/testdata/8.0.0/transitive_invalidation_for_tool_new.pb.zstd

@@ -862,6 +862,61 @@ old
 new
 -- pkg/in2.txt --
 new
+`,
+			bazelVersions: []string{"8.0.0"},
+		},
+		{
+			name: "transitive_invalidation_for_tool",
+			baseline: `
+-- MODULE.bazel --
+-- pkg/BUILD --
+genrule(
+    name = "library",
+    outs = ["lib.out"],
+    srcs = ["lib.in"],
+    cmd = "cp $< $@",
+)
+
+genrule(
+    name = "tool",
+	outs = ["tool.out"],
+	srcs = [":library"],
+    cmd = "cp $< $@",
+	executable = True,
+    tags = ["manual"],
+)
+
+genrule(
+    name = "tool_via_tool",
+	outs = ["tool_via_tool.out"],
+    tools = [":tool"],
+    cmd = "cp $(location :tool) $@",
+    executable = True,
+)
+
+genrule(
+	name = "gen",
+	outs = ["gen.out"],
+    tools = [":tool"],
+	cmd = "cp $(location :tool) $@",
+)
+
+genrule(
+    name = "toplevel",
+	outs = ["toplevel.out"],
+	srcs = [
+        ":library",
+        ":gen",
+        ":tool_via_tool",
+    ],
+	cmd = "cat $(SRCS) > $@",
+)
+-- pkg/lib.in --
+old
+`,
+			changes: `
+-- pkg/lib.in --
+new
 `,
 			bazelVersions: []string{"8.0.0"},
 		},