Skip to content

Commit

Permalink
fix: Use consistent labels for cquery (#244)
Browse files Browse the repository at this point in the history
  • Loading branch information
honnix authored Oct 9, 2024
1 parent fe58e24 commit a0975b9
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 29 deletions.
2 changes: 1 addition & 1 deletion MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ workspace.
targets. For example, one wants to specify `maven`
here if they user rules_jvm_external so that
individual third party dependency change won't
invalidate all targets in the mono repo.
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`
-h, --help Show this help message and exit.
--ignoredRuleHashingAttributes=<ignoredRuleHashingAttributes>
Attributes that should be ignored when hashing rule
Expand Down
20 changes: 3 additions & 17 deletions cli/src/main/kotlin/com/bazel_diff/bazel/BazelQueryService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class BazelQueryService(
val targets = outputFile.inputStream().buffered().use { proto ->
if (useCquery) {
val cqueryResult = AnalysisProtosV2.CqueryResult.parseFrom(proto)
cqueryResult.resultsList.filter { inCompatibleTargetSet(it, compatibleTargetSet) }.map { it.target }
cqueryResult.resultsList.filter { it.target.rule.name in compatibleTargetSet }.map { it.target }
} else {
mutableListOf<Build.Target>().apply {
while (true) {
Expand All @@ -60,15 +60,6 @@ class BazelQueryService(
return targets
}

private fun inCompatibleTargetSet(
target: AnalysisProtosV2.ConfiguredTarget,
compatibleTargetSet: Set<String>
): Boolean {
val name = target.target.rule.name
return name in compatibleTargetSet ||
name.startsWith("@") && !name.startsWith("@@") && "@${name}" in compatibleTargetSet
}

@OptIn(ExperimentalCoroutinesApi::class)
private suspend fun runQuery(
query: String,
Expand Down Expand Up @@ -111,13 +102,7 @@ class BazelQueryService(
# printed
return ""
if "IncompatiblePlatformProvider" not in providers(target):
label = str(target.label)
# normalize label to be consistent with content inside proto
if label.startswith("@//"):
return label[1:]
if label.startswith("@@//"):
return label[2:]
return label
return str(target.label)
return ""
""".trimIndent())
add(cqueryOutputFile.toString())
Expand All @@ -137,6 +122,7 @@ class BazelQueryService(
}
if (useCquery) {
addAll(cqueryOptions)
add("--consistent_labels")
} else {
addAll(commandOptions)
}
Expand Down
6 changes: 5 additions & 1 deletion cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class BazelRule(private val rule: Build.Rule) {
val name: String = rule.name

private fun transformRuleInput(fineGrainedHashExternalRepos: Set<String>, ruleInput: String): String {
if (ruleInput.startsWith("@") && fineGrainedHashExternalRepos.none { ruleInput.startsWith("@$it") || ruleInput.startsWith("@@${it}") }) {
if (isNotMainRepo(ruleInput) && ruleInput.startsWith("@") && fineGrainedHashExternalRepos.none { ruleInput.startsWith("@$it") || ruleInput.startsWith("@@${it}") }) {
val splitRule = ruleInput.split("//".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray()
if (splitRule.size == 2) {
var externalRule = splitRule[0]
Expand All @@ -47,4 +47,8 @@ class BazelRule(private val rule: Build.Rule) {
}
return ruleInput
}

private fun isNotMainRepo(ruleInput: String): Boolean {
return !ruleInput.startsWith("//") && !ruleInput.startsWith("@//") && !ruleInput.startsWith("@@//")
}
}
23 changes: 19 additions & 4 deletions cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ class SourceFileHasher : KoinComponent {
): ByteArray {
return sha256 {
val name = sourceFileTarget.name
val filenamePath = if (name.startsWith("//")) {
val filenameSubstring = name.substring(2)
val index = isMainRepo(name);
val filenamePath = if (index != -1) {
val filenameSubstring = name.substring(index)
Paths.get(filenameSubstring.removePrefix(":").replace(':', '/'))
} else if (name.startsWith("@")) {
val parts = if (name.startsWith("@@")) {
Expand Down Expand Up @@ -95,14 +96,28 @@ class SourceFileHasher : KoinComponent {

fun softDigest(sourceFileTarget: BazelSourceFileTarget, modifiedFilepaths: Set<Path> = emptySet()): ByteArray? {
val name = sourceFileTarget.name
if (!name.startsWith("//")) return null
val index = isMainRepo(name)
if (index == -1) return null

val filenameSubstring = name.substring(2)
val filenameSubstring = name.substring(index)
val filenamePath = filenameSubstring.replaceFirst(":".toRegex(), "/")
val absoluteFilePath = Paths.get(workingDirectory.toString(), filenamePath)
val file = absoluteFilePath.toFile()
if (!file.exists() || !file.isFile) return null

return digest(sourceFileTarget, modifiedFilepaths)
}

private fun isMainRepo(name: String): Int {
if (name.startsWith("//")) {
return 2;
}
if (name.startsWith("@//")) {
return 3;
}
if (name.startsWith("@@//")) {
return 4;
}
return -1;
}
}
19 changes: 14 additions & 5 deletions cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ class E2ETest {
assertThat(actual).isEqualTo(expected)
}

@Test
fun testFineGrainedHashBzlMod() {
private fun testFineGrainedHashBzlMod(extraGenerateHashesArgs: List<String>, fineGrainedHashExternalRepo: String, expectedResultFile: String) {
// The difference between these two snapshots is simply upgrading the Guava version.
// Following is the diff. (The diff on maven_install.json is omitted)
//
Expand Down Expand Up @@ -161,11 +160,11 @@ class E2ETest {
val cli = CommandLine(BazelDiff())
//From
cli.execute(
"generate-hashes", "-w", workingDirectoryA.absolutePath, "-b", bazelPath, "--fineGrainedHashExternalRepos", "bazel_diff_maven", from.absolutePath
listOf("generate-hashes", "-w", workingDirectoryA.absolutePath, "-b", bazelPath, "--fineGrainedHashExternalRepos", fineGrainedHashExternalRepo, from.absolutePath) + extraGenerateHashesArgs
)
//To
cli.execute(
"generate-hashes", "-w", workingDirectoryB.absolutePath, "-b", bazelPath, "--fineGrainedHashExternalRepos", "bazel_diff_maven", to.absolutePath
listOf("generate-hashes", "-w", workingDirectoryB.absolutePath, "-b", bazelPath, "--fineGrainedHashExternalRepos", fineGrainedHashExternalRepo, to.absolutePath) + extraGenerateHashesArgs
)
//Impacted targets
cli.execute(
Expand All @@ -174,11 +173,21 @@ class E2ETest {

val actual: Set<String> = impactedTargetsOutput.readLines().filter { it.isNotBlank() }.toSet()
val expected: Set<String> =
javaClass.getResourceAsStream("/fixture/fine-grained-hash-bzlmod-test-impacted-targets.txt").use { it.bufferedReader().readLines().filter { it.isNotBlank() }.toSet() }
javaClass.getResourceAsStream(expectedResultFile).use { it.bufferedReader().readLines().filter { it.isNotBlank() }.toSet() }

assertThat(actual).isEqualTo(expected)
}

@Test
fun testFineGrainedHashBzlMod() {
testFineGrainedHashBzlMod(emptyList(), "bazel_diff_maven", "/fixture/fine-grained-hash-bzlmod-test-impacted-targets.txt")
}

@Test
fun testFineGrainedHashBzlModCquery() {
testFineGrainedHashBzlMod(listOf("--useCquery"), "@rules_jvm_external~~maven~maven", "/fixture/fine-grained-hash-bzlmod-cquery-test-impacted-targets.txt")
}

// TODO: re-enable the test after https://github.com/bazelbuild/bazel/issues/21010 is fixed
@Ignore("cquery mode is broken with Bazel 7 because --transition=lite is crashes due to https://github.com/bazelbuild/bazel/issues/21010")
@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
@@//src/main/java/com/integration:guava-user
@@rules_jvm_external~~maven~com_google_errorprone_error_prone_annotations_2_18_0//file:file
@@rules_jvm_external~~maven~com_google_guava_guava_32_0_0_jre//file:file
@@rules_jvm_external~~maven~com_google_j2objc_j2objc_annotations_2_8//file:file
@@rules_jvm_external~~maven~maven//:com_google_errorprone_error_prone_annotations
@@rules_jvm_external~~maven~maven//:com_google_errorprone_error_prone_annotations_2_18_0_extension
@@rules_jvm_external~~maven~maven//:com_google_guava_guava
@@rules_jvm_external~~maven~maven//:com_google_guava_guava_32_0_0_jre_extension
@@rules_jvm_external~~maven~maven//:com_google_j2objc_j2objc_annotations
@@rules_jvm_external~~maven~maven//:com_google_j2objc_j2objc_annotations_2_8_extension
@@rules_jvm_external~~maven~maven//:org_checkerframework_checker_qual
@@rules_jvm_external~~maven~maven//:org_checkerframework_checker_qual_3_33_0_extension
@@rules_jvm_external~~maven~org_checkerframework_checker_qual_3_33_0//file:file
Binary file modified cli/src/test/resources/fixture/fine-grained-hash-bzlmod-test-1.zip
Binary file not shown.
Binary file modified cli/src/test/resources/fixture/fine-grained-hash-bzlmod-test-2.zip
Binary file not shown.

0 comments on commit a0975b9

Please sign in to comment.