Skip to content

Commit 7c8e9b8

Browse files
honnixhonnix
authored
fix: Respect leading @ when apply fine grained filtering (#261)
1 parent 65a68b0 commit 7c8e9b8

File tree

5 files changed

+28
-31
lines changed

5 files changed

+28
-31
lines changed

‎README.md‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,15 @@ workspace.
185185
By default, external repos are treated as an opaque
186186
blob. If an external repo is specified here,
187187
bazel-diff instead computes the hash for individual
188-
targets. For example, one wants to specify `maven`
189-
here if they user rules_jvm_external so that
188+
targets. For example, one wants to specify `@maven`
189+
here if they use rules_jvm_external so that
190190
individual third party dependency change won't
191191
invalidate all targets in the mono repo. Note that
192-
when `--useCquery` is used, the canonical repo name
193-
must be provided but with single `@`, e.g.
194-
`@rules_jvm_external~~maven~maven`
192+
if `--useCquery` is true; or `--useCquery` is false but
193+
`--bazelCommandOptions=--consistent_labels` is provided,
194+
the canonical repo name must be provided,
195+
e.g. `@@maven` or `@@rules_jvm_external~~maven~maven` (bzlmod)
196+
instead of apparent name `@maven`
195197
-h, --help Show this help message and exit.
196198
--ignoredRuleHashingAttributes=<ignoredRuleHashingAttributes>
197199
Attributes that should be ignored when hashing rule

‎cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class BazelClient(
4141
} else {
4242
val buildTargetsQuery =
4343
listOf("//...:all-targets") +
44-
fineGrainedHashExternalRepos.map { "@$it//...:all-targets" }
44+
fineGrainedHashExternalRepos.map { "$it//...:all-targets" }
4545
queryService.query((repoTargetsQuery + buildTargetsQuery).joinToString(" + ") { "'$it'" })
4646
}
4747
val queryDuration = Calendar.getInstance().getTimeInMillis() - queryEpoch

‎cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,7 @@ class BazelRule(private val rule: Build.Rule) {
4949
): String {
5050
if (isNotMainRepo(ruleInput) &&
5151
ruleInput.startsWith("@") &&
52-
fineGrainedHashExternalRepos.none {
53-
ruleInput.startsWith("@$it") || ruleInput.startsWith("@@${it}")
54-
}) {
52+
fineGrainedHashExternalRepos.none { ruleInput.startsWith(it) }) {
5553
val splitRule = ruleInput.split("//".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray()
5654
if (splitRule.size == 2) {
5755
var externalRule = splitRule[0]

‎cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher {
2525
private val workingDirectory: Path
2626
private val logger: Logger
2727
private val relativeFilenameToContentHash: Map<String, String>?
28-
private val fineGrainedHashExternalRepos: Set<String>
28+
private val fineGrainedHashExternalRepoNames: Set<String>
2929
private val externalRepoResolver: ExternalRepoResolver
3030

3131
init {
@@ -38,7 +38,8 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher {
3838
this.workingDirectory = workingDirectory
3939
val contentHashProvider: ContentHashProvider by inject()
4040
relativeFilenameToContentHash = contentHashProvider.filenameToHash
41-
this.fineGrainedHashExternalRepos = fineGrainedHashExternalRepos
41+
this.fineGrainedHashExternalRepoNames =
42+
fineGrainedHashExternalRepos.map { it.replaceFirst("@+".toRegex(), "") }.toSet()
4243
val externalRepoResolver: ExternalRepoResolver by inject()
4344
this.externalRepoResolver = externalRepoResolver
4445
}
@@ -51,7 +52,8 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher {
5152
) {
5253
this.workingDirectory = workingDirectory
5354
this.relativeFilenameToContentHash = relativeFilenameToContentHash
54-
this.fineGrainedHashExternalRepos = fineGrainedHashExternalRepos
55+
this.fineGrainedHashExternalRepoNames =
56+
fineGrainedHashExternalRepos.map { it.replaceFirst("@+".toRegex(), "") }.toSet()
5557
this.externalRepoResolver = externalRepoResolver
5658
}
5759

@@ -67,18 +69,13 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher {
6769
val filenameSubstring = name.substring(index)
6870
Paths.get(filenameSubstring.removePrefix(":").replace(':', '/'))
6971
} else if (name.startsWith("@")) {
70-
val parts =
71-
if (name.startsWith("@@")) {
72-
name.substring(2).split("//")
73-
} else {
74-
name.substring(1).split("//")
75-
}
72+
val parts = name.replaceFirst("@+".toRegex(), "").split("//")
7673
if (parts.size != 2) {
7774
logger.w { "Invalid source label $name" }
7875
return@sha256
7976
}
8077
val repoName = parts[0]
81-
if (repoName !in fineGrainedHashExternalRepos) {
78+
if (repoName !in fineGrainedHashExternalRepoNames) {
8279
return@sha256
8380
}
8481
val relativePath = Paths.get(parts[1].removePrefix(":").replace(':', '/'))

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ class E2ETest {
138138
"-b",
139139
bazelPath,
140140
"--fineGrainedHashExternalRepos",
141-
"bazel_diff_maven",
141+
"@bazel_diff_maven",
142142
from.absolutePath)
143143
// To
144144
cli.execute(
@@ -148,7 +148,7 @@ class E2ETest {
148148
"-b",
149149
bazelPath,
150150
"--fineGrainedHashExternalRepos",
151-
"bazel_diff_maven",
151+
"@bazel_diff_maven",
152152
to.absolutePath)
153153
// Impacted targets
154154
cli.execute(
@@ -254,15 +254,15 @@ class E2ETest {
254254
fun testFineGrainedHashBzlMod() {
255255
testFineGrainedHashBzlMod(
256256
emptyList(),
257-
"bazel_diff_maven",
257+
"@bazel_diff_maven",
258258
"/fixture/fine-grained-hash-bzlmod-test-impacted-targets.txt")
259259
}
260260

261261
@Test
262262
fun testFineGrainedHashBzlModCquery() {
263263
testFineGrainedHashBzlMod(
264264
listOf("--useCquery"),
265-
"@rules_jvm_external~~maven~maven",
265+
"@@rules_jvm_external~~maven~maven",
266266
"/fixture/fine-grained-hash-bzlmod-cquery-test-impacted-targets.txt")
267267
}
268268

@@ -345,7 +345,7 @@ class E2ETest {
345345
"--cqueryCommandOptions",
346346
"--platforms=//:android",
347347
"--fineGrainedHashExternalRepos",
348-
"bazel_diff_maven,bazel_diff_maven_android",
348+
"@@bazel_diff_maven,@@bazel_diff_maven_android",
349349
from.absolutePath)
350350
// To
351351
cli.execute(
@@ -358,7 +358,7 @@ class E2ETest {
358358
"--cqueryCommandOptions",
359359
"--platforms=//:android",
360360
"--fineGrainedHashExternalRepos",
361-
"bazel_diff_maven,bazel_diff_maven_android",
361+
"@@bazel_diff_maven,@@bazel_diff_maven_android",
362362
to.absolutePath)
363363
// Impacted targets
364364
cli.execute(
@@ -391,7 +391,7 @@ class E2ETest {
391391
"--cqueryCommandOptions",
392392
"--platforms=//:jre",
393393
"--fineGrainedHashExternalRepos",
394-
"bazel_diff_maven,bazel_diff_maven_android",
394+
"@@bazel_diff_maven,@@bazel_diff_maven_android",
395395
from.absolutePath)
396396
// To
397397
cli.execute(
@@ -404,7 +404,7 @@ class E2ETest {
404404
"--cqueryCommandOptions",
405405
"--platforms=//:jre",
406406
"--fineGrainedHashExternalRepos",
407-
"bazel_diff_maven,bazel_diff_maven_android",
407+
"@@bazel_diff_maven,@@bazel_diff_maven_android",
408408
to.absolutePath)
409409
// Impacted targets
410410
cli.execute(
@@ -505,7 +505,7 @@ class E2ETest {
505505
"--cqueryCommandOptions",
506506
"--platforms=//:android",
507507
"--fineGrainedHashExternalRepos",
508-
"bazel_diff_maven,bazel_diff_maven_android",
508+
"@@bazel_diff_maven,@@bazel_diff_maven_android",
509509
from.absolutePath)
510510
// To
511511
cli.execute(
@@ -518,7 +518,7 @@ class E2ETest {
518518
"--cqueryCommandOptions",
519519
"--platforms=//:android",
520520
"--fineGrainedHashExternalRepos",
521-
"bazel_diff_maven,bazel_diff_maven_android",
521+
"@@bazel_diff_maven,@@bazel_diff_maven_android",
522522
to.absolutePath)
523523
// Impacted targets
524524
cli.execute(
@@ -552,7 +552,7 @@ class E2ETest {
552552
"--cqueryCommandOptions",
553553
"--platforms=//:jre",
554554
"--fineGrainedHashExternalRepos",
555-
"bazel_diff_maven,bazel_diff_maven_android",
555+
"@@bazel_diff_maven,@@bazel_diff_maven_android",
556556
from.absolutePath)
557557
// To
558558
cli.execute(
@@ -565,7 +565,7 @@ class E2ETest {
565565
"--cqueryCommandOptions",
566566
"--platforms=//:jre",
567567
"--fineGrainedHashExternalRepos",
568-
"bazel_diff_maven,bazel_diff_maven_android",
568+
"@@bazel_diff_maven,@@bazel_diff_maven_android",
569569
to.absolutePath)
570570
// Impacted targets
571571
cli.execute(

0 commit comments

Comments
 (0)