Convert Starlark options with Starlark#fromJava in build_options
#18988
Conversation
`cquery`'s `build_config` function now ensures that Starlark option values are converted to Starlark types before being added to the `dict` of all options, fixing a crash observed for list-typed build settings specified on the command line.
github-actions
bot
added
awaiting-review
Nit: As an aside, I didn't realize this function exists. Does it offer anything over a |
fmeum
changed the title
Starlark#fromJava in build_configStarlark#fromJava in build_options
|
I can't think of anything that couldn't also be done with |
gregestren
added
awaiting-PR-merge
| EOF | ||
|
|
||
| bazel cquery "//$pkg:bar" --output=starlark \ | ||
| --starlark:file=$pkg/expr.star > output 2>"$TEST_log" || fail "Expected success" | ||
|
|
||
| assert_contains "//$pkg:bar%None%False" output | ||
| assert_contains "//$pkg:bar%None%False%None" output |
There was a problem hiding this comment.
Why is this None instead of the build_setting_default of ["c"]? This isn't necessarily something that needs to be changed if this is the behavior we want. I guess this does allow differentiating between an unset value and a value set to the default.
There was a problem hiding this comment.
This is a consequence of how Starlark options are implemented: When they haven't been set anywhere, they aren't tracked and the dict of all options just won't contain any value for them, not even the default.
This makes sense given the decentralized nature of Bazel: Knowing all Starlark options and their default values would require loading all packages (and thus all repos) eagerly.
There was a problem hiding this comment.
Ah, that does make some sense, but then it is weird how then you can get the actual value when its set despite bar having no dependency on mylistflag.
There was a problem hiding this comment.
That's why build_options is "magic": It gives you access to the full internal view of all options that Bazel maintains. This has an entry for mylistflag simply because it has been set on the command-line. build_options doesn't care about dependencies here, which is why it is only made available in cquery, not in regular Starlark.
github-actions
bot
removed
the
awaiting-PR-merge
cquery'sbuild_optionsfunction now ensures that Starlark option values are converted to Starlark types before being added to thedictof all options, fixing a crash observed for list-typed build settings specified on the command line.Fixes #18977