From 097bb6f01523bcf900e63c8d7ee7fc22eb47672b Mon Sep 17 00:00:00 2001 From: Zach Yu Date: Wed, 22 May 2024 15:14:46 -0700 Subject: [PATCH] [export] Fix bazel protoc actions with `--experimental_sibling_repository_layout` Protoc actions are failing when compiling protobuf in the `cc|objc_grpc_library` rules if `--experimental_sibling_repository_layout` is enabled. The assumptions made when generating certain paths are on longer true if sibling layout is used. These fixes are based on observations of how sibling repositories are laid out. ---- DO NOT SUBMIT. This PR is for testing purposes only. [cl/636309539](http://cl/636309539) PiperOrigin-RevId: 636309539 --- bazel/generate_cc.bzl | 8 ++++++-- bazel/generate_objc.bzl | 5 +++-- bazel/protobuf.bzl | 21 ++++++++++++++++----- bazel/python_rules.bzl | 5 +++-- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/bazel/generate_cc.bzl b/bazel/generate_cc.bzl index c0eb9401ee4..14fb7465a07 100644 --- a/bazel/generate_cc.bzl +++ b/bazel/generate_cc.bzl @@ -23,6 +23,7 @@ load( "get_include_directory", "get_plugin_args", "get_proto_root", + "is_sibling_repository_layout", "proto_path_to_generated_filename", ) @@ -127,6 +128,7 @@ def generate_cc_impl(ctx): arguments.append("--cpp_out=" + ",".join(ctx.attr.flags) + ":" + dir_out) tools = [] + is_sibling_layout = is_sibling_repository_layout(ctx) arguments += [ "--proto_path={}".format(get_include_directory(i)) for i in includes @@ -134,14 +136,16 @@ def generate_cc_impl(ctx): # Include the output directory so that protoc puts the generated code in the # right directory. - arguments.append("--proto_path={0}{1}".format(dir_out, proto_root)) + arguments.append("--proto_path={}".format(dir_out)) arguments += [_get_srcs_file_path(proto) for proto in protos] # create a list of well known proto files if the argument is non-None well_known_proto_files = [] if ctx.attr.well_known_protos: f = ctx.attr.well_known_protos.files.to_list()[0].dirname - if f != "external/com_google_protobuf/src/google/protobuf": + if f != "{}/com_google_protobuf/src/google/protobuf".format( + ".." if is_sibling_layout else "external", + ): print( "Error: Only @com_google_protobuf//:well_known_type_protos is supported", ) # buildifier: disable=print diff --git a/bazel/generate_objc.bzl b/bazel/generate_objc.bzl index d9518af7d88..477a4fc49b0 100644 --- a/bazel/generate_objc.bzl +++ b/bazel/generate_objc.bzl @@ -21,6 +21,7 @@ load( "//bazel:protobuf.bzl", "get_include_directory", "get_plugin_args", + "is_sibling_repository_layout", "proto_path_to_generated_filename", ) load(":grpc_util.bzl", "to_upper_camel_with_extension") @@ -60,7 +61,7 @@ def _generate_objc_impl(ctx): out_files = [ctx.actions.declare_file(out) for out in outs] dir_out = _join_directories([ str(ctx.genfiles_dir.path), - target_package, + ctx.label.package if ctx.label.workspace_root.startswith("../") else target_package, _GENERATED_PROTOS_DIR, ]) @@ -78,7 +79,7 @@ def _generate_objc_impl(ctx): arguments.append("--proto_path=.") arguments += [ - "--proto_path={}".format(get_include_directory(i)) + "--proto_path={}".format(get_include_directory(i, is_sibling_repository_layout(ctx))) for i in protos ] diff --git a/bazel/protobuf.bzl b/bazel/protobuf.bzl index 9568e627a77..6fefde55af7 100644 --- a/bazel/protobuf.bzl +++ b/bazel/protobuf.bzl @@ -58,7 +58,10 @@ def get_proto_root(workspace_root): Returns: The directory relative to which generated include paths should be. """ - if workspace_root: + + # When sibling repository layout is enabled (workspace_root = "../"), the output root + # should be just the genfiles directory, which already includes the repo name. + if workspace_root and not workspace_root.startswith("../"): return "/{}".format(workspace_root) else: return "" @@ -87,7 +90,7 @@ def proto_path_to_generated_filename(proto_path, fmt_str): """ return fmt_str.format(_strip_proto_extension(proto_path)) -def get_include_directory(source_file): +def get_include_directory(source_file, is_sibling_layout = False): """Returns the include directory path for the source_file. All of the include statements within the given source_file are calculated @@ -98,6 +101,7 @@ def get_include_directory(source_file): Args: source_file: A proto file. + is_sibling_layout: If `--experimental_sibling_repository_layout` is enabled. Returns: The include directory path for the source_file. @@ -113,7 +117,10 @@ def get_include_directory(source_file): if not source_file.is_source and directory.startswith(source_file.root.path): prefix_len = len(source_file.root.path) + 1 - if directory.startswith("external", prefix_len): + # This path is hit when proto targets are built as @grpc//:xxx + # instead of //:xxx. + # For that we have 2 cases: --experimental_sibling_repository_layout ("..") or not ("external") + if directory.startswith(".." if is_sibling_layout else "external", prefix_len): external_separator = directory.find("/", prefix_len) repository_separator = directory.find("/", external_separator + 1) return directory[:repository_separator] @@ -302,14 +309,14 @@ def get_out_dir(protos, context): Returns: The value of --_out= argument. """ - at_least_one_virtual = 0 + at_least_one_virtual = False for proto in protos: if is_in_virtual_imports(proto): at_least_one_virtual = True elif at_least_one_virtual: fail("Proto sources must be either all virtual imports or all real") if at_least_one_virtual: - out_dir = get_include_directory(protos[0]) + out_dir = get_include_directory(protos[0], is_sibling_repository_layout(context)) ws_root = protos[0].owner.workspace_root prefix = "/" + _make_prefix(protos[0].owner) + _VIRTUAL_IMPORTS[1:] @@ -339,3 +346,7 @@ def is_in_virtual_imports(source_file, virtual_folder = _VIRTUAL_IMPORTS): True if source_file is located under _virtual_imports, False otherwise. """ return not source_file.is_source and virtual_folder in source_file.path + +def is_sibling_repository_layout(ctx): + config = ctx.configuration + return hasattr(config, "is_sibling_repository_layout") and config.is_sibling_repository_layout() diff --git a/bazel/python_rules.bzl b/bazel/python_rules.bzl index 9c19d7ac2f2..40ee79ddbf2 100644 --- a/bazel/python_rules.bzl +++ b/bazel/python_rules.bzl @@ -23,6 +23,7 @@ load( "get_proto_arguments", "get_staged_proto_file", "includes_from_deps", + "is_sibling_repository_layout", "is_well_known", "protos_from_context", ) @@ -72,7 +73,7 @@ def _gen_py_aspect_impl(target, context): "--python_out={}".format(out_dir.path), "--pyi_out={}".format(out_dir.path), ] + [ - "--proto_path={}".format(get_include_directory(i)) + "--proto_path={}".format(get_include_directory(i, is_sibling_repository_layout(context))) for i in includes.to_list() ] + [ "--proto_path={}".format(context.genfiles_dir.path), @@ -203,7 +204,7 @@ def _generate_pb2_grpc_src_impl(context): ) arguments += [ - "--proto_path={}".format(get_include_directory(i)) + "--proto_path={}".format(get_include_directory(i, is_sibling_repository_layout(context))) for i in includes ] arguments.append("--proto_path={}".format(context.genfiles_dir.path))