fix: stackoverflow on circular dependencies (#137)

* throws on circular dependency

* fix test flakyness due to parallel

* add @VisibleForTesting annotation

Author: fa93hws

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

@@ -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 {
@@ -38,7 +56,8 @@ class RuleHasher : KoinComponent {
                             allRulesMap,
                             ruleHashes,
                             sourceDigests,
-                            seedHash
+                            seedHash,
+                            depPathClone,
                         )
                         safePutBytes(ruleInputHash)
                     }

cli/src/main/kotlin/com/bazel_diff/hash/TargetHasher.kt

@@ -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])

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

@@ -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
@@ -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.*
@@ -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())
+            }
+        }
+    }
+
     @Test
     fun testGeneratedTargets() = runBlocking {
         val generator = createRuleTarget("rule1", emptyList(), "rule1Digest")