Allows for controlling with attributes are ignored during rule hashing (

#182) * WIP: Ignore location attribute Related Different workspaces produce different hashes #180 * properly inject argument * get tests running
Tinder · Jun 9, 2023 · 6752eff · 6752eff
1 parent e226c1c
commit 6752eff
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 21 deletions.
diff --git a/README.md b/README.md
@@ -121,6 +121,9 @@ workspace.
                             individual third party dependency change won't
                             invalidate all targets in the mono repo.
   -h, --help              Show this help message and exit.
+      --ignoredRuleHashingAttributes=<ignoredRuleHashingAttributes>
+                          Attributes that should be ignored when hashing rule
+                            targets.
   -k, --[no-]keep_going   This flag controls if `bazel query` will be executed
                             with the `--keep_going` flag or not. Disabling this
                             flag allows you to catch configuration issues in

diff --git a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt
@@ -7,16 +7,16 @@ import com.google.devtools.build.lib.query2.proto.proto2api.Build
 // Ignore generator_location when computing a target's hash since it is likely to change and does not
 // affect a target's generated actions. Internally, Bazel also does this when computing a target's hash:
 // https://github.com/bazelbuild/bazel/blob/6971b016f1e258e3bb567a0f9fe7a88ad565d8f2/src/main/java/com/google/devtools/build/lib/query2/query/output/SyntheticAttributeHashCalculator.java#L78-L81
-private val IGNORED_ATTRS = arrayOf("generator_location")
+private val DEFAULT_IGNORED_ATTRS = arrayOf("generator_location")
 
 class BazelRule(private val rule: Build.Rule) {
-    val digest: ByteArray by lazy {
-        sha256 {
+    fun digest(ignoredAttrs: Set<String>): ByteArray {
+        return sha256 {
             safePutBytes(rule.ruleClassBytes.toByteArray())
             safePutBytes(rule.nameBytes.toByteArray())
             safePutBytes(rule.skylarkEnvironmentHashCodeBytes.toByteArray())
             for (attribute in rule.attributeList) {
-                if (!IGNORED_ATTRS.contains(attribute.name))
+                if (!DEFAULT_IGNORED_ATTRS.contains(attribute.name) && !ignoredAttrs.contains(attribute.name))
                     safePutBytes(attribute.toByteArray())
             }
         }

diff --git a/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt b/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt
@@ -110,6 +110,14 @@ class GenerateHashesCommand : Callable<Int> {
     )
     var outputPath: File? = null
 
+    @CommandLine.Option(
+        names = ["--ignoredRuleHashingAttributes"],
+        description = ["Attributes that should be ignored when hashing rule targets."],
+        scope = CommandLine.ScopeType.INHERIT,
+        converter = [OptionsConverter::class],
+    )
+    var ignoredRuleHashingAttributes: Set<String> = emptySet()
+
     @CommandLine.Spec
     lateinit var spec: CommandLine.Model.CommandSpec
 
@@ -134,7 +142,7 @@ class GenerateHashesCommand : Callable<Int> {
             )
         }
 
-        return when (GenerateHashesInteractor().execute(seedFilepaths, outputPath)) {
+        return when (GenerateHashesInteractor().execute(seedFilepaths, outputPath, ignoredRuleHashingAttributes)) {
             true -> CommandLine.ExitCode.OK
             false -> CommandLine.ExitCode.SOFTWARE
         }.also { stopKoin() }

diff --git a/cli/src/main/kotlin/com/bazel_diff/hash/BuildGraphHasher.kt b/cli/src/main/kotlin/com/bazel_diff/hash/BuildGraphHasher.kt
@@ -26,7 +26,10 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
     private val sourceFileHasher: SourceFileHasher by inject()
     private val logger: Logger by inject()
 
-    fun hashAllBazelTargetsAndSourcefiles(seedFilepaths: Set<Path> = emptySet()): Map<String, String> {
+    fun hashAllBazelTargetsAndSourcefiles(
+        seedFilepaths: Set<Path> = emptySet(),
+        ignoredAttrs: Set<String> = emptySet()
+    ): Map<String, String> {
         /**
          * Bazel will lock parallel queries but this is still allowing us to hash source files while executing a parallel query
          */
@@ -56,7 +59,12 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
             Pair(sourceDigestsFuture.await(), targetsTask.await())
         }
         val seedForFilepaths = createSeedForFilepaths(seedFilepaths)
-        return hashAllTargets(seedForFilepaths, sourceDigests, allTargets)
+        return hashAllTargets(
+            seedForFilepaths,
+            sourceDigests,
+            allTargets,
+            ignoredAttrs
+        )
     }
 
     private fun hashSourcefiles(targets: List<Build.Target>): ConcurrentMap<String, ByteArray> {
@@ -94,7 +102,8 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
     private fun hashAllTargets(
         seedHash: ByteArray,
         sourceDigests: ConcurrentMap<String, ByteArray>,
-        allTargets: List<BazelTarget>
+        allTargets: List<BazelTarget>,
+        ignoredAttrs: Set<String>
     ): Map<String, String> {
         val ruleHashes: ConcurrentMap<String, ByteArray> = ConcurrentHashMap()
         val targetToRule: MutableMap<String, BazelRule> = HashMap()
@@ -107,7 +116,8 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
                     targetToRule,
                     sourceDigests,
                     ruleHashes,
-                    seedHash
+                    seedHash,
+                    ignoredAttrs
                 )
                 Pair(target.name, targetDigest.toHexString())
             }

diff --git a/cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.kt b/cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.kt
@@ -30,7 +30,8 @@ class RuleHasher(private val useCquery: Boolean, private val fineGrainedHashExte
         ruleHashes: ConcurrentMap<String, ByteArray>,
         sourceDigests: ConcurrentMap<String, ByteArray>,
         seedHash: ByteArray?,
-        depPath: LinkedHashSet<String>?
+        depPath: LinkedHashSet<String>?,
+        ignoredAttrs: Set<String>
     ): ByteArray {
         val depPathClone = if (depPath != null) LinkedHashSet(depPath) else LinkedHashSet()
         if (depPathClone.contains(rule.name)) {
@@ -40,7 +41,7 @@ class RuleHasher(private val useCquery: Boolean, private val fineGrainedHashExte
         ruleHashes[rule.name]?.let { return it }
 
         val finalHashValue = sha256 {
-            safePutBytes(rule.digest)
+            safePutBytes(rule.digest(ignoredAttrs))
             safePutBytes(seedHash)
 
             for (ruleInput in rule.ruleInputList(useCquery, fineGrainedHashExternalRepos)) {
@@ -60,6 +61,7 @@ class RuleHasher(private val useCquery: Boolean, private val fineGrainedHashExte
                             sourceDigests,
                             seedHash,
                             depPathClone,
+                            ignoredAttrs
                         )
                         safePutBytes(ruleInputHash)
                     }

diff --git a/cli/src/main/kotlin/com/bazel_diff/hash/TargetHasher.kt b/cli/src/main/kotlin/com/bazel_diff/hash/TargetHasher.kt
@@ -14,7 +14,8 @@ class TargetHasher : KoinComponent {
         allRulesMap: Map<String, BazelRule>,
         sourceDigests: ConcurrentMap<String, ByteArray>,
         ruleHashes: ConcurrentMap<String, ByteArray>,
-        seedHash: ByteArray?
+        seedHash: ByteArray?,
+        ignoredAttrs: Set<String>
     ): ByteArray {
         return when (target) {
             is BazelTarget.GeneratedFile -> {
@@ -30,12 +31,21 @@ class TargetHasher : KoinComponent {
                         ruleHashes,
                         sourceDigests,
                         seedHash,
-                        depPath = null
+                        depPath = null,
+                        ignoredAttrs
                     )
                 }
             }
             is BazelTarget.Rule -> {
-                ruleHasher.digest(target.rule, allRulesMap, ruleHashes, sourceDigests, seedHash, depPath = null)
+                ruleHasher.digest(
+                    target.rule,
+                    allRulesMap,
+                    ruleHashes,
+                    sourceDigests,
+                    seedHash,
+                    depPath = null,
+                    ignoredAttrs
+                )
             }
             is BazelTarget.SourceFile -> sha256 {
                 safePutBytes(sourceDigests[target.sourceFileName])

diff --git a/cli/src/main/kotlin/com/bazel_diff/interactor/GenerateHashesInteractor.kt b/cli/src/main/kotlin/com/bazel_diff/interactor/GenerateHashesInteractor.kt
@@ -18,7 +18,7 @@ class GenerateHashesInteractor : KoinComponent {
     private val logger: Logger by inject()
     private val gson: Gson by inject()
 
-    fun execute(seedFilepaths: File?, outputPath: File?): Boolean {
+    fun execute(seedFilepaths: File?, outputPath: File?, ignoredRuleHashingAttributes: Set<String>): Boolean {
         return try {
             val epoch = Calendar.getInstance().getTimeInMillis()
             var seedFilepathsSet: Set<Path> = when {
@@ -31,7 +31,10 @@ class GenerateHashesInteractor : KoinComponent {
                 }
                 else -> emptySet()
             }
-            val hashes = buildGraphHasher.hashAllBazelTargetsAndSourcefiles(seedFilepathsSet)
+            val hashes = buildGraphHasher.hashAllBazelTargetsAndSourcefiles(
+                seedFilepathsSet,
+                ignoredRuleHashingAttributes
+            )
             when (outputPath) {
                 null -> FileWriter(FileDescriptor.out)
                 else -> FileWriter(outputPath)

diff --git a/cli/src/test/kotlin/com/bazel_diff/bazel/BazelRuleTest.kt b/cli/src/test/kotlin/com/bazel_diff/bazel/BazelRuleTest.kt
@@ -19,7 +19,7 @@ class BazelRuleTest {
                 .setRuleClass("java_library")
                 .setName("libbar")
                 .build()
-        assertThat(BazelRule(rule1Pb).digest).isNotEqualTo(BazelRule(rule2Pb).digest)
+        assertThat(BazelRule(rule1Pb).digest(emptySet())).isNotEqualTo(BazelRule(rule2Pb).digest(emptySet()))
     }
     @Test
     fun testIgnoreAttributes() {
@@ -41,6 +41,6 @@ class BazelRuleTest {
                         .setStringValue("path/to/BUILD:111:1").build())
                 .build()
 
-        assertThat(BazelRule(rule1Pb).digest).isEqualTo(BazelRule(rule2Pb).digest)
+        assertThat(BazelRule(rule1Pb).digest(emptySet())).isEqualTo(BazelRule(rule2Pb).digest(emptySet()))
     }
-}
+}
diff --git a/cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt b/cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt
@@ -162,7 +162,7 @@ class BuildGraphHasherTest : KoinTest {
         assertThat(hash).hasSize(3)
         val oldHash = hash["rule3"]!!
 
-        whenever(generator.rule.digest).thenReturn("newDigest".toByteArray())
+        whenever(generator.rule.digest(emptySet())).thenReturn("newDigest".toByteArray())
         hash = hasher.hashAllBazelTargetsAndSourcefiles()
         assertThat(hash).hasSize(3)
         val newHash = hash["rule3"]!!
@@ -175,7 +175,7 @@ class BuildGraphHasherTest : KoinTest {
         val rule = mock<BazelRule>()
         whenever(rule.name).thenReturn(name)
         whenever(rule.ruleInputList(false, emptySet())).thenReturn(inputs)
-        whenever(rule.digest).thenReturn(digest.toByteArray())
+        whenever(rule.digest(emptySet())).thenReturn(digest.toByteArray())
         whenever(target.rule).thenReturn(rule)
         whenever(target.name).thenReturn(name)
         return target