Skip to content

feat: demonstrate python grpc #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: demonstrate python grpc #58

wants to merge 1 commit into from

Conversation

alexeagle
Copy link
Member

Ported from https://github.com/chrisirhc/precompiled-grpc-in-bazel-python

FYI @chrisirhc - I haven't gotten it working here since it overrides the protoc toolchain for all languages, breaking the non-python ones.

@amol-mandhane
Copy link

I've managed to get this working in a different way. It requires some changes to grpc's python_rules which I have forked and modified here, and needs a release of grpc python plugin which I've put in the same repo.

@alexeagle
Copy link
Member Author

@amol-mandhane that's neat.

I'm not keen on signing up for long-term maintainence of a fork of something so complex though. We could propose this to the grpc team, I doubt they'll accept PRs but it would be great if they did.

My philosophy here is basically "why does Bazel have to be weird". proto/gRPC can work with other build tools, without the protobuf or gRPC teams owning special plugins. So I'd like to make design decisions that are motivated exclusively by "how does this work in the mainstream use case".

@amol-mandhane
Copy link

amol-mandhane commented Apr 4, 2025

As far as the bazel part goes, I reckon I can get it upstream in grpc since the patch is relatively small. However, it needs prebuilt plugins, which are not released by grpc properly, and I don't think that will change.

Using protoc from grpcio-tools package as shown in the example repo above is a bit too breaking for non-Python things.

How do you feel about users building and caching grpc plugins on their own? It's not a particularly great option, but I don't see anything better.

@chrisirhc
Copy link

chrisirhc commented Apr 15, 2025

FYI @chrisirhc - I haven't gotten it working here since it overrides the protoc toolchain for all languages, breaking the non-python ones.

In my testing (#60), it was able to run the protoc compiler from Python (grpcio_tools) and still pass. But I understand that it is indeed overriding the protoc for these other languages which is not desirable.

@antspy
Copy link

antspy commented Jun 21, 2025

Hi,

Just stumbled upon this - thank you for all the hard work you're doing!

Is there a roadmap / plan to when we expect this to land, or should we try to look for alternatives?

@amol-mandhane
Copy link

I have an alternative (mentioned in the comment above) here, but don't have the time currently to merge it upstream in grpc repo. If you have a python-only workspace, you can use the solution created by Chris in the link above.

@antspy
Copy link

antspy commented Jun 24, 2025

Thank you for your comment! I will wait for a proper resolution, in the meanwhile I settles on a (hacky) solution of invoking the gRPC binary outside of bazel, but I will be on the lookout for a proper solution :)

Thank you!

@ramilmsh
Copy link

@amol-mandhane

i've condensed your changes into a patch over grpc 1.73 from bcr (it's important, cause bcr applies a patch and this patch is on top of that, not the raw repo)

i have then used the fact that we were compiling grpc python plugin anyway and uploaded results of

bazel build @com_github_grpc_grpc//src/compiler:grpc_python_plugin

to gcs and proceeded to patch it back into our repo via rules_multitool. thus, I can #NeverCompileProtocAgain starting now

thanks a lot for your help and to @alexeagle for your work on this!


single_version_override(
    module_name = "grpc",
    patches = ["grpc.patch"],
)

a wrapper for the grpc rules, with updated defaults

load("@com_github_grpc_grpc//bazel:python_rules.bzl", _py_grpc_library = "py_grpc_library", _py_proto_library = "py_proto_library")

py_proto_library = _py_proto_library

def py_grpc_library(
        name,
        srcs,
        deps,
        strip_prefixes = [],
        grpc_library = "@pip//grpcio",
        grpc_plugin = "@multitool//tools/grpc_python",
        **kwargs):
    _py_grpc_library(
        name = name,
        srcs = srcs,
        deps = deps,
        strip_prefixes = strip_prefixes,
        grpc_library = grpc_library,
        grpc_plugin = grpc_plugin,
        **kwargs
    )
grpc.patch
--- bazel/python_rules.bzl
+++ bazel/python_rules.bzl
@@ -11,13 +11,15 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-"""Generates and compiles Python gRPC stubs from proto_library rules."""
+"""Generates and compiles Python gRPC stubs from proto_library rules.
 
+Fork of https://github.com/grpc/grpc/blob/master/bazel/python_rules.bzl
+that uses standard toolchain based proto deps.
+"""
+
 load("@com_google_protobuf//bazel:py_proto_library.bzl", protobuf_py_proto_library = "py_proto_library")
-load("@rules_proto//proto:defs.bzl", "ProtoInfo")
-load("@rules_python//python:py_info.bzl", "PyInfo")
 load(
-    "//bazel:protobuf.bzl",
+    "@com_github_grpc_grpc//bazel:protobuf.bzl",
     "declare_out_files",
     "get_include_directory",
     "get_out_dir",
@@ -28,11 +30,15 @@
     "is_well_known",
     "protos_from_context",
 )
+load("@rules_proto//proto:defs.bzl", "ProtoInfo")
+load("@rules_python//python:py_info.bzl", "PyInfo")
 
 _VIRTUAL_IMPORTS = "/_virtual_imports/"
 _GENERATED_PROTO_FORMAT = "{}_pb2.py"
 _GENERATED_PROTO_STUB_FORMAT = "{}_pb2.pyi"
 _GENERATED_GRPC_PROTO_FORMAT = "{}_pb2_grpc.py"
+_PROTO_TOOLCHAIN = "@com_google_protobuf//bazel/private:python_toolchain_type"
+_DEFAULT_PROTOBUF_LIBRARY = "@com_google_protobuf//:protobuf_python"
 
 PyProtoInfo = provider(
     "The Python outputs from the Protobuf compiler.",
@@ -42,6 +48,12 @@
     },
 )
 
+def _protoc_executable(context):
+    return context.toolchains[_PROTO_TOOLCHAIN].proto.proto_compiler.executable
+
+def _protobuf_library(context):
+    return context.toolchains[_PROTO_TOOLCHAIN].proto.runtime or Label(_DEFAULT_PROTOBUF_LIBRARY)
+
 def _merge_pyinfos(pyinfos):
     return PyInfo(
         transitive_sources = depset(transitive = [p.transitive_sources for p in pyinfos]),
@@ -53,7 +65,7 @@
     if is_well_known(str(context.label)):
         return [
             PyProtoInfo(
-                py_info = context.attr._protobuf_library[PyInfo],
+                py_info = _protobuf_library(context)[PyInfo],
                 generated_py_srcs = [],
             ),
         ]
@@ -67,7 +79,7 @@
                  declare_out_files(protos, context, _GENERATED_PROTO_STUB_FORMAT))
     generated_py_srcs = out_files
 
-    tools = [context.executable._protoc]
+    tools = [_protoc_executable(context)]
 
     out_dir = get_out_dir(protos, context)
 
@@ -87,7 +99,7 @@
         inputs = protos + includes.to_list(),
         tools = tools,
         outputs = out_files,
-        executable = context.executable._protoc,
+        executable = _protoc_executable(context),
         arguments = arguments,
         mnemonic = "ProtocInvocation",
     )
@@ -101,7 +113,7 @@
         py_info = _merge_pyinfos(
             [
                 py_info,
-                context.attr._protobuf_library[PyInfo],
+                _protobuf_library(context)[PyInfo],
             ] + [dep[PyProtoInfo].py_info for dep in context.rule.attr.deps],
         ),
         generated_py_srcs = generated_py_srcs,
@@ -111,17 +123,7 @@
     implementation = _gen_py_aspect_impl,
     attr_aspects = ["deps"],
     fragments = ["py"],
-    attrs = {
-        "_protoc": attr.label(
-            default = Label("@com_google_protobuf//:protoc"),
-            executable = True,
-            cfg = "exec",
-        ),
-        "_protobuf_library": attr.label(
-            default = Label("@com_google_protobuf//:protobuf_python"),
-            providers = [PyInfo],
-        ),
-    },
+    toolchains = [_PROTO_TOOLCHAIN],
 )
 
 def _generate_py_impl(context):
@@ -150,7 +152,7 @@
     py_info = PyInfo(transitive_sources = depset(direct = py_sources), imports = depset(direct = imports))
     out_pyinfo = _merge_pyinfos([py_info, context.attr.deps[0][PyProtoInfo].py_info])
 
-    runfiles = context.runfiles(files = out_pyinfo.transitive_sources.to_list()).merge(context.attr._protobuf_library[DefaultInfo].data_runfiles)
+    runfiles = context.runfiles(files = out_pyinfo.transitive_sources.to_list()).merge(_protobuf_library(context)[DefaultInfo].data_runfiles)
     return [
         DefaultInfo(
             files = out_pyinfo.transitive_sources,
@@ -167,18 +169,10 @@
             providers = [ProtoInfo],
             aspects = [_gen_py_aspect],
         ),
-        "_protoc": attr.label(
-            default = Label("@com_google_protobuf//:protoc"),
-            executable = True,
-            cfg = "exec",
-        ),
-        "_protobuf_library": attr.label(
-            default = Label("@com_google_protobuf//:protobuf_python"),
-            providers = [PyInfo],
-        ),
         "imports": attr.string_list(),
     },
     implementation = _generate_py_impl,
+    toolchains = [_PROTO_TOOLCHAIN],
 )
 
 def py_proto_library(name, deps, use_protobuf = True, **kwargs):
@@ -207,11 +201,15 @@
     plugin_flags = ["grpc_2_0"] + context.attr.strip_prefixes
 
     arguments = []
-    tools = [context.executable._protoc, context.executable._grpc_plugin]
+    tools = [_protoc_executable(context), context.executable.grpc_plugin]
     out_dir = get_out_dir(protos, context)
-    out_path = out_dir.path
+    if out_dir.import_path:
+        # is virtual imports
+        out_path = get_include_directory(out_files[0])
+    else:
+        out_path = out_dir.path
     arguments += get_plugin_args(
-        context.executable._grpc_plugin,
+        context.executable.grpc_plugin,
         plugin_flags,
         out_path,
         False,
@@ -227,7 +225,7 @@
         inputs = protos + includes,
         tools = tools,
         outputs = out_files,
-        executable = context.executable._protoc,
+        executable = _protoc_executable(context),
         arguments = arguments,
         mnemonic = "ProtocInvocation",
     )
@@ -279,22 +277,19 @@
             providers = [PyInfo],
         ),
         "strip_prefixes": attr.string_list(),
-        "_grpc_plugin": attr.label(
+        "grpc_plugin": attr.label(
             executable = True,
+            providers = ["files_to_run"],
             cfg = "exec",
             default = Label("//src/compiler:grpc_python_plugin"),
         ),
-        "_protoc": attr.label(
-            executable = True,
-            cfg = "exec",
-            default = Label("@com_google_protobuf//:protoc"),
-        ),
         "grpc_library": attr.label(
             default = Label("//src/python/grpcio/grpc:grpcio"),
             providers = [PyInfo],
         ),
     },
     implementation = _generate_pb2_grpc_src_impl,
+    toolchains = [_PROTO_TOOLCHAIN],
 )
 
 def py_grpc_library(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants