Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,39 @@ package com.bazel_diff.hash
import com.bazel_diff.bazel.BazelRule
import com.bazel_diff.bazel.BazelSourceFileTarget
import com.bazel_diff.log.Logger
import com.google.common.annotations.VisibleForTesting
import org.koin.core.component.KoinComponent
import org.koin.core.component.inject
import java.util.concurrent.ConcurrentMap

class RuleHasher : KoinComponent {
private val logger: Logger by inject()
private val sourceFileHasher: SourceFileHasher by inject()
@VisibleForTesting
class CircularDependencyException(message: String) : Exception(message)


private fun raiseCircularDependency(depPath: LinkedHashSet<String>, begin: String): CircularDependencyException {
val sb = StringBuilder().appendLine("Circular dependency detected: ").append(begin).append(" -> ")
val circularPath = depPath.toList().takeLastWhile { it != begin }
circularPath.forEach { sb.append(it).append(" -> ") }
sb.append(begin)
return CircularDependencyException(sb.toString())
}

fun digest(
rule: BazelRule,
allRulesMap: Map<String, BazelRule>,
ruleHashes: ConcurrentMap<String, ByteArray>,
sourceDigests: ConcurrentMap<String, ByteArray>,
seedHash: ByteArray?
seedHash: ByteArray?,
depPath: LinkedHashSet<String>?
): ByteArray {
val depPathClone = if (depPath != null) LinkedHashSet(depPath) else LinkedHashSet()
if (depPathClone.contains(rule.name)) {
throw raiseCircularDependency(depPathClone, rule.name)
}
depPathClone.add(rule.name)
ruleHashes[rule.name]?.let { return it }

val finalHashValue = sha256 {
Expand All @@ -38,7 +56,8 @@ class RuleHasher : KoinComponent {
allRulesMap,
ruleHashes,
sourceDigests,
seedHash
seedHash,
depPathClone,
)
safePutBytes(ruleInputHash)
}
Expand Down
5 changes: 3 additions & 2 deletions cli/src/main/kotlin/com/bazel_diff/hash/TargetHasher.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@ class TargetHasher : KoinComponent {
allRulesMap,
ruleHashes,
sourceDigests,
seedHash
seedHash,
depPath = null
)
}
}
is BazelTarget.Rule -> {
ruleHasher.digest(target.rule, allRulesMap, ruleHashes, sourceDigests, seedHash)
ruleHasher.digest(target.rule, allRulesMap, ruleHashes, sourceDigests, seedHash, depPath = null)
}
is BazelTarget.SourceFile -> sha256 {
safePutBytes(sourceDigests[target.sourceFileName])
Expand Down
24 changes: 24 additions & 0 deletions cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
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
Expand All @@ -18,6 +21,7 @@ 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.*
Expand Down Expand Up @@ -129,6 +133,26 @@ class BuildGraphHasherTest : KoinTest {
)
}

@Test
fun testCircularDependency() = runBlocking {
val rule3 = createRuleTarget("rule3", listOf("rule2", "rule4"), "digest3")
val rule4 = createRuleTarget("rule4", listOf("rule1", "rule3"), "digest4")
defaultTargets.add(rule3)
defaultTargets.add(rule4)
whenever(bazelClientMock.queryAllTargets()).thenReturn(defaultTargets)
whenever(bazelClientMock.queryAllSourcefileTargets()).thenReturn(emptyList())
assertThat {
hasher.hashAllBazelTargetsAndSourcefiles()
}.isFailure().all {
isInstanceOf(RuleHasher.CircularDependencyException::class)
// 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())
Comment on lines +150 to +151
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way for this?

}
}
}

@Test
fun testGeneratedTargets() = runBlocking {
val generator = createRuleTarget("rule1", emptyList(), "rule1Digest")
Expand Down