Skip to content

Commit ca357ff

Browse files
honnixhonnix
authored
fix: Fix cquery targets filtering (#251)
1 parent 4610cc6 commit ca357ff

7 files changed

+58
-27
lines changed

‎MODULE.bazel.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.bazel_diff.bazel
22

33
import com.bazel_diff.log.Logger
4-
import com.google.devtools.build.lib.query2.proto.proto2api.Build
54
import java.util.Calendar
65
import org.koin.core.component.KoinComponent
76
import org.koin.core.component.inject
@@ -42,7 +41,7 @@ class BazelClient(
4241
// labels.
4342
(queryService.query("deps(//...:all-targets)", useCquery = true) +
4443
queryService.query(repoTargetsQuery.joinToString(" + ") { "'$it'" }))
45-
.distinctBy { it.rule.name }
44+
.distinctBy { it.name }
4645
} else {
4746
val buildTargetsQuery =
4847
listOf("//...:all-targets") +
@@ -51,16 +50,6 @@ class BazelClient(
5150
}
5251
val queryDuration = Calendar.getInstance().getTimeInMillis() - queryEpoch
5352
logger.i { "All targets queried in $queryDuration" }
54-
return targets.mapNotNull { target: Build.Target ->
55-
when (target.type) {
56-
Build.Target.Discriminator.RULE -> BazelTarget.Rule(target)
57-
Build.Target.Discriminator.SOURCE_FILE -> BazelTarget.SourceFile(target)
58-
Build.Target.Discriminator.GENERATED_FILE -> BazelTarget.GeneratedFile(target)
59-
else -> {
60-
logger.w { "Unsupported target type in the build graph: ${target.type.name}" }
61-
null
62-
}
63-
}
64-
}
53+
return targets
6554
}
6655
}

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

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class BazelQueryService(
2424
) : KoinComponent {
2525
private val logger: Logger by inject()
2626

27-
suspend fun query(query: String, useCquery: Boolean = false): List<Build.Target> {
27+
suspend fun query(query: String, useCquery: Boolean = false): List<BazelTarget> {
2828
// Unfortunately, there is still no direct way to tell if a target is compatible or not with the
2929
// proto output
3030
// by itself. So we do an extra cquery with the trick at
@@ -46,16 +46,18 @@ class BazelQueryService(
4646
if (useCquery) {
4747
val cqueryResult = AnalysisProtosV2.CqueryResult.parseFrom(proto)
4848
cqueryResult.resultsList
49-
.filter { it.target.rule.name in compatibleTargetSet }
50-
.map { it.target }
49+
.mapNotNull { toBazelTarget(it.target) }
50+
.filter { it.name in compatibleTargetSet }
5151
} else {
52-
mutableListOf<Build.Target>().apply {
53-
while (true) {
54-
val target = Build.Target.parseDelimitedFrom(proto) ?: break
55-
// EOF
56-
add(target)
57-
}
58-
}
52+
mutableListOf<Build.Target>()
53+
.apply {
54+
while (true) {
55+
val target = Build.Target.parseDelimitedFrom(proto) ?: break
56+
// EOF
57+
add(target)
58+
}
59+
}
60+
.mapNotNull { toBazelTarget(it) }
5961
}
6062
}
6163

@@ -102,8 +104,6 @@ class BazelQueryService(
102104
"""
103105
def format(target):
104106
if providers(target) == None:
105-
# skip printing non-target results. That is, source files and generated files won't be
106-
# printed
107107
return ""
108108
if "IncompatiblePlatformProvider" not in providers(target):
109109
return str(target.label)
@@ -152,4 +152,16 @@ class BazelQueryService(
152152
throw RuntimeException("Bazel query failed, exit code ${result.resultCode}")
153153
return outputFile
154154
}
155+
156+
private fun toBazelTarget(target: Build.Target): BazelTarget? {
157+
return when (target.type) {
158+
Build.Target.Discriminator.RULE -> BazelTarget.Rule(target)
159+
Build.Target.Discriminator.SOURCE_FILE -> BazelTarget.SourceFile(target)
160+
Build.Target.Discriminator.GENERATED_FILE -> BazelTarget.GeneratedFile(target)
161+
else -> {
162+
logger.w { "Unsupported target type in the build graph: ${target.type.name}" }
163+
null
164+
}
165+
}
166+
}
155167
}

‎cli/src/test/resources/fixture/cquery-test-android-code-change-android-impacted-targets.txt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
@@//src/main/java/com/integration:GuavaUserAndroid.java
12
@@//src/main/java/com/integration:android
3+
@@//src/main/java/com/integration:android-src.jar
4+
@@//src/main/java/com/integration:android.jar
5+
@@//src/main/java/com/integration:android_deploy-src.jar
6+
@@//src/main/java/com/integration:android_deploy.jar
7+
@@//src/main/java/com/integration:android_deploy.jar.unstripped
28
@@//src/main/java/com/integration:android_deployjars_internal_rule
39
@@//src/main/java/com/integration:guava-user
10+
@@//src/main/java/com/integration:libguava-user-src.jar
11+
@@//src/main/java/com/integration:libguava-user.jar

‎cli/src/test/resources/fixture/cquery-test-android-code-change-jre-impacted-targets.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
@@//src/main/java/com/integration:GuavaUserAndroid.java

‎cli/src/test/resources/fixture/cquery-test-guava-upgrade-android-impacted-targets.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,19 @@
11
//external:bazel_diff_maven_android
22
@@//src/main/java/com/integration:android
3+
@@//src/main/java/com/integration:android-src.jar
4+
@@//src/main/java/com/integration:android.jar
5+
@@//src/main/java/com/integration:android_deploy-src.jar
6+
@@//src/main/java/com/integration:android_deploy.jar
7+
@@//src/main/java/com/integration:android_deploy.jar.unstripped
38
@@//src/main/java/com/integration:android_deployjars_internal_rule
49
@@//src/main/java/com/integration:guava-user
10+
@@//src/main/java/com/integration:libguava-user-src.jar
11+
@@//src/main/java/com/integration:libguava-user.jar
512
@@bazel_diff_maven_android//:com_google_errorprone_error_prone_annotations
613
@@bazel_diff_maven_android//:com_google_guava_guava
714
@@bazel_diff_maven_android//:com_google_j2objc_j2objc_annotations
815
@@bazel_diff_maven_android//:org_checkerframework_checker_qual
16+
@@bazel_diff_maven_android//:v1/https/jcenter.bintray.com/com/google/errorprone/error_prone_annotations/2.18.0/error_prone_annotations-2.18.0.jar
17+
@@bazel_diff_maven_android//:v1/https/jcenter.bintray.com/com/google/guava/guava/32.0.0-android/guava-32.0.0-android.jar
18+
@@bazel_diff_maven_android//:v1/https/jcenter.bintray.com/com/google/j2objc/j2objc-annotations/2.8/j2objc-annotations-2.8.jar
19+
@@bazel_diff_maven_android//:v1/https/jcenter.bintray.com/org/checkerframework/checker-qual/3.33.0/checker-qual-3.33.0.jar

‎cli/src/test/resources/fixture/fine-grained-hash-bzlmod-cquery-test-impacted-targets.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,23 @@
11
@@//src/main/java/com/integration:guava-user
2+
@@//src/main/java/com/integration:libguava-user-src.jar
3+
@@//src/main/java/com/integration:libguava-user.jar
24
@@rules_jvm_external~~maven~com_google_errorprone_error_prone_annotations_2_18_0//file:file
5+
@@rules_jvm_external~~maven~com_google_errorprone_error_prone_annotations_2_18_0//file:v1/com/google/errorprone/error_prone_annotations/2.18.0/error_prone_annotations-2.18.0.jar
36
@@rules_jvm_external~~maven~com_google_guava_guava_32_0_0_jre//file:file
7+
@@rules_jvm_external~~maven~com_google_guava_guava_32_0_0_jre//file:v1/com/google/guava/guava/32.0.0-jre/guava-32.0.0-jre.jar
48
@@rules_jvm_external~~maven~com_google_j2objc_j2objc_annotations_2_8//file:file
9+
@@rules_jvm_external~~maven~com_google_j2objc_j2objc_annotations_2_8//file:v1/com/google/j2objc/j2objc-annotations/2.8/j2objc-annotations-2.8.jar
10+
@@rules_jvm_external~~maven~maven//:com/google/errorprone/error_prone_annotations/2.18.0/error_prone_annotations-2.18.0.jar
11+
@@rules_jvm_external~~maven~maven//:com/google/guava/guava/32.0.0-jre/guava-32.0.0-jre.jar
12+
@@rules_jvm_external~~maven~maven//:com/google/j2objc/j2objc-annotations/2.8/j2objc-annotations-2.8.jar
513
@@rules_jvm_external~~maven~maven//:com_google_errorprone_error_prone_annotations
614
@@rules_jvm_external~~maven~maven//:com_google_errorprone_error_prone_annotations_2_18_0_extension
715
@@rules_jvm_external~~maven~maven//:com_google_guava_guava
816
@@rules_jvm_external~~maven~maven//:com_google_guava_guava_32_0_0_jre_extension
917
@@rules_jvm_external~~maven~maven//:com_google_j2objc_j2objc_annotations
1018
@@rules_jvm_external~~maven~maven//:com_google_j2objc_j2objc_annotations_2_8_extension
19+
@@rules_jvm_external~~maven~maven//:org/checkerframework/checker-qual/3.33.0/checker-qual-3.33.0.jar
1120
@@rules_jvm_external~~maven~maven//:org_checkerframework_checker_qual
1221
@@rules_jvm_external~~maven~maven//:org_checkerframework_checker_qual_3_33_0_extension
1322
@@rules_jvm_external~~maven~org_checkerframework_checker_qual_3_33_0//file:file
23+
@@rules_jvm_external~~maven~org_checkerframework_checker_qual_3_33_0//file:v1/org/checkerframework/checker-qual/3.33.0/checker-qual-3.33.0.jar

0 commit comments

Comments
 (0)