Skip to content

Commit dcf0e27

Browse files
tinder-maxwellelliotttinder-maxwellelliott
committed
Remove canQueryWorkspace
This is always true
1 parent 252bfe3 commit dcf0e27

4 files changed

Lines changed: 45 additions & 71 deletions

File tree

‎MODULE.bazel‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module(
22
name = "bazel-diff",
3-
version = "17.0.0",
3+
version = "17.0.1",
44
compatibility_level = 0,
55
)
66

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,7 @@ class GetImpactedTargetsCommand : Callable<Int> {
159159
outputWriter,
160160
targetType,
161161
fromData.moduleGraphJson,
162-
toData.moduleGraphJson,
163-
canQueryWorkspace = true)
162+
toData.moduleGraphJson)
164163
} else {
165164
CalculateImpactedTargetsInteractor()
166165
.execute(
@@ -169,8 +168,7 @@ class GetImpactedTargetsCommand : Callable<Int> {
169168
outputWriter,
170169
targetType,
171170
fromData.moduleGraphJson,
172-
toData.moduleGraphJson,
173-
canQueryWorkspace = true)
171+
toData.moduleGraphJson)
174172
}
175173
CommandLine.ExitCode.OK
176174
} catch (e: IOException) {

‎cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt‎

Lines changed: 29 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,25 @@ class CalculateImpactedTargetsInteractor : KoinComponent {
3535
outputWriter: Writer,
3636
targetTypes: Set<String>?,
3737
fromModuleGraphJson: String? = null,
38-
toModuleGraphJson: String? = null,
39-
canQueryWorkspace: Boolean = false
38+
toModuleGraphJson: String? = null
4039
) {
4140
/** This call might be faster if end hashes is a sorted map */
4241
val typeFilter = TargetTypeFilter(targetTypes, to)
4342

44-
// Only engage module change detection if workspace is available for querying
45-
// Without workspace, fall back to hash comparison which correctly handles fine-grained external repo hashing
46-
val impactedTargets = if (canQueryWorkspace) {
47-
// Quick check: if module graph JSON is identical, skip module change detection entirely
48-
val moduleGraphChanged = fromModuleGraphJson != toModuleGraphJson
49-
50-
// Detect module changes and query for impacted targets
51-
val changedModules = if (moduleGraphChanged) {
52-
detectChangedModules(fromModuleGraphJson, toModuleGraphJson)
53-
} else {
54-
emptySet()
55-
}
43+
// Quick check: if module graph JSON is identical, skip module change detection entirely
44+
val moduleGraphChanged = fromModuleGraphJson != toModuleGraphJson
5645

57-
if (changedModules.isNotEmpty()) {
58-
logger.i { "Module changes detected - querying for targets that depend on changed modules" }
59-
queryTargetsDependingOnModules(changedModules, to)
60-
} else {
61-
computeSimpleImpactedTargets(from, to)
62-
}
46+
// Detect module changes and query for impacted targets
47+
val changedModules = if (moduleGraphChanged) {
48+
detectChangedModules(fromModuleGraphJson, toModuleGraphJson)
49+
} else {
50+
emptySet()
51+
}
52+
53+
val impactedTargets = if (changedModules.isNotEmpty()) {
54+
logger.i { "Module changes detected - querying for targets that depend on changed modules" }
55+
queryTargetsDependingOnModules(changedModules, to)
6356
} else {
64-
// Without workspace, use hash comparison (supports fine-grained external repo hashing)
6557
computeSimpleImpactedTargets(from, to)
6658
}
6759

@@ -94,35 +86,27 @@ class CalculateImpactedTargetsInteractor : KoinComponent {
9486
outputWriter: Writer,
9587
targetTypes: Set<String>?,
9688
fromModuleGraphJson: String? = null,
97-
toModuleGraphJson: String? = null,
98-
canQueryWorkspace: Boolean = false
89+
toModuleGraphJson: String? = null
9990
) {
10091
val typeFilter = TargetTypeFilter(targetTypes, to)
10192

102-
// Only engage module change detection if workspace is available for querying
103-
// Without workspace, fall back to hash comparison which correctly handles fine-grained external repo hashing
104-
val impactedTargets = if (canQueryWorkspace) {
105-
// Quick check: if module graph JSON is identical, skip module change detection entirely
106-
val moduleGraphChanged = fromModuleGraphJson != toModuleGraphJson
107-
108-
// Detect module changes and query for impacted targets
109-
val changedModules = if (moduleGraphChanged) {
110-
detectChangedModules(fromModuleGraphJson, toModuleGraphJson)
111-
} else {
112-
emptySet()
113-
}
93+
// Quick check: if module graph JSON is identical, skip module change detection entirely
94+
val moduleGraphChanged = fromModuleGraphJson != toModuleGraphJson
11495

115-
if (changedModules.isNotEmpty()) {
116-
logger.i { "Module changes detected - querying for targets that depend on changed modules" }
117-
val moduleImpactedTargets = queryTargetsDependingOnModules(changedModules, to)
118-
// Mark module-impacted targets with distance 0, then compute distances from there
119-
val moduleImpactedHashes = from.filterKeys { !moduleImpactedTargets.contains(it) }
120-
computeAllDistances(moduleImpactedHashes, to, depEdges)
121-
} else {
122-
computeAllDistances(from, to, depEdges)
123-
}
96+
// Detect module changes and query for impacted targets
97+
val changedModules = if (moduleGraphChanged) {
98+
detectChangedModules(fromModuleGraphJson, toModuleGraphJson)
99+
} else {
100+
emptySet()
101+
}
102+
103+
val impactedTargets = if (changedModules.isNotEmpty()) {
104+
logger.i { "Module changes detected - querying for targets that depend on changed modules" }
105+
val moduleImpactedTargets = queryTargetsDependingOnModules(changedModules, to)
106+
// Mark module-impacted targets with distance 0, then compute distances from there
107+
val moduleImpactedHashes = from.filterKeys { !moduleImpactedTargets.contains(it) }
108+
computeAllDistances(moduleImpactedHashes, to, depEdges)
124109
} else {
125-
// Without workspace, use hash comparison (supports fine-grained external repo hashing)
126110
computeAllDistances(from, to, depEdges)
127111
}
128112

‎cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest {
255255

256256
@Test
257257
fun testModuleChangesWithoutWorkspace() {
258-
// When module changes occur but no workspace is provided, fall back to hash comparison
259-
// This correctly supports fine-grained external repo hashing where only changed external targets are marked
258+
// When module changes occur and query service is not available, all targets are marked as impacted
260259
val startHashes = mapOf(
261260
"//:target1" to TargetHash("", "hash1", "hash1"),
262261
"//:target2" to TargetHash("", "hash2", "hash2"),
@@ -304,13 +303,12 @@ class CalculateImpactedTargetsInteractorTest : KoinTest {
304303
outputWriter = outputWriter,
305304
targetTypes = null,
306305
fromModuleGraphJson = fromModuleGraph,
307-
toModuleGraphJson = toModuleGraph,
308-
canQueryWorkspace = false
306+
toModuleGraphJson = toModuleGraph
309307
)
310308

311309
val output = outputWriter.toString().trim().split("\n")
312-
// Without workspace, falls back to hash comparison - only external target is impacted
313-
assertThat(output).containsExactlyInAnyOrder("@@abseil-cpp~20240722.0//:strings")
310+
// Module changes detected but no query service available - all targets including external are impacted
311+
assertThat(output).containsExactlyInAnyOrder("//:target1", "//:target2", "@@abseil-cpp~20240722.0//:strings")
314312
}
315313

316314
@Test
@@ -363,8 +361,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest {
363361
outputWriter = outputWriter,
364362
targetTypes = null,
365363
fromModuleGraphJson = fromModuleGraph,
366-
toModuleGraphJson = toModuleGraph,
367-
canQueryWorkspace = true
364+
toModuleGraphJson = toModuleGraph
368365
)
369366

370367
val output = outputWriter.toString().trim().split("\n")
@@ -403,8 +400,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest {
403400
outputWriter = outputWriter,
404401
targetTypes = null,
405402
fromModuleGraphJson = moduleGraph,
406-
toModuleGraphJson = moduleGraph, // Same module graph
407-
canQueryWorkspace = false
403+
toModuleGraphJson = moduleGraph // Same module graph
408404
)
409405

410406
val output = outputWriter.toString().trim().split("\n")
@@ -414,8 +410,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest {
414410

415411
@Test
416412
fun testModuleChangesWithDistances() {
417-
// Test executeWithDistances with module changes but no workspace
418-
// Without workspace, falls back to hash comparison
413+
// Test executeWithDistances with module changes - when query service is not available, all targets are marked as impacted
419414
val startHashes = mapOf(
420415
"//:1" to TargetHash("", "//:1", "//:1"),
421416
"//:2" to TargetHash("", "//:2", "//:2"),
@@ -462,15 +457,14 @@ class CalculateImpactedTargetsInteractorTest : KoinTest {
462457
outputWriter = outputWriter,
463458
targetTypes = null,
464459
fromModuleGraphJson = fromModuleGraph,
465-
toModuleGraphJson = toModuleGraph,
466-
canQueryWorkspace = false
460+
toModuleGraphJson = toModuleGraph
467461
)
468462

469463
val output = outputWriter.toString()
470-
// Without workspace, falls back to hash comparison - only //:1 changed
464+
// Module changes detected but no query service available - all targets are marked as impacted
471465
assertThat(output).contains("//:1")
472-
assertThat(output).doesNotContain("//:2")
473-
assertThat(output).doesNotContain("//:3")
466+
assertThat(output).contains("//:2")
467+
assertThat(output).contains("//:3")
474468
assertThat(output).contains("\"targetDistance\": 0")
475469
assertThat(output).contains("\"packageDistance\": 0")
476470
}
@@ -497,8 +491,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest {
497491
outputWriter = outputWriter,
498492
targetTypes = null,
499493
fromModuleGraphJson = null, // Missing module graph
500-
toModuleGraphJson = null,
501-
canQueryWorkspace = true
494+
toModuleGraphJson = null
502495
)
503496

504497
val output = outputWriter.toString().trim().split("\n")
@@ -541,8 +534,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest {
541534
outputWriter = outputWriter,
542535
targetTypes = null,
543536
fromModuleGraphJson = moduleGraph, // Identical module graphs
544-
toModuleGraphJson = moduleGraph,
545-
canQueryWorkspace = true
537+
toModuleGraphJson = moduleGraph
546538
)
547539

548540
val output = outputWriter.toString().trim().split("\n")

0 commit comments

Comments
 (0)