Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,11 @@ class CalculateImpactedTargetsInteractor : KoinComponent {
}

/**
* Queries Bazel to find all targets that depend on the changed modules.
* Queries Bazel to find all workspace targets that depend on any changed module.
*
* This uses an efficient module-level query approach:
* 1. Identifies which external repository each changed module corresponds to
* 2. Uses `rdeps(//..., @@module~version//...)` to find workspace targets depending on each module
* 3. Returns the union of all impacted targets
* Maps every changed module to its matching bzlmod canonical repos, then issues a
* single `rdeps(//..., @@a//... + @@b//... + ...)` query. Bazel executes the union
* in one analysis pass, avoiding per-repo subprocess fan-out.
*
* @param changedModuleKeys Set of changed module keys (e.g., "abseil-cpp@20240722.0")
* @param allTargets Map of all targets from the final revision
Expand All @@ -292,72 +291,59 @@ class CalculateImpactedTargetsInteractor : KoinComponent {
changedModuleKeys: Set<String>,
allTargets: Map<String, TargetHash>
): Set<String> {
return try {
// Inject BazelQueryService if available
val queryService: BazelQueryService? = try {
inject<BazelQueryService>().value
} catch (e: Exception) {
null
}

if (queryService == null) {
logger.w { "BazelQueryService not available - cannot query for module dependencies" }
return allTargets.keys
}

val impactedTargets = mutableSetOf<String>()

for (moduleKey in changedModuleKeys) {
// Extract module name from key (e.g., "abseil-cpp" from "abseil-cpp@20240722.0")
val moduleName = moduleKey.substringBefore("@")
logger.i { "Querying targets depending on module: $moduleName (key: $moduleKey)" }

// Find the canonical repository name for this module from allTargets
// Bzlmod repos look like: @@abseil-cpp~20240116.2//... or @@rules_jvm_external~~maven~maven//...
val moduleRepos = allTargets.keys
.filter { it.startsWith("@@") && it.contains(moduleName) }
.map { it.substring(2).substringBefore("//") } // Extract repo name
.toSet()
val queryService: BazelQueryService? = try {
inject<BazelQueryService>().value
} catch (e: Exception) {
null
}

if (moduleRepos.isEmpty()) {
logger.w { "No external repository found for module $moduleName" }
continue
}
if (queryService == null) {
logger.w { "BazelQueryService not available - cannot query for module dependencies" }
return allTargets.keys
}

logger.i { "Found ${moduleRepos.size} repositories for module $moduleName: ${moduleRepos.joinToString(", ")}" }

// Query workspace targets that depend on any target in the changed module repo
for (repoName in moduleRepos) {
try {
// Use rdeps to find all workspace targets depending on this module
// rdeps(universe, target_set) finds all targets in universe that depend on target_set
val queryExpression = "rdeps(//..., @@$repoName//...)"
logger.i { "Executing query: $queryExpression" }

val rdeps = runBlocking { queryService.query(queryExpression, useCquery = false) }
val rdepLabels = rdeps.map { it.name }.filter { !it.startsWith("@@") } // Filter to workspace targets only

logger.i { "Found ${rdepLabels.size} workspace targets depending on @@$repoName" }
impactedTargets.addAll(rdepLabels)
} catch (e: Exception) {
logger.w { "Failed to query rdeps for @@$repoName: ${e.message}" }
logger.w { "Conservatively marking all targets as impacted for this module" }
// On error for this module, add all workspace targets
impactedTargets.addAll(allTargets.keys.filter { !it.startsWith("@@") })
}
}
// Map every changed module to its matching bzlmod canonical repos. A single module
// name can match multiple canonical repos (e.g. rules_jvm_external matches
// rules_jvm_external~~maven~maven, rules_jvm_external~~toolchains~...). Log per
// module so an operator can attribute a pathologically large impacted set back to
// a specific module bump.
val moduleRepos = mutableSetOf<String>()
for (moduleKey in changedModuleKeys) {
val moduleName = moduleKey.substringBefore("@")
val matched = allTargets.keys
.filter { it.startsWith("@@") && it.contains(moduleName) }
.map { it.substring(2).substringBefore("//") }
if (matched.isEmpty()) {
logger.w { "No external repository matched module $moduleKey" }
} else {
logger.i { "Module $moduleKey matched ${matched.size} repos: ${matched.joinToString(", ")}" }
moduleRepos.addAll(matched)
}
}

// Add directly changed targets from hash comparison (e.g., code changes)
val directlyChanged = computeSimpleImpactedTargets(emptyMap(), allTargets)
impactedTargets.addAll(directlyChanged)
if (moduleRepos.isEmpty()) {
logger.i { "No external repositories matched any changed module" }
return computeSimpleImpactedTargets(emptyMap(), allTargets)
}

logger.i { "Total targets impacted by module changes: ${impactedTargets.size}" }
impactedTargets
logger.i { "Querying rdeps for ${moduleRepos.size} repositories across ${changedModuleKeys.size} changed modules" }

val impactedTargets = mutableSetOf<String>()
try {
// Single unioned rdeps query: bazel executes the union in one analysis pass.
val queryExpression = "rdeps(//..., ${moduleRepos.joinToString(" + ") { "@@$it//..." }})"
val rdeps = runBlocking { queryService.query(queryExpression, useCquery = false) }
val rdepLabels = rdeps.map { it.name }.filter { !it.startsWith("@@") }
logger.i { "Found ${rdepLabels.size} workspace targets depending on changed modules" }
impactedTargets.addAll(rdepLabels)
} catch (e: Exception) {
logger.e(e) { "Error querying targets depending on modules" }
// On error, conservatively mark all targets as impacted
allTargets.keys
logger.e(e) { "Unioned rdeps query failed - conservatively marking all workspace targets impacted" }
impactedTargets.addAll(allTargets.keys.filter { !it.startsWith("@@") })
}

impactedTargets.addAll(computeSimpleImpactedTargets(emptyMap(), allTargets))

logger.i { "Total targets impacted by module changes: ${impactedTargets.size}" }
return impactedTargets
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,21 @@ package com.bazel_diff.interactor

import assertk.assertThat
import assertk.assertions.*
import com.bazel_diff.bazel.BazelQueryService
import com.bazel_diff.bazel.BazelTarget
import com.bazel_diff.hash.TargetHash
import com.bazel_diff.testModule
import java.io.StringWriter
import org.junit.Rule
import org.junit.Test
import org.koin.core.context.loadKoinModules
import org.koin.dsl.module
import org.koin.test.KoinTest
import org.koin.test.KoinTestRule
import org.mockito.junit.MockitoJUnit
import org.mockito.kotlin.any
import org.mockito.kotlin.doAnswer
import org.mockito.kotlin.mock

class CalculateImpactedTargetsInteractorTest : KoinTest {
@get:Rule val mockitoRule = MockitoJUnit.rule()
Expand Down Expand Up @@ -639,4 +646,67 @@ class CalculateImpactedTargetsInteractorTest : KoinTest {
// Only target1 should be impacted (hash changed) - module logic was skipped
assertThat(output).containsExactly("//:target1")
}

@Test
fun testUnionsRdepsAcrossChangedModules() {
// Guard against regressing to per-repo fan-out. Two changed modules matching two
// canonical repos should produce a single unioned rdeps query, not two.
val captured = mutableListOf<String>()
val fakeQueryService: BazelQueryService = mock {
onBlocking { query(any(), any()) } doAnswer {
captured.add(it.getArgument(0))
emptyList<BazelTarget>()
}
}
loadKoinModules(module { single { fakeQueryService } })

val from = mapOf(
"//:target1" to TargetHash("", "a", "a"),
"@@abseil-cpp~20240116.2//:strings" to TargetHash("", "e1", "e1"),
"@@aspect_bazel_lib~2.22.5//:copy_to_bin" to TargetHash("", "e2", "e2"),
)
val to = mapOf(
"//:target1" to TargetHash("", "a", "a"),
"@@abseil-cpp~20240722.0//:strings" to TargetHash("", "e1b", "e1b"),
"@@aspect_bazel_lib~2.23.0//:copy_to_bin" to TargetHash("", "e2b", "e2b"),
)
val fromGraph = """
{
"key": "root", "name": "root", "version": "", "apparentName": "root",
"dependencies": [
{"key": "abseil-cpp@20240116.2", "name": "abseil-cpp", "version": "20240116.2", "apparentName": "abseil-cpp"},
{"key": "aspect_bazel_lib@2.22.5", "name": "aspect_bazel_lib", "version": "2.22.5", "apparentName": "aspect_bazel_lib"}
]
}
""".trimIndent()
val toGraph = """
{
"key": "root", "name": "root", "version": "", "apparentName": "root",
"dependencies": [
{"key": "abseil-cpp@20240722.0", "name": "abseil-cpp", "version": "20240722.0", "apparentName": "abseil-cpp"},
{"key": "aspect_bazel_lib@2.23.0", "name": "aspect_bazel_lib", "version": "2.23.0", "apparentName": "aspect_bazel_lib"}
]
}
""".trimIndent()

val outputWriter = StringWriter()
CalculateImpactedTargetsInteractor().execute(
from = from,
to = to,
outputWriter = outputWriter,
targetTypes = null,
fromModuleGraphJson = fromGraph,
toModuleGraphJson = toGraph,
)

// Exactly one query - the whole point of this test. The per-repo loop would emit two.
assertThat(captured).hasSize(1)
val queryExpression = captured.single()
assertThat(queryExpression).startsWith("rdeps(//..., ")
assertThat(queryExpression).endsWith(")")
// Both matched canonical repos must appear, joined by " + ".
assertThat(queryExpression).contains("@@abseil-cpp~20240722.0//...")
assertThat(queryExpression).contains("@@aspect_bazel_lib~2.23.0//...")
assertThat(queryExpression).contains(" + ")
}
}