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

Incompatibility between generated files with protoc 3.1.0 and runtime deps com.google.protobuf:protobuf-java:3.5.1 #4314

Closed
bogdandrutu opened this issue Feb 15, 2018 · 9 comments · Fixed by #4504

Comments

@bogdandrutu
Copy link

Here is a commit that shows the problem:
bogdandrutu/cloud-trace-java@ad047fc

The proto file is generated using protoc:3.1.0 and the tests are run with com.google.protobuf:protobuf-java:3.5.1

The error that I get:
Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.02 sec <<< FAILURE!
getCurrent(com.google.cloud.trace.service.AppEngineSpanContextHandleTest) Time elapsed: 0.02 sec <<< ERROR!
java.lang.UnsupportedOperationException
at com.google.protos.cloud.trace.TraceId$TraceIdProto.dynamicMethod(TraceId.java:319)
at com.google.protobuf.GeneratedMessageLite.dynamicMethod(GeneratedMessageLite.java:332)
at com.google.protobuf.GeneratedMessageLite.isInitialized(GeneratedMessageLite.java:1458)
at com.google.protobuf.GeneratedMessageLite.isInitialized(GeneratedMessageLite.java:254)
at com.google.protobuf.GeneratedMessageLite.checkMessageInitialized(GeneratedMessageLite.java:1596)
at com.google.protobuf.GeneratedMessageLite.parseFrom(GeneratedMessageLite.java:1677)
at com.google.protos.cloud.trace.TraceId$TraceIdProto.parseFrom(TraceId.java:123)
at com.google.cloud.trace.service.AppEngineSpanContextHandle.getSpanContext(AppEngineSpanContextHandle.java:60)
at com.google.cloud.trace.service.AppEngineSpanContextHandle.getCurrentSpanContext(AppEngineSpanContextHandle.java:43)
at com.google.cloud.trace.service.AppEngineSpanContextHandleTest.getCurrent(AppEngineSpanContextHandleTest.java:63)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
at org.junit.runners.ParentRunner.run(ParentRunner.java:300)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)

@bogdandrutu
Copy link
Author

Also forgot to mention that this problem is related to the use of: optimize_for = LITE_RUNTIME. When that option is removed the tests pass without a problem.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Feb 15, 2018

optimize_for = LITE_RUNTIME is not supported by protobuf-java any more. To use lite runtime, use the protoc-gen-java-lite plugin with com.google.protobuf:protobuf-java-lite instead:
https://mvnrepository.com/artifact/com.google.protobuf/protoc-gen-javalite
https://mvnrepository.com/artifact/com.google.protobuf/protobuf-lite

With the plugin in PATH, run protoc with --javalite_out=... instead of --java_out=...

@bogdandrutu
Copy link
Author

@xfxyjwf sure, but what are we going to do with artifacts that were pushed to maven using non-lite version and because of how dependency management works in Java they pick a transient dependency on a newer protobuf-java and break their application?

@bogdandrutu
Copy link
Author

@xfxyjwf another question that I have is why the compilation does not fail if I set the lite option but I compile using the non-lite protoc?

This caused that we pushed artifacts to maven that include a version of the generated files using the non-lite protoc (3.1.0) and when these artifacts are used with protobuf-java (3.5.1) they crash at runtime.

I think what you should do is to crash the compilation if I try to compile the proto file which uses the lite option but the non-lite compiler. That way these kind of mistakes will not affect others.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Feb 15, 2018

If you use non-lite version everywhere, we guarantee the generated code of older protoc will work with newer version of protobuf-java runtime. So no problem there.

However, we no longer support 'optimize_for = LITE_RUNTIME' any more with protobuf-java so there is no guarantee of such compatibilities if you have 'optimize_for = LITE_RUNTIME' in your proto. And that's the exception you are seeing here.

And yes, I agree that we should start to disallow 'optimize_for = LITE_RUNTIME' for Java so you will get a compile failure instead of runtime failure.

@bogdandrutu
Copy link
Author

I think this issue should become a request to disallow 'optimize_for = LITE_RUNTIME' for Java.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Apr 13, 2018

It turns out we can't disallow 'optimize_for = LITE_RUNTIME' because the option is used by other languages as well. In #4504 I updated protoc to print a warning instead.

@ejona86
Copy link
Contributor

ejona86 commented Apr 13, 2018

@xfxyjwf, that puts us in a weird situation. If the option is required for other languages and broken for Java, then how are we supposed to resolve issues? It seems protoc should generate code for full protobuf and ignore the option for Java other than provide the warning.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Apr 17, 2018

@ejona86 Yes, we do need to change protoc to ignore 'optimize_for = LITE_RUNTIME' instead. Some cleanup needs to be done before we can switch away from the current 'generating broken code with a warning' situation. See b/77980691

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

Successfully merging a pull request may close this issue.

3 participants