From e731659b6eb2606449df355cb135c756077cda77 Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Sat, 8 Apr 2023 12:47:49 -0700 Subject: [PATCH 1/5] Add --fineGrainedHashExternalRepos This flag makes bazel-diff generate fine-grained hash for the explicitly mentioned external repos. This way, bazel-diff is smarter when handling external repos like `@maven` in a monorepo setting. --- .../com/bazel_diff/bazel/BazelClient.kt | 14 ++++--- .../kotlin/com/bazel_diff/bazel/BazelRule.kt | 10 +++-- .../bazel_diff/cli/GenerateHashesCommand.kt | 38 ++++++++++++------- .../converter/CommaSeparatedValueConverter.kt | 9 +++++ .../main/kotlin/com/bazel_diff/di/Modules.kt | 29 +++++++------- .../kotlin/com/bazel_diff/hash/RuleHasher.kt | 18 +++++---- cli/src/test/kotlin/com/bazel_diff/Modules.kt | 4 +- .../bazel_diff/hash/BuildGraphHasherTest.kt | 6 +-- 8 files changed, 75 insertions(+), 53 deletions(-) create mode 100644 cli/src/main/kotlin/com/bazel_diff/cli/converter/CommaSeparatedValueConverter.kt diff --git a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt index 631d079..d6d39bc 100644 --- a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt +++ b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt @@ -4,26 +4,28 @@ import com.bazel_diff.log.Logger import com.google.devtools.build.lib.query2.proto.proto2api.Build import org.koin.core.component.KoinComponent import org.koin.core.component.inject -import java.util.concurrent.ConcurrentMap -import java.util.Calendar +import java.util.* -class BazelClient : KoinComponent { +class BazelClient(private val fineGrainedHashExternalRepos: Set) : KoinComponent { private val logger: Logger by inject() private val queryService: BazelQueryService by inject() suspend fun queryAllTargets(): List { val queryEpoch = Calendar.getInstance().getTimeInMillis() - val targets = queryService.query("'//external:all-targets' + '//...:all-targets'") + + val query = listOf("//external:all-targets", "//...:all-targets") + fineGrainedHashExternalRepos.map { "@$it//...:all-targets" } + val targets = queryService.query(query.joinToString(" + ") { "'$it'" }) val queryDuration = Calendar.getInstance().getTimeInMillis() - queryEpoch logger.i { "All targets queried in $queryDuration" } return targets.mapNotNull { target: Build.Target -> when (target.type) { Build.Target.Discriminator.RULE -> BazelTarget.Rule(target) Build.Target.Discriminator.SOURCE_FILE -> BazelTarget.SourceFile( - target + target ) + Build.Target.Discriminator.GENERATED_FILE -> BazelTarget.GeneratedFile( - target + target ) else -> { logger.w { "Unsupported target type in the build graph: ${target.type.name}" } 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 069fdff..6566c12 100644 --- a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt +++ b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt @@ -21,13 +21,15 @@ class BazelRule(private val rule: Build.Rule) { } } } - val ruleInputList: List - get() = rule.ruleInputList.map { ruleInput: String -> transformRuleInput(ruleInput) } + + fun ruleInputList(fineGrainedHashExternalRepos: Set): List { + return rule.ruleInputList.map { ruleInput: String -> transformRuleInput(fineGrainedHashExternalRepos, ruleInput) } + } val name: String = rule.name - private fun transformRuleInput(ruleInput: String): String { - if (ruleInput.startsWith("@")) { + private fun transformRuleInput(fineGrainedHashExternalRepos: Set, ruleInput: String): String { + if (ruleInput.startsWith("@") && fineGrainedHashExternalRepos.none { ruleInput.startsWith("@$it") }) { val splitRule = ruleInput.split("//".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray() if (splitRule.size == 2) { var externalRule = splitRule[0] 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 9ad3f8b..222a270 100644 --- a/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt +++ b/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt @@ -1,5 +1,6 @@ package com.bazel_diff.cli +import com.bazel_diff.cli.converter.CommaSeparatedValueConverter import com.bazel_diff.cli.converter.NormalisingPathConverter import com.bazel_diff.cli.converter.OptionsConverter import com.bazel_diff.di.hasherModule @@ -57,18 +58,26 @@ class GenerateHashesCommand : Callable { var bazelStartupOptions: List = emptyList() @CommandLine.Option( - names = ["-co", "--bazelCommandOptions"], - description = ["Additional space separated Bazel command options used when invoking Bazel"], - scope = CommandLine.ScopeType.INHERIT, - converter = [OptionsConverter::class], + names = ["-co", "--bazelCommandOptions"], + description = ["Additional space separated Bazel command options used when invoking Bazel"], + scope = CommandLine.ScopeType.INHERIT, + converter = [OptionsConverter::class], ) var bazelCommandOptions: List = emptyList() @CommandLine.Option( - names = ["-k", "--keep_going"], - negatable = true, - description = ["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 your Bazel graph, but may not work for some Bazel setups. Defaults to `true`"], - scope = CommandLine.ScopeType.INHERIT + names = ["--fineGrainedHashExternalRepos"], + description = ["Comma separate list of external repos in which fine-grained hashes are computed for the targets. By default, external repos are treated as an opaque blob. If an external repo is specified here, bazel-diff instead computes the hash for individual 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."], + scope = CommandLine.ScopeType.INHERIT, + converter = [CommaSeparatedValueConverter::class], + ) + var fineGrainedHashExternalRepos: Set = emptySet() + + @CommandLine.Option( + names = ["-k", "--keep_going"], + negatable = true, + description = ["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 your Bazel graph, but may not work for some Bazel setups. Defaults to `true`"], + scope = CommandLine.ScopeType.INHERIT ) var keepGoing = true @@ -94,12 +103,13 @@ class GenerateHashesCommand : Callable { startKoin { modules( hasherModule( - workspacePath, - bazelPath, - contentHashPath, - bazelStartupOptions, - bazelCommandOptions, - keepGoing, + workspacePath, + bazelPath, + contentHashPath, + bazelStartupOptions, + bazelCommandOptions, + keepGoing, + fineGrainedHashExternalRepos, ), loggingModule(parent.verbose), serialisationModule(), diff --git a/cli/src/main/kotlin/com/bazel_diff/cli/converter/CommaSeparatedValueConverter.kt b/cli/src/main/kotlin/com/bazel_diff/cli/converter/CommaSeparatedValueConverter.kt new file mode 100644 index 0000000..a4db71d --- /dev/null +++ b/cli/src/main/kotlin/com/bazel_diff/cli/converter/CommaSeparatedValueConverter.kt @@ -0,0 +1,9 @@ +package com.bazel_diff.cli.converter + +import picocli.CommandLine.ITypeConverter + +class CommaSeparatedValueConverter : ITypeConverter> { + override fun convert(value: String): Set { + return value.split(",").dropLastWhile { it.isEmpty() }.toSet() + } +} diff --git a/cli/src/main/kotlin/com/bazel_diff/di/Modules.kt b/cli/src/main/kotlin/com/bazel_diff/di/Modules.kt index 6f19466..626c996 100644 --- a/cli/src/main/kotlin/com/bazel_diff/di/Modules.kt +++ b/cli/src/main/kotlin/com/bazel_diff/di/Modules.kt @@ -17,28 +17,29 @@ import java.io.File import java.nio.file.Path fun hasherModule( - workingDirectory: Path, - bazelPath: Path, - contentHashPath: File?, - startupOptions: List, - commandOptions: List, - keepGoing: Boolean?, + workingDirectory: Path, + bazelPath: Path, + contentHashPath: File?, + startupOptions: List, + commandOptions: List, + keepGoing: Boolean?, + fineGrainedHashExternalRepos: Set, ): Module = module { val debug = System.getProperty("DEBUG", "false").equals("true") single { BazelQueryService( - workingDirectory, - bazelPath, - startupOptions, - commandOptions, - keepGoing, - debug + workingDirectory, + bazelPath, + startupOptions, + commandOptions, + keepGoing, + debug ) } - single { BazelClient() } + single { BazelClient(fineGrainedHashExternalRepos) } single { BuildGraphHasher(get()) } single { TargetHasher() } - single { RuleHasher() } + single { RuleHasher(fineGrainedHashExternalRepos) } single { SourceFileHasher() } single(named("working-directory")) { workingDirectory } single { ContentHashProvider(contentHashPath) } 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 1f9f328..1d08c41 100644 --- a/cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.kt +++ b/cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.kt @@ -8,9 +8,10 @@ import org.koin.core.component.KoinComponent import org.koin.core.component.inject import java.util.concurrent.ConcurrentMap -class RuleHasher : KoinComponent { +class RuleHasher(private val fineGrainedHashExternalRepos: Set) : KoinComponent { private val logger: Logger by inject() private val sourceFileHasher: SourceFileHasher by inject() + @VisibleForTesting class CircularDependencyException(message: String) : Exception(message) @@ -42,7 +43,7 @@ class RuleHasher : KoinComponent { safePutBytes(rule.digest) safePutBytes(seedHash) - for (ruleInput in rule.ruleInputList) { + for (ruleInput in rule.ruleInputList(fineGrainedHashExternalRepos)) { safePutBytes(ruleInput.toByteArray()) val inputRule = allRulesMap[ruleInput] @@ -50,14 +51,15 @@ class RuleHasher : KoinComponent { inputRule == null && sourceDigests.containsKey(ruleInput) -> { safePutBytes(sourceDigests[ruleInput]) } + inputRule?.name != null && inputRule.name != rule.name -> { val ruleInputHash = digest( - inputRule, - allRulesMap, - ruleHashes, - sourceDigests, - seedHash, - depPathClone, + inputRule, + allRulesMap, + ruleHashes, + sourceDigests, + seedHash, + depPathClone, ) safePutBytes(ruleInputHash) } diff --git a/cli/src/test/kotlin/com/bazel_diff/Modules.kt b/cli/src/test/kotlin/com/bazel_diff/Modules.kt index db51971..b388d16 100644 --- a/cli/src/test/kotlin/com/bazel_diff/Modules.kt +++ b/cli/src/test/kotlin/com/bazel_diff/Modules.kt @@ -15,10 +15,10 @@ import java.nio.file.Paths fun testModule(): Module = module { single { SilentLogger } - single { BazelClient() } + single { BazelClient(emptySet()) } single { BuildGraphHasher(get()) } single { TargetHasher() } - single { RuleHasher() } + single { RuleHasher(emptySet()) } single { SourceFileHasher() } single { GsonBuilder().setPrettyPrinting().create() } single(named("working-directory")) { Paths.get("working-directory") } 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 5c28487..68d1804 100644 --- a/cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt @@ -1,10 +1,8 @@ package com.bazel_diff.hash import assertk.all -import assertk.assertAll import assertk.assertThat import assertk.assertions.* -import assertk.assertions.any import com.bazel_diff.bazel.BazelClient import com.bazel_diff.bazel.BazelRule import com.bazel_diff.bazel.BazelTarget @@ -21,10 +19,8 @@ import org.koin.test.mock.MockProviderRule import org.koin.test.mock.declareMock import org.mockito.Mockito import org.mockito.junit.MockitoJUnit -import org.mockito.kotlin.any import org.mockito.kotlin.mock import org.mockito.kotlin.whenever -import java.util.* class BuildGraphHasherTest : KoinTest { @@ -178,7 +174,7 @@ class BuildGraphHasherTest : KoinTest { val target = mock() val rule = mock() whenever(rule.name).thenReturn(name) - whenever(rule.ruleInputList).thenReturn(inputs) + whenever(rule.ruleInputList(emptySet())).thenReturn(inputs) whenever(rule.digest).thenReturn(digest.toByteArray()) whenever(target.rule).thenReturn(rule) whenever(target.name).thenReturn(name) From 5fbc05744ac4fe47af32e31bf42bf423ace820f9 Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Sat, 8 Apr 2023 14:43:35 -0700 Subject: [PATCH 2/5] fix formatting --- .../bazel_diff/cli/GenerateHashesCommand.kt | 40 +++++++++---------- .../main/kotlin/com/bazel_diff/di/Modules.kt | 26 ++++++------ .../kotlin/com/bazel_diff/hash/RuleHasher.kt | 14 ++++--- .../bazel_diff/hash/BuildGraphHasherTest.kt | 2 +- 4 files changed, 42 insertions(+), 40 deletions(-) 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 222a270..bd76628 100644 --- a/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt +++ b/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt @@ -58,26 +58,26 @@ class GenerateHashesCommand : Callable { var bazelStartupOptions: List = emptyList() @CommandLine.Option( - names = ["-co", "--bazelCommandOptions"], - description = ["Additional space separated Bazel command options used when invoking Bazel"], - scope = CommandLine.ScopeType.INHERIT, - converter = [OptionsConverter::class], + names = ["-co", "--bazelCommandOptions"], + description = ["Additional space separated Bazel command options used when invoking Bazel"], + scope = CommandLine.ScopeType.INHERIT, + converter = [OptionsConverter::class], ) var bazelCommandOptions: List = emptyList() @CommandLine.Option( - names = ["--fineGrainedHashExternalRepos"], - description = ["Comma separate list of external repos in which fine-grained hashes are computed for the targets. By default, external repos are treated as an opaque blob. If an external repo is specified here, bazel-diff instead computes the hash for individual 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."], - scope = CommandLine.ScopeType.INHERIT, - converter = [CommaSeparatedValueConverter::class], + names = ["--fineGrainedHashExternalRepos"], + description = ["Comma separate list of external repos in which fine-grained hashes are computed for the targets. By default, external repos are treated as an opaque blob. If an external repo is specified here, bazel-diff instead computes the hash for individual 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."], + scope = CommandLine.ScopeType.INHERIT, + converter = [CommaSeparatedValueConverter::class], ) var fineGrainedHashExternalRepos: Set = emptySet() @CommandLine.Option( - names = ["-k", "--keep_going"], - negatable = true, - description = ["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 your Bazel graph, but may not work for some Bazel setups. Defaults to `true`"], - scope = CommandLine.ScopeType.INHERIT + names = ["-k", "--keep_going"], + negatable = true, + description = ["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 your Bazel graph, but may not work for some Bazel setups. Defaults to `true`"], + scope = CommandLine.ScopeType.INHERIT ) var keepGoing = true @@ -98,18 +98,18 @@ class GenerateHashesCommand : Callable { lateinit var spec: CommandLine.Model.CommandSpec override fun call(): Int { - validate(contentHashPath=contentHashPath) + validate(contentHashPath = contentHashPath) startKoin { modules( hasherModule( - workspacePath, - bazelPath, - contentHashPath, - bazelStartupOptions, - bazelCommandOptions, - keepGoing, - fineGrainedHashExternalRepos, + workspacePath, + bazelPath, + contentHashPath, + bazelStartupOptions, + bazelCommandOptions, + keepGoing, + fineGrainedHashExternalRepos, ), loggingModule(parent.verbose), serialisationModule(), diff --git a/cli/src/main/kotlin/com/bazel_diff/di/Modules.kt b/cli/src/main/kotlin/com/bazel_diff/di/Modules.kt index 626c996..c3e9433 100644 --- a/cli/src/main/kotlin/com/bazel_diff/di/Modules.kt +++ b/cli/src/main/kotlin/com/bazel_diff/di/Modules.kt @@ -17,23 +17,23 @@ import java.io.File import java.nio.file.Path fun hasherModule( - workingDirectory: Path, - bazelPath: Path, - contentHashPath: File?, - startupOptions: List, - commandOptions: List, - keepGoing: Boolean?, - fineGrainedHashExternalRepos: Set, + workingDirectory: Path, + bazelPath: Path, + contentHashPath: File?, + startupOptions: List, + commandOptions: List, + keepGoing: Boolean?, + fineGrainedHashExternalRepos: Set, ): Module = module { val debug = System.getProperty("DEBUG", "false").equals("true") single { BazelQueryService( - workingDirectory, - bazelPath, - startupOptions, - commandOptions, - keepGoing, - debug + workingDirectory, + bazelPath, + startupOptions, + commandOptions, + keepGoing, + debug ) } single { BazelClient(fineGrainedHashExternalRepos) } 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 1d08c41..704dfe3 100644 --- a/cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.kt +++ b/cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.kt @@ -54,15 +54,16 @@ class RuleHasher(private val fineGrainedHashExternalRepos: Set) : KoinCo inputRule?.name != null && inputRule.name != rule.name -> { val ruleInputHash = digest( - inputRule, - allRulesMap, - ruleHashes, - sourceDigests, - seedHash, - depPathClone, + inputRule, + allRulesMap, + ruleHashes, + sourceDigests, + seedHash, + depPathClone, ) safePutBytes(ruleInputHash) } + else -> { val heuristicDigest = sourceFileHasher.softDigest(BazelSourceFileTarget(ruleInput, ByteArray(0))) when { @@ -71,6 +72,7 @@ class RuleHasher(private val fineGrainedHashExternalRepos: Set) : KoinCo sourceDigests[ruleInput] = heuristicDigest safePutBytes(heuristicDigest) } + else -> logger.w { "Unable to calculate digest for input $ruleInput for rule ${rule.name}" } } } 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 68d1804..e4e2ba3 100644 --- a/cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt @@ -144,7 +144,7 @@ class BuildGraphHasherTest : KoinTest { // they are run in parallel, so we don't know whether rule3 or rule4 will be processed first message().matchesPredicate { it!!.contains("\\brule3 -> rule4 -> rule3\\b".toRegex()) || - it.contains("\\brule4 -> rule3 -> rule4\\b".toRegex()) + it.contains("\\brule4 -> rule3 -> rule4\\b".toRegex()) } } } From e9c33c9f818266cb8f51edd8b261c97df39d51ea Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Sat, 8 Apr 2023 14:44:44 -0700 Subject: [PATCH 3/5] fix formatting again --- cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt index d6d39bc..32f3521 100644 --- a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt +++ b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt @@ -21,12 +21,13 @@ class BazelClient(private val fineGrainedHashExternalRepos: Set) : KoinC when (target.type) { Build.Target.Discriminator.RULE -> BazelTarget.Rule(target) Build.Target.Discriminator.SOURCE_FILE -> BazelTarget.SourceFile( - target + target ) Build.Target.Discriminator.GENERATED_FILE -> BazelTarget.GeneratedFile( - target + target ) + else -> { logger.w { "Unsupported target type in the build graph: ${target.type.name}" } null From af642a90fb78bb9e99c16b7af2d2754e1ff65cc0 Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Mon, 10 Apr 2023 11:36:49 -0700 Subject: [PATCH 4/5] add test and rename flag to --fine_grained_hash_external_repos --- .../bazel_diff/cli/GenerateHashesCommand.kt | 2 +- .../CommaSeparatedValueConverterTest.kt | 13 ++++ .../test/kotlin/com/bazel_diff/e2e/E2ETest.kt | 68 ++++++++++++++---- ...fine-grained-hash-external-repo-test-1.zip | Bin 0 -> 7070 bytes ...fine-grained-hash-external-repo-test-2.zip | Bin 0 -> 7070 bytes ...sh-external-repo-test-impacted-targets.txt | 10 +++ 6 files changed, 80 insertions(+), 13 deletions(-) create mode 100644 cli/src/test/kotlin/com/bazel_diff/cli/converter/CommaSeparatedValueConverterTest.kt create mode 100644 cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-1.zip create mode 100644 cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-2.zip create mode 100644 cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-impacted-targets.txt 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 bd76628..834beb9 100644 --- a/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt +++ b/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt @@ -66,7 +66,7 @@ class GenerateHashesCommand : Callable { var bazelCommandOptions: List = emptyList() @CommandLine.Option( - names = ["--fineGrainedHashExternalRepos"], + names = ["--fine_grained_hash_external_repos"], description = ["Comma separate list of external repos in which fine-grained hashes are computed for the targets. By default, external repos are treated as an opaque blob. If an external repo is specified here, bazel-diff instead computes the hash for individual 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."], scope = CommandLine.ScopeType.INHERIT, converter = [CommaSeparatedValueConverter::class], diff --git a/cli/src/test/kotlin/com/bazel_diff/cli/converter/CommaSeparatedValueConverterTest.kt b/cli/src/test/kotlin/com/bazel_diff/cli/converter/CommaSeparatedValueConverterTest.kt new file mode 100644 index 0000000..e853876 --- /dev/null +++ b/cli/src/test/kotlin/com/bazel_diff/cli/converter/CommaSeparatedValueConverterTest.kt @@ -0,0 +1,13 @@ +package com.bazel_diff.cli.converter + +import assertk.assertThat +import assertk.assertions.isEqualTo +import org.junit.Test + +class CommaSeparatedValueConverterTest { + @Test + fun testConverter() { + val converter = CommaSeparatedValueConverter() + assertThat(converter.convert("a,b,c,d")).isEqualTo(listOf("a", "b", "c", "d")) + } +} 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 75aab4d..790aa09 100644 --- a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt @@ -1,28 +1,16 @@ package com.bazel_diff.e2e import assertk.assertThat -import assertk.assertions.contains import assertk.assertions.containsExactlyInAnyOrder -import assertk.assertions.containsOnly import assertk.assertions.isEqualTo -import com.bazel_diff.Main import com.bazel_diff.cli.BazelDiff -import com.bazel_diff.interactor.DeserialiseHashesInteractor -import com.bazel_diff.testModule -import com.google.gson.Gson import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder -import org.koin.core.context.startKoin import picocli.CommandLine import java.io.File -import java.io.FileInputStream -import java.io.IOException -import java.nio.file.Path import java.nio.file.Paths -import java.nio.file.StandardCopyOption import java.util.zip.ZipFile -import java.util.zip.ZipInputStream class E2ETest { @@ -63,6 +51,62 @@ class E2ETest { assertThat(actual).isEqualTo(expected) } + @Test + fun testFineGrainedHashExternalRepo() { + // The difference between these two snapshot is simply upgrading Guava version. Following + // is the diff. + // + // diff --git a/integration/WORKSPACE b/integration/WORKSPACE + // index 617a8d6..2cb3c7d 100644 + // --- a/integration/WORKSPACE + // +++ b/integration/WORKSPACE + // @@ -15,7 +15,7 @@ maven_install( + // name = "bazel_diff_maven", + // artifacts = [ + // "junit:junit:4.12", + // - "com.google.guava:guava:31.0-jre", + // + "com.google.guava:guava:31.1-jre", + // ], + // repositories = [ + // "http://uk.maven.org/maven2", + // + // The project contains a single target that depends on Guava: + // //src/main/java/com/integration:guava-user + // + // So this target, its derived targets, and all other changed external targets should be + // the only impacted targets. + val projectA = extractFixtureProject("/fixture/fine-grained-hash-external-repo-test-1.zip") + val projectB = extractFixtureProject("/fixture/fine-grained-hash-external-repo-test-2.zip") + + val workingDirectoryA = projectA + val workingDirectoryB = projectB + val bazelPath = "bazel" + val outputDir = temp.newFolder() + val from = File(outputDir, "starting_hashes.json") + val to = File(outputDir, "final_hashes.json") + val impactedTargetsOutput = File(outputDir, "impacted_targets.txt") + + val cli = CommandLine(BazelDiff()) + //From + cli.execute( + "generate-hashes", "-w", workingDirectoryA.absolutePath, "-b", bazelPath, "--fine_grained_hash_external_repos", "bazel_diff_maven", from.absolutePath + ) + //To + cli.execute( + "generate-hashes", "-w", workingDirectoryB.absolutePath, "-b", bazelPath, "--fine_grained_hash_external_repos", "bazel_diff_maven", to.absolutePath + ) + //Impacted targets + cli.execute( + "get-impacted-targets", "-sh", from.absolutePath, "-fh", to.absolutePath, "-o", impactedTargetsOutput.absolutePath + ) + + val actual: Set = impactedTargetsOutput.readLines().filter { it.isNotBlank() }.toSet() + val expected: Set = + javaClass.getResourceAsStream("/fixture/fine-grained-hash-external-repo-test-impacted-targets.txt").use { it.bufferedReader().readLines().filter { it.isNotBlank() }.toSet() } + + assertThat(actual).isEqualTo(expected) + } + private fun extractFixtureProject(path: String): File { val testProject = temp.newFolder() val fixtureCopy = temp.newFile() diff --git a/cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-1.zip b/cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-1.zip new file mode 100644 index 0000000000000000000000000000000000000000..b6c3fdf8170adfb35da65e86c0b9efde9d15f35c GIT binary patch literal 7070 zcmdT|2UJtZ8V)EBx`rZ)Gy?&|5PDVVK|@4Zz@SD95Sr2hf=d$%LO>*_s30gsI#OH< zJ_{-xga|6df{0RGRD4R&@NQ%wkkH(-``+7`bIEYe{NK!9?>A$KUa^u31OkCU6?QRp zLj*SudEh?{;Nb#sf)K9mM0Zy&Z$E-9h7H7`Yp(8$HY+e|q?p$=yfI@fQynq0ZXrR? zsv~&lSQgCE4&~vC+8%aLWev#k4`9#_U;uBXu>rtFwy13;GnmtJr#A6UxW>VU1q8}x z1pzZTK|3t0(KeQb#>koZp5#HV`%;3{cP9ihqodKX&4c?BVZX)`-1$yjlRv|rr%%`$+COy zh46JF_t|svN+G|&Oh1>qT^LlkmD44}_U6vY2fKnHPNybDT#kOb;i7pMa_xEAi7f4n zc-;@w#Cy8Z6Qx?|XL@_@zwcmoi;EPg-jODpuD8@(& zrqczAcWS6t>YniS3CLN;DqjJ|`#>%X9}BqHTJxde1NdW6Xrf21L~NplkL&Gs{-sia zLs8qy9a9CwOi52)jk>-z9W>>9&f^p5nEGZ_%lntdk)s+)PW6Svj;+3u5{<>1@?S?} z?=05jJr{T#(|3&6bac{(U2Srh=i9dZSI%uJa+vEM1CnICQkLML$(SFMMMB z!>4iTw~@}4iTdl$qS9Wz+OsE|`f=>vW3RYhz0~{On@nwr|2FDD37`1-nfyLza zf$NeJ)N$&U&tt<)4%BbYk{nW>Wq%F7GugCrucy=dy^1Av*jw0r1-wt6W(eDso+@oE z6)eM)ohoZB6TFHMRN$g|(f$e@jWI%AO1HGiamxq#NRv5o*Q~w;N$XLQh6=hf-ApJe z#u}NM-V+u1;}>IHPT3O3$Cw>*KwIa6tbpW+$>R-OegM$F^0$ z<9Aa9523giQ)EWE6(z?Fqplz-#H)oJE`IJPd9zy2FEO+SF;V0BC1GMnTB~Er?^olW zdq}o_VUcWeF~F{pJ;IAO!LD{$DP97OMxx->juZ&#EVlI_R8Dwv2{;C61t&XF1W7qq zOeJ)KFs=klLfXS0J5q#6H?XahP$l8e5^yXM4~IBWL`XGQOchj3_(BOd4(SfJcA|)q z+OVxvP%YuxCEycCUpU!`B1Y=NVydAVgGHP9`>n@hoDWCEP*OpzhU+hb~>Xki?Lh-!uz zCaqk?;7FSHC`&bmg?8IJSFx~~*V$rW4X=B}!kU*SzMLigmN~VT(VDo@ezJMe{&MFQ zSTdrW_$OXX@Wwh+jEXmKDB;`QJyQ^$Lhgf?F z&4=^rBO53Eb+6v2LG@(1ZJ>IxpDJ`#UmMFj9v?GWaGam?_bs{j_EC>vs3aL2nfTus z^xG8ZRSozT=+z7)7D$Q_2+90q3kV%3#Ya)sQ`(F;mQsb05MlZ)*=@>1F zQ|b)y={)9n$NLSp)?c!*-Q_)xVM&Oa#M&%00_WFHMkoE>y(;o%olqzqW}Q$i_Fgfe zTRglX+-Kcc&pVK9Kf6~{3(P!89ru{{&l#lJcH?2GcHO?P)E(W2V5!*d1ejz49KUuU zEsT?RaH_=GX4pS`5SyJFoN~T5H~8XtxxC=a^R9WpIp?$ULY3{_AHL~vxAHspG@WCn z<|_d@=kMpFG(+ANo=Ripw5gtWcdzBl=bJs>10RTAju3=N$jRGtIf4@N3c7m{39f#4 zqPzDzTb??6=&Dqf_X0ZBs-Z>E__|*Ue2LRG&G0q&O!% z)*{gDeCTrx#dAq~9sc#FLJqyxc;p`+#v<@Gvf}H*QYSW>6v&|}_9t;e*GxXYuqpoPFpn3=~_@Up7#tenh+3f*AgGT0ij z{+_FSg3d<0N+gh?*+DS7ajx;9`4}Prt^Mw@(V(dGKtR!ebWrQp>=2f@TJVVS zS_K0dTmc@{CAF|2`nh|#ZXtLPfMD`w)TKd%@4hk*Tl41ZV%-WdH_@>hlyVPiM{v7P z7l^eS-rhm%@^%;=yt36hMa1pQkf~0a^x9u@O^58&gH(CeAN&xRK6X^H0dpo?+-B#X zzp#fzA78vOrT>OKhdv{T8{m~cI`X9Te9lj$A(tgrLE+RVm0;C-K%Y&`em5y zjXn3l`eCtD)Wnk>MK^_be9XC~s~(gGg^Fp)H5;D$An*?6tAln!AQ;;y2&R z+gc8Alv_ORkrg@X%y6b9eNE}y0jTB+JeohDp3St~=Ek#aLj`Yy6c6ZLc=p~%M3=~R z%YDXq2ApjARWCGT^+Fu{P6uVnA4*KuszSUhiSBfRpV(Rc&=gnruM`xw7jaeT+q)-V zox0?0pa3{(Os95pgX9r`uE@0udzf~oLL~STncc@64YZ4$A9I&mg=wXmF0jlBEXpLb zjOq)?S2lYKxS$cJzLJXq%#Eu!%lf?g%Bx;{-B4GMhb>du?Q=lCt2_~M`TbQBa5y8=Gm~9n3T-#-6O*=V%2?&DxgsvC6Q{A?Y%6!JZErI!Rr@4=u|kYo zPK>IasqtG2PVqgcXNN_rclute#ao^kH2GXyi_!A8Jl10%Ckf+lqpp6UBH4jFTRfyO zE^|24B)npMYFPAovx39jRzokx2fcJ$P#=pdsEOBb0TKV!95cLi(6+chDWUj)nqqzR zx?eImj7-+xEq$rBlwHyQ^Z{D4Bv5{Yp+W7p7#D!LPlrs zb2b2`Dg#VKt5|a#G~3n~ATAcISaBy+ynyc(^1ub78kqxKY_Y73tt-^m>=iV!bVMUi zFyoWAww%Ca`{h%^; zfr)ixJ1Gi5$vXE+GINTB*4YL9n))rn?dq?w{Y|Q))+g47Sf?vEdxb})W!)0RaYvO7 z4>*NHK?7UuGH;9S>UFv*(aDqIzX=<9`#|Hi*s@1l^tL&_P8+&$a+myv zZ8_>zMJ(fpw&>3Na)g$KZ;j6z#4XV*z_sDRH=2OOK!C;mX!G1}(gBz=hn8iuHU(f0 zjOKHe| zD2?~gMJ0KrL>XD&!t%6YLYKFgEzekw76zg5LORG1CJ=@iwJ;EkC(?m#&kHm=mu46X z-MUk*X5@`D(5WzC<(*}sA1cyzHyd>EAYvp~bYwNYrH~h(Ta24$WIyyJ#+F=S(aJ(! zh&hmGq-3hmVz8cnDP88JDAS&upQOuGmjhzN21oV@@s96vMk_TyO p67VVn6;*<=vy-Z$GC|#iK+w=8s5z=>JF98w;5D_>wN-%^{{d!R#qwJ+)(!YcQ%w!;DY}lPgD5YZ=0HD2Z0dnH=qw zY=`8EVxn9XD<#tEuw~GA|I;kPOkwx4`@Zi!&*yI*|IhRLKELbv{lCv+hh4H%7z6@= zL2sNBoQKI?J{rJ(dcY$L5&|K1c~iW1`S}Nuop1smzT-A164tuN`X1fpZqqYs{&L;- z)()*yNv!UBB9xH>vvbDy1YovAg(Fvj?EU}-eFg^bW|{y1Y~h62Y&C;9J(sP-$>P&@ z_ws>2h5R63rVz-*)&c8iXKsm}neR(`Ph4@N_WT94 zWqO46+^mX=T7^o#$WIOQjf`C!Z5X{Z_Vd9L=R*1&uLimv%PtV6yYARsVzG}&9l!`Y ze>8PczM^BTN70wUV2I|!l}T^u%NWQbE?Fv8gHgD&I26{h9tt5<#VGI(!M$ad z+E?_fldFMm=~ETb4F(^5qvc`lBtXx+5Rk44Yk~EieJ<+@@996(c3V*};gnCqQj&!k z(`Dm{Tv}3<3w_FI@W}y_04M_-gzmU~Dx`Q-ZR|0XYQbuiJCz@73Ub0*oN>2`0@9ls zVpDoV&y+CMs~aXlE>;FK-~P$rr~a#oy`qfub*sfgl3q~X?A}p(VioZ?WYT1PI?>3v zTFCaNojJR8U;E{^heZl2zI$a{$tuOI{^lrj^n!;h={JdIWypl^_9jc+Utd?*=DFY3^dIA9SXD{!IJ zK`kjX!c0Z~rPVNWt!sq!yNQ=ODw(6zBU5kcluRN&y{mNVF)@vZ3=fMO>uxp8R5Xd6 z7=QC_oH_ZvyLF=RN_$+^)1DnWqM2_QzcYG7dY+nm8aT~tPM#e9r0sSylEG|zUu`?} zY3Pd51aqAEVU#h_?8cmIPjky`&;1yEW2$-UPG67LJGDxk@s;@9MPiR0or5@)9V=@q zlP<@VA1iMwm%faX))Z#?vHr>&POw1#m~CgD=T!*w_surwZL|6&G^@u=nQI!)bZeQM z1P63}c7I&#=U<$CK^_jLsVQ?j!5UrDZ1njzC%bS~Z%p=+AprfSgstd{+0OQbc}{Jb zKEG|$^pQzQ*oZ!tZBNhh!l5+fWW)^cBPEaBl`85?0*^-aBPQy7Kcr3!s~B```t5S^ zV;`l?4}3};o@V&vYKO##R`?a3OBG7N@n{U(!JRHiJ&A9-167A?CNp zkE?;Mg%C=?RJ1Gnfjb>Sy^3$Efoek{OTmd~B3#mgE=#S)<7%PnAZJR!Noa4lg9lxX z+JSGYg&IJvmx7O=1K>0dx;*tI9#;qb7V@MNoQw{GKk%R{QpfOZbx>2tr&4eVItDID zqAOFuuDE)rB}AbNoQjTzJCNv5s;p~UJ#-UfLm8NcPKDD*bXBT`E3N^Gg%Bhum=>6M z+R{Y~4rTd|@zn|18g{(cB_CD)Yp#4$)2{>aQ7wxTU(6E!$ee}?Sbaj-Ak8Li&oxpj z>@=d2@)urBh$XsJjVUwYZqpeCw)j!+Y|cU49!erBBiAvs~J=m!bvKey!k>SG?jFlnj;8s)z; z7<4Q$sT~R`GN~UrTBIaTCZColA4e4?m;aM$pwpxY4ipR>_P-o3GLy8s#x5jX>KZFf z((aDf+x@-o4gY5%27ilU+pqOMfTbZSC=EGS1R-#chE4mwdsXbuKcQJN!at!~;=g3V zxMXBW^xoAceQ!u^{Kq5_BmIEwF$2ATc*TEF*m&KkRI}dO_Iv^j!sEdFiL1jx8Gvsi_f0#LkuP^Mo5xJ$iv@vF@lO;6!i9^kaq%BFh5-51v`(docLg;bi->uxcRwgOicji!Y}Lm*WgY^K8Im!z%N`~T|7@^bPC?{A1SKG*f0>OSi=_=EMCc}4DBl(QVDJQm>a=2*9uWN zG5avUf=j@oyPy@0lt6F4U7N^$WFVRRxov4eir>}|6=>OzTVh;A6QQ{GfiiC4oylHj z?gr!SMz(ZOdi~u-US0amKSS2*#PCL=4wY5=^EVE=qC#~=*Mz@`&1M`{YQmj}R&d<< zDhT3Z`%*kvhdy}!?mcRifqA3RAefX`b$xsB04X^D=P2v<-bcZb*{RtZ-tFSi4hy7Y!n#sh3Q_Yus=+zgsvUKX#KHiHUy4kD<-7YDK)7hT$1f8o; z@mj;lZbYO3qkhcjb^%Na03Q86VV>i(-{$7CQ&W}L0cBCpt?1mH zv4~#TEq1#t3(SNBOlzO$shLE$1s)I0)rdKoZBUDNS{mQ&1wXR&+MSJri@#)GMEoer z%bwpn0_!%WZ3gXwV;Dva8=903N%h9An%0>&!mK+L3OR_v8$RB-tc#t`a~E5MS*4pJ zu*eE5$0M|e>ML2OWBnWm!9Ae*D$NTpH?NZH8VhdgEPwKG?cJgRe7W}ay&8|W8=?B2sHt&DhFJ}s#tRaG~3rWATQ>vSV<{JKj6EC4mo2{uNu-TkjQ_pZ3$}S zPH77}cPs(}vrMVnbcB$**Z7#!ThpGY*kz;YqNK=`<{bxV8af!nFh*G|=T;svx52pY zo2mikl~y^aiPhy>>6)RZjc%2m&nuBx?Hsy4bMllqN3DneO z4PQ;!rtxNT9?HI$ZyeDP-@Qkj+}bpG?{SlY9hMKcHvIaHCg3p;;ITj27w$JX0K7Yg zwaZwI4d4ol=B6P>mK`JRn@o=EVPMF6ugNXXebdH~hXF<|aPP(e<28iE^Vn@U@G~j` z;%0r*PUID3@ji~Ik|fdZ^0u?(x$Du_L0G(y19FH5gsVn<9f-vfIY8IH z2sC>y%`g^@ciExl=8Y^+cAD^u&2rIa6It5L2Aw~NxCs^qd7bz|$X}sb+{$yaAI=ht z1(%q&vTzpSO(Yg6VK-I|HcBjH%NHribY$5<;3g>?JLG^hW=<<#3<3v!{%~+J160&u%Q)&S}}jZ@9U;R#@_n#b-SEg?+)R z1&b1Jv@qoXnY|~z0LYFpH$`9pvNzg+c>(8-D)-lvgIXxXZR>0`7yP!`VFi}4z+`}) z0*qRuK_Cr~r>DD~2a323h0<|Hkx@jlr#?}iq^oD(ZiqBM8tRi!o=5{DlKy`H3q<)| literal 0 HcmV?d00001 diff --git a/cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-impacted-targets.txt b/cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-impacted-targets.txt new file mode 100644 index 0000000..7a63b2f --- /dev/null +++ b/cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-impacted-targets.txt @@ -0,0 +1,10 @@ +//external:bazel_diff_maven +//src/main/java/com/integration:guava-user +//src/main/java/com/integration:libguava-user-src.jar +//src/main/java/com/integration:libguava-user.jar +@bazel_diff_maven//:com_google_errorprone_error_prone_annotations +@bazel_diff_maven//:com_google_errorprone_error_prone_annotations_2_11_0 +@bazel_diff_maven//:com_google_guava_guava +@bazel_diff_maven//:com_google_guava_guava_31_1_jre +@bazel_diff_maven//:v1/https/jcenter.bintray.com/com/google/errorprone/error_prone_annotations/2.11.0/error_prone_annotations-2.11.0.jar +@bazel_diff_maven//:v1/https/jcenter.bintray.com/com/google/guava/guava/31.1-jre/guava-31.1-jre.jar \ No newline at end of file From 1f8b4f384314adfd77cd097a312a1e3495015f0b Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Mon, 10 Apr 2023 12:50:08 -0700 Subject: [PATCH 5/5] rename flag back to fineGrainedHashExternalRepos --- .../main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt | 2 +- cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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 834beb9..bd76628 100644 --- a/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt +++ b/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt @@ -66,7 +66,7 @@ class GenerateHashesCommand : Callable { var bazelCommandOptions: List = emptyList() @CommandLine.Option( - names = ["--fine_grained_hash_external_repos"], + names = ["--fineGrainedHashExternalRepos"], description = ["Comma separate list of external repos in which fine-grained hashes are computed for the targets. By default, external repos are treated as an opaque blob. If an external repo is specified here, bazel-diff instead computes the hash for individual 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."], scope = CommandLine.ScopeType.INHERIT, converter = [CommaSeparatedValueConverter::class], 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 790aa09..ea0e6cc 100644 --- a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt @@ -89,11 +89,11 @@ class E2ETest { val cli = CommandLine(BazelDiff()) //From cli.execute( - "generate-hashes", "-w", workingDirectoryA.absolutePath, "-b", bazelPath, "--fine_grained_hash_external_repos", "bazel_diff_maven", from.absolutePath + "generate-hashes", "-w", workingDirectoryA.absolutePath, "-b", bazelPath, "--fineGrainedHashExternalRepos", "bazel_diff_maven", from.absolutePath ) //To cli.execute( - "generate-hashes", "-w", workingDirectoryB.absolutePath, "-b", bazelPath, "--fine_grained_hash_external_repos", "bazel_diff_maven", to.absolutePath + "generate-hashes", "-w", workingDirectoryB.absolutePath, "-b", bazelPath, "--fineGrainedHashExternalRepos", "bazel_diff_maven", to.absolutePath ) //Impacted targets cli.execute(