fix serialization warnings in generated code when compiling with Java 18 and above #10561
Conversation
… version 18 and above Java 18 added additional linter checks as part of https://bugs.openjdk.java.net/browse/JDK-8274336 and https://bugs.openjdk.java.net/browse/JDK-8274335 These additional checks cause generated protobuf code to raise the following compiler warning: "non-transient instance field of a serializable class declared with a non-serializable type" This change fixes the code generation to annotate the generated fields with the necessary suppressions to avoid false positives. All the code generated from src/google/protobuf/*.proto now compiles successfully without serialization warnings in Java 18. fixes protocolbuffers#9673
|
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. |
| @@ -335,6 +335,7 @@ void ImmutableMapFieldGenerator::GenerateMembers(io::Printer* printer) const { | |||
| " $value_default_value$);\n" | |||
| "}\n"); | |||
| printer->Print(variables_, | |||
| "@SuppressWarnings(\"serial\")\n" | |||
There was a problem hiding this comment.
this may not actually be a false-positive, since I could not find any evidence that any MapField types are actually serializable.
There was a problem hiding this comment.
I believe that our custom serialization format is implemented in such a way that we dump a bunch of details into the serialization that are not actually used in the deserialization (which is really accomplished through proto deserialization). This is probably fine until we can fix the serialization format to drop things that shouldn't be there.
There was a problem hiding this comment.
sounds good, thanks for checking
| @@ -335,6 +335,7 @@ void ImmutableMapFieldGenerator::GenerateMembers(io::Printer* printer) const { | |||
| " $value_default_value$);\n" | |||
| "}\n"); | |||
| printer->Print(variables_, | |||
| "@SuppressWarnings(\"serial\")\n" | |||
There was a problem hiding this comment.
I believe that our custom serialization format is implemented in such a way that we dump a bunch of details into the serialization that are not actually used in the deserialization (which is really accomplished through proto deserialization). This is probably fine until we can fix the serialization format to drop things that shouldn't be there.
|
@fowles any details on what the aarch64 failure might be? |
Most likely a flake or bad sync point. Can you rebase onto tip of tree and I will rerun? |
|
I kicked off a rerun of the failing test, let's see if maybe that will fix it. I believe the test infrastructure will run tests with the latest |
|
@googleberg I didn't see this change included in the latest release. Is there anything else we need to do? I'm not very familiar with the process. |
|
@googleberg @acozzette @fowles I was hoping this would make it into one of the recent releases but I may not have followed the right process. Should I be submitting a new PR to cherry-pick this change into the 21.x branch to make sure it makes it into the next release? |
|
@xvrl this would naturally end up in the next minor-version release (3.22). We don't typically merge non-critical changes into patch releases. We are hoping to establish a more regular cadence of minor releases this year. |
|
👍 thanks for clarifying |
### What changes were proposed in this pull request? This pr aims upgrade `protobuf-java` from 3.21.12 to 3.22.0 ### Why are the changes needed? The new version bring some improvements like: - Use bit-field int values in buildPartial to skip work on unset groups of fields. (protocolbuffers/protobuf@2326aef) - Fix serialization warnings in generated code when compiling with Java 18 and above (protocolbuffers/protobuf#10561) - Enable Text format parser to skip unknown short-formed repeated fields. (protocolbuffers/protobuf@6dbd413) - Add serialVersionUID to ByteString and subclasses (protocolbuffers/protobuf#10718) and some bug fix like: - Mark default instance as immutable first to avoid race during static initialization of default instances. (protocolbuffers/protobuf#10770) - Fix Timestamps fromDate for negative 'exact second' java.sql.Timestamps (protocolbuffers/protobuf#10321) - Fix Timestamps.fromDate to correctly handle java.sql.Timestamps before unix epoch (protocolbuffers/protobuf#10126) - Fix bug in nested builder caching logic where cleared sub-field builders would remain dirty after a clear and build in a parent layer. protocolbuffers/protobuf#10624 The release notes as follows: - https://github.com/protocolbuffers/protobuf/releases/tag/v22.0 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Pass GitHub Actions Closes #40084 from LuciferYang/SPARK-42490. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Sean Owen <srowen@gmail.com>
fix serialization warnings in generated code when compiling with Java 18 and above
Java 18 added additional linter checks as part of
https://bugs.openjdk.java.net/browse/JDK-8274336 and https://bugs.openjdk.java.net/browse/JDK-8274335
These additional checks cause generated protobuf code to raise the following compiler warning: "non-transient instance field of a serializable class declared with a non-serializable type"
This change fixes the code generation to annotate the generated fields with the necessary suppressions to avoid false positives.
All the code generated from src/google/protobuf/*.proto now compiles successfully without serialization warnings in Java 18.
fixes #9673