From f30e59a38bf51e9a15ecd5c23148287eedcd6c17 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 3 Oct 2023 03:32:01 -0700 Subject: [PATCH] Use proto toolchains in cc_proto_library Retrieve proto_lang_toolchain using toolchains in cc_proto_library when proto toolchain resolution is enabled. Issue: https://github.com/bazelbuild/rules_proto/issues/179 PiperOrigin-RevId: 570334276 Change-Id: If7b7e25254bf3036fdcad67d36474b8fe64a8ffd --- .../common/cc/cc_proto_library.bzl | 30 ++++++++++--------- .../builtins_bzl/common/cc/semantics.bzl | 1 + .../rules/cpp/proto/CcProtoLibraryTest.java | 28 +++++++++++++++++ 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_proto_library.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_proto_library.bzl index fa7f896264c386..956b9750db9c01 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_proto_library.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_proto_library.bzl @@ -15,7 +15,8 @@ """Starlark implementation of cc_proto_library""" load(":common/cc/cc_helper.bzl", "cc_helper") -load(":common/proto/proto_common.bzl", "ProtoLangToolchainInfo", proto_common = "proto_common_do_not_use") +load(":common/proto/proto_common.bzl", "toolchains", "ProtoLangToolchainInfo", proto_common = "proto_common_do_not_use") +load(":common/cc/semantics.bzl", "semantics") ProtoInfo = _builtins.toplevel.ProtoInfo CcInfo = _builtins.toplevel.CcInfo @@ -24,15 +25,15 @@ cc_common = _builtins.toplevel.cc_common ProtoCcFilesInfo = provider(fields = ["files"], doc = "Provide cc proto files.") ProtoCcHeaderInfo = provider(fields = ["headers"], doc = "Provide cc proto headers.") -def _are_srcs_excluded(ctx, target): - return not proto_common.experimental_should_generate_code(target[ProtoInfo], ctx.attr._aspect_cc_proto_toolchain[ProtoLangToolchainInfo], "cc_proto_library", target.label) +def _are_srcs_excluded(proto_toolchain, target): + return not proto_common.experimental_should_generate_code(target[ProtoInfo], proto_toolchain, "cc_proto_library", target.label) -def _get_feature_configuration(ctx, target, cc_toolchain, proto_info): +def _get_feature_configuration(ctx, target, cc_toolchain, proto_info, proto_toolchain): requested_features = list(ctx.features) unsupported_features = list(ctx.disabled_features) unsupported_features.append("parse_headers") unsupported_features.append("layering_check") - if not _are_srcs_excluded(ctx, target) and len(proto_info.direct_sources) != 0: + if not _are_srcs_excluded(proto_toolchain, target) and len(proto_info.direct_sources) != 0: requested_features.append("header_modules") else: unsupported_features.append("header_modules") @@ -78,12 +79,13 @@ def _get_strip_include_prefix(ctx, proto_info): def _aspect_impl(target, ctx): cc_toolchain = cc_helper.find_cpp_toolchain(ctx) + proto_toolchain = toolchains.find_toolchain(ctx, "_aspect_cc_proto_toolchain", semantics.CC_PROTO_TOOLCHAIN) proto_info = target[ProtoInfo] - feature_configuration = _get_feature_configuration(ctx, target, cc_toolchain, proto_info) + feature_configuration = _get_feature_configuration(ctx, target, cc_toolchain, proto_info, proto_toolchain) proto_configuration = ctx.fragments.proto deps = [] - runtime = ctx.attr._aspect_cc_proto_toolchain[ProtoLangToolchainInfo].runtime + runtime = proto_toolchain.runtime if runtime != None: deps.append(runtime) deps.extend(getattr(ctx.rule.attr, "deps", [])) @@ -100,7 +102,7 @@ def _aspect_impl(target, ctx): textual_hdrs = [] additional_exported_hdrs = [] - if _are_srcs_excluded(ctx, target): + if _are_srcs_excluded(proto_toolchain, target): header_provider = None # Hack: This is a proto_library for descriptor.proto or similar. @@ -155,7 +157,7 @@ def _aspect_impl(target, ctx): proto_common.compile( actions = ctx.actions, proto_info = proto_info, - proto_lang_toolchain_info = ctx.attr._aspect_cc_proto_toolchain[ProtoLangToolchainInfo], + proto_lang_toolchain_info = proto_toolchain, generated_files = outputs, experimental_output_files = "multiple", ) @@ -250,12 +252,11 @@ cc_proto_aspect = aspect( required_providers = [ProtoInfo], provides = [CcInfo], attrs = { - "_aspect_cc_proto_toolchain": attr.label( - default = configuration_field(fragment = "proto", name = "proto_toolchain_for_cc"), - ), "_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"), - }, - toolchains = cc_helper.use_cpp_toolchain(), + } | toolchains.if_legacy_toolchain({"_aspect_cc_proto_toolchain": attr.label( + default = configuration_field(fragment = "proto", name = "proto_toolchain_for_cc"), + )}), + toolchains = cc_helper.use_cpp_toolchain() + toolchains.use_toolchain(semantics.CC_PROTO_TOOLCHAIN), ) def _impl(ctx): @@ -283,4 +284,5 @@ cc_proto_library = rule( ), }, provides = [CcInfo], + toolchains = toolchains.use_toolchain(semantics.CC_PROTO_TOOLCHAIN), ) diff --git a/src/main/starlark/builtins_bzl/common/cc/semantics.bzl b/src/main/starlark/builtins_bzl/common/cc/semantics.bzl index 14cb4f4c422404..9f8e8334e98f88 100644 --- a/src/main/starlark/builtins_bzl/common/cc/semantics.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/semantics.bzl @@ -198,4 +198,5 @@ semantics = struct( get_coverage_env = _get_coverage_env, get_proto_aspects = _get_proto_aspects, incompatible_disable_objc_library_transition = _incompatible_disable_objc_library_transition, + CC_PROTO_TOOLCHAIN = "@rules_cc//cc/proto:toolchain_type", ) diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java index 232c258aaeea52..2511c8d2868b8f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java @@ -56,6 +56,9 @@ public class CcProtoLibraryTest extends BuildViewTestCase { @Before public void setUp() throws Exception { MockProtoSupport.setup(mockToolsConfig); + scratch.file( + "third_party/bazel_rules/rules_cc/cc/proto/BUILD", + "toolchain_type(name = 'toolchain_type', visibility = ['//visibility:public'])"); scratch.file("protobuf/WORKSPACE"); scratch.overwriteFile( "protobuf/BUILD", @@ -82,6 +85,31 @@ public void setUp() throws Exception { invalidatePackages(); // A dash of magic to re-evaluate the WORKSPACE file. } + @Test + public void protoToolchainResolution_enabled() throws Exception { + setBuildLanguageOptions("--incompatible_enable_proto_toolchain_resolution"); + getAnalysisMock() + .ccSupport() + .setupCcToolchainConfig( + mockToolsConfig, + CcToolchainConfig.builder() + .withFeatures( + CppRuleClasses.SUPPORTS_DYNAMIC_LINKER, + CppRuleClasses.SUPPORTS_INTERFACE_SHARED_LIBRARIES)); + scratch.file( + "x/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "cc_proto_library(name = 'foo_cc_proto', deps = ['foo_proto'])", + "proto_library(name = 'foo_proto', srcs = ['foo.proto'])"); + assertThat(prettyArtifactNames(getFilesToBuild(getConfiguredTarget("//x:foo_cc_proto")))) + .containsExactly( + "x/foo.pb.h", + "x/foo.pb.cc", + "x/libfoo_proto.a", + "x/libfoo_proto.ifso", + "x/libfoo_proto.so"); + } + @Test public void basic() throws Exception { getAnalysisMock()