Skip to content

Commit a3cb64f

Browse files
rdarkrdark
committed
fix: tolerate stderr-polluted moduleGraphJson in ModuleGraphParser
bazel-diff 17.0.1..18.0.5 configured BazelModService.getModuleGraphJson() with stdout=CAPTURE, stderr=CAPTURE. In Process.kt, captureAll triggers ProcessBuilder.redirectErrorStream(true), physically merging stderr into stdout. The captured moduleGraphJson therefore contained bazel's INFO lines ("INFO: Invocation ID: ...", "Loading: 0 packages loaded", etc.) prepended to the actual JSON output. PR #330 (shipped in v18.1.0) correctly switched stderr to SILENT but broke format compatibility: any CI pipeline re-using a base graph from 17.0.1..18.0.5 and a head graph from 18.1.0+ hits parseModuleGraph's try/catch on the polluted side, gets emptyMap(), and cascades into findChangedModules reporting every head-side module as "added". The downstream queryTargetsDependingOnModules then spawns thousands of bazel query rdeps(...) subprocesses. Make parseModuleGraph tolerant: attempt the whole-string parse first, and on failure retry from the first '{' to end-of-string. On second failure fall through to the existing emptyMap() behaviour. Clean input continues to parse in one attempt with no extra allocation. Tests: - ModuleGraphParserTest: positive case for stderr-prefixed input. - StderrPollutionRegressionTest: end-to-end regression guard covering parse, findChangedModules, and the CalculateImpactedTargetsInteractor dispatch path. Confirms the cross-version comparison now short-circuits to computeSimpleImpactedTargets instead of the rdeps fan-out.
1 parent 2f6b341 commit a3cb64f

4 files changed

Lines changed: 198 additions & 1 deletion

File tree

‎cli/BUILD‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,12 @@ kt_jvm_test(
114114
runtime_deps = [":cli-test-lib"],
115115
)
116116

117+
kt_jvm_test(
118+
name = "StderrPollutionRegressionTest",
119+
test_class = "com.bazel_diff.bazel.StderrPollutionRegressionTest",
120+
runtime_deps = [":cli-test-lib"],
121+
)
122+
117123
kt_jvm_test(
118124
name = "E2ETest",
119125
timeout = "long",

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,24 @@ class ModuleGraphParser {
2424
/**
2525
* Parses the JSON output from `bazel mod graph --output=json`.
2626
*
27+
* Tolerates a non-JSON prefix (e.g. leaked stderr from bazel-diff
28+
* 17.0.1..18.0.5, which captured stderr into moduleGraphJson via
29+
* Process.kt's captureAll -> ProcessBuilder.redirectErrorStream(true)).
30+
*
2731
* @param json The JSON string from bazel mod graph
2832
* @return A map of module keys to Module objects
2933
*/
3034
fun parseModuleGraph(json: String): Map<String, Module> {
3135
val modules = mutableMapOf<String, Module>()
3236

3337
try {
34-
val root = JsonParser.parseString(json).asJsonObject
38+
val root = try {
39+
JsonParser.parseString(json).asJsonObject
40+
} catch (_: Exception) {
41+
val start = json.indexOf('{')
42+
if (start < 0) return emptyMap()
43+
JsonParser.parseString(json.substring(start)).asJsonObject
44+
}
3545
extractModules(root, modules)
3646
} catch (e: Exception) {
3747
// If parsing fails, return empty map

‎cli/src/test/kotlin/com/bazel_diff/bazel/ModuleGraphParserTest.kt‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,30 @@ class ModuleGraphParserTest {
9090
assertThat(googletest!!.version).isEqualTo("1.14.0")
9191
}
9292

93+
@Test
94+
fun parseModuleGraph_withStderrPrefix_extractsModules() {
95+
val cleanJson =
96+
"""
97+
{
98+
"key": "<root>",
99+
"name": "ws",
100+
"version": "",
101+
"apparentName": "ws",
102+
"dependencies": [
103+
{"key": "a@1", "name": "a", "version": "1", "apparentName": "a"}
104+
]
105+
}
106+
"""
107+
.trimIndent()
108+
val polluted = "INFO: Invocation ID: abc\nLoading: 0 packages loaded\n$cleanJson"
109+
110+
val clean = parser.parseModuleGraph(cleanJson)
111+
val actual = parser.parseModuleGraph(polluted)
112+
113+
assertThat(actual).hasSize(2)
114+
assertThat(actual).isEqualTo(clean)
115+
}
116+
93117
@Test
94118
fun parseModuleGraph_withInvalidJson_returnsEmptyMap() {
95119
val json = "{ invalid json"

‎cli/src/test/kotlin/com/bazel_diff/bazel/StderrPollutionRegressionTest.kt‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
package com.bazel_diff.bazel
2+
3+
import assertk.assertThat
4+
import assertk.assertions.hasSize
5+
import assertk.assertions.isEmpty
6+
import assertk.assertions.isEqualTo
7+
import assertk.assertions.isNotEqualTo
8+
import com.bazel_diff.hash.TargetHash
9+
import com.bazel_diff.interactor.CalculateImpactedTargetsInteractor
10+
import com.bazel_diff.testModule
11+
import java.io.StringWriter
12+
import org.junit.Rule
13+
import org.junit.Test
14+
import org.koin.test.KoinTest
15+
import org.koin.test.KoinTestRule
16+
17+
/**
18+
* Regression guard for the 17.0.1..18.0.5 -> 18.1.0+ stderr-pollution compatibility break
19+
* introduced by PR #330. See `ModuleGraphParser.parseModuleGraph` for the fix.
20+
*/
21+
class StderrPollutionRegressionTest : KoinTest {
22+
@get:Rule val koinTestRule = KoinTestRule.create { modules(testModule()) }
23+
24+
private val parser = ModuleGraphParser()
25+
26+
// Shape of the stderr lines 17.0.1..18.0.5 captured into `moduleGraphJson`.
27+
private val stderrPrefix =
28+
"""
29+
Computing main repo mapping:
30+
Loading:
31+
Loading: 0 packages loaded
32+
Analyzing: 0 targets (0 packages loaded, 0 targets configured)
33+
INFO: Invocation ID: 4d8d5c62-1f1c-4f72-9a3e-5fbd5e6ac3d2
34+
INFO: Current date is 2026-04-20
35+
""".trimIndent()
36+
37+
private val cleanGraphJson =
38+
"""
39+
{
40+
"key": "<root>",
41+
"name": "my-workspace",
42+
"version": "",
43+
"apparentName": "my-workspace",
44+
"dependencies": [
45+
{"key": "bazel_tools@_", "name": "bazel_tools", "version": "_", "apparentName": "bazel_tools"},
46+
{"key": "abseil-cpp@20240116.2", "name": "abseil-cpp", "version": "20240116.2", "apparentName": "com_google_absl"},
47+
{"key": "aspect_bazel_lib@2.22.5", "name": "aspect_bazel_lib", "version": "2.22.5", "apparentName": "aspect_bazel_lib"},
48+
{"key": "rules_jvm_external@6.10", "name": "rules_jvm_external", "version": "6.10", "apparentName": "rules_jvm_external"},
49+
{"key": "rules_python@1.8.4", "name": "rules_python", "version": "1.8.4", "apparentName": "rules_python"},
50+
{"key": "googletest@1.14.0", "name": "googletest", "version": "1.14.0", "apparentName": "com_google_googletest"}
51+
]
52+
}
53+
""".trimIndent()
54+
55+
private val pollutedGraphJson = "$stderrPrefix\n$cleanGraphJson"
56+
57+
@Test
58+
fun `18_0_x polluted moduleGraphJson now parses correctly`() {
59+
val clean = parser.parseModuleGraph(cleanGraphJson)
60+
val result = parser.parseModuleGraph(pollutedGraphJson)
61+
62+
assertThat(result).hasSize(7)
63+
assertThat(result).isEqualTo(clean)
64+
}
65+
66+
@Test
67+
fun `18_1_0 clean moduleGraphJson parses successfully`() {
68+
val result = parser.parseModuleGraph(cleanGraphJson)
69+
70+
assertThat(result).hasSize(7)
71+
}
72+
73+
@Test
74+
fun `semantically identical graph across bazel-diff versions reports no module changes`() {
75+
val fromGraph = parser.parseModuleGraph(pollutedGraphJson)
76+
val toGraph = parser.parseModuleGraph(cleanGraphJson)
77+
78+
val changed = parser.findChangedModules(fromGraph, toGraph)
79+
80+
assertThat(changed).isEmpty()
81+
}
82+
83+
@Test
84+
fun `string compare at line 44 fires because polluted != clean even when modules are identical`() {
85+
// Documents that the naive byte compare in CalculateImpactedTargetsInteractor.execute
86+
// still fires cross-version, so downstream dispatch must keep handling it gracefully.
87+
assertThat(pollutedGraphJson).isNotEqualTo(cleanGraphJson)
88+
}
89+
90+
@Test
91+
fun `large head graph produces no spurious fan-out`() {
92+
val deps = (1..100).joinToString(",\n") { i ->
93+
""" {"key": "mod$i@1.0", "name": "mod$i", "version": "1.0", "apparentName": "mod$i"}"""
94+
}
95+
val bigHeadGraph = """
96+
{
97+
"key": "<root>",
98+
"name": "my-workspace",
99+
"version": "",
100+
"apparentName": "my-workspace",
101+
"dependencies": [
102+
$deps
103+
]
104+
}
105+
""".trimIndent()
106+
val bigPolluted = "$stderrPrefix\n$bigHeadGraph"
107+
108+
val fromGraph = parser.parseModuleGraph(bigPolluted)
109+
val toGraph = parser.parseModuleGraph(bigHeadGraph)
110+
val changed = parser.findChangedModules(fromGraph, toGraph)
111+
112+
assertThat(changed).isEmpty()
113+
}
114+
115+
@Test
116+
fun `end-to-end - semantically identical graph compared across versions reports no impacted targets`() {
117+
val hashes = mapOf(
118+
"//:target1" to TargetHash("", "unchanged1", "unchanged1"),
119+
"//:target2" to TargetHash("", "unchanged2", "unchanged2"),
120+
"//:target3" to TargetHash("", "unchanged3", "unchanged3"),
121+
)
122+
123+
val outputWriter = StringWriter()
124+
CalculateImpactedTargetsInteractor().execute(
125+
from = hashes,
126+
to = hashes,
127+
outputWriter = outputWriter,
128+
targetTypes = null,
129+
fromModuleGraphJson = pollutedGraphJson,
130+
toModuleGraphJson = cleanGraphJson,
131+
)
132+
133+
val impacted = outputWriter.toString().trim().split("\n").filter { it.isNotEmpty() }
134+
assertThat(impacted).isEmpty()
135+
}
136+
137+
@Test
138+
fun `end-to-end - same bazel-diff version on both sides is fast path`() {
139+
val hashes = mapOf(
140+
"//:target1" to TargetHash("", "unchanged1", "unchanged1"),
141+
"//:target2" to TargetHash("", "unchanged2", "unchanged2"),
142+
)
143+
144+
val outputWriter = StringWriter()
145+
CalculateImpactedTargetsInteractor().execute(
146+
from = hashes,
147+
to = hashes,
148+
outputWriter = outputWriter,
149+
targetTypes = null,
150+
fromModuleGraphJson = cleanGraphJson,
151+
toModuleGraphJson = cleanGraphJson,
152+
)
153+
154+
val impacted = outputWriter.toString().trim().split("\n").filter { it.isNotEmpty() }
155+
assertThat(impacted).isEmpty()
156+
}
157+
}

0 commit comments

Comments
 (0)