rules-proto-grpc / rules_proto_grpc

Bazel rules for building Protobuf and gRPC code and libraries from proto_library targets
https://rules-proto-grpc.com
Apache License 2.0
253 stars 160 forks source link

Bzlmod integration #269

Closed jbgcarnes closed 3 months ago

jbgcarnes commented 1 year ago

Description

I know it's been mentioned a few times tangentially in a few other issues, but as bzlmod's are going to be the standard in Bazel 7.0 (released later this year), it would be useful for the project to support bzlmod and potentially be on the bazel central repo.

Is there a roadmap or path forward in that direction for the project? And if so, what are some of the steps to getting it there (I might have a few cycles I can use to submit mrs)

aaliddell commented 1 year ago

It's something that needs to happen eventually and would solve so many of the dependency tree problems, but the protobuf and grpc modules in the BCR are presently effectively unsupported by Google (IIRC there's an issue somewhere stating they don't have time to support it) and therefore have not kept up to date as new releases occur. So unfortunately at present we are better off not supporting bzlmod, as non-bzlmod protobuf/grpc is more officially supported. Additionally, some of the languages used here do not yet have stable rules modules in the BCR, so a number of languages will be dropped.

When bzlmod support is eventually added, this will be as v6 of rules_proto_grpc that drops support for non-bzlmod usage, with the existing v5 releases available for that use-case. Step 1 on this process will be a minimum viable module that maybe just supports a single language.

So, realistically we are waiting for Google's Bazel/bzlmod team to prod Google's protobuf/grpc teams into officially supporting bzlmod, as nobody external really has the power nor time to make these changes.

jtcarnes commented 1 year ago

Eloquently put, well thought out, reasonable and what I didn't want to hear lol. Makes total sense. I'll hold tight then. Thank you for the well reasoned answer!

oh-tarnished commented 1 year ago

Hey Guys, I just checked right now what are all missing so this project can move to bzlmod pretty much everything that we require are in place with proto and protobuf right now we can do partial transition to MODULE system. where we can use both WORKSPACE AND MODULE.

any plans ?

aaliddell commented 1 year ago

Here's a small sample of the latest upstream issues that affect the viability of implementing bzlmod support here:

It looks like at present C++, Go and perhaps Python + Swift could be made to work under bzlmod, with the outdated protobuf & grpc versions presently in BCR. For other languages the support just isn't there yet

aaliddell commented 1 year ago

Some experimental bzlmod work is happening in the next branch. Only C++ and Python have been migrated as of today, but it works! I've already hit a number of issues related to the lack of proper upstream bzlmod support in the protobuf and grpc repos, which made Python in particular a bit of a hack. If you are using any of the languages that are available though, any feedback would always be welcome. Once there's a handful of the core languages working, I'll cut an alpha release so that it can be used directly with BCR.

BEWARE: There are sharp edges, things will break & the interface is by no means stable

linzhp commented 11 months ago

Thanks for the work on the next branch. We (Uber) is using that on our Python monorepo. It works! Looking forward to seeing it in Bazel Central Repository.

aaliddell commented 11 months ago

Good to hear! Hopefully there’s no more breaking changes before it lands in BCR alpha but can’t promise anything 🤞. I need grpc-gateway and Buf rules over the line as a minimum to issue the first release to cover requirements.

One thing to beware of with the grpc rules and python under bzlmod is that they don’t use the rules_python toolchains, so you may end up building the extension against the wrong python headers. This could be worked around under WORKSPACE but not under bzlmod, so needs https://github.com/grpc/grpc/issues/24665. Until that is resolved you need your system Python to match your toolchain Python version

linzhp commented 11 months ago

I assume you meant this:

https://github.com/rules-proto-grpc/rules_proto_grpc/blob/563df6e91df2dcf95b5898ae61180224b6c0b5ff/modules/python/python_grpc_library.bzl#L37-L38

I actually prefer the pip package grpcio over @grpc//src/python/grpcio/grpc:grpcio, because the pip package is prebuilt, and generally works better due to the complexity of building C libraries from source. Also, the Python projects using importing grpcio will be resolved to the pip package by default, which comes with version resolution and lock in requirements.txt. If rules-proto-grpc uses @grpc//src/python/grpcio/grpc:grpcio, it means there will be two versions of grpcio in the same Python project, which can lead to unpredictable behavior.

aaliddell commented 11 months ago

Oh I forgot I put that workaround in 🤣

The reason the @grpc version is preferred here is that the plugin used to generate the python sources comes from @grpc repo, as the raw plugin is not distributed in the pip wheel (only a wrapped protoc is bundled, which is useless here 😠). If you have a version skew between the pip dep and the bzlmod dep then I believe there’s no guarantee the generated python sources will work.

So we’re stuck in a position where neither the bzlmod dep nor the wheel provide both a working plugin and a working runtime…

linzhp commented 11 months ago

Let me clarify: Python gRPC projects using rules-proto-grpc needs to depend on gRPC in two places:

  1. rules-proto-grpc needs the gRPC plugin to generate Python sources
  2. Python sources (both generated and hand-crafted) importing grpcio package need gRPC to build and run

https://github.com/grpc/grpc/issues/24665 doesn't affect 2 when we use the pre-built grpcio pip package, right?

aaliddell commented 11 months ago

Correct, but you need to match those two versions (in theory anyway; in practice it seems like you can get away with it sometimes since presumably not much changes in these files).

linzhp commented 11 months ago

We ran into troubles in building grpcio from source but no trouble in build grpc plugin. So in another internal repo without bzlmod, we had to patch rules-proto-grpc so it uses the grpcio pip package.

aaliddell commented 11 months ago

Ok, if the plugin was bundled directly in the grpcio wheel it’d solve all the problems of version skew and rebuild, but sadly getting that change done upstream would likely be an uphill battle

linzhp commented 11 months ago

Actually, the plugin is published in a different wheel grpcio-tools. It's recommended by the official tutorial. grpcio-tools requires grpcio. So we can rely on pip to keep both in sync.

aaliddell commented 11 months ago

The grpcio-tools wheel doesn't contain the plugin in a usable form, which is what I meant earlier about "only a wrapped protoc is bundled, which is useless here".

The problem is that wheel doesn't contain a standard protoc plugin that we can integrate in these rules but instead contains an entire separate implementation of the protoc compiler wrapping the grpc python plugin as a shared library extension. I can understand why they do this for user friendliness (by not needing your own protoc), but it is the only language that does this and does not integrate with any other tooling that expects a normal plugin (such as these rules). The plugin we need is buried within that .so, but last time I tried to convert it into a usable form it I gave up.

aaliddell commented 10 months ago

I am going to cut an alpha release with languages in the current state they are in the next branch, which will end up closing this issue. I have added an issue tracking further language support here: #299