Merge pull request grpc#18955 from vam-google/master

Make cc_grpc_library compatible with native proto_library and cc_proto_library rules
tmatsuo · May 8, 2019 · 0803c79 · 0803c79
2 parents 26c43ed + 0f7f745
commit 0803c79
Show file tree

Hide file tree

Showing 7 changed files with 201 additions and 118 deletions.
diff --git a/bazel/BUILD b/bazel/BUILD
@@ -17,15 +17,3 @@ licenses(["notice"])  # Apache v2
 package(default_visibility = ["//:__subpackages__"])
 
 load(":cc_grpc_library.bzl", "cc_grpc_library")
-
-proto_library(
-    name = "well_known_protos_list",
-    srcs = ["@com_google_protobuf//:well_known_protos"],
-)
-
-cc_grpc_library(
-    name = "well_known_protos",
-    srcs = "well_known_protos_list",
-    proto_only = True,
-    deps = [],
-)
diff --git a/bazel/cc_grpc_library.bzl b/bazel/cc_grpc_library.bzl
@@ -1,71 +1,105 @@
 """Generates and compiles C++ grpc stubs from proto_library rules."""
 
 load("//bazel:generate_cc.bzl", "generate_cc")
+load("//bazel:protobuf.bzl", "well_known_proto_libs")
 
-def cc_grpc_library(name, srcs, deps, proto_only, well_known_protos, generate_mocks = False, use_external = False, **kwargs):
-  """Generates C++ grpc classes from a .proto file.
+def cc_grpc_library(
+        name,
+        srcs,
+        deps,
+        proto_only = False,
+        well_known_protos = False,
+        generate_mocks = False,
+        use_external = False,
+        grpc_only = False,
+        **kwargs):
+    """Generates C++ grpc classes for services defined in a proto file.
 
-  Assumes the generated classes will be used in cc_api_version = 2.
+    If grpc_only is True, this rule is compatible with proto_library and
+    cc_proto_library native rules such that it expects proto_library target
+    as srcs argument and generates only grpc library classes, expecting
+    protobuf messages classes library (cc_proto_library target) to be passed in
+    deps argument. By default grpc_only is False which makes this rule to behave
+    in a backwards-compatible mode (trying to generate both proto and grpc
+    classes).
 
-  Arguments:
-      name: name of rule.
-      srcs: a single proto_library, which wraps the .proto files with services.
-      deps: a list of C++ proto_library (or cc_proto_library) which provides
-        the compiled code of any message that the services depend on.
-      well_known_protos: Should this library additionally depend on well known
-        protos
-      use_external: When True the grpc deps are prefixed with //external. This
-        allows grpc to be used as a dependency in other bazel projects.
-      generate_mocks: When True, Google Mock code for client stub is generated.
-      **kwargs: rest of arguments, e.g., compatible_with and visibility.
-  """
-  if len(srcs) > 1:
-    fail("Only one srcs value supported", "srcs")
+    Assumes the generated classes will be used in cc_api_version = 2.
 
-  proto_target = "_" + name + "_only"
-  codegen_target = "_" + name + "_codegen"
-  codegen_grpc_target = "_" + name + "_grpc_codegen"
-  proto_deps = ["_" + dep + "_only" for dep in deps if dep.find(':') == -1]
-  proto_deps += [dep.split(':')[0] + ':' + "_" + dep.split(':')[1] + "_only" for dep in deps if dep.find(':') != -1]
+    Args:
+        name (str): Name of rule.
+        srcs (list): A single .proto file which contains services definitions,
+          or if grpc_only parameter is True, a single proto_library which
+          contains services descriptors.
+        deps (list): A list of C++ proto_library (or cc_proto_library) which
+          provides the compiled code of any message that the services depend on.
+        proto_only (bool): If True, create only C++ proto classes library,
+          avoid creating C++ grpc classes library (expect it in deps).
+          Deprecated, use native cc_proto_library instead. False by default.
+        well_known_protos (bool): Should this library additionally depend on
+          well known protos. Deprecated, the well known protos should be
+          specified as explicit dependencies of the proto_library target
+          (passed in srcs parameter) instead. False by default.
+        generate_mocks (bool): when True, Google Mock code for client stub is
+          generated. False by default.
+        use_external (bool): Not used.
+        grpc_only (bool): if True, generate only grpc library, expecting
+          protobuf messages library (cc_proto_library target) to be passed as
+          deps. False by default (will become True by default eventually).
+        **kwargs: rest of arguments, e.g., compatible_with and visibility
+    """
+    if len(srcs) > 1:
+        fail("Only one srcs value supported", "srcs")
+    if grpc_only and proto_only:
+        fail("A mutualy exclusive configuration is specified: grpc_only = True and proto_only = True")
 
-  native.proto_library(
-      name = proto_target,
-      srcs = srcs,
-      deps = proto_deps,
-      **kwargs
-  )
+    extra_deps = []
+    proto_targets = []
 
-  generate_cc(
-      name = codegen_target,
-      srcs = [proto_target],
-      well_known_protos = well_known_protos,
-      **kwargs
-  )
+    if not grpc_only:
+        proto_target = "_" + name + "_only"
+        cc_proto_target = name if proto_only else "_" + name + "_cc_proto"
 
-  if not proto_only:
-    plugin = "@com_github_grpc_grpc//:grpc_cpp_plugin"
-    generate_cc(
-        name = codegen_grpc_target,
-        srcs = [proto_target],
-        plugin = plugin,
-        well_known_protos = well_known_protos,
-        generate_mocks = generate_mocks,
-        **kwargs
-    )
-    grpc_deps  = ["@com_github_grpc_grpc//:grpc++_codegen_proto",
-                  "//external:protobuf"]
-    native.cc_library(
-        name = name,
-        srcs = [":" + codegen_grpc_target, ":" + codegen_target],
-        hdrs = [":" + codegen_grpc_target, ":" + codegen_target],
-        deps = deps + grpc_deps,
-        **kwargs
-    )
-  else:
-    native.cc_library(
-        name = name,
-        srcs = [":" + codegen_target],
-        hdrs = [":" + codegen_target],
-        deps = deps + ["//external:protobuf"],
-        **kwargs
-    )
+        proto_deps = ["_" + dep + "_only" for dep in deps if dep.find(":") == -1]
+        proto_deps += [dep.split(":")[0] + ":" + "_" + dep.split(":")[1] + "_only" for dep in deps if dep.find(":") != -1]
+        if well_known_protos:
+            proto_deps += well_known_proto_libs()
+
+        native.proto_library(
+            name = proto_target,
+            srcs = srcs,
+            deps = proto_deps,
+            **kwargs
+        )
+
+        native.cc_proto_library(
+            name = cc_proto_target,
+            deps = [":" + proto_target],
+            **kwargs
+        )
+        extra_deps.append(":" + cc_proto_target)
+        proto_targets.append(proto_target)
+    else:
+        if not srcs:
+            fail("srcs cannot be empty", "srcs")
+        proto_targets += srcs
+
+    if not proto_only:
+        codegen_grpc_target = "_" + name + "_grpc_codegen"
+        generate_cc(
+            name = codegen_grpc_target,
+            srcs = proto_targets,
+            plugin = "@com_github_grpc_grpc//:grpc_cpp_plugin",
+            well_known_protos = well_known_protos,
+            generate_mocks = generate_mocks,
+            **kwargs
+        )
+
+        native.cc_library(
+            name = name,
+            srcs = [":" + codegen_grpc_target],
+            hdrs = [":" + codegen_grpc_target],
+            deps = deps +
+                   extra_deps +
+                   ["@com_github_grpc_grpc//:grpc++_codegen_proto"],
+            **kwargs
+        )
diff --git a/bazel/generate_cc.bzl b/bazel/generate_cc.bzl
@@ -18,20 +18,30 @@ _GRPC_PROTO_MOCK_HEADER_FMT = "{}_mock.grpc.pb.h"
 _PROTO_HEADER_FMT = "{}.pb.h"
 _PROTO_SRC_FMT = "{}.pb.cc"
 
-def _strip_package_from_path(label_package, path):
+def _strip_package_from_path(label_package, file):
+    prefix_len = 0
+    if not file.is_source and file.path.startswith(file.root.path):
+        prefix_len = len(file.root.path) + 1
+
+    path = file.path
     if len(label_package) == 0:
         return path
-    if not path.startswith(label_package + "/"):
+    if not path.startswith(label_package + "/", prefix_len):
         fail("'{}' does not lie within '{}'.".format(path, label_package))
-    return path[len(label_package + "/"):]
+    return path[prefix_len + len(label_package + "/"):]
+
+def _get_srcs_file_path(file):
+    if not file.is_source and file.path.startswith(file.root.path):
+        return file.path[len(file.root.path) + 1:]
+    return file.path
 
 def _join_directories(directories):
     massaged_directories = [directory for directory in directories if len(directory) != 0]
     return "/".join(massaged_directories)
 
 def generate_cc_impl(ctx):
     """Implementation of the generate_cc rule."""
-    protos = [f for src in ctx.attr.srcs for f in src.proto.direct_sources]
+    protos = [f for src in ctx.attr.srcs for f in src.proto.check_deps_sources]
     includes = [
         f
         for src in ctx.attr.srcs
@@ -46,37 +56,37 @@ def generate_cc_impl(ctx):
     if ctx.executable.plugin:
         outs += [
             proto_path_to_generated_filename(
-                _strip_package_from_path(label_package, proto.path),
+                _strip_package_from_path(label_package, proto),
                 _GRPC_PROTO_HEADER_FMT,
             )
             for proto in protos
         ]
         outs += [
             proto_path_to_generated_filename(
-                _strip_package_from_path(label_package, proto.path),
+                _strip_package_from_path(label_package, proto),
                 _GRPC_PROTO_SRC_FMT,
             )
             for proto in protos
         ]
         if ctx.attr.generate_mocks:
             outs += [
                 proto_path_to_generated_filename(
-                    _strip_package_from_path(label_package, proto.path),
+                    _strip_package_from_path(label_package, proto),
                     _GRPC_PROTO_MOCK_HEADER_FMT,
                 )
                 for proto in protos
             ]
     else:
         outs += [
             proto_path_to_generated_filename(
-                _strip_package_from_path(label_package, proto.path),
+                _strip_package_from_path(label_package, proto),
                 _PROTO_HEADER_FMT,
             )
             for proto in protos
         ]
         outs += [
             proto_path_to_generated_filename(
-                _strip_package_from_path(label_package, proto.path),
+                _strip_package_from_path(label_package, proto),
                 _PROTO_SRC_FMT,
             )
             for proto in protos
@@ -102,7 +112,7 @@ def generate_cc_impl(ctx):
     # Include the output directory so that protoc puts the generated code in the
     # right directory.
     arguments += ["--proto_path={0}{1}".format(dir_out, proto_root)]
-    arguments += [proto.path for proto in protos]
+    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 = []

diff --git a/bazel/protobuf.bzl b/bazel/protobuf.bzl
@@ -2,6 +2,22 @@
 
 _PROTO_EXTENSION = ".proto"
 
+def well_known_proto_libs():
+    return [
+        "@com_google_protobuf//:any_proto",
+        "@com_google_protobuf//:api_proto",
+        "@com_google_protobuf//:compiler_plugin_proto",
+        "@com_google_protobuf//:descriptor_proto",
+        "@com_google_protobuf//:duration_proto",
+        "@com_google_protobuf//:empty_proto",
+        "@com_google_protobuf//:field_mask_proto",
+        "@com_google_protobuf//:source_context_proto",
+        "@com_google_protobuf//:struct_proto",
+        "@com_google_protobuf//:timestamp_proto",
+        "@com_google_protobuf//:type_proto",
+        "@com_google_protobuf//:wrappers_proto",
+    ]
+
 def get_proto_root(workspace_root):
     """Gets the root protobuf directory.
 
@@ -42,12 +58,16 @@ def proto_path_to_generated_filename(proto_path, fmt_str):
 
 def _get_include_directory(include):
     directory = include.path
-    if directory.startswith("external"):
-        external_separator = directory.find("/")
+    prefix_len = 0
+    if not include.is_source and directory.startswith(include.root.path):
+        prefix_len = len(include.root.path) + 1
+
+    if directory.startswith("external", prefix_len):
+        external_separator = directory.find("/", prefix_len)
         repository_separator = directory.find("/", external_separator + 1)
         return directory[:repository_separator]
     else:
-        return "."
+        return include.root.path if include.root.path else "."
 
 def get_include_protoc_args(includes):
     """Returns protoc args that imports protos relative to their import root.

diff --git a/bazel/python_rules.bzl b/bazel/python_rules.bzl
@@ -130,21 +130,6 @@ def _generate_py(well_known_protos, **kwargs):
     else:
         __generate_py(**kwargs)
 
-_WELL_KNOWN_PROTO_LIBS = [
-    "@com_google_protobuf//:any_proto",
-    "@com_google_protobuf//:api_proto",
-    "@com_google_protobuf//:compiler_plugin_proto",
-    "@com_google_protobuf//:descriptor_proto",
-    "@com_google_protobuf//:duration_proto",
-    "@com_google_protobuf//:empty_proto",
-    "@com_google_protobuf//:field_mask_proto",
-    "@com_google_protobuf//:source_context_proto",
-    "@com_google_protobuf//:struct_proto",
-    "@com_google_protobuf//:timestamp_proto",
-    "@com_google_protobuf//:type_proto",
-    "@com_google_protobuf//:wrappers_proto",
-]
-
 def py_proto_library(
         name,
         deps,
@@ -167,8 +152,6 @@ def py_proto_library(
     codegen_target = "_{}_codegen".format(name)
     codegen_grpc_target = "_{}_grpc_codegen".format(name)
 
-    well_known_proto_rules = _WELL_KNOWN_PROTO_LIBS if well_known_protos else []
-
     _generate_py(
         name = codegen_target,
         deps = deps,