bazel

bazel copied to clipboard

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() for DictLikeView, in turn allowing DictLikeView to extend Map (see https://github.com/bazelbuild/bazel/issues/13605#issuecomment-939146471 for why we want to extend Map and not go with a custom map-like creature)
• ... which would immediately fix json; for proto, we'd have to allow it to support arbitrary mappings, not just Dict.

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