Skip to content

Commit 74f1114

Browse files
tgengtgeng
authored
Add --fineGrainedHashExternalRepos (#174)
* 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. * fix formatting * fix formatting again * add test and rename flag to --fine_grained_hash_external_repos * rename flag back to fineGrainedHashExternalRepos
1 parent 7f08714 commit 74f1114

13 files changed

+125
-33
lines changed

‎cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt

+7-4
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,17 @@ import com.bazel_diff.log.Logger
44
import com.google.devtools.build.lib.query2.proto.proto2api.Build
55
import org.koin.core.component.KoinComponent
66
import org.koin.core.component.inject
7-
import java.util.concurrent.ConcurrentMap
8-
import java.util.Calendar
7+
import java.util.*
98

10-
class BazelClient : KoinComponent {
9+
class BazelClient(private val fineGrainedHashExternalRepos: Set<String>) : KoinComponent {
1110
private val logger: Logger by inject()
1211
private val queryService: BazelQueryService by inject()
1312

1413
suspend fun queryAllTargets(): List<BazelTarget> {
1514
val queryEpoch = Calendar.getInstance().getTimeInMillis()
16-
val targets = queryService.query("'//external:all-targets' + '//...:all-targets'")
15+
16+
val query = listOf("//external:all-targets", "//...:all-targets") + fineGrainedHashExternalRepos.map { "@$it//...:all-targets" }
17+
val targets = queryService.query(query.joinToString(" + ") { "'$it'" })
1718
val queryDuration = Calendar.getInstance().getTimeInMillis() - queryEpoch
1819
logger.i { "All targets queried in $queryDuration" }
1920
return targets.mapNotNull { target: Build.Target ->
@@ -22,9 +23,11 @@ class BazelClient : KoinComponent {
2223
Build.Target.Discriminator.SOURCE_FILE -> BazelTarget.SourceFile(
2324
target
2425
)
26+
2527
Build.Target.Discriminator.GENERATED_FILE -> BazelTarget.GeneratedFile(
2628
target
2729
)
30+
2831
else -> {
2932
logger.w { "Unsupported target type in the build graph: ${target.type.name}" }
3033
null

‎cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt

+6-4
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,15 @@ class BazelRule(private val rule: Build.Rule) {
2121
}
2222
}
2323
}
24-
val ruleInputList: List<String>
25-
get() = rule.ruleInputList.map { ruleInput: String -> transformRuleInput(ruleInput) }
24+
25+
fun ruleInputList(fineGrainedHashExternalRepos: Set<String>): List<String> {
26+
return rule.ruleInputList.map { ruleInput: String -> transformRuleInput(fineGrainedHashExternalRepos, ruleInput) }
27+
}
2628

2729
val name: String = rule.name
2830

29-
private fun transformRuleInput(ruleInput: String): String {
30-
if (ruleInput.startsWith("@")) {
31+
private fun transformRuleInput(fineGrainedHashExternalRepos: Set<String>, ruleInput: String): String {
32+
if (ruleInput.startsWith("@") && fineGrainedHashExternalRepos.none { ruleInput.startsWith("@$it") }) {
3133
val splitRule = ruleInput.split("//".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray()
3234
if (splitRule.size == 2) {
3335
var externalRule = splitRule[0]

‎cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt

+11-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.bazel_diff.cli
22

3+
import com.bazel_diff.cli.converter.CommaSeparatedValueConverter
34
import com.bazel_diff.cli.converter.NormalisingPathConverter
45
import com.bazel_diff.cli.converter.OptionsConverter
56
import com.bazel_diff.di.hasherModule
@@ -64,6 +65,14 @@ class GenerateHashesCommand : Callable<Int> {
6465
)
6566
var bazelCommandOptions: List<String> = emptyList()
6667

68+
@CommandLine.Option(
69+
names = ["--fineGrainedHashExternalRepos"],
70+
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."],
71+
scope = CommandLine.ScopeType.INHERIT,
72+
converter = [CommaSeparatedValueConverter::class],
73+
)
74+
var fineGrainedHashExternalRepos: Set<String> = emptySet()
75+
6776
@CommandLine.Option(
6877
names = ["-k", "--keep_going"],
6978
negatable = true,
@@ -89,7 +98,7 @@ class GenerateHashesCommand : Callable<Int> {
8998
lateinit var spec: CommandLine.Model.CommandSpec
9099

91100
override fun call(): Int {
92-
validate(contentHashPath=contentHashPath)
101+
validate(contentHashPath = contentHashPath)
93102

94103
startKoin {
95104
modules(
@@ -100,6 +109,7 @@ class GenerateHashesCommand : Callable<Int> {
100109
bazelStartupOptions,
101110
bazelCommandOptions,
102111
keepGoing,
112+
fineGrainedHashExternalRepos,
103113
),
104114
loggingModule(parent.verbose),
105115
serialisationModule(),

‎cli/src/main/kotlin/com/bazel_diff/cli/converter/CommaSeparatedValueConverter.kt

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package com.bazel_diff.cli.converter
2+
3+
import picocli.CommandLine.ITypeConverter
4+
5+
class CommaSeparatedValueConverter : ITypeConverter<Set<String>> {
6+
override fun convert(value: String): Set<String> {
7+
return value.split(",").dropLastWhile { it.isEmpty() }.toSet()
8+
}
9+
}

‎cli/src/main/kotlin/com/bazel_diff/di/Modules.kt

+3-2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ fun hasherModule(
2323
startupOptions: List<String>,
2424
commandOptions: List<String>,
2525
keepGoing: Boolean?,
26+
fineGrainedHashExternalRepos: Set<String>,
2627
): Module = module {
2728
val debug = System.getProperty("DEBUG", "false").equals("true")
2829
single {
@@ -35,10 +36,10 @@ fun hasherModule(
3536
debug
3637
)
3738
}
38-
single { BazelClient() }
39+
single { BazelClient(fineGrainedHashExternalRepos) }
3940
single { BuildGraphHasher(get()) }
4041
single { TargetHasher() }
41-
single { RuleHasher() }
42+
single { RuleHasher(fineGrainedHashExternalRepos) }
4243
single { SourceFileHasher() }
4344
single(named("working-directory")) { workingDirectory }
4445
single { ContentHashProvider(contentHashPath) }

‎cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.kt

+6-2
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ import org.koin.core.component.KoinComponent
88
import org.koin.core.component.inject
99
import java.util.concurrent.ConcurrentMap
1010

11-
class RuleHasher : KoinComponent {
11+
class RuleHasher(private val fineGrainedHashExternalRepos: Set<String>) : KoinComponent {
1212
private val logger: Logger by inject()
1313
private val sourceFileHasher: SourceFileHasher by inject()
14+
1415
@VisibleForTesting
1516
class CircularDependencyException(message: String) : Exception(message)
1617

@@ -42,14 +43,15 @@ class RuleHasher : KoinComponent {
4243
safePutBytes(rule.digest)
4344
safePutBytes(seedHash)
4445

45-
for (ruleInput in rule.ruleInputList) {
46+
for (ruleInput in rule.ruleInputList(fineGrainedHashExternalRepos)) {
4647
safePutBytes(ruleInput.toByteArray())
4748

4849
val inputRule = allRulesMap[ruleInput]
4950
when {
5051
inputRule == null && sourceDigests.containsKey(ruleInput) -> {
5152
safePutBytes(sourceDigests[ruleInput])
5253
}
54+
5355
inputRule?.name != null && inputRule.name != rule.name -> {
5456
val ruleInputHash = digest(
5557
inputRule,
@@ -61,6 +63,7 @@ class RuleHasher : KoinComponent {
6163
)
6264
safePutBytes(ruleInputHash)
6365
}
66+
6467
else -> {
6568
val heuristicDigest = sourceFileHasher.softDigest(BazelSourceFileTarget(ruleInput, ByteArray(0)))
6669
when {
@@ -69,6 +72,7 @@ class RuleHasher : KoinComponent {
6972
sourceDigests[ruleInput] = heuristicDigest
7073
safePutBytes(heuristicDigest)
7174
}
75+
7276
else -> logger.w { "Unable to calculate digest for input $ruleInput for rule ${rule.name}" }
7377
}
7478
}

‎cli/src/test/kotlin/com/bazel_diff/Modules.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ import java.nio.file.Paths
1515

1616
fun testModule(): Module = module {
1717
single<Logger> { SilentLogger }
18-
single { BazelClient() }
18+
single { BazelClient(emptySet()) }
1919
single { BuildGraphHasher(get()) }
2020
single { TargetHasher() }
21-
single { RuleHasher() }
21+
single { RuleHasher(emptySet()) }
2222
single { SourceFileHasher() }
2323
single { GsonBuilder().setPrettyPrinting().create() }
2424
single(named("working-directory")) { Paths.get("working-directory") }

‎cli/src/test/kotlin/com/bazel_diff/cli/converter/CommaSeparatedValueConverterTest.kt

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package com.bazel_diff.cli.converter
2+
3+
import assertk.assertThat
4+
import assertk.assertions.isEqualTo
5+
import org.junit.Test
6+
7+
class CommaSeparatedValueConverterTest {
8+
@Test
9+
fun testConverter() {
10+
val converter = CommaSeparatedValueConverter()
11+
assertThat(converter.convert("a,b,c,d")).isEqualTo(listOf("a", "b", "c", "d"))
12+
}
13+
}

‎cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt

+56-12
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,16 @@
11
package com.bazel_diff.e2e
22

33
import assertk.assertThat
4-
import assertk.assertions.contains
54
import assertk.assertions.containsExactlyInAnyOrder
6-
import assertk.assertions.containsOnly
75
import assertk.assertions.isEqualTo
8-
import com.bazel_diff.Main
96
import com.bazel_diff.cli.BazelDiff
10-
import com.bazel_diff.interactor.DeserialiseHashesInteractor
11-
import com.bazel_diff.testModule
12-
import com.google.gson.Gson
137
import org.junit.Rule
148
import org.junit.Test
159
import org.junit.rules.TemporaryFolder
16-
import org.koin.core.context.startKoin
1710
import picocli.CommandLine
1811
import java.io.File
19-
import java.io.FileInputStream
20-
import java.io.IOException
21-
import java.nio.file.Path
2212
import java.nio.file.Paths
23-
import java.nio.file.StandardCopyOption
2413
import java.util.zip.ZipFile
25-
import java.util.zip.ZipInputStream
2614

2715

2816
class E2ETest {
@@ -63,6 +51,62 @@ class E2ETest {
6351
assertThat(actual).isEqualTo(expected)
6452
}
6553

54+
@Test
55+
fun testFineGrainedHashExternalRepo() {
56+
// The difference between these two snapshot is simply upgrading Guava version. Following
57+
// is the diff.
58+
//
59+
// diff --git a/integration/WORKSPACE b/integration/WORKSPACE
60+
// index 617a8d6..2cb3c7d 100644
61+
// --- a/integration/WORKSPACE
62+
// +++ b/integration/WORKSPACE
63+
// @@ -15,7 +15,7 @@ maven_install(
64+
// name = "bazel_diff_maven",
65+
// artifacts = [
66+
// "junit:junit:4.12",
67+
// - "com.google.guava:guava:31.0-jre",
68+
// + "com.google.guava:guava:31.1-jre",
69+
// ],
70+
// repositories = [
71+
// "http://uk.maven.org/maven2",
72+
//
73+
// The project contains a single target that depends on Guava:
74+
// //src/main/java/com/integration:guava-user
75+
//
76+
// So this target, its derived targets, and all other changed external targets should be
77+
// the only impacted targets.
78+
val projectA = extractFixtureProject("/fixture/fine-grained-hash-external-repo-test-1.zip")
79+
val projectB = extractFixtureProject("/fixture/fine-grained-hash-external-repo-test-2.zip")
80+
81+
val workingDirectoryA = projectA
82+
val workingDirectoryB = projectB
83+
val bazelPath = "bazel"
84+
val outputDir = temp.newFolder()
85+
val from = File(outputDir, "starting_hashes.json")
86+
val to = File(outputDir, "final_hashes.json")
87+
val impactedTargetsOutput = File(outputDir, "impacted_targets.txt")
88+
89+
val cli = CommandLine(BazelDiff())
90+
//From
91+
cli.execute(
92+
"generate-hashes", "-w", workingDirectoryA.absolutePath, "-b", bazelPath, "--fineGrainedHashExternalRepos", "bazel_diff_maven", from.absolutePath
93+
)
94+
//To
95+
cli.execute(
96+
"generate-hashes", "-w", workingDirectoryB.absolutePath, "-b", bazelPath, "--fineGrainedHashExternalRepos", "bazel_diff_maven", to.absolutePath
97+
)
98+
//Impacted targets
99+
cli.execute(
100+
"get-impacted-targets", "-sh", from.absolutePath, "-fh", to.absolutePath, "-o", impactedTargetsOutput.absolutePath
101+
)
102+
103+
val actual: Set<String> = impactedTargetsOutput.readLines().filter { it.isNotBlank() }.toSet()
104+
val expected: Set<String> =
105+
javaClass.getResourceAsStream("/fixture/fine-grained-hash-external-repo-test-impacted-targets.txt").use { it.bufferedReader().readLines().filter { it.isNotBlank() }.toSet() }
106+
107+
assertThat(actual).isEqualTo(expected)
108+
}
109+
66110
private fun extractFixtureProject(path: String): File {
67111
val testProject = temp.newFolder()
68112
val fixtureCopy = temp.newFile()

‎cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt

+2-6
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
package com.bazel_diff.hash
22

33
import assertk.all
4-
import assertk.assertAll
54
import assertk.assertThat
65
import assertk.assertions.*
7-
import assertk.assertions.any
86
import com.bazel_diff.bazel.BazelClient
97
import com.bazel_diff.bazel.BazelRule
108
import com.bazel_diff.bazel.BazelTarget
@@ -21,10 +19,8 @@ import org.koin.test.mock.MockProviderRule
2119
import org.koin.test.mock.declareMock
2220
import org.mockito.Mockito
2321
import org.mockito.junit.MockitoJUnit
24-
import org.mockito.kotlin.any
2522
import org.mockito.kotlin.mock
2623
import org.mockito.kotlin.whenever
27-
import java.util.*
2824

2925

3026
class BuildGraphHasherTest : KoinTest {
@@ -148,7 +144,7 @@ class BuildGraphHasherTest : KoinTest {
148144
// they are run in parallel, so we don't know whether rule3 or rule4 will be processed first
149145
message().matchesPredicate {
150146
it!!.contains("\\brule3 -> rule4 -> rule3\\b".toRegex()) ||
151-
it.contains("\\brule4 -> rule3 -> rule4\\b".toRegex())
147+
it.contains("\\brule4 -> rule3 -> rule4\\b".toRegex())
152148
}
153149
}
154150
}
@@ -178,7 +174,7 @@ class BuildGraphHasherTest : KoinTest {
178174
val target = mock<BazelTarget.Rule>()
179175
val rule = mock<BazelRule>()
180176
whenever(rule.name).thenReturn(name)
181-
whenever(rule.ruleInputList).thenReturn(inputs)
177+
whenever(rule.ruleInputList(emptySet())).thenReturn(inputs)
182178
whenever(rule.digest).thenReturn(digest.toByteArray())
183179
whenever(target.rule).thenReturn(rule)
184180
whenever(target.name).thenReturn(name)

‎cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-1.zip

6.9 KB
Binary file not shown.

‎cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-2.zip

6.9 KB
Binary file not shown.

‎cli/src/test/resources/fixture/fine-grained-hash-external-repo-test-impacted-targets.txt

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
//external:bazel_diff_maven
2+
//src/main/java/com/integration:guava-user
3+
//src/main/java/com/integration:libguava-user-src.jar
4+
//src/main/java/com/integration:libguava-user.jar
5+
@bazel_diff_maven//:com_google_errorprone_error_prone_annotations
6+
@bazel_diff_maven//:com_google_errorprone_error_prone_annotations_2_11_0
7+
@bazel_diff_maven//:com_google_guava_guava
8+
@bazel_diff_maven//:com_google_guava_guava_31_1_jre
9+
@bazel_diff_maven//:v1/https/jcenter.bintray.com/com/google/errorprone/error_prone_annotations/2.11.0/error_prone_annotations-2.11.0.jar
10+
@bazel_diff_maven//:v1/https/jcenter.bintray.com/com/google/guava/guava/31.1-jre/guava-31.1-jre.jar

0 commit comments

Comments
 (0)