fix: omit stderr from metadata when generating hashes by Ahajha · Pull Request #330 · Tinder/bazel-diff · GitHub
Skip to content

fix: omit stderr from metadata when generating hashes#330

Merged
tinder-maxwellelliott merged 1 commit into
Tinder:masterfrom
Ahajha:trotta/nostderr
Apr 13, 2026
Merged

fix: omit stderr from metadata when generating hashes#330
tinder-maxwellelliott merged 1 commit into
Tinder:masterfrom
Ahajha:trotta/nostderr

Conversation

@Ahajha

@Ahajha Ahajha commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Occasionally I see INFO: Checking for file changes...\n in the metadata when generating hashes, which is extra useless info that causes bazel-diff to think the whole graph has changed. This silences the stderr so that it never gets added to the metadata.

I can't reproduce the issue exactly, but I've been running this patch internally for a little bit and it seems to be working.

Fixes #329

Occasionally I see `INFO: Checking for file changes...\n` in the metadata when generating hashes, which is extra useless info that causes bazel-diff to think the whole graph has changed. This silences the stderr so that it never gets added to the metadata.

Fixes Tinder#329
@Ahajha Ahajha marked this pull request as ready for review April 11, 2026 22:30

@tinder-maxwellelliott tinder-maxwellelliott left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you!

@tinder-maxwellelliott tinder-maxwellelliott merged commit b213ee4 into Tinder:master Apr 13, 2026
15 checks passed
rdark added a commit to rdark/bazel-diff that referenced this pull request Apr 27, 2026
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 Tinder#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.
tinder-maxwellelliott pushed a commit that referenced this pull request May 13, 2026
)

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Arbitrary Bazel output can cause full rebuilds

2 participants