Use proto_toolchain in proto_library

Change --incompatible_enable_proto_toolchain_resolution flag to a loading time flag. This is partial revert of 11cf1b7. We need a load time flag, because toolchain type might not exist at current versions of rules repos users use.

When the flag is set, proto_library defines the toolchain and doesn't use an implicit dependency at the same time.

In Bazel the flag may only be flipped after all the rules_* repo are upgraded to define toolchain type and protobuf repository registers the toolchains.

Issue: bazelbuild/rules_proto#179
PiperOrigin-RevId: 567296665
Change-Id: Ib3fc27751dd14589fa403f45d2cbbad22537b9a3

src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java

@@ -712,6 +712,18 @@ public final class BuildLanguageOptions extends OptionsBase {
       help = "If enabled, targets that have unknown attributes set to None fail.")
   public boolean incompatibleFailOnUnknownAttributes;
 
+  // Flip when dependencies to rules_* repos are upgraded and protobuf registers toolchains
+  @Option(
+      name = "incompatible_enable_proto_toolchain_resolution",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+      help =
+          "If true, proto lang rules define toolchains from rules_proto, rules_java, rules_cc"
+              + " repositories.")
+  public boolean incompatibleEnableProtoToolchainResolution;
+
   /**
    * An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should
    * never accumulate a large number of these and being able to shortcut on object identity makes a
@@ -813,6 +825,9 @@ public StarlarkSemantics toStarlarkSemantics() {
                 INCOMPATIBLE_DISABLE_OBJC_LIBRARY_TRANSITION,
                 incompatibleDisableObjcLibraryTransition)
             .setBool(INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES, incompatibleFailOnUnknownAttributes)
+            .setBool(
+                INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION,
+                incompatibleEnableProtoToolchainResolution)
             .build();
     return INTERNER.intern(semantics);
   }
@@ -906,6 +921,8 @@ public StarlarkSemantics toStarlarkSemantics() {
       "-incompatible_disable_objc_library_transition";
   public static final String INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES =
       "+incompatible_fail_on_unknown_attributes";
+  public static final String INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION =
+      "-incompatible_enable_proto_toolchain_resolution";
 
   // non-booleans
   public static final StarlarkSemantics.Key<String> EXPERIMENTAL_BUILTINS_BZL_PATH =

src/main/java/com/google/devtools/build/lib/rules/proto/BUILD

@@ -32,18 +32,17 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
-        "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
         "//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment",
         "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection",
         "//src/main/java/com/google/devtools/build/lib/analysis/starlark/annotations",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/packages/semantics",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/proto",
         "//src/main/java/com/google/devtools/build/lib/util:filetype",
-        "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/com/google/devtools/common/options",
         "//src/main/java/net/starlark/java/annot",
         "//src/main/java/net/starlark/java/eval",

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

@@ -14,11 +14,28 @@
 
 package com.google.devtools.build.lib.rules.proto;
 
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.packages.BuiltinRestriction;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.starlarkbuildapi.proto.ProtoCommonApi;
+import net.starlark.java.annot.StarlarkMethod;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.StarlarkThread;
 
 /** Protocol buffers support for Starlark. */
 public class BazelProtoCommon implements ProtoCommonApi {
   public static final BazelProtoCommon INSTANCE = new BazelProtoCommon();
 
   protected BazelProtoCommon() {}
+
+  @StarlarkMethod(
+      name = "incompatible_enable_proto_toolchain_resolution",
+      useStarlarkThread = true,
+      documented = false)
+  public boolean getDefineProtoToolchains(StarlarkThread thread) throws EvalException {
+    BuiltinRestriction.failIfCalledOutsideAllowlist(thread, ImmutableSet.of());
+    return thread
+        .getSemantics()
+        .getBool(BuildLanguageOptions.INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION);
+  }
 }

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

@@ -46,17 +46,6 @@ public class ProtoConfiguration extends Fragment implements ProtoConfigurationAp
 
   /** Command line options. */
   public static class Options extends FragmentOptions {
-    @Option(
-        name = "incompatible_enable_proto_toolchain_resolution",
-        defaultValue = "false",
-        documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
-        effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
-        metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
-        help =
-            "If true, proto lang rules use toolchain resolution to find the toolchain. The flags"
-                + " proto_compiler and proto_toolchain_for_* are a no-op.")
-    public boolean enableProtoToolchainResolution;
-
     @Option(
         name = "protocopt",
         allowMultiple = true,
@@ -175,7 +164,6 @@ public static class Options extends FragmentOptions {
     @Override
     public FragmentOptions getExec() {
       Options exec = (Options) super.getExec();
-      exec.enableProtoToolchainResolution = enableProtoToolchainResolution;
       exec.protoCompiler = protoCompiler;
       exec.protocOpts = protocOpts;
       exec.experimentalProtoDescriptorSetsIncludeSourceInfo =
@@ -196,12 +184,10 @@ public FragmentOptions getExec() {
   private final ImmutableList<String> protocOpts;
   private final ImmutableList<String> ccProtoLibraryHeaderSuffixes;
   private final ImmutableList<String> ccProtoLibrarySourceSuffixes;
-  private final boolean enableProtoToolchainResolution;
   private final Options options;
 
   public ProtoConfiguration(BuildOptions buildOptions) {
     Options options = buildOptions.get(Options.class);
-    this.enableProtoToolchainResolution = options.enableProtoToolchainResolution;
     this.protocOpts = ImmutableList.copyOf(options.protocOpts);
     this.ccProtoLibraryHeaderSuffixes = ImmutableList.copyOf(options.ccProtoLibraryHeaderSuffixes);
     this.ccProtoLibrarySourceSuffixes = ImmutableList.copyOf(options.ccProtoLibrarySourceSuffixes);
@@ -246,11 +232,7 @@ public boolean runExperimentalProtoExtraActions() {
       defaultLabel = ProtoConstants.DEFAULT_PROTOC_LABEL)
   @Nullable
   public Label protoCompiler() {
-    if (enableProtoToolchainResolution) {
-      return null;
-    } else {
-      return options.protoCompiler;
-    }
+    return options.protoCompiler;
   }
 
   @StarlarkConfigurationField(
@@ -259,11 +241,7 @@ public Label protoCompiler() {
       defaultLabel = ProtoConstants.DEFAULT_JAVA_PROTO_LABEL)
   @Nullable
   public Label protoToolchainForJava() {
-    if (enableProtoToolchainResolution) {
-      return null;
-    } else {
-      return options.protoToolchainForJava;
-    }
+    return options.protoToolchainForJava;
   }
 
   @StarlarkConfigurationField(
@@ -272,11 +250,7 @@ public Label protoToolchainForJava() {
       defaultLabel = ProtoConstants.DEFAULT_J2OBJC_PROTO_LABEL)
   @Nullable
   public Label protoToolchainForJ2objc() {
-    if (enableProtoToolchainResolution) {
-      return null;
-    } else {
-      return options.protoToolchainForJ2objc;
-    }
+    return options.protoToolchainForJ2objc;
   }
 
   @StarlarkConfigurationField(
@@ -285,11 +259,7 @@ public Label protoToolchainForJ2objc() {
       defaultLabel = ProtoConstants.DEFAULT_JAVA_LITE_PROTO_LABEL)
   @Nullable
   public Label protoToolchainForJavaLite() {
-    if (enableProtoToolchainResolution) {
-      return null;
-    } else {
-      return options.protoToolchainForJavaLite;
-    }
+    return options.protoToolchainForJavaLite;
   }
 
   @StarlarkConfigurationField(
@@ -298,11 +268,7 @@ public Label protoToolchainForJavaLite() {
       defaultLabel = ProtoConstants.DEFAULT_CC_PROTO_LABEL)
   @Nullable
   public Label protoToolchainForCc() {
-    if (enableProtoToolchainResolution) {
-      return null;
-    } else {
-      return options.protoToolchainForCc;
-    }
+    return options.protoToolchainForCc;
   }
 
   @StarlarkMethod(name = "strict_proto_deps", useStarlarkThread = true, documented = false)

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

@@ -208,15 +208,22 @@ def _write_descriptor_set(ctx, proto_info, deps, exports, descriptor_set):
                 map_each = proto_common.get_import_path,
                 join_with = ":",
             )
-    proto_lang_toolchain_info = proto_common.ProtoLangToolchainInfo(
-        out_replacement_format_flag = "--descriptor_set_out=%s",
-        output_files = "single",
-        mnemonic = "GenProtoDescriptorSet",
-        progress_message = "Generating Descriptor Set proto_library %{label}",
-        proto_compiler = ctx.executable._proto_compiler,
-        protoc_opts = ctx.fragments.proto.experimental_protoc_opts,
-        plugin = None,
-    )
+    if semantics.INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION:
+        toolchain = ctx.toolchains[semantics.PROTO_TOOLCHAIN_TYPE]
+        if not toolchain:
+            fail("Protocol compiler toolchain could not be resolved.")
+        proto_lang_toolchain_info = toolchain.proto
+    else:
+        proto_lang_toolchain_info = proto_common.ProtoLangToolchainInfo(
+            out_replacement_format_flag = "--descriptor_set_out=%s",
+            output_files = "single",
+            mnemonic = "GenProtoDescriptorSet",
+            progress_message = "Generating Descriptor Set proto_library %{label}",
+            proto_compiler = ctx.executable._proto_compiler,
+            protoc_opts = ctx.fragments.proto.experimental_protoc_opts,
+            plugin = None,
+        )
+
     proto_common.compile(
         ctx.actions,
         proto_info,
@@ -228,7 +235,7 @@ def _write_descriptor_set(ctx, proto_info, deps, exports, descriptor_set):
 
 proto_library = rule(
     _proto_library_impl,
-    attrs = dict({
+    attrs = {
         "srcs": attr.label_list(
             allow_files = [".proto", ".protodevel"],
             flags = ["DIRECT_COMPILE_TIME_INPUT"],
@@ -249,14 +256,16 @@ proto_library = rule(
             flags = ["SKIP_CONSTRAINTS_OVERRIDE"],
         ),
         "licenses": attr.license() if hasattr(attr, "license") else attr.string_list(),
+    } | ({} if semantics.INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION else {
         "_proto_compiler": attr.label(
             cfg = "exec",
             executable = True,
             allow_files = True,
             default = configuration_field("proto", "proto_compiler"),
         ),
-    }, **semantics.EXTRA_ATTRIBUTES),
+    }) | semantics.EXTRA_ATTRIBUTES,
     fragments = ["proto"] + semantics.EXTRA_FRAGMENTS,
     provides = [ProtoInfo],
     exec_groups = semantics.EXEC_GROUPS,
+    toolchains = semantics.PROTO_TOOLCHAIN,
 )

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

@@ -19,7 +19,18 @@ Proto Semantics
 def _preprocess(ctx):
     pass
 
+_PROTO_TOOLCHAIN_TYPE = "@rules_proto//proto:toolchain_type"
+
+def _get_proto_toolchain():
+    if _builtins.toplevel.proto_common.incompatible_enable_proto_toolchain_resolution():
+        return [_builtins.toplevel.config_common.toolchain_type(_PROTO_TOOLCHAIN_TYPE, mandatory = False)]
+    else:
+        return []
+
 semantics = struct(
+    PROTO_TOOLCHAIN_TYPE = _PROTO_TOOLCHAIN_TYPE,
+    PROTO_TOOLCHAIN = _get_proto_toolchain(),
+    INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION = _builtins.toplevel.proto_common.incompatible_enable_proto_toolchain_resolution(),
     PROTO_COMPILER_LABEL = "@bazel_tools//tools/proto:protoc",
     EXTRA_ATTRIBUTES = {
         "import_prefix": attr.string(),

src/test/java/com/google/devtools/build/lib/packages/util/MockProtoSupport.java

@@ -34,13 +34,19 @@ private MockProtoSupport() {
   public static void setup(MockToolsConfig config) throws IOException {
     createNetProto2(config);
     setupWorkspace(config);
-    config.append("tools/proto/toolchains/BUILD");
+    registerProtoToolchain(config);
   }
 
-  /**
-   * Create a dummy "net/proto2 compiler and proto APIs for all languages
-   * and versions.
-   */
+  private static void registerProtoToolchain(MockToolsConfig config) throws IOException {
+    config.append("WORKSPACE", "register_toolchains('tools/proto/toolchains:all')");
+    config.append(
+        "tools/proto/toolchains/BUILD",
+        TestConstants.LOAD_PROTO_TOOLCHAIN,
+        "proto_toolchain(name = 'protoc_sources',"
+            + "proto_compiler = '//net/proto2/compiler/public:protocol_compiler')");
+  }
+
+  /** Create a dummy "net/proto2 compiler and proto APIs for all languages and versions. */
   private static void createNetProto2(MockToolsConfig config) throws IOException {
     config.create(
         "net/proto2/compiler/public/BUILD",
@@ -203,8 +209,12 @@ public static void setupWorkspace(MockToolsConfig config) throws IOException {
           "    path = 'third_party/bazel_rules/rules_proto',",
           ")");
     }
+
     config.create("third_party/bazel_rules/rules_proto/WORKSPACE");
-    config.create("third_party/bazel_rules/rules_proto/proto/BUILD", "licenses(['notice'])");
+    config.create(
+        "third_party/bazel_rules/rules_proto/proto/BUILD",
+        "licenses(['notice'])",
+        "toolchain_type(name = 'toolchain_type', visibility = ['//visibility:public'])");
     config.create(
         "third_party/bazel_rules/rules_proto/proto/defs.bzl",
         "def _add_tags(kargs):",
@@ -216,5 +226,58 @@ public static void setupWorkspace(MockToolsConfig config) throws IOException {
         "",
         "def proto_library(**kargs): native.proto_library(**_add_tags(kargs))",
         "def proto_lang_toolchain(**kargs): native.proto_lang_toolchain(**_add_tags(kargs))");
+    config.create(
+        "third_party/bazel_rules/rules_proto/proto/proto_toolchain.bzl",
+        "load(':proto_toolchain_rule.bzl', _proto_toolchain_rule = 'proto_toolchain')",
+        "def proto_toolchain(*, name, proto_compiler, exec_compatible_with = []):",
+        "  _proto_toolchain_rule(name = name, proto_compiler = proto_compiler)",
+        "  native.toolchain(",
+        "    name = name + '_toolchain',",
+        "    toolchain_type = '" + TestConstants.PROTO_TOOLCHAIN + "',",
+        "    exec_compatible_with = exec_compatible_with,",
+        "    target_compatible_with = [],",
+        "    toolchain = name,",
+        "  )");
+    config.create(
+        "third_party/bazel_rules/rules_proto/proto/proto_toolchain_rule.bzl",
+        "ProtoLangToolchainInfo = proto_common_do_not_use.ProtoLangToolchainInfo",
+        "def _impl(ctx):",
+        "  return [",
+        "    DefaultInfo(",
+        "      files = depset(),",
+        "      runfiles = ctx.runfiles(),",
+        "    ),",
+        "    platform_common.ToolchainInfo(",
+        "      proto = ProtoLangToolchainInfo(",
+        "        out_replacement_format_flag = ctx.attr.command_line,",
+        "        output_files = ctx.attr.output_files,",
+        "        plugin = None,",
+        "        runtime = None,",
+        "        proto_compiler = ctx.attr.proto_compiler.files_to_run,",
+        "        protoc_opts = ctx.fragments.proto.experimental_protoc_opts,",
+        "        progress_message = ctx.attr.progress_message,",
+        "        mnemonic = ctx.attr.mnemonic,",
+        "        allowlist_different_package = None,",
+        "      ),",
+        "    ),",
+        "  ]",
+        "proto_toolchain = rule(",
+        "  _impl,",
+        "  attrs = {",
+        "     'progress_message': attr.string(default = ",
+        "          'Generating Descriptor Set proto_library %{label}'),",
+        "    'mnemonic': attr.string(default = 'GenProtoDescriptorSet'),",
+        "    'command_line': attr.string(default = '--descriptor_set_out=%s'),",
+        "    'output_files': attr.string(values = ['single', 'multiple', 'legacy'],",
+        "       default = 'single'),",
+        "    'proto_compiler': attr.label(",
+        "       cfg = 'exec',",
+        "       executable = True,",
+        "       allow_files = True,",
+        "     ),",
+        "  },",
+        "  provides = [platform_common.ToolchainInfo],",
+        "  fragments = ['proto'],",
+        ")");
   }
 }

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

@@ -64,6 +64,7 @@ public final void setUpMocks() throws Exception {
     scratch.file("proto/BUILD", "licenses(['notice'])", "exports_files(['compiler'])");
 
     mockToolchains();
+    invalidatePackages();
 
     actionsTestUtil = actionsTestUtil();
   }