[7.1.0] Fixes for experimental extend rule and subrule functionality (#…

…21237)

Contains cherry-picks:

d083f3d Reify outputs of initializer within the package
723a3ac Allow reading the name of the target in the initializer
48b975a Fix dict types in initializers
c519916 Make Google proto_library and extension of Bazel
proto_library


Fixes: #20782

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -516,7 +516,9 @@ public static StarlarkRuleFunction createRule(
     }
 
     // Verify the child against parent's allowlist
-    if (parent != null && parent.getExtendableAllowlist() != null) {
+    if (parent != null
+        && parent.getExtendableAllowlist() != null
+        && !bzlFile.getRepository().getNameWithAt().equals("@_builtins")) {
       builder.addAllowlistChecker(EXTEND_RULE_ALLOWLIST_CHECKER);
       Attribute.Builder<Label> allowlistAttr =
           attr("$allowlist_extend_rule", LABEL)
@@ -1071,7 +1073,7 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
           // The less magic the better. Do not give in those temptations!
           Dict.Builder<String, Object> initializerKwargs = Dict.builder();
           for (var attr : currentRuleClass.getAttributes()) {
-            if (attr.isPublic() && attr.starlarkDefined()) {
+            if ((attr.isPublic() && attr.starlarkDefined()) || attr.getName().equals("name")) {
               if (kwargs.containsKey(attr.getName())) {
                 Object value = kwargs.get(attr.getName());
                 if (value == Starlark.NONE) {
@@ -1101,6 +1103,12 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
                   : Dict.cast(ret, String.class, Object.class, "rule's initializer return value");
 
           for (var arg : newKwargs.keySet()) {
+            if (arg.equals("name")) {
+              if (!kwargs.get("name").equals(newKwargs.get("name"))) {
+                throw Starlark.errorf("Initializer can't change the name of the target");
+              }
+              continue;
+            }
             checkAttributeName(arg);
             if (arg.startsWith("_")) {
               // allow setting private attributes from initializers in builtins
@@ -1126,8 +1134,11 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
                         currentRuleClass.getName(),
                         attr,
                         value,
-                        // Reify to the location of the initializer definition
-                        currentRuleClass.getLabelConverterForInitializer());
+                        // Reify to the location of the initializer definition (except for outputs)
+                        attr.getType() == BuildType.OUTPUT
+                                || attr.getType() == BuildType.OUTPUT_LIST
+                            ? pkgContext.getBuilder().getLabelConverter()
+                            : currentRuleClass.getLabelConverterForInitializer());
             kwargs.putEntry(nativeName, reifiedValue);
           }
         }

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java

@@ -465,7 +465,7 @@ public boolean isDefaultExecutableCreated() {
    */
   public void close() {
     // Check super was called
-    if (ruleClassUnderEvaluation.getStarlarkParent() != null && !superCalled) {
+    if (ruleClassUnderEvaluation.getStarlarkParent() != null && !superCalled && !isForAspect()) {
       ruleContext.ruleError("'super' was not called.");
     }
 
@@ -595,7 +595,7 @@ public Object callParent(StarlarkThread thread) throws EvalException, Interrupte
       BuiltinRestriction.failIfCalledOutsideAllowlist(thread, ALLOWLIST_RULE_EXTENSION_API);
     }
     checkMutable("super()");
-    if (aspectDescriptor != null) {
+    if (isForAspect()) {
       throw Starlark.errorf("Can't use 'super' call in an aspect.");
     }
     if (ruleClassUnderEvaluation.getStarlarkParent() == null) {

src/main/java/com/google/devtools/build/lib/packages/Type.java

@@ -606,9 +606,24 @@ public Map<KeyT, ValueT> convert(Object x, Object what, LabelConverter labelConv
     @Override
     public Object copyAndLiftStarlarkValue(
         Object x, Object what, @Nullable LabelConverter labelConverter) throws ConversionException {
-      return Dict.immutableCopyOf(convert(x, what, labelConverter));
+      if (!(x instanceof Map)) {
+        throw new ConversionException(this, x, what);
+      }
+      Map<?, ?> o = (Map<?, ?>) x;
+      // It's possible that #convert() calls transform non-equal keys into equal ones so we can't
+      // just use ImmutableMap.Builder() here (that throws on collisions).
+      LinkedHashMap<Object, Object> result = new LinkedHashMap<>();
+      for (Map.Entry<?, ?> elem : o.entrySet()) {
+        result.put(
+            keyType.copyAndLiftStarlarkValue(elem.getKey(), "dict key element", labelConverter),
+            valueType.copyAndLiftStarlarkValue(
+                elem.getValue(), "dict value element", labelConverter));
+      }
+      return Dict.immutableCopyOf(result);
     }
 
+
+
     @Override
     public Map<KeyT, ValueT> concat(Iterable<Map<KeyT, ValueT>> iterable) {
       Dict.Builder<KeyT, ValueT> output = new Dict.Builder<>();

src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConstants.java

@@ -43,7 +43,7 @@ public final class ProtoConstants {
    * it with the .proto file that violates strict proto deps.
    */
   static final String STRICT_PROTO_DEPS_VIOLATION_MESSAGE =
-      "%%s is imported, but %1$s doesn't directly depend on a proto_library that 'srcs' it.";
+      "--direct_dependencies_violation_msg=%%s is imported, but %1$s doesn't directly depend on a proto_library that 'srcs' it.";
 
   private ProtoConstants() {}
 }

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java

@@ -466,8 +466,8 @@ Object provider(Object doc, Object fields, Object init, StarlarkThread thread)
             doc =
                 "Experimental: the Stalark function initializing the attributes of the rule. "
                     + "<p>The function is called at load time for each instance of the rule. It's "
-                    + "called with values of public attributes defined by the rule (not with "
-                    + "generic attributes, for example <code>name</code> or <code>tags</code>). "
+                    + "called with <code>name</code> and the values of public attributes defined by"
+                    + "the rule (not with generic attributes, for example <code>tags</code>). "
                     + "<p>It has to return a dictionary from the attribute names to the desired "
                     + "values. The attributes that are not returned are unaffected. Returning "
                     + "<code>None</code> as value results in using the default value specified in "

src/main/starlark/builtins_bzl/bazel/exports.bzl

@@ -19,6 +19,7 @@ load("@_builtins//:common/java/java_import.bzl", "java_import")
 load("@_builtins//:common/java/java_library.bzl", "JAVA_LIBRARY_ATTRS", "bazel_java_library_rule", "java_library", "make_sharded_java_library")
 load("@_builtins//:common/java/java_plugin.bzl", "java_plugin")
 load("@_builtins//:common/java/proto/java_proto_library.bzl", "java_proto_library")
+load("@_builtins//:common/proto/proto_library.bzl", "proto_library")
 load("@_builtins//:common/python/py_binary_macro.bzl", "py_binary")
 load("@_builtins//:common/python/py_internal.bzl", "py_internal")
 load("@_builtins//:common/python/py_library_macro.bzl", "py_library")
@@ -38,6 +39,7 @@ exported_toplevels = {
     "py_internal": py_internal,
 }
 exported_rules = {
+    "proto_library": proto_library,
     "java_library": java_library,
     "java_plugin": java_plugin,
     "java_import": java_import,

src/main/starlark/builtins_bzl/common/exports.bzl

@@ -35,7 +35,6 @@ load("@_builtins//:common/objc/objc_library.bzl", "objc_library")
 load("@_builtins//:common/proto/proto_common.bzl", "proto_common_do_not_use")
 load("@_builtins//:common/proto/proto_info.bzl", "ProtoInfo")
 load("@_builtins//:common/proto/proto_lang_toolchain.bzl", "proto_lang_toolchain")
-load("@_builtins//:common/proto/proto_library.bzl", "proto_library")
 load("@_builtins//:common/python/providers.bzl", "PyCcLinkParamsProvider", "PyInfo", "PyRuntimeInfo")
 load("@_builtins//:common/python/py_runtime_macro.bzl", "py_runtime")
 load(":common/java/java_common.bzl", "java_common")
@@ -75,7 +74,6 @@ exported_rules = {
     "objc_import": objc_import,
     "objc_library": objc_library,
     "j2objc_library": j2objc_library,
-    "proto_library": proto_library,
     "cc_shared_library": cc_shared_library,
     "cc_binary": cc_binary,
     "cc_test": cc_test,

src/main/starlark/builtins_bzl/common/proto/proto_library.bzl

@@ -36,7 +36,7 @@ def _check_srcs_package(target_package, srcs):
 def _get_import_prefix(ctx):
     """Gets and verifies import_prefix attribute if it is declared."""
 
-    import_prefix = ctx.attr.import_prefix if hasattr(ctx.attr, "import_prefix") else ""
+    import_prefix = ctx.attr.import_prefix
 
     if not paths.is_normalized(import_prefix):
         fail("should be normalized (without uplevel references or '.' path segments)", attr = "import_prefix")
@@ -61,8 +61,6 @@ def _get_strip_import_prefix(ctx):
     return strip_import_prefix.removesuffix("/")
 
 def _proto_library_impl(ctx):
-    semantics.preprocess(ctx)
-
     # Verifies attributes.
     _check_srcs_package(ctx.label.package, ctx.attr.srcs)
     srcs = ctx.files.srcs
@@ -247,6 +245,7 @@ proto_library = rule(
             providers = [ProtoInfo],
         ),
         "strip_import_prefix": attr.string(default = "/"),
+        "import_prefix": attr.string(),
         "allow_exports": attr.label(
             cfg = "exec",
             providers = [PackageSpecificationInfo],
@@ -263,9 +262,8 @@ proto_library = rule(
             allow_files = True,
             default = configuration_field("proto", "proto_compiler"),
         ),
-    }) | semantics.EXTRA_ATTRIBUTES,
-    fragments = ["proto"] + semantics.EXTRA_FRAGMENTS,
+    }),
+    fragments = ["proto"],
     provides = [ProtoInfo],
-    exec_groups = semantics.EXEC_GROUPS,
     toolchains = toolchains.use_toolchain(semantics.PROTO_TOOLCHAIN),
 )

src/main/starlark/builtins_bzl/common/proto/proto_semantics.bzl

@@ -16,17 +16,8 @@
 Proto Semantics
 """
 
-def _preprocess(ctx):
-    pass
-
 semantics = struct(
     PROTO_TOOLCHAIN = "@rules_proto//proto:toolchain_type",
-    PROTO_COMPILER_LABEL = "@bazel_tools//tools/proto:protoc",
-    EXTRA_ATTRIBUTES = {
-        "import_prefix": attr.string(),
-    },
-    EXTRA_FRAGMENTS = [],
-    preprocess = _preprocess,
     # This constant is used in ProtoCompileActionBuilder to generate an error message that's
     # displayed when a strict proto deps violation occurs.
     #
@@ -37,6 +28,5 @@ semantics = struct(
         "--direct_dependencies_violation_msg=" +
         "%%s is imported, but %s doesn't directly depend on a proto_library that 'srcs' it."
     ),
-    EXEC_GROUPS = {},
     allowlist_different_package = None,
 )

src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java

@@ -225,7 +225,7 @@ public void testDescriptorSetOutput_strictDeps() throws Exception {
         .containsAtLeast("--direct_dependencies", "x/nodeps.proto")
         .inOrder();
     assertThat(getGeneratingSpawnAction(getDescriptorOutput("//x:nodeps")).getRemainingArguments())
-        .contains(String.format(ProtoCompileActionBuilder.STRICT_DEPS_FLAG_TEMPLATE, "//x:nodeps"));
+        .contains(String.format(ProtoConstants.STRICT_PROTO_DEPS_VIOLATION_MESSAGE, "//x:nodeps"));
 
     assertThat(
             getGeneratingSpawnAction(getDescriptorOutput("//x:withdeps")).getRemainingArguments())
@@ -275,7 +275,7 @@ public void testDescriptorSetOutput_strictDeps_disabled() throws Exception {
       assertThat(arg).doesNotContain("--direct_dependencies=");
       assertThat(arg)
           .doesNotContain(
-              String.format(ProtoCompileActionBuilder.STRICT_DEPS_FLAG_TEMPLATE, "//x:foo_proto"));
+              String.format(ProtoConstants.STRICT_PROTO_DEPS_VIOLATION_MESSAGE, "//x:foo_proto"));
     }
   }