Conversation
bolinfest
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9c9743233
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ./.github/scripts/run-bazel-query-ci.sh \ | ||
| --keep_going \ | ||
| --output=label \ | ||
| "$query" >"$query_stdout" 2>"$query_stderr"; then | ||
| cat "$query_stderr" >&2 | ||
| rm -f "$query_stdout" "$query_stderr" | ||
| exit 1 | ||
| fi | ||
|
|
||
| cat "$query_stdout" | ||
| rm -f "$query_stdout" "$query_stderr" | ||
| -- "$query" |
There was a problem hiding this comment.
read_query_labels now streams run-bazel-query-ci.sh output directly, but this function is consumed via process substitution (done < <(read_query_labels ...)), where Bash set -e does not fail the parent script when the producer exits non-zero. With --keep_going, bazel query can emit partial labels before failing; those labels are then used as final_build_targets, so the Windows lint job can continue with an incomplete target set instead of failing closed. The previous implementation buffered stdout and only emitted labels on success, which avoided this silent partial-target behavior.
Useful? React with 👍 / 👎.
etraut-openai
left a comment
etraut-openai
left a comment
There was a problem hiding this comment.
Nice find! I don't have experience with bazel, but this looks good to me.
2 participants
Why
A rerun of the Windows Bazel clippy job after #19161 had exactly the cache behavior we wanted in BuildBuddy: zero action-cache misses. Even so, the GitHub job still took a little over five minutes.
The problem was that the job was paying for two separate Bazel startup paths:
bazel queryto discover extra lint targetsbazel build --config=clippy ...invocationOn Windows, that query was bypassing the CI Bazel wrapper, so it did not reuse the same
--output_user_root, CI config, or remote-cache setup as the real build. In practice that meant the rerun could still cold-start a separate Bazel server before the actual clippy build even began.What
.github/scripts/run-bazel-query-ci.shto run CI-side Bazel queries with the same startup and cache-related flags as the main Bazel commandscripts/list-bazel-clippy-targets.shto use that helper for manualrust_testtarget discoverytools/argument-comment-lint/list-bazel-targets.shto use the same helper.github/scripts/run-argument-comment-lint-bazel.shso its Windows-only query path also goes through the shared helperThis keeps the target-discovery queries aligned with the later build/test invocation instead of treating them as a separate cold Bazel session.
Verification
bash -n .github/scripts/run-bazel-query-ci.shbash -n scripts/list-bazel-clippy-targets.shbash -n tools/argument-comment-lint/list-bazel-targets.shbash -n .github/scripts/run-argument-comment-lint-bazel.shrun-bazel-query-ci.shand verified it forwards--output_user_root,--config=ci-windows, the BuildBuddy auth header, and the repository cache flagsDocs
No documentation updates are needed.