protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
65.5k stars 15.46k forks source link

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

Closed be-hase closed 1 month ago

be-hase commented 3 months ago

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

ejona86 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

@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 commented 3 months ago

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

epkugelmass commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

jchadwick-buf commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

emintz commented 2 months ago

Does 4.28.rc1 fix the problem?

emintz commented 2 months ago

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 commented 2 months ago

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 commented 1 month ago

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.

protocolbuffers/protobuf@6bf01c51a0b92278958f0169d330d64a08dbb4ec

googleberg commented 1 month ago

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 commented 1 month ago

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! :tada:

be-hase commented 1 month ago

I have confirmed that the issue has been fixed in my reproducible repository as well. https://github.com/protocolbuffers/protobuf/issues/17247#issuecomment-2192866635

zhangskz commented 1 month ago

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

Wyverald commented 1 month ago

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
bjoernmayer commented 1 month ago

FYI 27.4 was released

4.27.4 tagged artifacts are still missing though: https://mvnrepository.com/artifact/com.google.protobuf/protoc https://mvnrepository.com/artifact/com.google.protobuf/protobuf-kotlin https://mvnrepository.com/artifact/com.google.protobuf/protobuf-java https://mvnrepository.com/artifact/com.google.protobuf/protobuf-java-util

Second edit: They are there now

estomagordo commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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

be-hase commented 1 month ago

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

googleberg commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@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.