Skip to content

Commit 5b29fb6

Browse files
tgengtgeng
authored
Fix computing external repo fine-grained hashes (#175)
1 parent d976e2f commit 5b29fb6

File tree

6 files changed

+95
-32
lines changed

6 files changed

+95
-32
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ class BazelClient(private val fineGrainedHashExternalRepos: Set<String>) : KoinC
3838

3939
suspend fun queryAllSourcefileTargets(): List<Build.Target> {
4040
val queryEpoch = Calendar.getInstance().getTimeInMillis()
41-
val targets = queryService.query("kind('source file', //...:all-targets)")
41+
val allReposToQuery = listOf("@") + fineGrainedHashExternalRepos.map { "@$it" }
42+
val targets = queryService.query("kind('source file', ${allReposToQuery.joinToString(" + ") { "'$it//...:all-targets'" }})")
4243
val queryDuration = Calendar.getInstance().getTimeInMillis() - queryEpoch
4344
logger.i { "All source files queried in $queryDuration" }
4445

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,19 @@ import com.bazel_diff.hash.TargetHasher
99
import com.bazel_diff.io.ContentHashProvider
1010
import com.bazel_diff.log.Logger
1111
import com.bazel_diff.log.StderrLogger
12+
import com.bazel_diff.process.Redirect
13+
import com.bazel_diff.process.process
1214
import com.google.gson.GsonBuilder
15+
import kotlinx.coroutines.ExperimentalCoroutinesApi
16+
import kotlinx.coroutines.runBlocking
1317
import org.koin.core.module.Module
1418
import org.koin.core.qualifier.named
1519
import org.koin.dsl.module
1620
import java.io.File
1721
import java.nio.file.Path
22+
import java.nio.file.Paths
1823

24+
@OptIn(ExperimentalCoroutinesApi::class)
1925
fun hasherModule(
2026
workingDirectory: Path,
2127
bazelPath: Path,
@@ -40,8 +46,20 @@ fun hasherModule(
4046
single { BuildGraphHasher(get()) }
4147
single { TargetHasher() }
4248
single { RuleHasher(fineGrainedHashExternalRepos) }
43-
single { SourceFileHasher() }
49+
single { SourceFileHasher(fineGrainedHashExternalRepos) }
4450
single(named("working-directory")) { workingDirectory }
51+
single(named("output-base")) {
52+
val result = runBlocking {
53+
process(
54+
bazelPath.toString(), "info", "output_base",
55+
stdout = Redirect.CAPTURE,
56+
workingDirectory = workingDirectory.toFile(),
57+
stderr = Redirect.PRINT,
58+
destroyForcibly = true,
59+
)
60+
}
61+
Paths.get(result.output.single())
62+
}
4563
single { ContentHashProvider(contentHashPath) }
4664
}
4765

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

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,49 +13,69 @@ class SourceFileHasher : KoinComponent {
1313
private val workingDirectory: Path
1414
private val logger: Logger
1515
private val relativeFilenameToContentHash: Map<String, String>?
16+
private val outputBase: Path
17+
private val fineGrainedHashExternalRepos: Set<String>
18+
1619
init {
1720
val logger: Logger by inject()
1821
this.logger = logger
1922
}
2023

21-
constructor() {
24+
constructor(fineGrainedHashExternalRepos: Set<String> = emptySet()) {
2225
val workingDirectory: Path by inject(qualifier = named("working-directory"))
2326
this.workingDirectory = workingDirectory
2427
val contentHashProvider: ContentHashProvider by inject()
2528
relativeFilenameToContentHash = contentHashProvider.filenameToHash
29+
val outputBase: Path by inject(qualifier = named("output-base"))
30+
this.outputBase = outputBase
31+
this.fineGrainedHashExternalRepos = fineGrainedHashExternalRepos
2632
}
2733

28-
constructor(workingDirectory: Path, relativeFilenameToContentHash: Map<String, String>?) {
34+
constructor(workingDirectory: Path, outputBase: Path, relativeFilenameToContentHash: Map<String, String>?, fineGrainedHashExternalRepos: Set<String> = emptySet()) {
2935
this.workingDirectory = workingDirectory
36+
this.outputBase = outputBase
3037
this.relativeFilenameToContentHash = relativeFilenameToContentHash
38+
this.fineGrainedHashExternalRepos = fineGrainedHashExternalRepos
3139
}
3240

3341
fun digest(sourceFileTarget: BazelSourceFileTarget): ByteArray {
3442
return sha256 {
3543
val name = sourceFileTarget.name
36-
if (name.startsWith("//")) {
44+
val filenamePath = if (name.startsWith("//")) {
3745
val filenameSubstring = name.substring(2)
38-
val filenamePath = filenameSubstring.replaceFirst(
39-
":".toRegex(),
40-
if (filenameSubstring.startsWith(":")) "" else "/"
41-
)
42-
if (relativeFilenameToContentHash?.contains(filenamePath) == true) {
43-
val contentHash = relativeFilenameToContentHash.getValue(filenamePath)
44-
safePutBytes(contentHash.toByteArray())
45-
} else {
46-
val absoluteFilePath = Paths.get(workingDirectory.toString(), filenamePath)
47-
val file = absoluteFilePath.toFile()
48-
if (file.exists()) {
49-
if (file.isFile) {
50-
putFile(file)
51-
}
52-
} else {
53-
logger.w { "File $absoluteFilePath not found" }
46+
Paths.get(filenameSubstring.removePrefix(":").replace(':', '/'))
47+
} else if (name.startsWith("@")) {
48+
val parts = name.substring(1).split("//")
49+
if (parts.size != 2) {
50+
logger.w { "Invalid source label $name" }
51+
return@sha256
52+
}
53+
val repoName = parts[0]
54+
if (repoName !in fineGrainedHashExternalRepos) {
55+
return@sha256
56+
}
57+
val relativePath = Paths.get(parts[1].removePrefix(":").replace(':', '/'))
58+
outputBase.resolve("external/$repoName").resolve(relativePath)
59+
} else {
60+
return@sha256
61+
}
62+
val filenamePathString = filenamePath.toString()
63+
if (relativeFilenameToContentHash?.contains(filenamePathString) == true) {
64+
val contentHash = relativeFilenameToContentHash.getValue(filenamePathString)
65+
safePutBytes(contentHash.toByteArray())
66+
} else {
67+
val absoluteFilePath = workingDirectory.resolve(filenamePath)
68+
val file = absoluteFilePath.toFile()
69+
if (file.exists()) {
70+
if (file.isFile) {
71+
putFile(file)
5472
}
73+
} else {
74+
logger.w { "File $absoluteFilePath not found" }
5575
}
56-
safePutBytes(sourceFileTarget.seed)
57-
safePutBytes(name.toByteArray())
5876
}
77+
safePutBytes(sourceFileTarget.seed)
78+
safePutBytes(name.toByteArray())
5979
}
6080
}
6181

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.bazel_diff
22

33
import com.bazel_diff.bazel.BazelClient
4+
import com.bazel_diff.bazel.BazelQueryService
45
import com.bazel_diff.hash.BuildGraphHasher
56
import com.bazel_diff.hash.RuleHasher
67
import com.bazel_diff.hash.SourceFileHasher
@@ -22,6 +23,7 @@ fun testModule(): Module = module {
2223
single { SourceFileHasher() }
2324
single { GsonBuilder().setPrettyPrinting().create() }
2425
single(named("working-directory")) { Paths.get("working-directory") }
26+
single(named("output-base")) { Paths.get("output-base") }
2527
single { ContentHashProvider(null) }
2628
}
2729

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

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import java.nio.file.Paths
1717

1818
internal class SourceFileHasherTest: KoinTest {
1919
private val repoAbsolutePath = Paths.get("").toAbsolutePath()
20+
private val outputBasePath = Files.createTempDirectory("SourceFileHasherTest")
2021
private val fixtureFileTarget = "//cli/src/test/kotlin/com/bazel_diff/hash/fixture:foo.ts"
2122
private val fixtureFileContent: ByteArray
2223
private val seed = "seed".toByteArray()
@@ -34,7 +35,7 @@ internal class SourceFileHasherTest: KoinTest {
3435

3536
@Test
3637
fun testHashConcreteFile() = runBlocking {
37-
val hasher = SourceFileHasher(repoAbsolutePath, null)
38+
val hasher = SourceFileHasher(repoAbsolutePath, outputBasePath, null)
3839
val bazelSourceFileTarget = BazelSourceFileTarget(fixtureFileTarget, seed)
3940
val actual = hasher.digest(bazelSourceFileTarget).toHexString()
4041
val expected = sha256 {
@@ -45,9 +46,27 @@ internal class SourceFileHasherTest: KoinTest {
4546
assertThat(actual).isEqualTo(expected)
4647
}
4748

49+
@Test
50+
fun testHashConcreteFileInExternalRepo() = runBlocking {
51+
val hasher = SourceFileHasher(repoAbsolutePath, outputBasePath, null, setOf("external_repo"))
52+
val externalRepoFilePath = outputBasePath.resolve("external/external_repo/path/to/my_file.txt")
53+
Files.createDirectories(externalRepoFilePath.parent)
54+
val externalRepoFileTarget = "@external_repo//path/to:my_file.txt"
55+
val externalRepoFileContent = "hello world"
56+
externalRepoFilePath.toFile().writeText(externalRepoFileContent)
57+
val bazelSourceFileTarget = BazelSourceFileTarget(externalRepoFileTarget, seed)
58+
val actual = hasher.digest(bazelSourceFileTarget).toHexString()
59+
val expected = sha256 {
60+
safePutBytes(externalRepoFileContent.toByteArray())
61+
safePutBytes(seed)
62+
safePutBytes(externalRepoFileTarget.toByteArray())
63+
}.toHexString()
64+
assertThat(actual).isEqualTo(expected)
65+
}
66+
4867
@Test
4968
fun testSoftHashConcreteFile() = runBlocking {
50-
val hasher = SourceFileHasher(repoAbsolutePath, null)
69+
val hasher = SourceFileHasher(repoAbsolutePath, outputBasePath, null)
5170
val bazelSourceFileTarget = BazelSourceFileTarget(fixtureFileTarget, seed)
5271
val actual = hasher.softDigest(bazelSourceFileTarget)?.toHexString()
5372
val expected = sha256 {
@@ -60,7 +79,7 @@ internal class SourceFileHasherTest: KoinTest {
6079

6180
@Test
6281
fun testSoftHashNonExistedFile() = runBlocking {
63-
val hasher = SourceFileHasher(repoAbsolutePath, null)
82+
val hasher = SourceFileHasher(repoAbsolutePath, outputBasePath, null)
6483
val bazelSourceFileTarget = BazelSourceFileTarget("//i/do/not/exist", seed)
6584
val actual = hasher.softDigest(bazelSourceFileTarget)
6685
assertThat(actual).isNull()
@@ -69,7 +88,7 @@ internal class SourceFileHasherTest: KoinTest {
6988
@Test
7089
fun testSoftHashExternalTarget() = runBlocking {
7190
val target = "@bazel-diff//some:file"
72-
val hasher = SourceFileHasher(repoAbsolutePath, null)
91+
val hasher = SourceFileHasher(repoAbsolutePath, outputBasePath, null)
7392
val bazelSourceFileTarget = BazelSourceFileTarget(target, seed)
7493
val actual = hasher.softDigest(bazelSourceFileTarget)
7594
assertThat(actual).isNull()
@@ -78,7 +97,7 @@ internal class SourceFileHasherTest: KoinTest {
7897
@Test
7998
fun testHashNonExistedFile() = runBlocking {
8099
val target = "//i/do/not/exist"
81-
val hasher = SourceFileHasher(repoAbsolutePath, null)
100+
val hasher = SourceFileHasher(repoAbsolutePath, outputBasePath, null)
82101
val bazelSourceFileTarget = BazelSourceFileTarget(target, seed)
83102
val actual = hasher.digest(bazelSourceFileTarget).toHexString()
84103
val expected = sha256 {
@@ -91,7 +110,7 @@ internal class SourceFileHasherTest: KoinTest {
91110
@Test
92111
fun testHashExternalTarget() = runBlocking {
93112
val target = "@bazel-diff//some:file"
94-
val hasher = SourceFileHasher(repoAbsolutePath, null)
113+
val hasher = SourceFileHasher(repoAbsolutePath, outputBasePath, null)
95114
val bazelSourceFileTarget = BazelSourceFileTarget(target, seed)
96115
val actual = hasher.digest(bazelSourceFileTarget).toHexString()
97116
val expected = sha256 {}.toHexString()
@@ -101,7 +120,7 @@ internal class SourceFileHasherTest: KoinTest {
101120
@Test
102121
fun testHashWithProvidedContentHash() = runBlocking {
103122
val filenameToContentHash = hashMapOf("cli/src/test/kotlin/com/bazel_diff/hash/fixture/foo.ts" to "foo-content-hash")
104-
val hasher = SourceFileHasher(repoAbsolutePath, filenameToContentHash)
123+
val hasher = SourceFileHasher(repoAbsolutePath, outputBasePath, filenameToContentHash)
105124
val bazelSourceFileTarget = BazelSourceFileTarget(fixtureFileTarget, seed)
106125
val actual = hasher.digest(bazelSourceFileTarget).toHexString()
107126
val expected = sha256 {
@@ -115,7 +134,7 @@ internal class SourceFileHasherTest: KoinTest {
115134
@Test
116135
fun testHashWithProvidedContentHashButNotInKey() = runBlocking {
117136
val filenameToContentHash = hashMapOf("cli/src/test/kotlin/com/bazel_diff/hash/fixture/bar.ts" to "foo-content-hash")
118-
val hasher = SourceFileHasher(repoAbsolutePath, filenameToContentHash)
137+
val hasher = SourceFileHasher(repoAbsolutePath, outputBasePath, filenameToContentHash)
119138
val bazelSourceFileTarget = BazelSourceFileTarget(fixtureFileTarget, seed)
120139
val actual = hasher.digest(bazelSourceFileTarget).toHexString()
121140
val expected = sha256 {
@@ -130,7 +149,7 @@ internal class SourceFileHasherTest: KoinTest {
130149
fun testHashWithProvidedContentHashWithLeadingColon() = runBlocking {
131150
val targetName = "//:cli/src/test/kotlin/com/bazel_diff/hash/fixture/bar.ts"
132151
val filenameToContentHash = hashMapOf("cli/src/test/kotlin/com/bazel_diff/hash/fixture/bar.ts" to "foo-content-hash")
133-
val hasher = SourceFileHasher(repoAbsolutePath, filenameToContentHash)
152+
val hasher = SourceFileHasher(repoAbsolutePath, outputBasePath, filenameToContentHash)
134153
val bazelSourceFileTarget = BazelSourceFileTarget(targetName, seed)
135154
val actual = hasher.digest(bazelSourceFileTarget).toHexString()
136155
val expected = sha256 {

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22
//src/main/java/com/integration:guava-user
33
//src/main/java/com/integration:libguava-user-src.jar
44
//src/main/java/com/integration:libguava-user.jar
5+
@bazel_diff_maven//:BUILD
56
@bazel_diff_maven//:com_google_errorprone_error_prone_annotations
67
@bazel_diff_maven//:com_google_errorprone_error_prone_annotations_2_11_0
78
@bazel_diff_maven//:com_google_guava_guava
89
@bazel_diff_maven//:com_google_guava_guava_31_1_jre
10+
@bazel_diff_maven//:pin
11+
@bazel_diff_maven//:pin.sh
912
@bazel_diff_maven//:v1/https/jcenter.bintray.com/com/google/errorprone/error_prone_annotations/2.11.0/error_prone_annotations-2.11.0.jar
1013
@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)