diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 4f9d0885a27d63..f75c7bf3b03af6 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -696,6 +696,18 @@ public final class BuildLanguageOptions extends OptionsBase { + " Label.relative) can be used.") public boolean enableDeprecatedLabelApis; + // 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 @@ -795,6 +807,9 @@ public StarlarkSemantics toStarlarkSemantics() { INCOMPATIBLE_DISABLE_OBJC_LIBRARY_TRANSITION, incompatibleDisableObjcLibraryTransition) .setBool(INCOMPATIBLE_ENABLE_DEPRECATED_LABEL_APIS, enableDeprecatedLabelApis) + .setBool( + INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION, + incompatibleEnableProtoToolchainResolution) .build(); return INTERNER.intern(semantics); } @@ -891,6 +906,8 @@ public StarlarkSemantics toStarlarkSemantics() { "-incompatible_disable_objc_library_transition"; public static final String INCOMPATIBLE_ENABLE_DEPRECATED_LABEL_APIS = "+incompatible_enable_deprecated_label_apis"; + public static final String INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION = + "-incompatible_enable_proto_toolchain_resolution"; // non-booleans public static final StarlarkSemantics.Key EXPERIMENTAL_BUILTINS_BZL_PATH = diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD b/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD index af13958a45ed23..d79f08378391b4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD @@ -40,6 +40,7 @@ java_library( "//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", diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoCommon.java b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoCommon.java index e41380310763ba..77ac10a2292d3b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoCommon.java @@ -22,6 +22,12 @@ import net.starlark.java.annot.StarlarkMethod; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.StarlarkList; +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. */ @@ -87,4 +93,15 @@ public ProtoInfo protoInfo( Depset.cast(transitiveDescriptorSets, Artifact.class, "transitive_descriptor_set"), Depset.cast(exportedSources, ProtoSource.class, "exported_sources")); } + + @StarlarkMethod( + name = "incompatible_enable_proto_toolchain_resolution", + useStarlarkThread = true, + documented = false) + public boolean getDefineProtoToolchains(StarlarkThread thread) throws EvalException { + ProtoCommon.checkPrivateStarlarkificationAllowlist(thread); + return thread + .getSemantics() + .getBool(BuildLanguageOptions.INCOMPATIBLE_ENABLE_PROTO_TOOLCHAIN_RESOLUTION); + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java index beba72dd0c5f55..9582ed93be901e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java @@ -56,17 +56,6 @@ public static class Options extends FragmentOptions { + "information, see https://github.com/bazelbuild/bazel/issues/9215") public boolean generatedProtosInVirtualImports; - @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, @@ -196,7 +185,6 @@ public static class Options extends FragmentOptions { @Override public FragmentOptions getHost() { Options host = (Options) super.getHost(); - host.enableProtoToolchainResolution = enableProtoToolchainResolution; host.protoCompiler = protoCompiler; host.protocOpts = protocOpts; host.experimentalProtoDescriptorSetsIncludeSourceInfo = @@ -218,12 +206,10 @@ public FragmentOptions getHost() { private final ImmutableList protocOpts; private final ImmutableList ccProtoLibraryHeaderSuffixes; private final ImmutableList 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); @@ -268,11 +254,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( @@ -281,20 +263,12 @@ public Label protoCompiler() { defaultLabel = ProtoConstants.DEFAULT_JAVA_PROTO_LABEL) @Nullable public Label protoToolchainForJava() { - if (enableProtoToolchainResolution) { - return null; - } else { - return options.protoToolchainForJava; - } + return options.protoToolchainForJava; } @Nullable public Label protoToolchainForJ2objc() { - if (enableProtoToolchainResolution) { - return null; - } else { - return options.protoToolchainForJ2objc; - } + return options.protoToolchainForJ2objc; } @StarlarkConfigurationField( @@ -303,11 +277,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( @@ -316,11 +286,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) 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 5ce86c3bfc4ba5..b6f392f2caeb88 100644 --- a/src/main/starlark/builtins_bzl/common/proto/proto_library.bzl +++ b/src/main/starlark/builtins_bzl/common/proto/proto_library.bzl @@ -251,15 +251,22 @@ def _write_descriptor_set(ctx, direct_sources, deps, exports, proto_info, descri args.add("--allowed_public_imports=") else: args.add_joined("--allowed_public_imports", public_import_protos, map_each = _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, @@ -271,7 +278,7 @@ def _write_descriptor_set(ctx, direct_sources, deps, exports, proto_info, descri proto_library = rule( _proto_library_impl, - attrs = dict({ + attrs = { "srcs": attr.label_list( allow_files = [".proto", ".protodevel"], flags = ["DIRECT_COMPILE_TIME_INPUT"], @@ -288,14 +295,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], output_to_genfiles = True, # TODO(b/204266604) move to bin dir + toolchains = 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 8634dd6dcb7963..47d7fbf75089f5 100644 --- a/src/main/starlark/builtins_bzl/common/proto/proto_semantics.bzl +++ b/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(), diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockProtoSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockProtoSupport.java index 28262363162cbe..187c3596b889a7 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockProtoSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockProtoSupport.java @@ -40,13 +40,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", @@ -209,8 +215,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):", @@ -222,5 +232,57 @@ 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,", + " ),", + " ),", + " ]", + "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'],", + ")"); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/proto/StarlarkJavaLiteProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/proto/StarlarkJavaLiteProtoLibraryTest.java index 09f9ba4492c12d..1767b8cf7cb158 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/java/proto/StarlarkJavaLiteProtoLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/java/proto/StarlarkJavaLiteProtoLibraryTest.java @@ -65,6 +65,7 @@ public final void setUpMocks() throws Exception { scratch.file("proto/BUILD", "licenses(['notice'])", "exports_files(['compiler'])"); mockToolchains(); + invalidatePackages(); actionsTestUtil = actionsTestUtil(); } 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 bace5876b98158..42e5e5662bfc39 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 @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.packages.util.MockProtoSupport; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.vfs.FileSystemUtils; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.List; import org.junit.Before; import org.junit.Ignore; @@ -54,6 +55,19 @@ public void setUp() throws Exception { invalidatePackages(); } + @Test + public void protoToolchainResolution_enabled() throws Exception { + setBuildLanguageOptions("--incompatible_enable_proto_toolchain_resolution"); + scratch.file( + "x/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "proto_library(name='foo', srcs=['foo.proto'])"); + + getDescriptorOutput("//x:foo"); + + assertNoEvents(); + } + @Test public void createsDescriptorSets() throws Exception { scratch.file( @@ -1011,6 +1025,7 @@ public void testStripImportPrefixForExternalRepositories() throws Exception { .contains("-Iy/z/q.proto=" + genfiles + "/external/foo/x/y/_virtual_imports/q/y/z/q.proto"); } + @CanIgnoreReturnValue private Artifact getDescriptorOutput(String label) throws Exception { return getFirstArtifactEndingWith(getFilesToBuild(getConfiguredTarget(label)), ".proto.bin"); } diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java index c2061065eede90..60443846d6b585 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java @@ -26,6 +26,9 @@ public class TestConstants { public static final String LOAD_PROTO_LIBRARY = "load('@rules_proto//proto:defs.bzl', 'proto_library')"; + public static final String PROTO_TOOLCHAIN = "@rules_proto//proto:toolchain_type"; + public static final String LOAD_PROTO_TOOLCHAIN = + "load('@rules_proto//proto:proto_toolchain.bzl', 'proto_toolchain')"; public static final String LOAD_PROTO_LANG_TOOLCHAIN = "load('@rules_proto//proto:defs.bzl', 'proto_lang_toolchain')";