From a0975b9e907bb0ad200498ee12cf2c1851551452 Mon Sep 17 00:00:00 2001 From: Honnix Date: Wed, 9 Oct 2024 22:24:38 +0200 Subject: [PATCH] fix: Use consistent labels for cquery (#244) --- MODULE.bazel.lock | 2 +- README.md | 5 +++- .../com/bazel_diff/bazel/BazelQueryService.kt | 20 +++------------ .../kotlin/com/bazel_diff/bazel/BazelRule.kt | 6 ++++- .../com/bazel_diff/hash/SourceFileHasher.kt | 23 +++++++++++++++--- .../test/kotlin/com/bazel_diff/e2e/E2ETest.kt | 19 +++++++++++---- ...sh-bzlmod-cquery-test-impacted-targets.txt | 13 ++++++++++ .../fine-grained-hash-bzlmod-test-1.zip | Bin 8168 -> 8704 bytes .../fine-grained-hash-bzlmod-test-2.zip | Bin 8216 -> 8752 bytes 9 files changed, 59 insertions(+), 29 deletions(-) create mode 100644 cli/src/test/resources/fixture/fine-grained-hash-bzlmod-cquery-test-impacted-targets.txt diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 94d5605..60b8394 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -127,7 +127,7 @@ "@@rules_jvm_external~//:extensions.bzl%maven": { "general": { "bzlTransitiveDigest": "aME0tyUxYd+PGhICmzT9zEnIgZNf05hZhsfDD5v0JXM=", - "usagesDigest": "+dpflq+TpnHw1wMaSLFQaN4wOlDKqv7WllhW2Ba0Eg0=", + "usagesDigest": "RiESu1RzypJ/tRsAUBEwT8uAvAL02bjB08j6rSK2pT8=", "recordedFileInputs": { "@@rules_jvm_external~//rules_jvm_external_deps_install.json": "cafb5d2d8119391eb2b322ce3840d3352ea82d496bdb8cbd4b6779ec4d044dda", "@@//maven_install.json": "8ab3e635bbd52fcff5900e530933fbdc41c45d1d5b2a2aef8af1becf7a7d0784" diff --git a/README.md b/README.md index c7bb1ca..916f111 100644 --- a/README.md +++ b/README.md @@ -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= Attributes that should be ignored when hashing rule diff --git a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelQueryService.kt b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelQueryService.kt index f240136..39bf49d 100644 --- a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelQueryService.kt +++ b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelQueryService.kt @@ -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().apply { while (true) { @@ -60,15 +60,6 @@ class BazelQueryService( return targets } - private fun inCompatibleTargetSet( - target: AnalysisProtosV2.ConfiguredTarget, - compatibleTargetSet: Set - ): 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, @@ -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()) @@ -137,6 +122,7 @@ class BazelQueryService( } if (useCquery) { addAll(cqueryOptions) + add("--consistent_labels") } else { addAll(commandOptions) } diff --git a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt index 3ef015a..6ed2ccc 100644 --- a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt +++ b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt @@ -37,7 +37,7 @@ class BazelRule(private val rule: Build.Rule) { val name: String = rule.name private fun transformRuleInput(fineGrainedHashExternalRepos: Set, 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] @@ -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("@@//") + } } diff --git a/cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt b/cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt index e988af7..c26c8e6 100644 --- a/cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt +++ b/cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt @@ -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("@@")) { @@ -95,9 +96,10 @@ class SourceFileHasher : KoinComponent { fun softDigest(sourceFileTarget: BazelSourceFileTarget, modifiedFilepaths: Set = 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() @@ -105,4 +107,17 @@ class SourceFileHasher : KoinComponent { 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; + } } diff --git a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt index 7dbfcad..042940e 100644 --- a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt @@ -123,8 +123,7 @@ class E2ETest { assertThat(actual).isEqualTo(expected) } - @Test - fun testFineGrainedHashBzlMod() { + private fun testFineGrainedHashBzlMod(extraGenerateHashesArgs: List, 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) // @@ -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( @@ -174,11 +173,21 @@ class E2ETest { val actual: Set = impactedTargetsOutput.readLines().filter { it.isNotBlank() }.toSet() val expected: Set = - 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 diff --git a/cli/src/test/resources/fixture/fine-grained-hash-bzlmod-cquery-test-impacted-targets.txt b/cli/src/test/resources/fixture/fine-grained-hash-bzlmod-cquery-test-impacted-targets.txt new file mode 100644 index 0000000..4632b12 --- /dev/null +++ b/cli/src/test/resources/fixture/fine-grained-hash-bzlmod-cquery-test-impacted-targets.txt @@ -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 diff --git a/cli/src/test/resources/fixture/fine-grained-hash-bzlmod-test-1.zip b/cli/src/test/resources/fixture/fine-grained-hash-bzlmod-test-1.zip index f96c205f526ba259c7c96ee2de19d0ee60c37ffe..bfecb4d08c2e99bda92294b8db072d80a16e0d45 100644 GIT binary patch delta 3413 zcmbW3TWAw$6vt0&qfJd?ZcQ|e4Y&$r(QZ||D_U)Zm};_WsUoPcou;i#ib-m>!-Z?+Not6Njt|Sk4(NLyL}z0#IR@-NgMXuMQ=ocH zZ8rV;3Mn`yRIQTpf6ukFRbc!AJme}WL#^@FP^2TuWg!YeIDKHgk_=YYd+Tb{;#e_$ zIn=WAAAIf!EZ&IEUhl7|udA)x73}8PvlN9AxWL26V_}kRgcs`wUiEGr?k;_`8d+=c z6p=d8lxSm`lfX7S9o-7w;7~F-wCTmmrOk*0uhZUi`Kn&m5U0By1LG%6=a$4dE}_scAB#_w)JJ4G7qSXCVc-J>4Zil%_pl z-$p@Di$3PNoig^?3}HNWF^6UqzZp0HLj6_EhQgiAf-oszNWMPtd?i2)zs zDqp@HL@t0CKJeA&(YZ2KGk#Vv&8mJap)!M^f#=C_E)xUvln6cL_El-?97lyN1`;X} z!X7uSP_=aAD`3rszJhxMv{)q@8Nyl+4f)?hvOzyG@LI}Z?T7}=H3Mx<+KQu{iI*-< z6xapi(MF{P97|Y4utp0&#=DeJyeLw+X{>e8wfd078ksVZCXO~Rn%HRtp7Jxq5kDJ; z#tIpYeYHZynA8$u+s2cTCNBenT4WPZ8JE(|6JY@C2zApqW-eFc{t{JN8dfI9M>fDs z>#WwIU@0x~znr0%?1Yl`G5mEk{z5AlTC+?Ox@;zu++@YEK1S=~2}MiTY?^4Sq0zLv zrP)e_+tb~eXsol*v>t~d;RlN^s&| Ls0iTbH1g>i?2_Qz delta 2800 zcma)8U2GIp6ut{Au(fozE!$sVX;G=9!)*6w+A0LZHj!={R;9tDiDh>7&UQ<;i_=Y! zMl!|+6Rn1jNq8{C#5C~*v=V*tMP7);puQOs9(?hq28bk(=o_3fcmDRyF6`ah%2O_@FsB!PF{`iN*Z<(Diw;AVqvkYQAaB?adpDj zh=?a3Frb_j1o+YEW0PYJvReB8^K+;E0Q5%zLYe_Pb$o1ka-5~=P>hSM8-wn_f~(iH zLEzvH!RO@h*D|kXUwV1mB~&7D-6b4&^Vh%>fN?(y1hWu=T3n<;L6hN;7v|)pIU%>Q z02L4_bg3c?Ibz}&t+*^#sJJX^Vx@FOoHhBgl>%KXh{}kR8XcvIJd}`9Nm)(h<8ejK zD@sxw&gJ7HIg!3i6|G#Zh_fY#RegLtxpO?*OvvX4327rCo*+ESy&c?V#G%9EnfV*yeuPWFB_BkQ844*+oR7|^Hd+W8-Sm7#GdnI z&gScdp&Q@+4Y~gYL$DX1U-k{KDj1i${^?~i&5+lAcq+35$V;#XnW!<-*Jhz_cAo9V zt}|?N&yH8m!MAnT2@{*%W@T#J?Kp2D{goTARNs%a^qZZqPsK!kJUjKxDhPfLTd<%H zV(nG}V)TM1fz|&m-#Q7nw_r;awy@`E3%kE##xtS-aTeYgoWVvKhu6`>+D&^>&#==r zVNe#q0~!g_1L;S?z4aQ{b|pUXC*=PVG%#jD`<+rxhVMpM?LJrwhi(GuYd~Q`K+w3q zqsKw+Yx!9ZY(k5{Rm;NgcOYBKXK2}sO=kgl-$EXvPrzFZ7t897f`2lwEv~{Eyz<6Y z9i6u}m-`F6)-*a_eBSC@jlbX^e&fRmbZ&MF$UC3SfB4x^D74=PUwNDn^5x~s3$?BC z-r8iWcX#R5{jtm9FDW`2TM>BV!-V&@`c!l{mwLgXR{5Py-HiRQ@glea-+2Z2yHr05 za6pg9gK81uc6%u0cw>DLa7T}b5FzN5X^&UhR5sO|Tbfih)mp^(qVM?xS9ZqYoYTWv zYc&I+kJhGgM(Uv!+mM=WftS9D&78P7KUh|}!?Ruz1BM|E| zz=EGVnf7HT1SfM>l=vLkZi;`h(39Ch?lGI|nJOLreKkItD^&`dW7kguJlm7Tx>v;L z3Aajh2aI!bQ-lrJy(j_K?)Ue*ronrNy`4xj?S9<35y7p9=z`DuHbRbc5i&p|RZ0xW hsgd+>QdK27qUKa35zk39ElY{CMCIJ*u$)XL9s#~4NCW@? diff --git a/cli/src/test/resources/fixture/fine-grained-hash-bzlmod-test-2.zip b/cli/src/test/resources/fixture/fine-grained-hash-bzlmod-test-2.zip index d1d025c579edd46960622244b8eac90dd36b1294..0d66ae649a7cb9039918bb710230b14c147862e7 100644 GIT binary patch delta 3414 zcmbW3T}V@57{}kGGpAGMK3vVIU>{hRnP8t7E-jc-XCGqGg-tz8O*h)K4%lNJoTj+UAl-lNPSpY6G@R`OWH!qbhKNo3qW(AmQJYc@o(I4H9K0$l z8r+v$DZnr7MFKGRKH27lU>}_P2tduHX*FSp$gpr5wPy*)?rra0%>v*H%-lKxLTnu| z$h_t|t5}sCC-j8&k~M%t2$-nv68=xjS(G7XiQ0)ZHp~^jd?gBT5z?Rq)-np##aqqF zSxF?zl9a-u_`#6D%ZW0+R0>#~!VPBYOM~9If(lI(#FxU|%Q3?Sl}bmr0@{4YE11Ke zof6py5ZZ!BNNyU=2K-3H$|;MsBNB8%gGh6dR~+d~Z20m*0-FFHX;f0c<%ER=yC)qX z-nzt5Y)GU+(`e@+YwZFS?PKCdlDO2sNMgShv4o!i4*S^(G!{>3Y?BW0#w3*(-8L4F zG=8Zll#@sCE+wBAf+0kY&|?xu&1FgSqo~x<&{8@6(jnNiMvEK;httCE*%W239#euo z22WQjFEo#$`2-sGNS7*+o1i$_$4GtrC1{H}oh%w{Xe4dU5UG@#%`(fP(auKF1~bC6 v^|DB`#gQbRF-*dSrlfKxOM$`1?!NizUt%)&B6Akgn7h^qV?)>eYUD><2*$aH% zcg{WM+%H@mIsd9Q8QZz5lHj_rHv8tqSI*E7oN@0Ve$u1Jt8^}>PQC6U6%Rk$qG~pi z$f)X({b7l>VU8GhLt(g4n=#MH0sRHcy#K_dx5tge>(Qnm0>2_}B+qjkM z1`cjNd`?ZAh@G5#ZTN^o*sKX_4x#7WA1cNHjQb!Ej7kW}^gI<(IT^kZ!nC|NEhwvV zPy!)O7xO~I784h8=_NT&#U(i><}(Xo+~CLaDLS7L)q#F#aFD8Uq_+99HFo zs`e#cQWD_-MWpXjHJ8oi#dro{6<&X0|GwX}Dnd>V5mHC|`mZfPpV6FaW9wBKU(8*Q z`(jQN?bIwW)^WIt62`;x_+Znd1 zb?@;N_~{DlUIQE5ZC**=AN;^TdMnpus$Orae%@$>b$up!XL64?0rvxlbu_a5wi6ML?GpNV~|>224H0>obUXmAM2X&gRC18X*I+n!;kEkd^} zf(w)rrWMjZ6bO}SVBHttQGCYtpn+`weH!XCZ@CuuvyoM9eKX*{3z)9}gY5u0{Zh!P z#v9f5wTc~RFt~~t82%op=JMFtWXz(oaJ*|FkI_frRK&w3HCw?u71$C-VH1wLs})A) zea+!MfMZdY^Tg+U&e3=b7xB6tRv@_3DIjlq7XRtz!%%3i2_F4F7v#&!ITvc1<^5@g z{-~u{GvAI?7H>(>&Dg4euTKJm_qNhhbT^0kjY%!=I*mFRdt>7xa0Is80{lBFodvj` zOQaKSVLR@OtcM!>lKmJGPg+zq)tplrR5sO@YV@BwUhL({&RCpl zx>(=W8UfKmi&Hrhbx|+WAvIdgCBai&mz=F6?f4VYR6951J