From 6752eff71a65e692ea214f889bcc19177330b5b2 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott <56700854+tinder-maxwellelliott@users.noreply.github.com> Date: Fri, 9 Jun 2023 09:38:50 -0400 Subject: [PATCH] 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 --- README.md | 3 +++ .../kotlin/com/bazel_diff/bazel/BazelRule.kt | 8 ++++---- .../bazel_diff/cli/GenerateHashesCommand.kt | 10 +++++++++- .../com/bazel_diff/hash/BuildGraphHasher.kt | 18 ++++++++++++++---- .../kotlin/com/bazel_diff/hash/RuleHasher.kt | 6 ++++-- .../kotlin/com/bazel_diff/hash/TargetHasher.kt | 16 +++++++++++++--- .../interactor/GenerateHashesInteractor.kt | 7 +++++-- .../com/bazel_diff/bazel/BazelRuleTest.kt | 6 +++--- .../bazel_diff/hash/BuildGraphHasherTest.kt | 4 ++-- 9 files changed, 57 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index fd6db4a..770a44d 100644 --- 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= + 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 index 18b422e..6e0769e 100644 --- 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): 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 index f45f11e..8babffe 100644 --- 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 { ) 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 = emptySet() + @CommandLine.Spec lateinit var spec: CommandLine.Model.CommandSpec @@ -134,7 +142,7 @@ class GenerateHashesCommand : Callable { ) } - 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 index 0212311..12b8f48 100644 --- 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 = emptySet()): Map { + fun hashAllBazelTargetsAndSourcefiles( + seedFilepaths: Set = emptySet(), + ignoredAttrs: Set = emptySet() + ): Map { /** * 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): ConcurrentMap { @@ -94,7 +102,8 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent { private fun hashAllTargets( seedHash: ByteArray, sourceDigests: ConcurrentMap, - allTargets: List + allTargets: List, + ignoredAttrs: Set ): Map { val ruleHashes: ConcurrentMap = ConcurrentHashMap() val targetToRule: MutableMap = 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 index b13bba0..78a8a1d 100644 --- 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, sourceDigests: ConcurrentMap, seedHash: ByteArray?, - depPath: LinkedHashSet? + depPath: LinkedHashSet?, + ignoredAttrs: Set ): 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 index b3c4564..5eab92a 100644 --- 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, sourceDigests: ConcurrentMap, ruleHashes: ConcurrentMap, - seedHash: ByteArray? + seedHash: ByteArray?, + ignoredAttrs: Set ): 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 index 0b2ffd2..453c392 100644 --- 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): Boolean { return try { val epoch = Calendar.getInstance().getTimeInMillis() var seedFilepathsSet: Set = 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 index 85b2233..03d16dd 100644 --- 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())) } -} \ No newline at end of file +} diff --git a/cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt b/cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt index 82bfa8e..854e388 100644 --- 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() 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