fix(bes): write the caller's --build_event_binary_file - #1389
Draft
mcook42 wants to merge 4 commits into
Draft
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
✨ Aspect Workflows Tasks📅 Fri Aug 14 15:03:49 UTC 2026 ✅ 2 successful tasks
⏱ Last updated Fri Aug 14 15:11:51 UTC 2026 · 📊 GitHub API quota 9/15,000 (0% used, resets in 58m) |
mcook42
force-pushed
the
fix/bep-file-not-written
branch
from
August 13, 2026 15:57
7fd7cde to
d799a68
Compare
`Build::spawn` appends the CLI's own `--build_event_binary_file` after every user flag. Bazel's option is single-valued, so last-wins meant a caller who asked for a BEP file silently got nothing — the file was created by whoever made the temp path and never written to. The IntelliJ Bazel plugin drives sync on exactly that flag, so even with the target-pattern collision fixed it would parse an empty file. Collect a file sink for the caller's path instead of stripping or reordering flags. The CLI's path still wins on the command line; the caller's file is re-created from Bazel's own byte stream, since file sinks share the BES reader's raw-bytes path. Placed in `collect_bes_sinks` so all eight bazel-spawning tasks are covered — every one of them hit the same clobbering. Two consequences: `--build_event_binary_file_upload_mode` no longer governs that file (the caller's existing `sink.wait()` completes it, before the task concludes), and `--build_event_json_file` / `--build_event_text_file` are untouched — different flags, so Bazel still writes those itself.
The unit tests cover collect_bes_sinks in isolation, but nothing exercised it end to end: that a caller's --build_event_binary_file, forwarded in the shape the IntelliJ Bazel plugin produces (Bazel's own flag spelling behind --bazel-flag), actually lands on disk. Extend the existing test-flags-task step in both pipelines with one aspect build in that shape, asserting a non-empty BEP at the caller's path. Split out of the combined IDE/BSP smoke-test commit in #1370 — the --target_pattern_file half goes with that fix's own PR.
mcook42
force-pushed
the
fix/bep-file-not-written
branch
from
August 13, 2026 22:26
d799a68 to
81b53d8
Compare
…p-file-not-written
Two prior commits (81b53d8, dd4a1dc) each independently extended the test-flags-task comment block with their own IDE/BSP explanation. The merge (87e5694) interleaved both without deduplicating their shared boilerplate, leaving a garbled, self-contradicting comment. Merge the two explanations into one coherent paragraph; no script changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #1370 (fix 2 of 3). See that PR's description for full context on all three defects found in the same investigation.
A caller's
--build_event_binary_filewas silently never written.Build::spawnappends the CLI's own--build_event_binary_fileafter every user flag; Bazel's option is single-valued, so last-wins meant a caller who asked for a BEP file got a path that was created but never written to. The IntelliJ Bazel plugin parses sync results from that file, so even with the--target_pattern_fileconflict fixed separately, it would have read an empty file.Fixed by collecting a file sink for the caller's path in
collect_bes_sinks, rather than stripping or reordering flags. The CLI's path still wins on the command line; the caller's file is recreated from Bazel's own byte stream, since file sinks share the BES reader's raw-bytes path.Test plan
bazel_bep_fileresolution and thecollect_bes_sinksfile-sink append inbuild_events_test.axl. Verified locally:bazel run //crates/aspect-cli:aspect-cli -- dev test-bes-sinks→build_events_test.axl: OK (14 tests).test-flags-taskin both.buildkite/pipeline.yamland.github/workflows/ci-workflows.yamlwith oneaspect buildin the plugin's exact shape (--bazel-flag=--build_event_binary_file=...), asserting a non-empty BEP at the caller's path.--build_event_binary_filehalf of fix(cli): IDEs with plugins using BSP can now use aspect cli instead of vanilla bazel #1370's combined IDE/BSP smoke-test commit — the--target_pattern_filehalf went to that fix's own PR instead, since a single combined step would make each split PR's CI depend on the other's fix also being merged.Before / after (captured on
main)WARNING: BES was not properly closed