fix: bxl target patterns didn't accept multiple args#1279
Conversation
I was surprised to learn that my code which the target exprs were used as a list no longer worked after a buck2 upgrade. These are documented as being a list, yet they were not actually parsed as such! https://buck2.build/docs/api/bxl/cli_args/#target_expr ``` def _collect_nix_deps(ctx): print(ctx.cli_args) pass main = bxl.main( impl = _collect_nix_deps, cli_args = { "target": bxl.cli_args.target_expr("Target(s) to build and import"), }, doc = """ Extracts the direct/buck2-transitive Nix dependency list of a buck2 target into a messy json file. """, ) ``` Before: ``` $ cargo r -p buck2 -- bxl test.bxl:main -- --target //:foo --target //:bar Compiling buck2_bxl v0.1.0 (/Users/jade/co/buck2/app/buck2_bxl) Compiling buck2 v0.1.0 (/Users/jade/co/buck2/app/buck2) Finished `dev` profile [optimized + debuginfo] target(s) in 5.84s Running `target/debug/buck2 bxl 'test.bxl:main' -- --target '//:foo' --target '//:bar'` buck2 daemon constraint mismatch: Version mismatch; killing daemon... Starting new buck2 daemon... Connected to new buck2 daemon. error: the argument '--target <target>' cannot be used multiple times ``` After: ``` Compiling buck2_bxl v0.1.0 (/Users/jade/co/buck2/app/buck2_bxl) Compiling buck2 v0.1.0 (/Users/jade/co/buck2/app/buck2) Finished `dev` profile [optimized + debuginfo] target(s) in 9.08s Running `target/debug/buck2 bxl 'test.bxl:main' -- --target '//:buck2' --target '//:buck2_bundle'` buck2 daemon constraint mismatch: Version mismatch; killing daemon... Starting new buck2 daemon... Connected to new buck2 daemon. struct(target=[gh_facebook_buck2//:buck2, gh_facebook_buck2//:buck2_bundle]) ``` I have no way to run the tests (facebook#1027), so you will have to forgive my attempt to add a test case here; try getting Claude to fix the bits I missed. The OSS build is down due to April Fools, so while I've tested this locally with some build fix hacks, I don't think the external CI will say much useful.
4ed31a8 to
ccc251a
Compare
|
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D99137385. (Because this pull request was imported automatically, there will not be any future comments.) |
|
I wonder if this is a case where it would be better to just update the documentation. The documentation says: "Takes an arg from the cli, and treats it as a target pattern, e.g. "cell//foo:bar", "cell//foo:", or "cell//foo/...". We will get a list of TargetLabel in bxl." The "we will get a list" part is about the fact that a single expression like "cell//foo:" can resolve to multiple targets, and that bxl gets the resolved targets. I'd have expected this to give both the resolved list and the original expression, but :/ I think (but not sure) it's intentional that this doesn't accept multiple arguments, and where you'd want to accept multiples you'd use a cli_args.list(cli_args.target_expr()). The cli_args.list() documentation deserves some update... like how do i actually pass a list? does it mean i can pass the arg multiple times? I wonder if we were previously swallowing this and fixed it to give an error. |
I was surprised to learn that my code which the target exprs were used as a list no longer worked after a buck2 upgrade. These are documented as being a list, yet they were not actually parsed as such! https://buck2.build/docs/api/bxl/cli_args/#target_expr
I actually wonder if buck2 was eating the subsequent list elements before (... but I remember it working before??), because this parser seems to never have contemplated multiple target args.
Before:
After:
I have no way to run the tests
(#1027), so you will have to
forgive my attempt to add a test case here; try getting Claude to fix
the bits I missed.
The OSS build is down due to April Fools, so while I've tested this
locally with some build fix hacks, I don't think the external CI will
say much useful.