Skip to content
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

Incompatible with protobuf v4 #579

Closed
jchadwick-buf opened this issue Jul 31, 2024 · 5 comments
Closed

Incompatible with protobuf v4 #579

jchadwick-buf opened this issue Jul 31, 2024 · 5 comments

Comments

@jchadwick-buf
Copy link

I know there was some discussion about this but now that #533 is closed without resolution it's unclear what the future holds.

The status has changed slightly since before. Protobuf v27.2 introduced some backwards compatibility with the older gencode, apparently allowing gRPC to function with Protobuf v27.2. However, this backwards compatibility doesn't extend to GeneratedMessageV3$Builder, thus the gencode used in cel-java runs into problems when using Protobuf v27.2. This is because the new shims give source compatibility with old gencode, but old gencode that is compiled against the old runtime will reference classes that still don't exist. It seems like a future version of protobuf may resolve this and offer a cleaner path forward, but it hasn't happened yet. See the discussion over here: protocolbuffers/protobuf#17247

Meanwhile, though, on May 23rd, Protobuf v27 actually gave people a reason to upgrade: Protobuf v27 is the first version of Protobuf that enables Protobuf Editions by default. While Editions is mostly backwards compatible, using Protobuf Editions in protobuf schemas requires new gencode, and new gencode requires the new runtime.

This whole situation is a bit of a mess. With Protobuf v27.3 having been cut a few hours ago with no sign of any improvements to gencode compatibility, we are at a bit of a standstill. I wonder if the gencode compatibility improvements are perhaps good enough that cel-java can update? It will be a breaking change, but at least there shouldn't be a hold up with grpc-java.

Can't imagine this is an exciting issue to have to deal with but it's one that I don't think can be ignored so forgive me for having to raise it.

For people searching, this error can manifest in two ways:

  • From source code depending on old bytecode gencode with old runtime. This will give a compile-time error: error: cannot access Builder: class file for com.google.protobuf.GeneratedMessageV3$Builder not found
  • From bytecode depending on old bytecode gencode with old runtime. This will give a class not found error at runtime instead.

(When compiling old gencode from source with Protobuf v27.2, this issue should not occur.)

@snazy
Copy link
Member

snazy commented Aug 1, 2024

@jchadwick-buf thanks for your detailed issue report!

We've already seen the "v4 problem" a while ago via #533 and the "JLBP-7 violation" - and it's sad to hear that it's still a promise to have an ABI compatible upgrade path.

Our own use cases depend on the cel-standalone artifact, which has the necessary shaded dependencies.

The biggest issue I've seen when I naively tried to "just bump" to v4 were surprisingly the CEL conformance tests, which also suffered that time from the v4-upgrade issue.

I think, we have to bump protobuf (and gRPC for the conformance tests) to some newer version. Let me give that a try in the next days.

@snazy
Copy link
Member

snazy commented Aug 5, 2024

Pre-requisite: #584

@snazy
Copy link
Member

snazy commented Aug 13, 2024

Fixed via #588

@snazy snazy closed this as completed Aug 13, 2024
@jchadwick-buf
Copy link
Author

Thanks for making quick work of this! I can confirm that it's working great so far.

@snazy
Copy link
Member

snazy commented Aug 15, 2024

Glad it help!

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

No branches or pull requests

2 participants