Fix #259 / #227 .bzl macro changes now invalidate targets

MODULE.bazel

@@ -1,6 +1,6 @@
 module(
     name = "bazel-diff",
-    version = "19.0.1",
+    version = "20.0.0",
     compatibility_level = 0,
 )
 

README.md

@@ -259,7 +259,8 @@ workspace.
 
 ```terminal
 Missing required options: '--startingHashes=<startingHashesJSONPath>', '--finalHashes=<finalHashesJSONPath>', '--workspacePath=<workspacePath>'
-Usage: bazel-diff get-impacted-targets [-v] [--[no-]noBazelrc] [-b=<bazelPath>]
+Usage: bazel-diff get-impacted-targets [-v] [--[no-]excludeExternalTargets] [--
+                                       [no-]noBazelrc] [-b=<bazelPath>]
                                        [-d=<depsMappingJSONPath>]
                                        -fh=<finalHashesJSONPath>
                                        [-o=<outputPath>]
@@ -275,6 +276,17 @@ Command-line utility to analyze the state of the bazel build graph
                          Path to the file where dependency edges are. If
                            specified, build graph distance metrics will be
                            computed from the given hash data.
+      --[no-]excludeExternalTargets
+                         If true, drop labels starting with '//external:' from
+                           the impacted-targets output. These synthetic labels
+                           are produced for bzlmod-managed external repos so
+                           generate-hashes can detect dep changes, but they are
+                           not buildable in bzlmod-only mode (Bazel 8.6.0+ with
+                           --enable_workspace=false) and will fail downstream
+                           `bazel build`. See https://github.
+                           com/Tinder/bazel-diff/issues/326. When unset,
+                           defaults to true if Bzlmod is detected (via `bazel
+                           mod graph`), false otherwise.
       -fh, --finalHashes=<finalHashesJSONPath>
                          The path to the JSON file of target hashes for the
                            final revision. Run 'generate-hashes' to get this
@@ -320,7 +332,7 @@ First, add the following snippet to your project:
 #### Bzlmod snippet
 
 ```bazel
-bazel_dep(name = "bazel-diff", version = "19.0.1")
+bazel_dep(name = "bazel-diff", version = "20.0.0")
 ```
 
 You can now run the tool with:

cli/src/main/kotlin/com/bazel_diff/hash/BuildGraphHasher.kt

@@ -55,8 +55,22 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
         }
     val seedForFilepaths =
         runBlocking(Dispatchers.IO) { createSeedForFilepaths(seedFilepaths) }
+    val seedForBzlFiles = createSeedForBzlFiles(allTargets, modifiedFilepaths)
+    // Only mix the bzl-files seed when at least one main-repo `.bzl` was actually hashed.
+    // This keeps the seed (and therefore every target's hash) byte-for-byte stable for
+    // workspaces that don't load any tracked .bzl files, preserving cached hashes from
+    // before this change.
+    val combinedSeed =
+        if (seedForBzlFiles != null) {
+          sha256 {
+            safePutBytes(seedForFilepaths)
+            safePutBytes(seedForBzlFiles)
+          }
+        } else {
+          seedForFilepaths
+        }
     return hashAllTargets(
-        seedForFilepaths, sourceDigests, allTargets, ignoredAttrs, modifiedFilepaths)
+        combinedSeed, sourceDigests, allTargets, ignoredAttrs, modifiedFilepaths)
   }
 
   private fun hashSourcefiles(
@@ -171,4 +185,53 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
       }
     }
   }
+
+  /**
+   * Builds a seed-hash contribution from the contents of every `.bzl` (and `.scl`) file the
+   * workspace's BUILD files load.
+   *
+   * Background: Bazel pre-7 populated [Build.Rule.skylark_environment_hash_code] in the query
+   * proto, so any change to a `.bzl` file loaded by a rule's BUILD file naturally bubbled into
+   * that rule's hash. Bazel 7+ leaves that field empty, which is the root cause of issues
+   * [#259](https://github.com/Tinder/bazel-diff/issues/259) and
+   * [#227](https://github.com/Tinder/bazel-diff/issues/227): editing a macro body (e.g. adding
+   * `print()`) no longer invalidated any caller because the emitted rule attrs were identical.
+   *
+   * Fix: walk every queried SourceFile target's `subincludeList` (the `Build.SourceFile.subinclude`
+   * proto field, which already lists every `.bzl` the BUILD file loaded), softDigest each main-repo
+   * `.bzl`, and roll the union of digests into the workspace-wide seed. This is intentionally
+   * conservative -- a single `.bzl` edit re-hashes every target -- because `.bzl` edits are rare
+   * and missing them silently is the worse failure mode. Per-package precision would require
+   * mapping each rule to its package's BUILD file, which isn't a direct dep relationship in the
+   * query proto.
+   *
+   * External-repo `.bzl` files (`@repo//...`, `@@canonical//...`) are skipped because
+   * [SourceFileHasher.softDigest] returns null for non-main-repo labels, which keeps the seed
+   * stable across BCR fetches that don't actually change repo contents.
+   */
+  private fun createSeedForBzlFiles(
+      allTargets: List<BazelTarget>,
+      modifiedFilepaths: Set<Path>
+  ): ByteArray? {
+    val subincludes =
+        allTargets
+            .asSequence()
+            .filterIsInstance<BazelTarget.SourceFile>()
+            .flatMap { it.subincludeList.asSequence() }
+            .toSortedSet()
+    val tracked = mutableListOf<Pair<String, ByteArray>>()
+    for (label in subincludes) {
+      val digest =
+          sourceFileHasher.softDigest(
+              BazelSourceFileTarget(label, ByteArray(0)), modifiedFilepaths)
+      if (digest != null) tracked += label to digest
+    }
+    if (tracked.isEmpty()) return null
+    return sha256 {
+      for ((label, digest) in tracked) {
+        safePutBytes(label.toByteArray())
+        safePutBytes(digest)
+      }
+    }
+  }
 }

cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt

@@ -1446,6 +1446,84 @@ class E2ETest {
         .isEqualTo(true)
   }
 
+  // ------------------------------------------------------------------------
+  // Regression coverage for https://github.com/Tinder/bazel-diff/issues/259 and #227.
+  // ------------------------------------------------------------------------
+  // Both issues describe the same underlying gap: when a BUILD file `load()`s a .bzl
+  // macro, editing the .bzl macro body in a way that does not change the generated rule's
+  // attrs is not reflected in `bazel-diff get-impacted-targets`. The user in #259
+  // noticed this regression after upgrading to Bazel 7 -- Bazel pre-7 populated
+  // `Rule.skylark_environment_hash_code` in the query proto so .bzl-content changes
+  // bubbled in naturally; Bazel 7+ leaves that field empty, so bazel-diff missed the change.
+  //
+  // The fix in BuildGraphHasher.hashAllBazelTargetsAndSourcefiles walks every BUILD
+  // source file's `subincludeList` (the `subinclude` proto field, which lists every .bzl
+  // loaded by that BUILD), softDigests each main-repo .bzl file, and mixes the union of
+  // digests into the seed hash. This restores the pre-Bazel-7 behaviour: a `.bzl`-only
+  // edit invalidates the targets that depend on it.
+  //
+  // The reproducer workspace `macro_invalidation` has:
+  //   - `miniature.bzl` defines a `miniature(name, src)` macro that wraps `native.genrule`.
+  //   - `BUILD` does `load(":miniature.bzl", "miniature")` and calls `miniature(...)` to
+  //     produce `//:logo_miniature`.
+  //
+  // The test mutates `miniature.bzl` to add a `print()` call inside the macro body -- this
+  // does not change any attribute of the emitted `native.genrule`, so a fix that only looks
+  // at rule attrs would miss it. The user's example in #259 was exactly this pattern.
+  @Test
+  fun testMacroBzlChangeImpactsCallers_regressionForIssue259And227() {
+    val workspaceA = copyTestWorkspace("macro_invalidation")
+    val workspaceB = copyTestWorkspace("macro_invalidation")
+
+    // Mutate only the macro body in B by adding a `print()` call. This deliberately does
+    // not touch the genrule attrs the macro emits, so the bug shows up only via the .bzl
+    // file content rather than any rule-attribute hash diff.
+    val bzlInB = File(workspaceB, "miniature.bzl")
+    val originalBzl = bzlInB.readText()
+    val mutatedBzl = originalBzl.replace(
+        "def miniature(name, src, **kwargs):",
+        "def miniature(name, src, **kwargs):\n    print(\"miniature: \" + name)")
+    assertThat(mutatedBzl != originalBzl).isEqualTo(true)
+    bzlInB.writeText(mutatedBzl)
+
+    val outputDir = temp.newFolder()
+    val from = File(outputDir, "starting_hashes.json")
+    val to = File(outputDir, "final_hashes.json")
+    val impactedTargetsOutput = File(outputDir, "impacted_targets.txt")
+
+    val cli = CommandLine(BazelDiff())
+    assertThat(
+            cli.execute(
+                "generate-hashes",
+                "-w", workspaceA.absolutePath,
+                "-b", "bazel",
+                from.absolutePath))
+        .isEqualTo(0)
+    assertThat(
+            cli.execute(
+                "generate-hashes",
+                "-w", workspaceB.absolutePath,
+                "-b", "bazel",
+                to.absolutePath))
+        .isEqualTo(0)
+
+    assertThat(
+            cli.execute(
+                "get-impacted-targets",
+                "-w", workspaceB.absolutePath,
+                "-b", "bazel",
+                "-sh", from.absolutePath,
+                "-fh", to.absolutePath,
+                "-o", impactedTargetsOutput.absolutePath))
+        .isEqualTo(0)
+
+    val impacted = impactedTargetsOutput.readLines().filter { it.isNotBlank() }.toSet()
+    val callerImpacted = impacted.any { it.contains("logo_miniature") }
+    assertThat(callerImpacted)
+        .transform("//:logo_miniature should be impacted by miniature.bzl edit; got: $impacted") { it }
+        .isEqualTo(true)
+  }
+
   private fun copyTestWorkspace(path: String): File {
     val testProject = temp.newFolder()
 

cli/src/test/resources/workspaces/macro_invalidation/BUILD

@@ -0,0 +1,10 @@
+load(":miniature.bzl", "miniature")
+
+# Source consumed by the macro-generated genrule. Used so the test workspace builds
+# without external rules; the macro itself is what we're testing for invalidation.
+exports_files(["image.png"])
+
+miniature(
+    name = "logo_miniature",
+    src = "image.png",
+)

cli/src/test/resources/workspaces/macro_invalidation/MODULE.bazel

@@ -0,0 +1,4 @@
+module(
+    name = "macro_invalidation_test",
+    version = "0.0.0",
+)