Repository navigation
Validate that bazel_args holds flags, not a command - #37
Open
anakanemison wants to merge 1 commit into
Open
anakanemison wants to merge 1 commit into
anakanemison wants to merge 1 commit into
Conversation
Base automatically changed from
mschaller/u01-configure-file-case-insensitive
to
main
October 26, 2026 13:30
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. config.load now validates the list and raises ValueError naming the first bad entry; diff.py turns that into a one-line error. An entry must be a flag, or the value of the flag just before it when that flag has no '='. So the well-formed ["-c", "opt"] is accepted, which a simpler "every entry starts with -" rule would reject. SKILL.md shows one right and one wrong example. In the lab's later QGIS run with this change, every bazel_args entry was a valid flag. Lab-Proposal: P22 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Part 2 of 23 of a stacked series (tracking issue: #35). Based on
U01-configure-file-case-insensitive, the previous part: review and merge that first.Kind: fix. Lab proposal: P22.
What the migration runs showed
A qgis migration put a whole
bazel aquery ...command inbazel_args, which silently disabled the comparison of the default invocation. In a later run with this change (run 089), every entry was a valid flag.The change
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 aqueryfailed on a second query expression and produced no parityresult.
config.load now validates the list and raises ValueError naming the first
bad entry; diff.py turns that into a one-line error. An entry must be a
flag, or the value of the flag just before it when that flag has no '='.
So the well-formed ["-c", "opt"] is accepted, which a simpler "every entry
starts with -" rule would reject. SKILL.md shows one right and one wrong
example.
In the lab's later QGIS run with this change, every bazel_args entry was a
valid flag.
Testing
pytest tests/at this commit: 136 passed (130 onmain).🤖 Generated with Claude Code