Repository navigation
Check bazel_args with Bazel's own flag parser - #37
Merged
Merged
Conversation
Base automatically changed from
mschaller/u01-configure-file-case-insensitive
to
main
October 26, 2026 13:30
anakanemison
force-pushed
the
mschaller/u02-validate-bazel-args
branch
from
October 26, 2026 13:30
27bf8be to
4f689d1
Compare
armandomontanez
requested changes
Oct 28, 2026
| return name in self.exclude_targets | ||
|
|
||
|
|
||
| def validate_bazel_args(args) -> tuple: |
Contributor
There was a problem hiding this comment.
Just use Bazel's canonicalize-flags for this. IIRC it chokes when you enter a target pattern (for better or for worse).
Otherwise, we need to properly handle boolean flags which don't accept --keep_going true, which can mask problems like --keep_going //some/target:pattern
Contributor
Author
There was a problem hiding this comment.
Good call, implemented.
bazel_args is meant for the flags that make aquery mirror the real build (--config=release, -c opt). Anything that consumes it splices it into a bazel command line, but nothing checked its shape, and the SKILL.md text sits next to a literal `bazel aquery ...` line that is easy to paste in. Measured in any2bazel-lab: on a QGIS migration the agent recorded the whole aquery command as bazel_args. Its own diff never read the field, so the migration looked converged, while a consumer that spliced the field into `bazel aquery` failed on a second query expression and produced no parity result. SKILL.md now says the field holds flags, with one right and one wrong example, and runs `bazel canonicalize-flags --for_command=aquery` on it just before aquery. Bazel refuses anything that is not an aquery option, including a target pattern and a stray word after a boolean flag (`--keep_going true`), which a hand-written check would have to special-case. The one-liner hands the list to Bazel as argv (os.execvp), so a value with a space in it is not split. It does not check that a --config exists; aquery fails on one that does not. Checked against Bazel 9.3.1: it refused a whole aquery command and `--keep_going true`, and accepted `--copt=-DMSG="a b"`, `-c opt`, `--config=release` and a config with no bazel_args. The lab's later QGIS run, made with an earlier version of this change (a Python check), recorded only valid flags; this version has not been through a migration run. Lab-Proposal: P22 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anakanemison
force-pushed
the
mschaller/u02-validate-bazel-args
branch
from
November 2, 2026 17:50
4f689d1 to
ccf83d5
Compare
anakanemison
changed the title
armandomontanez
approved these changes
Nov 2, 2026
anakanemison
added a commit
that referenced
this pull request
Nov 9, 2026
) A Bazel target mapped through `target_map` to an excluded CMake name was skipped by the diff but missing from `excluded.bazel`, because the summary checked the raw label. A new helper, `_bazel_renames`, makes the rename decision once; the diff and `excluded_summary` both use it, and the per-side summaries later in this series are built on it. Stacked on #37 (tracking issue: #35). `pytest tests/`: 134 passed (132 on `main`). Lab proposal: support for P17. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
bazel_argsis spliced intobazel aquery, but nothing checked that it holds flags. SKILL.md now says it holds flags, with one right and one wrong example, and runsbazel canonicalize-flags --for_command=aqueryon it just before aquery, from a one-liner that hands the list to Bazel as argv, so no value is split on spaces. Bazel refuses a command, a target pattern or a stray word after a boolean flag (--keep_going true), and prints the accepted flags in canonical form.A QGIS migration in any2bazel-lab recorded a whole
bazel aquery ...command asbazel_args; the migration looked converged, while the lab's script that spliced the field intobazel aqueryfailed and produced no parity result. Checked against Bazel 9.3.1. This version has not been through a migration run.Stacked on
main(tracking issue: #35).pytest tests/: 132 passed (132 onmain). Lab proposal: P22.🤖 Generated with Claude Code