fix: Respect leading @ when apply fine grained filtering

Tinder · Jan 25, 2025 · f8ddde0 · f8ddde0
1 parent 65a68b0
commit f8ddde0
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 31 deletions.
diff --git a/README.md b/README.md
@@ -185,13 +185,15 @@ workspace.
                             By default, external repos are treated as an opaque
                             blob. If an external repo is specified here,
                             bazel-diff instead computes the hash for individual
-                            targets. For example, one wants to specify `maven`
-                            here if they user rules_jvm_external so that
+                            targets. For example, one wants to specify `@maven`
+                            here if they use rules_jvm_external so that
                             individual third party dependency change won't
                             invalidate all targets in the mono repo. Note that
-                            when `--useCquery` is used, the canonical repo name
-                            must be provided but with single `@`, e.g.
-                            `@rules_jvm_external~~maven~maven`
+                            if `--useCquery` is true; or `--useCquery` is false but
+                            `--bazelCommandOptions=--consistent_labels` is provided,
+                            the canonical repo name must be provided,
+                            e.g. `@@maven` or `@@rules_jvm_external~~maven~maven` (bzlmod)
+                            instead of apparent name `@maven`
   -h, --help              Show this help message and exit.
       --ignoredRuleHashingAttributes=<ignoredRuleHashingAttributes>
                           Attributes that should be ignored when hashing rule

diff --git a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt
@@ -41,7 +41,7 @@ class BazelClient(
         } else {
           val buildTargetsQuery =
               listOf("//...:all-targets") +
-                  fineGrainedHashExternalRepos.map { "@$it//...:all-targets" }
+                  fineGrainedHashExternalRepos.map { "$it//...:all-targets" }
           queryService.query((repoTargetsQuery + buildTargetsQuery).joinToString(" + ") { "'$it'" })
         }
     val queryDuration = Calendar.getInstance().getTimeInMillis() - queryEpoch

diff --git a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt
@@ -49,9 +49,7 @@ class BazelRule(private val rule: Build.Rule) {
   ): String {
     if (isNotMainRepo(ruleInput) &&
         ruleInput.startsWith("@") &&
-        fineGrainedHashExternalRepos.none {
-          ruleInput.startsWith("@$it") || ruleInput.startsWith("@@${it}")
-        }) {
+        fineGrainedHashExternalRepos.none { ruleInput.startsWith(it) }) {
       val splitRule = ruleInput.split("//".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray()
       if (splitRule.size == 2) {
         var externalRule = splitRule[0]

diff --git a/cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt b/cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt
@@ -25,7 +25,7 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher {
   private val workingDirectory: Path
   private val logger: Logger
   private val relativeFilenameToContentHash: Map<String, String>?
-  private val fineGrainedHashExternalRepos: Set<String>
+  private val fineGrainedHashExternalRepoNames: Set<String>
   private val externalRepoResolver: ExternalRepoResolver
 
   init {
@@ -38,7 +38,8 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher {
     this.workingDirectory = workingDirectory
     val contentHashProvider: ContentHashProvider by inject()
     relativeFilenameToContentHash = contentHashProvider.filenameToHash
-    this.fineGrainedHashExternalRepos = fineGrainedHashExternalRepos
+    this.fineGrainedHashExternalRepoNames =
+        fineGrainedHashExternalRepos.map { it.replaceFirst("@+".toRegex(), "") }.toSet()
     val externalRepoResolver: ExternalRepoResolver by inject()
     this.externalRepoResolver = externalRepoResolver
   }
@@ -51,7 +52,8 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher {
   ) {
     this.workingDirectory = workingDirectory
     this.relativeFilenameToContentHash = relativeFilenameToContentHash
-    this.fineGrainedHashExternalRepos = fineGrainedHashExternalRepos
+    this.fineGrainedHashExternalRepoNames =
+        fineGrainedHashExternalRepos.map { it.replaceFirst("@+".toRegex(), "") }.toSet()
     this.externalRepoResolver = externalRepoResolver
   }
 
@@ -67,18 +69,13 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher {
             val filenameSubstring = name.substring(index)
             Paths.get(filenameSubstring.removePrefix(":").replace(':', '/'))
           } else if (name.startsWith("@")) {
-            val parts =
-                if (name.startsWith("@@")) {
-                  name.substring(2).split("//")
-                } else {
-                  name.substring(1).split("//")
-                }
+            val parts = name.replaceFirst("@+".toRegex(), "").split("//")
             if (parts.size != 2) {
               logger.w { "Invalid source label $name" }
               return@sha256
             }
             val repoName = parts[0]
-            if (repoName !in fineGrainedHashExternalRepos) {
+            if (repoName !in fineGrainedHashExternalRepoNames) {
               return@sha256
             }
             val relativePath = Paths.get(parts[1].removePrefix(":").replace(':', '/'))

diff --git a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt
@@ -138,7 +138,7 @@ class E2ETest {
         "-b",
         bazelPath,
         "--fineGrainedHashExternalRepos",
-        "bazel_diff_maven",
+        "@bazel_diff_maven",
         from.absolutePath)
     // To
     cli.execute(
@@ -148,7 +148,7 @@ class E2ETest {
         "-b",
         bazelPath,
         "--fineGrainedHashExternalRepos",
-        "bazel_diff_maven",
+        "@bazel_diff_maven",
         to.absolutePath)
     // Impacted targets
     cli.execute(
@@ -254,15 +254,15 @@ class E2ETest {
   fun testFineGrainedHashBzlMod() {
     testFineGrainedHashBzlMod(
         emptyList(),
-        "bazel_diff_maven",
+        "@bazel_diff_maven",
         "/fixture/fine-grained-hash-bzlmod-test-impacted-targets.txt")
   }
 
   @Test
   fun testFineGrainedHashBzlModCquery() {
     testFineGrainedHashBzlMod(
         listOf("--useCquery"),
-        "@rules_jvm_external~~maven~maven",
+        "@@rules_jvm_external~~maven~maven",
         "/fixture/fine-grained-hash-bzlmod-cquery-test-impacted-targets.txt")
   }
 
@@ -345,7 +345,7 @@ class E2ETest {
         "--cqueryCommandOptions",
         "--platforms=//:android",
         "--fineGrainedHashExternalRepos",
-        "bazel_diff_maven,bazel_diff_maven_android",
+        "@@bazel_diff_maven,@@bazel_diff_maven_android",
         from.absolutePath)
     // To
     cli.execute(
@@ -358,7 +358,7 @@ class E2ETest {
         "--cqueryCommandOptions",
         "--platforms=//:android",
         "--fineGrainedHashExternalRepos",
-        "bazel_diff_maven,bazel_diff_maven_android",
+        "@@bazel_diff_maven,@@bazel_diff_maven_android",
         to.absolutePath)
     // Impacted targets
     cli.execute(
@@ -391,7 +391,7 @@ class E2ETest {
         "--cqueryCommandOptions",
         "--platforms=//:jre",
         "--fineGrainedHashExternalRepos",
-        "bazel_diff_maven,bazel_diff_maven_android",
+        "@@bazel_diff_maven,@@bazel_diff_maven_android",
         from.absolutePath)
     // To
     cli.execute(
@@ -404,7 +404,7 @@ class E2ETest {
         "--cqueryCommandOptions",
         "--platforms=//:jre",
         "--fineGrainedHashExternalRepos",
-        "bazel_diff_maven,bazel_diff_maven_android",
+        "@@bazel_diff_maven,@@bazel_diff_maven_android",
         to.absolutePath)
     // Impacted targets
     cli.execute(
@@ -505,7 +505,7 @@ class E2ETest {
         "--cqueryCommandOptions",
         "--platforms=//:android",
         "--fineGrainedHashExternalRepos",
-        "bazel_diff_maven,bazel_diff_maven_android",
+        "@@bazel_diff_maven,@@bazel_diff_maven_android",
         from.absolutePath)
     // To
     cli.execute(
@@ -518,7 +518,7 @@ class E2ETest {
         "--cqueryCommandOptions",
         "--platforms=//:android",
         "--fineGrainedHashExternalRepos",
-        "bazel_diff_maven,bazel_diff_maven_android",
+        "@@bazel_diff_maven,@@bazel_diff_maven_android",
         to.absolutePath)
     // Impacted targets
     cli.execute(
@@ -552,7 +552,7 @@ class E2ETest {
         "--cqueryCommandOptions",
         "--platforms=//:jre",
         "--fineGrainedHashExternalRepos",
-        "bazel_diff_maven,bazel_diff_maven_android",
+        "@@bazel_diff_maven,@@bazel_diff_maven_android",
         from.absolutePath)
     // To
     cli.execute(
@@ -565,7 +565,7 @@ class E2ETest {
         "--cqueryCommandOptions",
         "--platforms=//:jre",
         "--fineGrainedHashExternalRepos",
-        "bazel_diff_maven,bazel_diff_maven_android",
+        "@@bazel_diff_maven,@@bazel_diff_maven_android",
         to.absolutePath)
     // Impacted targets
     cli.execute(