incompatible_existing_rules_immutable_view

[tetromino]

I'm working on the implementation for https://github.com/bazelbuild/proposals/blob/main/designs/2021-06-15-improving-native.existing_rules.md (being reviewed by @brandjon offline).

The plan is to:

• add an experimental flag (--experimental_existing_rules_immutable_view --incompatible_existing_rules_immutable_view); if the flag is enabled, native.existing_rule and native.existing_rules will return immutable, lightweight dict-like view objects instead of mutable dicts;
• identify rule sets which currently rely on mutability or typename of the return value of existing_rule/s and migrate them so that they are compatible with both mutable and immutable behaviors of the functions;
• flip the flag.

[tetromino]

Migration notes for flag: --incompatible_existing_rules_immutable_view

native.existing_rule(), instead of a dict value, returns an immutable, dict-like view object (in other words, supporting dict-like iteration, len(x), name in x, x[name], x.get(name), x.items(), x.keys(), and x.values()) with the same content as before.

native.existing_rules(), instead of a dict value, returns an immutable, dict-like view object (in other words, supporting dict-like iteration, len(x), name in x, x[name], x.get(name), x.items(), x.keys(), and x.values()) with the same content as before.

As a result of these changes, native.existing_rule and native.existing_rules calls have better performance in most cases.

Migration

You can no longer mutate the value returned by native.existing_rule() or native.existing_rules(). Ideally, try to avoid mutation: that would help improve analysis-time performance. If your rules' logic requires you to mutate the value, you would need to explicitly construct a mutable dict using dict(x.items()):

     rule = native.existing_rule(some_name)
     if not rule:
         fail("%s not found." % some_name)
+    rule = dict(rule.items())  # Ensure rule is mutable
     # ...
     rule[attr_name] = value

Unfortunately, you can no longer pass directly the value returned by native.existing_rule() or native.existing_rules() to the dict builtin until #13605 is fixed. As a workaround, pass x.items() to dict instead:

     rule = native.existing_rule(some_name)
     if not rule:
         fail("%s not found." % some_name)
-    rule_copy = dict(rule)
+    rule_copy = dict(rule.items())  # TODO: use dict(rule) when https://github.com/bazelbuild/bazel/issues/13605 is fixed

Similarly, you can no longer pass the value returned by native.existing_rule() or native.existing_rules() to the ** operator. There are two workarounds. You may create a mutable dict using dict(x.items()), as explained above. Alternatively, you might switch your Starlark function to take a dict argument instead of **kwargs:

- def is_rule_valid(**kwargs):
-    return kwargs["foo"] == "bar"
+ def is_rule_valid(rule):
+    return rule["foo"] == "bar"
 
 def my_func():
     rule = native.existing_rule(some_name)
     if not rule:
         fail("%s not found." % some_name)
     if is_rule_valid(rule):
        # ...

[dws]

Would it make sense to adjust json.encode() to treat these immutable views as dicts?

I ask because I noticed that

json.encode_indent(native.existing_rules().values())

no longer produced the same output when --incompatible_existing_rules_immutable_view
was enabled in Bazel 5. Following the advice here I found that

json.encode_indent([dict(repo.items()) for repo in native.existing_rules().values()])

was compatible.

[tetromino]

@dws - thanks, filed #16256 as new issue. Very unfortunate that we overlooked this before the flag flip.