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

[Java] The release notes state that compatibility was restored in v27.2, but it is not compatible #17247

Closed
be-hase opened this issue Jun 26, 2024 · 30 comments
Assignees

Comments

@be-hase
Copy link
Contributor

be-hase commented Jun 26, 2024

Summary

When trying to use the code generated with protobuf v25.3 in a protobuf-java v4.27.2 environment, a compilation error occurs.

Cannot access com.google.protobuf.GeneratedMessageV3.Builder

CleanShot 2024-06-27 at 00 36 58

In v27.2, GeneratedMessageV3.Builder does not exist.

Links

@be-hase be-hase added the untriaged auto added to all issues by default when created. label Jun 26, 2024
@be-hase be-hase changed the title [Java] The release notes state that compatibility was restored in v27.2, but it is not compatible. [Java] The release notes state that compatibility was restored in v27.2, but it is not compatible Jun 26, 2024
@ejona86
Copy link
Contributor

ejona86 commented Jun 26, 2024

I'm suspecting this is an ABI incompatibility. When you compile generated code against v4.27.2 what can go into the class file is GeneratedMessage$Builder (since GeneratedMessageV3 extends GeneratedMessage). But if the generated code was compiled with v3.x, then the .class file will have GeneratedMessageV3$Builder which no longer exists.

@be-hase
Copy link
Contributor Author

be-hase commented Jun 27, 2024

Incidentally, an exception occurs at runtime in the reverse case(using the code generated with protobuf v27.2 in a protobuf-java v3.25.3 environment).
Moreover, regarding this, the protobuf team may not intentionally maintain forward compatibility.

@ejona86
Copy link
Contributor

ejona86 commented Jun 27, 2024

@be-hase, that would not be supported. Newer generated code can make use of newer runtime features. For basically all libraries in the ecosystem, downgrading dependencies is not directly supported.

@be-hase
Copy link
Contributor Author

be-hase commented Jun 27, 2024

@ejona86
I believe that libraries should always use the latest versions, so I have no problem with that.
(Of course, it would be nice to have forward compatibility if possible... haha)

@be-hase
Copy link
Contributor Author

be-hase commented Jun 27, 2024

I have prepared a reproduction repository, so please feel free to use it.
https://github.com/be-hase/protobuf-java-compatibility-issue

@epkugelmass
Copy link

What @be-hase noticed means that protobuf-java 4.27.1 is incompatible with grpc-java 1.64.0
See this reproducer: https://github.com/epkugelmass/protobuf-java-grpc-compat/tree/master

Even HealthCheckRequest.newBuilder().build() will fail to compile.

@zhangskz
Copy link
Member

zhangskz commented Jun 27, 2024

v4.27.2 should have re-established compatibility with v3.25.x gencode from source, but looks like additional shims will need to be added to further restore compatibility with v3.25.x gencode in jars compiled against 3.25.x runtime (per ejona86's prior comment).

Also can confirm that new gencode + old runtime is never allowed. You can see our full policies on this in https://protobuf.dev/support/cross-version-runtime-guarantee.

@be-hase
Copy link
Contributor Author

be-hase commented Jun 28, 2024

New Gencode + Old Runtime = Never Allowed

OK, I see.

v4.27.2 should have re-established compatibility with v3.25.x gencode from source, but looks like additional shims will need to be added to further restore compatibility with v3.25.x gencode in jars compiled against 3.25.x runtime (per ejona86's prior comment).

I am waiting for the shims to be added.

@pcj
Copy link
Contributor

pcj commented Jun 30, 2024

Another example (com/google/protobuf/GeneratedMessageV3$ExtendableMessageOrBuilder does not exist):

--protoc-gen-scala_out: java.lang.NoClassDefFoundError: com/google/protobuf/GeneratedMessageV3$ExtendableMessageOrBuilder
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1017)
	at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
	at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:862)
	at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:760)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:681)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:639)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:525)
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1017)
	at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
	at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:862)
	at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:760)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:681)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:639)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:525)
	at scalapb.options.Scalapb.<clinit>(Scalapb.java:23793)
	at scalapb.ScalaPbCodeGenerator$.registerExtensions(ScalaPbCodeGenerator.scala:13)
	at protocgen.CodeGenApp.run(CodeGenApp.scala:42)
	at protocgen.CodeGenApp.run$(CodeGenApp.scala:39)
	at scalapb.ScalaPbCodeGenerator$.run(ScalaPbCodeGenerator.scala:11)
	at protocgen.CodeGenApp.main(CodeGenApp.scala:27)
	at protocgen.CodeGenApp.main$(CodeGenApp.scala:26)
	at scalapb.ScalaPbCodeGenerator$.main(ScalaPbCodeGenerator.scala:11)
	at scalapb.ScalaPbCodeGenerator.main(ScalaPbCodeGenerator.scala)
Caused by: java.lang.ClassNotFoundException: com.google.protobuf.GeneratedMessageV3$ExtendableMessageOrBuilder
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:525)
	... 27 more

Using com.thesamet.scalapb:compilerplugin_2.12:0.11.17

@zhangskz zhangskz self-assigned this Jul 1, 2024
@zhangskz zhangskz added java 27.x and removed untriaged auto added to all issues by default when created. labels Jul 1, 2024
@zhangskz
Copy link
Member

zhangskz commented Jul 1, 2024

Thanks, we have a repro for this on our side and are aware that additional shims would be needed for the other inner classes (incl. GeneratedMessageV3.ExtendableMessageOrBuilder) as well

holtskinner pushed a commit to GoogleCloudPlatform/generative-ai that referenced this issue Jul 1, 2024
…xes protobuf dep. issue (#829)

Fixes #490  

### Changelog

- [x] Remove Cloud SQL flavor from upstream, keeping Vertex AI Search
(now agent builder). Rationale: all user feedback so far has been for
the Agent Builder flavor. GCP already has multiple Cloud SQL RAG sample
apps, including the [DB Retrieval
App](https://github.com/GoogleCloudPlatform/genai-databases-retrieval-app)
and this [Jump Start
Solution](https://cloud.google.com/architecture/ai-ml/generative-ai-rag).
Removing the Cloud SQL flavor helps reduce duplicated code and
maintenance overhead in this repo. If we hear user feedback or requests
for the Cloud SQL flavor to return, I will re-add.
- [x] Swap in Agent Builder naming for all instances of Vertex AI
Search.
- [x] Upgrade model to Gemini 1.5 Flash. 
- [x] Java backend - commit `MavenWrapper`. This is [somewhat
contentious](https://stackoverflow.com/questions/47240546/should-the-mvnw-files-be-added-to-the-repository)
but will help speed up deployment for new users and make sure folks are
using the right Maven version.
- [x] Java backend - Fix [protobuf forward-breaking
change](protocolbuffers/protobuf#17247) by
switching all GCP client libs to the latest `libraries-bom`. Remove
specific versions of clients and dependencies. This seemed to resolve
the problem, which I believe was caused by multiple underlying and
incompatible protobuf versions.

---------
zeroasterisk pushed a commit to zeroasterisk/generative-ai that referenced this issue Jul 4, 2024
…xes protobuf dep. issue (GoogleCloudPlatform#829)

Fixes GoogleCloudPlatform#490  

### Changelog

- [x] Remove Cloud SQL flavor from upstream, keeping Vertex AI Search
(now agent builder). Rationale: all user feedback so far has been for
the Agent Builder flavor. GCP already has multiple Cloud SQL RAG sample
apps, including the [DB Retrieval
App](https://github.com/GoogleCloudPlatform/genai-databases-retrieval-app)
and this [Jump Start
Solution](https://cloud.google.com/architecture/ai-ml/generative-ai-rag).
Removing the Cloud SQL flavor helps reduce duplicated code and
maintenance overhead in this repo. If we hear user feedback or requests
for the Cloud SQL flavor to return, I will re-add.
- [x] Swap in Agent Builder naming for all instances of Vertex AI
Search.
- [x] Upgrade model to Gemini 1.5 Flash. 
- [x] Java backend - commit `MavenWrapper`. This is [somewhat
contentious](https://stackoverflow.com/questions/47240546/should-the-mvnw-files-be-added-to-the-repository)
but will help speed up deployment for new users and make sure folks are
using the right Maven version.
- [x] Java backend - Fix [protobuf forward-breaking
change](protocolbuffers/protobuf#17247) by
switching all GCP client libs to the latest `libraries-bom`. Remove
specific versions of clients and dependencies. This seemed to resolve
the problem, which I believe was caused by multiple underlying and
incompatible protobuf versions.

---------
copybara-service bot pushed a commit to tink-crypto/tink-java that referenced this issue Jul 18, 2024
Protobuf 4.x introduced a breaking change. Version 27.2 introduced a set of stubs to make proto backward compatible (https://github.com/protocolbuffers/protobuf/releases/tag/v27.2) but this seems to be incomplete (protocolbuffers/protobuf#17247).

#31

PiperOrigin-RevId: 653514325
Change-Id: I10ac3cab3a8856864052619569e6be0492dca274
morambro added a commit to morambro/tink-java-1.14.1 that referenced this issue Jul 18, 2024
Protobuf 4.x introduced a breaking change. Version 27.2 introduced a set of stubs to make proto backward compatible (https://github.com/protocolbuffers/protobuf/releases/tag/v27.2) but this seems to be incomplete (protocolbuffers/protobuf#17247).

tink-crypto#31

PiperOrigin-RevId: 653514325
Change-Id: I10ac3cab3a8856864052619569e6be0492dca274
(cherry picked from commit 9fdb4e2)
@jchadwick-buf
Copy link
Contributor

I know it hasn't been that long, but it looks like another protobuf release has occurred and it doesn't seem like this issue is resolved yet. Are improvements to gencode backwards compatibility still on-track to hit a v27.x release, or are there blockers preventing this issue from being resolved?

@zhangskz
Copy link
Member

Improvements to gencode backward compatibilities are still being worked on and are planned for a v27.x release -- v27.3 made some unrelated fixes and doesn't include compat improvements yet. More extensive shims will be needed for binary compatibility with jars compiled against 25.x runtime.

@emintz
Copy link

emintz commented Aug 3, 2024

Could you please provide a projected release date? My clients are waiting for the fix.

@emintz
Copy link

emintz commented Aug 3, 2024

Does 4.28.rc1 fix the problem?

skewalramani-oss pushed a commit to skewalramani-oss/generative-ai that referenced this issue Aug 3, 2024
…xes protobuf dep. issue (GoogleCloudPlatform#829)

Fixes GoogleCloudPlatform#490  

### Changelog

- [x] Remove Cloud SQL flavor from upstream, keeping Vertex AI Search
(now agent builder). Rationale: all user feedback so far has been for
the Agent Builder flavor. GCP already has multiple Cloud SQL RAG sample
apps, including the [DB Retrieval
App](https://github.com/GoogleCloudPlatform/genai-databases-retrieval-app)
and this [Jump Start
Solution](https://cloud.google.com/architecture/ai-ml/generative-ai-rag).
Removing the Cloud SQL flavor helps reduce duplicated code and
maintenance overhead in this repo. If we hear user feedback or requests
for the Cloud SQL flavor to return, I will re-add.
- [x] Swap in Agent Builder naming for all instances of Vertex AI
Search.
- [x] Upgrade model to Gemini 1.5 Flash. 
- [x] Java backend - commit `MavenWrapper`. This is [somewhat
contentious](https://stackoverflow.com/questions/47240546/should-the-mvnw-files-be-added-to-the-repository)
but will help speed up deployment for new users and make sure folks are
using the right Maven version.
- [x] Java backend - Fix [protobuf forward-breaking
change](protocolbuffers/protobuf#17247) by
switching all GCP client libs to the latest `libraries-bom`. Remove
specific versions of clients and dependencies. This seemed to resolve
the problem, which I believe was caused by multiple underlying and
incompatible protobuf versions.

---------
@emintz
Copy link

emintz commented Aug 5, 2024

The answer to my previous question is NO, 4.28.rc1 does not fix the problem. From what I see on GitHub, version 4.29.0 will.

I'm trying to compile it locally without success. Could you please advise?

@jchadwick-buf
Copy link
Contributor

Firstly, please be mindful that when you reply to this issue it is notifying all of us following it. (I realize I'm doing that too, but alas.)

Secondly, I do not think the compatibility shims have been improved yet. e.g. GeneratedMessageV3 hasn't changed as of two months ago on main: https://github.com/protocolbuffers/protobuf/blob/main/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java - as far as I can tell, there is nothing you can build yet that will give you v3 gencode compatibility, even unreleased.

Finally, the latest status update suggests that the work to fix this is not blocked and is slated to come in a future v27.x release. There is no ETA right now.

Unfortunately, I think we'll just need to be patient for now. A future v27 protobuf release should give us a clean path forward.

@yu-shiba
Copy link

Good news, it looks like shim was added in v28.0-rc3.

Binary compatibility shims for GeneratedMessageV3, SingleFieldBuilderV3, RepeatedFieldBuilderV3, and their nested classes to restore binary compatibility with <=v3.x.x generated code built against v3.x.x prior to v4.26.0 breaking release.

6bf01c5

@googleberg
Copy link
Member

Hello Community,

Yes, we've added more-complete shims in PBJ 4.28.0-rc3.
We will also be cherrypicking the shims into a new patch release 4.27.4 (coming soon).

If possible please try out the 4.28.0-rc3 runtime with your PBJ 3.x gencode at your earliest convenience and let us know if you discover incompatibilities that we've missed during testing.

Thank you for your patience!
-Jerry

@ejona86
Copy link
Contributor

ejona86 commented Aug 23, 2024

Looks good in my testing with gRPC! I don't think we mix protos from different sources into one message tree, so maybe there could be something lurking there. But look great from what I can tell! 🎉

@be-hase
Copy link
Contributor Author

be-hase commented Aug 23, 2024

I have confirmed that the issue has been fixed in my reproducible repository as well.
#17247 (comment)

@zhangskz
Copy link
Member

We should be releasing v4.27.4 and v4.28.0 with the patch this week as well.

@Wyverald
Copy link
Contributor

I'm probably holding this wrong, but my attempt to get Bazel itself to use the latest protobuf didn't work.

Since no PR was sent to the Bazel Central Registry for protobuf 28.0-rc3, I had to use a git override to point it at the tag. But the error message says:

ERROR: no such package '@@[unknown repo 'maven' requested from @@protobuf~]//': The repository '@@[unknown repo 'maven' requested from @@protobuf~]' could not be resolved: No repository visible as '@maven' from repository '@@protobuf~'
ERROR: /private/var/tmp/_bazel_wyv/01cd51505ca764e566d7f93a5254e0db/external/protobuf~/java/util/BUILD.bazel:8:13: no such package '@@[unknown repo 'maven' requested from @@protobuf~]//': The repository '@@[unknown repo 'maven' requested from @@protobuf~]' could not be resolved: No repository visible as '@maven' from repository '@@protobuf~' and referenced by '@@protobuf~//java/util:util'
ERROR: Analysis of target '//src:bazel' failed; build aborted: Analysis failed

which indeed shows that the MODULE.bazel file in the 28.x branch still doesn't contain proper references to its maven dependencies.

I then tried 29.0-dev (basically tip-of-tree), and still got the same error message as before:

ERROR: /private/var/tmp/_bazel_wyv/01cd51505ca764e566d7f93a5254e0db/external/grpc-java~/protobuf/BUILD.bazel:3:13: Building external/grpc-java~/protobuf/libprotobuf.jar (6 source files) failed: (Exit 1): java failed: error executing Javac command (from target @@grpc-java~//protobuf:protobuf) external/rules_java~~toolchains~remotejdk21_macos_aarch64/bin/java '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ... (remaining 19 arguments skipped)
external/grpc-java~/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java:190: error: cannot access Builder
        .setCode(status.getCode().value());
        ^
  class file for com.google.protobuf.GeneratedMessageV3$Builder not found
Target //src:bazel failed to build

@estomagordo
Copy link

I am not having success with either 4.27.4 or 4.28.0.

Can others verify that the issue is resolved for them in these releases?

@ejona86
Copy link
Contributor

ejona86 commented Sep 3, 2024

@bjoernmayer, mvnrepository.com has no relationship with Maven Central. The site is not an authority for something being available. search.maven.org/central.sonatype.com are much better about indexing quickly, but even then stuff can show up in repo1 and be downloadable before it is indexed.

@estomagordo, both 4.27.4 and 4.28.0 worked fine for me.

Testing against different versions of proto-google-common-protos, it seems 4.27.4/4.28 is at least partly compatible back to 21.8 (Oct 2022). 21.7 fails (NoSuchMethodError: makeExtensionsImmutable()) and 21.9 succeeds.

@emintz
Copy link

emintz commented Sep 3, 2024

Version 4.28.0 from the Maven Repository works with the latest Cloud Pub/Sub Java library. Thank you so much.

@be-hase
Copy link
Contributor Author

be-hase commented Sep 4, 2024

I think it's okay to close this issue. thanks. :)

@googleberg
Copy link
Member

Excellent.

Thank you @be-hase, @ejona86 , @bjoernmayer, and @emintz for verifying.
Big thank you to @zhangskz who made the fixes happen.

Happy buffering.

@mjones-vsat
Copy link

mjones-vsat commented Sep 4, 2024

Certainly solved the majority of the issues I was seeing, thanks! Out of curiosity, does this look like the same thing? We are getting it with 4.28.0 off Central with Android instrumented tests.

Exception in thread "grpc-default-executor-79" java.lang.NoSuchMethodError: 'void com.google.testing.platform.proto.api.core.TestSuiteResultProto$TestSuiteMetaData.makeExtensionsImmutable()'
    at com.google.testing.platform.proto.api.core.TestSuiteResultProto$TestSuiteMetaData.<init>(TestSuiteResultProto.java:2759)
    at com.google.testing.platform.proto.api.core.TestSuiteResultProto$TestSuiteMetaData.<init>(TestSuiteResultProto.java:2674)
    at com.google.testing.platform.proto.api.core.TestSuiteResultProto$TestSuiteMetaData$1.parsePartialFrom(TestSuiteResultProto.java:3553)
    at com.google.testing.platform.proto.api.core.TestSuiteResultProto$TestSuiteMetaData$1.parsePartialFrom(TestSuiteResultProto.java:3547)
    at com.google.protobuf.AbstractParser.parsePartialFrom(AbstractParser.java:77)
    at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:97)
    at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:102)
    at com.google.protobuf.AbstractParser.parseFrom(AbstractParser.java:25)
    at com.google.protobuf.Any.unpack(Any.java:119)
    at com.android.build.gradle.internal.testing.utp.DdmlibTestResultAdapter.onTestResultEvent(DdmlibTestResultAdapter.kt:107)
    at com.android.build.gradle.internal.testing.utp.UtpTestUtilsKt$runUtpTestSuiteAndWait$2$postProcessCallback$1$1.onTestResultEvent(UtpTestUtils.kt:151)
    at com.android.build.gradle.internal.testing.utp.UtpTestUtilsKt$runUtpTestSuiteAndWait$testResultListener$1.onTestResultEvent(UtpTestUtils.kt:129)
    at com.android.build.gradle.internal.testing.utp.GradleAndroidTestResultListenerService$recordTestResultEvent$1.onNext(UtpTestResultListenerServer.kt:125)
    at com.android.build.gradle.internal.testing.utp.GradleAndroidTestResultListenerService$recordTestResultEvent$1.onNext(UtpTestResultListenerServer.kt:116)
    at io.grpc.stub.ServerCalls$StreamingServerCallHandler$StreamingServerCallListener.onMessage(ServerCalls.java:262)
    at io.grpc.internal.ServerCallImpl$ServerStreamListenerImpl.messagesAvailableInternal(ServerCallImpl.java:324)
    at io.grpc.internal.ServerCallImpl$ServerStreamListenerImpl.messagesAvailable(ServerCallImpl.java:309)
    at io.grpc.internal.ServerImpl$JumpToApplicationThreadServerStreamListener$1MessagesAvailable.runInContext(ServerImpl.java:833)
    at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37)
    at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:133)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
    at java.base/java.lang.Thread.run(Thread.java:833)

@ejona86
Copy link
Contributor

ejona86 commented Sep 4, 2024

@mjones-vsat, that looks similar to what I saw. Note protoc 21.8 (the oldest version that seemed to have some level of compatibility) fixed CVE-2022-3171. (Edit: seems it was 21.7 that fixed that CVE; off by one...) When you see that makeExtensionsImmutable() error, check if there's a newer release available for whichever library contains the generated code that threw the exception.

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

No branches or pull requests