From 3b01cdcc22b6d370331bd6e053105bf249e34da9 Mon Sep 17 00:00:00 2001 From: "Matt (Orion) Cook" Date: Thu, 30 Jul 2026 15:28:39 -0600 Subject: [PATCH 1/3] fix(bes): write the caller's --build_event_binary_file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- .../builtins/aspect/bazel/build_events.axl | 45 ++++++++++++++++++- .../aspect/bazel/build_events_test.axl | 45 ++++++++++++++++++- 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/crates/aspect-cli/src/builtins/aspect/bazel/build_events.axl b/crates/aspect-cli/src/builtins/aspect/bazel/build_events.axl index f873ba93f..93d8fd920 100644 --- a/crates/aspect-cli/src/builtins/aspect/bazel/build_events.axl +++ b/crates/aspect-cli/src/builtins/aspect/bazel/build_events.axl @@ -12,6 +12,11 @@ once rather than twice. Because that can leave no sink at all — and the Aspect Web UI link keys on an id only a sink mints — a task pairs the collect call with `bes_streamed_by_bazel`, which redirects the link to Bazel's invocation id. +`collect_bes_sinks` also covers the one Bazel-facing BEP flag the CLI would +otherwise clobber: a caller's `--build_event_binary_file` becomes a CLI file sink, +because `Build::spawn` appends the CLI's own path to that single-valued option +last and last wins. See `collect_bes_sinks` for what that changes. + The Aspect login JWT is attached to an Aspect-owned backend's sink metadata when the user has not supplied their own `authorization` header — see `aspect_endpoint_auth.axl` for the host gate and best-effort credential @@ -55,6 +60,22 @@ def bazel_bes_backend(rc, command: str) -> str: return "" return rc.flag_value("--bes_backend", command = command) or "" +def bazel_bep_file(rc, command: str) -> str: + """The `--build_event_binary_file` path the caller asked Bazel to write, or `""`. + + Reads through the run command, so it sees the flag wherever it came from — + `--bazel-flag=--build_event_binary_file=…`, a `.bazelrc`, or an expanded + `--config`. `rc` may be `None` for a caller with no run command to consult. + + Nobody but the caller sets this: the CLI's own BEP file is appended inside + `Build::spawn`, after rc expansion, and never passes through the rc. + + Public so `build_events_test.axl` can assert the resolution. + """ + if rc == None: + return "" + return rc.flag_value("--build_event_binary_file", command = command) or "" + def _drop_bazel_streamed(items: list, uri_of, bazel_backend: str) -> list: """`items` minus those whose `uri_of(item)` names the same endpoint as Bazel's own `bazel_backend`. @@ -106,16 +127,38 @@ def collect_bes_sinks(ctx, bazel_trait, rc, command: str = "build", extra_backen Sinks to the endpoint Bazel's own `--bes_backend` uploads to are dropped from both groups so the invocation is streamed once — see `_drop_bazel_streamed`. + Last comes a file sink for the caller's own `--build_event_binary_file`, when + they asked for one. Bazel cannot write two: the option is single-valued and + `Build::spawn` appends the CLI's path last, so the caller's file would + silently never be written. Writing it as a sink instead re-creates it from + Bazel's own byte stream — the file sinks share the BES reader's raw-bytes + path, so the result is what Bazel would have written. This is what lets IDE / + BSP tooling (the IntelliJ Bazel plugin) drive builds through the + `tools/bazel` wrapper and still get its BEP back. Two consequences worth + knowing: `--build_event_binary_file_upload_mode` no longer governs that file + (the caller's `wait()` on the returned sinks is what 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 one call a bazel-spawning task makes; pair it with `bes_streamed_by_bazel` to keep the Aspect Web UI link resolvable. The caller `wait()`s the returned sinks after the build. """ bazel_backend = bazel_bes_backend(rc, command) - return ( + sinks = ( collect_bes_from_args(ctx, extra_backends = extra_backends, bazel_backend = bazel_backend) + _drop_bazel_streamed(list(bazel_trait.build_event_sinks), lambda s: s.uri, bazel_backend) ) + # Not routed through `_drop_bazel_streamed`: a file is a local dump, not a + # second upload to an endpoint Bazel already streams to. + bep_file = bazel_bep_file(rc, command) + if bep_file: + trace.event("bes.caller_bep_file", fields = {"path": bep_file, "command": command}) + sinks.append(bazel.build_events.file(path = bep_file)) + + return sinks + def dropped_bes_backends(ctx, bazel_trait, rc, command: str = "build", extra_backends = []) -> list[str]: """The BES endpoints `collect_bes_sinks` skipped because Bazel uploads to them itself, deduped. Same inputs as that call — pass the same arguments. diff --git a/crates/aspect-cli/src/builtins/aspect/bazel/build_events_test.axl b/crates/aspect-cli/src/builtins/aspect/bazel/build_events_test.axl index 6e88f7a0a..302d2458d 100644 --- a/crates/aspect-cli/src/builtins/aspect/bazel/build_events_test.axl +++ b/crates/aspect-cli/src/builtins/aspect/bazel/build_events_test.axl @@ -1,13 +1,14 @@ """Tests for `bazel/build_events.axl` — the BES upload summary line, the gRPC-sink filter shared by the announce/summary helpers, sink collection, and the duplicate-stream drop that keeps the CLI from streaming to an endpoint Bazel's -own `--bes_backend` already uploads to. +own `--bes_backend` already uploads to, and the file-sink tee that keeps a +caller's `--build_event_binary_file` from being silently clobbered. Run with: aspect dev test-bes-sinks """ -load("@aspect//bazel/build_events.axl", "bazel_bes_backend", "bes_results_line", "bes_streamed_by_bazel", "bes_upload_line", "collect_bes_from_args", "collect_bes_sinks", "dropped_bes_backends", "grpc_backends") +load("@aspect//bazel/build_events.axl", "bazel_bep_file", "bazel_bes_backend", "bes_results_line", "bes_streamed_by_bazel", "bes_upload_line", "collect_bes_from_args", "collect_bes_sinks", "dropped_bes_backends", "grpc_backends") def _eq(label, got, want): if got != want: @@ -248,6 +249,44 @@ def _test_collect_bes_sinks(_): [None, "grpcs://bes.other.example.com"], ) +def _test_bazel_bep_file(_): + """`bazel_bep_file` reads the caller's `--build_event_binary_file` off the run + command, so it sees the flag from `--bazel-flag=`, a `.bazelrc`, or a + `--config` expansion alike.""" + _eq("caller asked for one", bazel_bep_file(_fake_rc({"--build_event_binary_file": "/tmp/bep.binpb"}), "build"), "/tmp/bep.binpb") + _eq("flag unset", bazel_bep_file(_fake_rc({}), "build"), "") + _eq("no run command", bazel_bep_file(None, "build"), "") + +def _test_collect_bes_sinks_tees_caller_bep_file(_): + """A caller's `--build_event_binary_file` becomes a trailing file sink, so the + path they asked for is written from Bazel's own byte stream. + + Without it their file is silently empty: the Bazel option is single-valued and + `Build::spawn` appends the CLI's own path last.""" + trait = _fake_trait([struct(uri = "grpcs://bes.other.example.com")]) + + def uris(rc_values): + ctx = _fake_ctx(bes_backends = []) + return [s.uri for s in collect_bes_sinks(ctx, trait, _fake_rc(rc_values))] + + _eq( + "file sink appended after the trait's own", + uris({"--build_event_binary_file": "/tmp/bep.binpb"}), + ["grpcs://bes.other.example.com", None], + ) + _eq("no BEP flag, no extra sink", uris({}), ["grpcs://bes.other.example.com"]) + + # The tee is a local dump, so it survives the duplicate-stream drop that + # suppresses a CLI sink to the endpoint Bazel itself uploads to. + duplicate = "grpcs://bes.acme.aspect.build" + ctx = _fake_ctx(bes_backends = [duplicate]) + rc = _fake_rc({"--bes_backend": duplicate, "--build_event_binary_file": "/tmp/bep.binpb"}) + _eq( + "kept while every gRPC sink is suppressed", + [s.uri for s in collect_bes_sinks(ctx, _fake_trait([struct(uri = duplicate)]), rc)], + [None], + ) + def _test_bes_streamed_by_bazel(_): """True only when Bazel's `--bes_backend` is the runner's own Aspect BES backend — the case where the Web UI still holds the invocation (under @@ -280,6 +319,8 @@ _UNIT_TESTS = [ _test_bazel_bes_backend, _test_collect_drops_duplicate_backends, _test_collect_bes_sinks, + _test_bazel_bep_file, + _test_collect_bes_sinks_tees_caller_bep_file, _test_bes_streamed_by_bazel, ] From 81b53d865c9b5ed8eb5e708dea8eaade490bfe23 Mon Sep 17 00:00:00 2001 From: "Matt (Orion) Cook" Date: Thu, 13 Aug 2026 09:17:54 -0600 Subject: [PATCH 2/3] ci: smoke-test the IDE/BSP --build_event_binary_file passthrough MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .buildkite/pipeline.yaml | 17 ++++++++++++++++- .github/workflows/ci-workflows.yaml | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index 905926452..cc4cde81d 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -329,7 +329,12 @@ steps: aspect test --task:name test-bk # Smoke-tests the new test-task flags. `--target-pattern-file` is forwarded # to Bazel verbatim, so the assertion is just that aspect resolves the file - # and the run completes. `--coverage` is exercised against an sh_test rather + # and the run completes. The IDE/BSP step re-runs a build in the shape the + # IntelliJ Bazel plugin produces through the tools/bazel wrapper — Bazel's + # own flag spellings behind `--bazel-flag` — and asserts the caller's BEP + # file is actually written. The CLI appends its own + # `--build_event_binary_file` last, and the Bazel option is single-valued. + # `--coverage` is exercised against an sh_test rather # than a rust_test: `toolchains_llvm_bootstrapped` 0.5.2 (pinned in # MODULE.bazel) ships without `libclang_rt.profile.a`, so any rust_test under # --collect_code_coverage fails CppLink on libld/libc shared libs (upstream @@ -358,6 +363,16 @@ steps: aspect test --task:name test-bk-target-pattern-file --target-pattern-file=$$PATTERNS rm -f $$PATTERNS + echo "--- :aspect: IDE/BSP shape — forwarded --build_event_binary_file" + BEP=$$(mktemp) + aspect build --task:name test-bk-ide-bep-passthrough \ + --bazel-flag=--build_event_binary_file=$$BEP \ + --bazel-flag=--build_event_binary_file_upload_mode=wait_for_upload_complete \ + --bazel-flag=--tool_tag=bazelbsp:3.2.0 \ + //examples/test_states:always_pass + test -s $$BEP || { echo "caller's --build_event_binary_file was not written"; exit 1; } + rm -f $$BEP + echo "--- :aspect: aspect test --coverage (+ --coverage-report + --coverage-tool)" REPORT=$$(mktemp) aspect test --task:name test-bk-coverage \ diff --git a/.github/workflows/ci-workflows.yaml b/.github/workflows/ci-workflows.yaml index 305197f54..beb3909e5 100644 --- a/.github/workflows/ci-workflows.yaml +++ b/.github/workflows/ci-workflows.yaml @@ -613,7 +613,12 @@ jobs: # Smoke-tests the new test-task flags. Ported from the Buildkite # `test-flags-task` step. `--target-pattern-file` is forwarded to Bazel # verbatim, so the assertion is just that aspect resolves the file and the run - # completes. `--coverage` is exercised against an sh_test rather than a + # completes. The IDE/BSP step re-runs a build in the shape the IntelliJ + # Bazel plugin produces through the tools/bazel wrapper — Bazel's own flag + # spellings behind `--bazel-flag` — and asserts the caller's BEP file is + # actually written. The CLI appends its own `--build_event_binary_file` + # last, and the Bazel option is single-valued. + # `--coverage` is exercised against an sh_test rather than a # rust_test: toolchains_llvm_bootstrapped 0.5.2 (pinned in MODULE.bazel) ships # without libclang_rt.profile.a, so any rust_test under --collect_code_coverage # fails CppLink (upstream hermeticbuild/hermetic-llvm#318, fixed in #468 — not @@ -638,6 +643,16 @@ jobs: aspect test --task:name test-gha-target-pattern-file --target-pattern-file="$PATTERNS" rm -f "$PATTERNS" + echo "--- IDE/BSP shape — forwarded --build_event_binary_file" + BEP=$(mktemp) + aspect build --task:name test-gha-ide-bep-passthrough \ + --bazel-flag=--build_event_binary_file="$BEP" \ + --bazel-flag=--build_event_binary_file_upload_mode=wait_for_upload_complete \ + --bazel-flag=--tool_tag=bazelbsp:3.2.0 \ + //examples/test_states:always_pass + test -s "$BEP" || { echo "caller's --build_event_binary_file was not written"; exit 1; } + rm -f "$BEP" + echo "--- aspect test --coverage (+ --coverage-report + --coverage-tool)" REPORT=$(mktemp) aspect test --task:name test-gha-coverage \ From 5dffa18a648963934fa19fb5d344b0cb0342ada0 Mon Sep 17 00:00:00 2001 From: "Matt (Orion) Cook" Date: Fri, 14 Aug 2026 08:56:50 -0600 Subject: [PATCH 3/3] ci: de-duplicate a merged comment in test-flags-task Two prior commits (81b53d86, dd4a1dcc) each independently extended the test-flags-task comment block with their own IDE/BSP explanation. The merge (87e56946) interleaved both without deduplicating their shared boilerplate, leaving a garbled, self-contradicting comment. Merge the two explanations into one coherent paragraph; no script changes. --- .buildkite/pipeline.yaml | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index 2257502f6..f75bab625 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -329,16 +329,12 @@ steps: aspect test --task:name test-bk # Smoke-tests the new test-task flags. `--target-pattern-file` is forwarded # to Bazel verbatim, so the assertion is just that aspect resolves the file - # and the run completes. The IDE/BSP step re-runs a build in the shape the + # and the run completes. The IDE/BSP steps re-run a build in the shape the # IntelliJ Bazel plugin produces through the tools/bazel wrapper — Bazel's - # own flag spellings behind `--bazel-flag` — and asserts the caller's BEP - # file is actually written. The CLI appends its own - # `--build_event_binary_file` last, and the Bazel option is single-valued. - # `--coverage` is exercised against an sh_test rather - # and the run completes. The IDE/BSP step re-runs the same pattern file in - # the shape the IntelliJ Bazel plugin produces through the tools/bazel - # wrapper — Bazel's own flag spellings behind `--bazel-flag` — asserting - # that the composed command line is one Bazel accepts. + # own flag spellings behind `--bazel-flag` — asserting that the composed + # command line is one Bazel accepts, and that the caller's + # `--build_event_binary_file` is actually written (the CLI appends its own + # last, and the Bazel option is single-valued). # `--coverage` is exercised against an sh_test rather # than a rust_test: `toolchains_llvm_bootstrapped` 0.5.2 (pinned in # MODULE.bazel) ships without `libclang_rt.profile.a`, so any rust_test under