protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

Add standalone protoc plugin binaries

Open jchadwick-buf opened this issue 2 years ago • 21 comments

This PR adds targets to build protoc plugins for all the built-in plugins. This will result in the following binaries being available:

  • protoc-gen-cpp
  • protoc-gen-csharp
  • protoc-gen-java
  • protoc-gen-kotlin
  • protoc-gen-objectivec
  • protoc-gen-php
  • protoc-gen-python
  • protoc-gen-pyi
  • protoc-gen-ruby
  • protoc-gen-rust

The goal of this PR is for these to be added as assets to each GitHub release, although we are unable to find the code that builds the assets for releases - if it is in this repository, let us know and we're happy to do any work required!

There's multiple motivations for this PR:

  • Optimally, people want to version their plugins separately from their compiler. That is, the version of protoc should be decoupled from the version of each plugin, as is the case with the rest of the Protobuf ecosystem outside of the built-in plugins. You should be able to, for example, take advantage of fixes in protoc v24.0 whilst continuing to use the Java generated code from v23.0, in case there are breaking changes you're unable to adopt yet.
  • This partially addresses https://github.com/protocolbuffers/protobuf/issues/5587 - the core motivation behind this issue is that when piping a FileDescriptorSet into protoc with --descriptor_set_in, and then invoking one of the built-in plugins, different code can result, as the value for json_name will be subtly different when taking an external FileDescriptorSet vs protoc producing one internally and then invoking one of the built-in plugins.
  • Selfishly, it makes life a lot easier for alternative Protobuf compilers. For example, for us with buf, to use the built-in plugins, we have a convoluted mechanism to proxy to protoc that cannot deal with issue #5587, and requires people to install both protoc and buf when they may just want one of the built-in plugins. Having separate, independently-versionable plugins would make our lives a lot easier.

The majority of this PR is something we have already been maintaining for our remote plugins feature within Buf, we're just hoping we can get this upstreamed, as we think it would benefit the Protobuf community.

Follow-ups that would be nice:

  • Adding to GitHub releases, as mentioned.
  • Get these plugins installed as part of the Homebrew protobuf formula. There's precedent for this with i.e. the grpc formula, which installs each language-specific plugin.
  • Change protoc to default to using an on-disk plugin vs a built-in plugin, that is if protoc-gen-cpp exists, use protoc-gen-cpp over the builtin cpp plugin. This allows people to independently version plugins as mentioned, and mirrors the behavior of the Well-Known Types (if a WKT is in the include path, it is used instead of the WKTs included as part of the protoc distribution). We're happy to put up a PR for this as well.

jchadwick-buf avatar Jul 13 '23 17:07 jchadwick-buf

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Jul 13 '23 17:07 google-cla[bot]

CLA has been signed.

ghost avatar Jul 13 '23 18:07 ghost

Optimally, people want to version their plugins separately from their compiler. That is, the version of protoc should be decoupled from the version of each plugin, as is the case with the rest of the Protobuf ecosystem outside of the built-in plugins. You should be able to, for example, take advantage of fixes in protoc v24.0 whilst continuing to use the Java generated code from v23.0, in case there are breaking changes you're unable to adopt yet.

Can you clarify if you are referring to the same language in v23 and v24 here, or do you mean something like "you want v24 java, but v23 C++"?

fowles avatar Jul 18 '23 16:07 fowles

Sorry, should have clarified here:

  • I don't think this PR is intending to make it so that v23 java and v24 java can be generated into the same codebase, that's a separate issue entirely heh.
  • It would be nice to easily be able to do v23 java and v24 c++ in the same monorepo as an example, something that is more difficult now if you have a single point of entry for protoc.
  • It would also be nice if the core compiler functionality of protoc would be able to work with older plugins, so for example there's a bug fix in the compiler of protoc in v24, but I continue to use protoc-gen-java v23.

Does that make sense?

bufdev avatar Jul 18 '23 17:07 bufdev

Sorry this has lingered for so long. There are some internal things that are moving around that make this direction look really promising, so odds are strong we will take this eventually, but moving the behemoth takes time :)

fowles avatar Sep 08 '23 15:09 fowles

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment.

This PR is labeled inactive because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Dec 08 '23 10:12 github-actions[bot]

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active or becomes active again, please reopen it.

This PR was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

github-actions[bot] avatar Dec 23 '23 10:12 github-actions[bot]

Oops, this PR probably shouldn't be closed. I could of course just open a new PR later, but the relevant discussion is probably worth keeping.

ghost avatar Jan 02 '24 17:01 ghost

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment.

This PR is labeled inactive because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Apr 02 '24 10:04 github-actions[bot]

This is still coming along internally.

googleberg avatar Apr 02 '24 14:04 googleberg

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment.

This PR is labeled inactive because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Jul 02 '24 10:07 github-actions[bot]

Bump to unmark inactive.

ghost avatar Jul 02 '24 18:07 ghost

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment.

This PR is labeled inactive because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Oct 01 '24 10:10 github-actions[bot]

Bumping again. I should probably rebase/update this too.

ghost avatar Oct 01 '24 14:10 ghost

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment.

This PR is labeled inactive because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Jan 10 '25 10:01 github-actions[bot]

Still relevant, needs to be rebased though.

ghost avatar Jan 15 '25 20:01 ghost

@jchadwick-buf - can you rebase this PR?

JasonLunn avatar Feb 03 '25 22:02 JasonLunn

Sorry for the delay. I have been a bit busy.

I've rebased. I see the Kotlin generator moved but otherwise not much needed to be changed, seems to be building. Please let me know if there's anything else that needs to be done.

ghost avatar Mar 26 '25 23:03 ghost

Looks close! The last test run looks like it only failed due to comparison with goldens:

Installed files do not match goldens!
If this is expected, please update the golden file: cmake/installed_bin_golden.txt
The following diffs were found:
--- cmake/installed_bin_golden.txt	2025-03-26 23:42:40.086630065 +0000
+++ static/bin.txt	2025-03-26 23:42:41.302627899 +0000
@@ -1,3 +1,13 @@
 protoc
+protoc-gen-cpp
+protoc-gen-csharp
+protoc-gen-java
+protoc-gen-kotlin
+protoc-gen-objectivec
+protoc-gen-php
+protoc-gen-pyi
+protoc-gen-python
+protoc-gen-ruby
+protoc-gen-rust
 protoc-gen-upb
 protoc-gen-upbdefs

JasonLunn avatar Mar 27 '25 14:03 JasonLunn

Thanks, I've applied those changes.

ghost avatar Apr 02 '25 17:04 ghost

I see that the CL failed internally. Let me know if there's anything I should do to help make this pass in google3; otherwise, I'll cross my fingers that it isn't anything too ridiculous and leave it to y'all.

ghost avatar Apr 04 '25 16:04 ghost