bazel
bazel copied to clipboard
`--incompatible_existing_rules_immutable_view` breaks json encoding of `existing_rule/s` objects
Originally posted by @dws in https://github.com/bazelbuild/bazel/issues/13907#issuecomment-1239478273
If --incompatible_existing_rules_immutable_view
is set, json.encode
(or encode_indent
) will encode an immutable native.existing_rule
or existing_rules
object as an array of field names, instead of a dict (because our DictLikeView
object is a StarlarkIterable
).
proto.encode_text
is worse: it will fail with Error in encode_text: [...] got ExistingRulesView, want string, int, bool, or struct
when trying to encode a struct one of whose fields has an existing_rule
object as value.
Although there is a workaround (encode dict(native.existing_rule("foo").items())
), this is obviously a regression compared to the situation before the flag flip.
I think a solution would have the following shape:
- fix https://github.com/bazelbuild/bazel/issues/13829
- ... allowing us to have a non-throwing version of
get()
forDictLikeView
, in turn allowingDictLikeView
to extendMap
(see https://github.com/bazelbuild/bazel/issues/13605#issuecomment-939146471 for why we want to extendMap
and not go with a custom map-like creature) - ... which would immediately fix
json
; forproto
, we'd have to allow it to support arbitrary mappings, not justDict
.
An alternative would be for DictLikeView
to implement Json.Encodable
(whose interface we'd need to change a bit), and the corresponding ProtoModule.Encodable
(which we'd need to add) - but IMHO that would be quite ugly. Especially the duplication between the two encodable interfaces.
@brandjon fyi
After looking at this a bit, proto.encode_text
cannot reasonably support the output of native.existing_rule()
: the encoder cannot produce any reasonable schema for a dict (or dict-like entity) whose values may be lists/tuples, as opposed to scalars. Which is a reasonable limitation, considering the proto format.
We do still want to fix json encoding, however.
@comius Is this something you can look into? This is also blocking the 6.0 release.
@meteorcloudy - I have a fix in review internally
Bazel 6.0 cut is scheduled on Oct 10th, do you think we can fix this in time?
I hope to submit the fix this week