From c51991600c73b9b488b08c8b68d156bce008b064 Mon Sep 17 00:00:00 2001
From: Googler <ilist@google.com>
Date: Tue, 7 Nov 2023 05:35:34 -0800
Subject: [PATCH] Make Google proto_library and extension of Bazel
 proto_library

PiperOrigin-RevId: 580146223
Change-Id: I13ac76086d56cd26ae0c661d1c36c45a2ee270d1
---
 .../starlark/StarlarkRuleClassFunctions.java  |  4 ++-
 .../starlark/StarlarkRuleContext.java         |  4 +--
 .../proto/ProtoCompileActionBuilder.java      | 25 -------------------
 .../build/lib/rules/proto/ProtoConstants.java |  2 +-
 .../starlark/builtins_bzl/bazel/exports.bzl   |  2 ++
 .../starlark/builtins_bzl/common/exports.bzl  |  2 --
 .../common/proto/proto_library.bzl            | 10 +++-----
 .../common/proto/proto_semantics.bzl          | 10 --------
 .../rules/proto/BazelProtoLibraryTest.java    |  4 +--
 9 files changed, 14 insertions(+), 49 deletions(-)
 delete mode 100644 src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
index 6412ae7deff673..60e4acead8d247 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
@@ -519,7 +519,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)
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java
index ff13237567a8ed..61959585c03dff 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java
@@ -455,7 +455,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.");
     }
 
@@ -585,7 +585,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) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
deleted file mode 100644
index 0b029d35e9c259..00000000000000
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
+++ /dev/null
@@ -1,25 +0,0 @@
-// Copyright 2016 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.rules.proto;
-
-import com.google.common.annotations.VisibleForTesting;
-
-/** Constructs actions to run the protocol compiler to generate sources from .proto files. */
-public class ProtoCompileActionBuilder {
-
-  @VisibleForTesting
-  public static final String STRICT_DEPS_FLAG_TEMPLATE =
-      "--direct_dependencies_violation_msg=" + ProtoConstants.STRICT_PROTO_DEPS_VIOLATION_MESSAGE;
-}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConstants.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConstants.java
index 089c9fe3bc9031..65329225c3e7a8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConstants.java
+++ b/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() {}
 }
diff --git a/src/main/starlark/builtins_bzl/bazel/exports.bzl b/src/main/starlark/builtins_bzl/bazel/exports.bzl
index 2c11416a181ea0..63d6b5b6e9db38 100644
--- a/src/main/starlark/builtins_bzl/bazel/exports.bzl
+++ b/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,
diff --git a/src/main/starlark/builtins_bzl/common/exports.bzl b/src/main/starlark/builtins_bzl/common/exports.bzl
index 4a3ac9d08ae233..5ba021419e02fc 100755
--- a/src/main/starlark/builtins_bzl/common/exports.bzl
+++ b/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_binary_deploy_jar.bzl", get_java_build_info = "get_build_info")
@@ -76,7 +75,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,
diff --git a/src/main/starlark/builtins_bzl/common/proto/proto_library.bzl b/src/main/starlark/builtins_bzl/common/proto/proto_library.bzl
index bb6ac60287947e..022ecdfa372b05 100644
--- a/src/main/starlark/builtins_bzl/common/proto/proto_library.bzl
+++ b/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),
 )
diff --git a/src/main/starlark/builtins_bzl/common/proto/proto_semantics.bzl b/src/main/starlark/builtins_bzl/common/proto/proto_semantics.bzl
index d34f5412f99354..80590a3583c7b1 100644
--- a/src/main/starlark/builtins_bzl/common/proto/proto_semantics.bzl
+++ b/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,
 )
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
index d9ca872aa9f74d..8f355152cc415f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
+++ b/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"));
     }
   }