From 648c20d60225f0fc551c28c6c57e997f6fd76fd9 Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Thu, 11 Jan 2024 03:50:30 -0800 Subject: [PATCH] Pass crate mapping from Bazel to protoc This will enable us to get the correct crate names for Rust gencode. The actual reading of the mapping file in protoc happens in the followup. PiperOrigin-RevId: 597509582 --- .github/workflows/test_rust.yml | 2 +- rust/BUILD | 4 +- rust/aspects.bzl | 100 +++++++++++++++++- rust/defs.bzl | 8 +- .../grandparent1.proto | 10 ++ .../grandparent2.proto | 10 ++ .../rust_proto_library_unit_test.bzl | 76 +++++++++++-- src/google/protobuf/compiler/main.cc | 2 +- 8 files changed, 192 insertions(+), 20 deletions(-) create mode 100644 rust/test/rust_proto_library_unit_test/grandparent1.proto create mode 100644 rust/test/rust_proto_library_unit_test/grandparent2.proto diff --git a/.github/workflows/test_rust.yml b/.github/workflows/test_rust.yml index d86acb0a00..a82ff2c434 100644 --- a/.github/workflows/test_rust.yml +++ b/.github/workflows/test_rust.yml @@ -23,7 +23,7 @@ jobs: - name: Run tests uses: protocolbuffers/protobuf-ci/bazel-docker@v2 with: - image: us-docker.pkg.dev/protobuf-build/containers/common/linux/bazel:6.3.0-91a0ac83e968068672bc6001a4d474cfd9a50f1d + image: "us-docker.pkg.dev/protobuf-build/containers/common/linux/bazel:7.0.0-a04396cc76704d4b7c722789e9c08df18f47df53" credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} bazel-cache: rust_linux bazel: >- diff --git a/rust/BUILD b/rust/BUILD index 62ae297d66..165640694b 100644 --- a/rust/BUILD +++ b/rust/BUILD @@ -127,7 +127,7 @@ rust_library( proto_lang_toolchain( name = "proto_rust_upb_toolchain", - command_line = "--rust_out=experimental-codegen=enabled,kernel=upb:$(OUT)", + command_line = "--rust_out=$(OUT)", progress_message = "Generating Rust proto_library %{label}", runtime = ":protobuf_upb", visibility = ["//visibility:public"], @@ -135,7 +135,7 @@ proto_lang_toolchain( proto_lang_toolchain( name = "proto_rust_cpp_toolchain", - command_line = "--rust_out=experimental-codegen=enabled,kernel=cpp:$(OUT)", + command_line = "--rust_out=$(OUT)", progress_message = "Generating Rust proto_library %{label}", runtime = ":protobuf_cpp", visibility = ["//visibility:public"], diff --git a/rust/aspects.bzl b/rust/aspects.bzl index 0399c12c6d..a873c6d2f8 100644 --- a/rust/aspects.bzl +++ b/rust/aspects.bzl @@ -15,18 +15,67 @@ proto_common = proto_common_do_not_use visibility(["//rust/..."]) +CrateMappingInfo = provider( + doc = "Struct mapping crate name to the .proto import paths", + fields = { + "crate_name": "Crate name of the proto_library target", + "import_paths": "Import path used in .proto files of dependants to import the .proto " + + "file of the current proto_library.", + }, +) RustProtoInfo = provider( doc = "Rust protobuf provider info", fields = { "dep_variant_info": "DepVariantInfo for the compiled Rust gencode (also covers its " + "transitive dependencies)", + "crate_mapping": "depset(CrateMappingInfo) containing mappings of all transitive " + + "dependencies of the current proto_library.", }, ) +def _register_crate_mapping_write_action(name, actions, crate_mappings): + """Registers an action that generates a crate mapping for a proto_library. + + Args: + name: The name of the target being analyzed. + actions: The context's actions object. + crate_mappings: depset(CrateMappingInfo) to be rendered. + This sequence should already have duplicates removed. + + Returns: + The generated `File` with the crate mapping. + """ + mapping_file = actions.declare_file( + "{}.rust_crate_mapping".format(name), + ) + content = actions.args() + content.set_param_file_format("multiline") + content.add_all(crate_mappings, map_each = _render_text_crate_mapping) + actions.write(content = content, output = mapping_file) + + return mapping_file + +def _render_text_crate_mapping(mapping): + """Renders the mapping to an easily parseable file for a crate mapping. + + Args: + mapping (CrateMappingInfo): A single crate mapping. + + Returns: + A string containing the crate mapping for the target in simple format: + \n + \n + \n + """ + crate_name = mapping.crate_name + import_paths = mapping.import_paths + return "\n".join(([crate_name, str(len(import_paths))] + list(import_paths))) + def _generate_rust_gencode( ctx, proto_info, proto_lang_toolchain, + crate_mapping, is_upb): """Generates Rust gencode @@ -37,6 +86,8 @@ def _generate_rust_gencode( proto_info (ProtoInfo): ProtoInfo of the proto_library target for which we are generating gencode proto_lang_toolchain (ProtoLangToolchainInfo): proto lang toolchain for Rust + crate_mapping (File): File containing the mapping from .proto file import path to its + corresponding containing Rust crate name. is_upb (Bool): True when generating gencode for UPB, False otherwise. Returns: rs_outputs (([File], [File]): tuple(generated Rust files, generated C++ thunks). @@ -55,10 +106,20 @@ def _generate_rust_gencode( proto_info = proto_info, extension = ".pb.thunks.cc", ) + additional_args = ctx.actions.args() + + additional_args.add( + "--rust_opt=experimental-codegen=enabled,kernel={},bazel_crate_mapping={}".format( + "upb" if is_upb else "cpp", + crate_mapping.path, + ), + ) proto_common.compile( actions = ctx.actions, proto_info = proto_info, + additional_inputs = depset(direct = [crate_mapping]), + additional_args = additional_args, generated_files = rs_outputs + cc_outputs, proto_lang_toolchain_info = proto_lang_toolchain, plugin_output = ctx.bin_dir.path, @@ -211,6 +272,12 @@ def _rust_cc_proto_aspect_impl(target, ctx): """Implements the Rust protobuf aspect logic for C++ kernel.""" return _rust_proto_aspect_common(target, ctx, is_upb = False) +def get_import_path(f): + if hasattr(proto_common, "get_import_path"): + return proto_common.get_import_path(f) + else: + return f.path + def _rust_proto_aspect_common(target, ctx, is_upb): if RustProtoInfo in target: return [] @@ -231,10 +298,25 @@ def _rust_proto_aspect_common(target, ctx, is_upb): unsupported_features = ctx.disabled_features, ) + proto_srcs = getattr(ctx.rule.files, "srcs", []) + proto_deps = getattr(ctx.rule.attr, "deps", []) + transitive_crate_mappings = [] + for dep in proto_deps: + rust_proto_info = dep[RustProtoInfo] + transitive_crate_mappings.append(rust_proto_info.crate_mapping) + + mapping_for_current_target = depset(transitive = transitive_crate_mappings) + crate_mapping_file = _register_crate_mapping_write_action( + target.label.name, + ctx.actions, + mapping_for_current_target, + ) + (gencode, thunks) = _generate_rust_gencode( ctx, target[ProtoInfo], proto_lang_toolchain, + crate_mapping_file, is_upb, ) @@ -259,17 +341,25 @@ def _rust_proto_aspect_common(target, ctx, is_upb): ) dep_variant_info_for_native_gencode = DepVariantInfo(cc_info = thunks_cc_info) - proto_dep = getattr(ctx.rule.attr, "deps", []) dep_variant_info = _compile_rust( ctx = ctx, attr = ctx.rule.attr, src = gencode[0], extra_srcs = gencode[1:], deps = [dep_variant_info_for_runtime, dep_variant_info_for_native_gencode] + ( - [proto_dep[0][RustProtoInfo].dep_variant_info] if proto_dep else [] + [proto_deps[0][RustProtoInfo].dep_variant_info] if proto_deps else [] ), ) - return [RustProtoInfo(dep_variant_info = dep_variant_info)] + return [RustProtoInfo( + dep_variant_info = dep_variant_info, + crate_mapping = depset( + direct = [CrateMappingInfo( + crate_name = target.label.name, + import_paths = tuple([get_import_path(f) for f in proto_srcs]), + )], + transitive = transitive_crate_mappings, + ), + )] def _make_proto_library_aspect(is_upb): return aspect( @@ -319,7 +409,9 @@ def _make_proto_library_aspect(is_upb): cfg = "exec", ), "_proto_lang_toolchain": attr.label( - default = Label(("//rust:proto_rust_upb_toolchain" if is_upb else "//rust:proto_rust_cpp_toolchain")), + default = Label( + "//rust:proto_rust_upb_toolchain" if is_upb else "//rust:proto_rust_cpp_toolchain", + ), ), }, fragments = ["cpp"], diff --git a/rust/defs.bzl b/rust/defs.bzl index d51829bb83..7541c9986f 100644 --- a/rust/defs.bzl +++ b/rust/defs.bzl @@ -3,13 +3,13 @@ Disclaimer: This project is experimental, under heavy development, and should not be used yet.""" -load("@rules_cc//cc:defs.bzl", "cc_proto_library") load( "//rust:aspects.bzl", "RustProtoInfo", "rust_cc_proto_library_aspect", "rust_upb_proto_library_aspect", ) +load("@rules_cc//cc:defs.bzl", "cc_proto_library") visibility([ "//experimental/...", @@ -72,7 +72,11 @@ def _rust_proto_library_impl(ctx): dep = deps[0] rust_proto_info = dep[RustProtoInfo] dep_variant_info = rust_proto_info.dep_variant_info - return [dep_variant_info.crate_info, dep_variant_info.dep_info, dep_variant_info.cc_info] + return [ + dep_variant_info.crate_info, + dep_variant_info.dep_info, + dep_variant_info.cc_info, + ] def _make_rust_proto_library(is_upb): return rule( diff --git a/rust/test/rust_proto_library_unit_test/grandparent1.proto b/rust/test/rust_proto_library_unit_test/grandparent1.proto new file mode 100644 index 0000000000..23c31f2552 --- /dev/null +++ b/rust/test/rust_proto_library_unit_test/grandparent1.proto @@ -0,0 +1,10 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2023 Google LLC. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +syntax = "proto2"; + +package third_party_protobuf_rust_test_rust_proto_library_unit_test; diff --git a/rust/test/rust_proto_library_unit_test/grandparent2.proto b/rust/test/rust_proto_library_unit_test/grandparent2.proto new file mode 100644 index 0000000000..23c31f2552 --- /dev/null +++ b/rust/test/rust_proto_library_unit_test/grandparent2.proto @@ -0,0 +1,10 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2023 Google LLC. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +syntax = "proto2"; + +package third_party_protobuf_rust_test_rust_proto_library_unit_test; diff --git a/rust/test/rust_proto_library_unit_test/rust_proto_library_unit_test.bzl b/rust/test/rust_proto_library_unit_test/rust_proto_library_unit_test.bzl index d43d8dcbf9..3e24f71b59 100644 --- a/rust/test/rust_proto_library_unit_test/rust_proto_library_unit_test.bzl +++ b/rust/test/rust_proto_library_unit_test/rust_proto_library_unit_test.bzl @@ -1,15 +1,58 @@ """This module contains unit tests for rust_proto_library and its aspect.""" load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") -load(":defs.bzl", "ActionsInfo", "attach_cc_aspect", "attach_upb_aspect") load("//rust:aspects.bzl", "RustProtoInfo") load("@rules_cc//cc:defs.bzl", "cc_proto_library") +load(":defs.bzl", "ActionsInfo", "attach_cc_aspect", "attach_upb_aspect") + +def _find_actions_with_mnemonic(actions, mnemonic): + actions = [a for a in actions if a.mnemonic == mnemonic] + if not actions: + fail("Couldn't find action with mnemonic {} among {}".format( + mnemonic, + [a.mnemonic for a in actions], + )) + return actions + +def _check_crate_mapping(actions, target_name): + fw_actions = _find_actions_with_mnemonic(actions, "FileWrite") + crate_mapping_action = None + for a in fw_actions: + outputs = a.outputs.to_list() + output = [o for o in outputs if o.basename == target_name + ".rust_crate_mapping"] + if output: + crate_mapping_action = a + if not crate_mapping_action: + fail("Couldn't find action outputting {}.rust_crate_mapping among {}".format( + target_name, + fw_actions, + )) + expected_content = """grandparent_proto +2 +rust/test/rust_proto_library_unit_test/grandparent1.proto +rust/test/rust_proto_library_unit_test/grandparent2.proto +parent_proto +1 +rust/test/rust_proto_library_unit_test/parent.proto +""" + if crate_mapping_action.content != expected_content: + fail("The crate mapping file content didn't match. Was: {}".format( + crate_mapping_action.content, + )) -def _find_action_with_mnemonic(actions, mnemonic): - action = [a for a in actions if a.mnemonic == mnemonic] - if not action: - fail("Couldn't find action with mnemonic {} among {}".format(mnemonic, actions)) - return action[0] + protoc_action = [ + a + for a in actions + for i in a.inputs.to_list() + if "rust_crate_mapping" in i.basename + ] + if not protoc_action: + fail("Couldn't find action with the rust_crate_mapping as input") + if protoc_action[0].mnemonic != "GenProto": + fail( + "Action that had rust_crate_mapping as input wasn't a GenProto action, but {}", + protoc_action[0].mnemonic, + ) def _find_rust_lib_input(inputs, target_name): inputs = inputs.to_list() @@ -54,7 +97,10 @@ def _rust_upb_aspect_test_impl(ctx): env = analysistest.begin(ctx) target_under_test = analysistest.target_under_test(env) actions = target_under_test[ActionsInfo].actions - rustc_action = _find_action_with_mnemonic(actions, "Rustc") + rustc_action = _find_actions_with_mnemonic(actions, "Rustc")[0] + + # The protoc action needs to be given the crate mapping file + _check_crate_mapping(actions, "child_proto") # The action needs to have the Rust runtime as an input _find_rust_lib_input(rustc_action.inputs, "protobuf") @@ -83,9 +129,11 @@ def _rust_cc_aspect_test_impl(ctx): env = analysistest.begin(ctx) target_under_test = analysistest.target_under_test(env) actions = target_under_test[ActionsInfo].actions - rustc_action = _find_action_with_mnemonic(actions, "Rustc") + rustc_action = _find_actions_with_mnemonic(actions, "Rustc")[0] - # The action needs to have the Rust runtime as an input + _check_crate_mapping(actions, "child_proto") + + # The rustc action needs to have the Rust runtime as an input _find_rust_lib_input(rustc_action.inputs, "protobuf") # The action needs to produce a .rlib artifact (sometimes .rmeta as well, not tested here). @@ -114,7 +162,15 @@ def rust_proto_library_unit_test(name): Args: name: name of the test suite""" - native.proto_library(name = "parent_proto", srcs = ["parent.proto"]) + native.proto_library( + name = "grandparent_proto", + srcs = ["grandparent1.proto", "grandparent2.proto"], + ) + native.proto_library( + name = "parent_proto", + srcs = ["parent.proto"], + deps = [":grandparent_proto"], + ) native.proto_library(name = "child_proto", srcs = ["child.proto"], deps = [":parent_proto"]) cc_proto_library(name = "child_cc_proto", deps = [":child_proto"]) diff --git a/src/google/protobuf/compiler/main.cc b/src/google/protobuf/compiler/main.cc index 0ade6ace80..b43b2f15d4 100644 --- a/src/google/protobuf/compiler/main.cc +++ b/src/google/protobuf/compiler/main.cc @@ -92,7 +92,7 @@ int ProtobufMain(int argc, char* argv[]) { // Rust rust::RustGenerator rust_generator; - cli.RegisterGenerator("--rust_out", &rust_generator, + cli.RegisterGenerator("--rust_out", "--rust_opt", &rust_generator, "Generate Rust sources."); return cli.Run(argc, argv); }