Bump error_prone version to 2.5.1 #13016
Conversation
Also update transitive dependencies. Change-Id: I7b35948d1c6c7ba7f1856405a7d4f2d0be76b9af
|
The procedure to bump https://github.com/davido/java_tools/releases/download/v12.1/java_tools-v12.1.zip and created this diff in bazel tree: diff --git a/distdir_deps.bzl b/distdir_deps.bzl
index 1739a25c2a..1f29d5cc11 100644
--- a/distdir_deps.bzl
+++ b/distdir_deps.bzl
@@ -264,11 +264,10 @@ DIST_DEPS = {
},
"remote_java_tools": {
"aliases": ["remote_java_tools_test", "remote_java_tools_for_testing"],
- "archive": "java_tools-v11.1.zip",
- "sha256": "12cffbb7c87622a6bd6e9231e81ecb9efdb118afbdd6e047ef06eeb3d72a7dc3",
+ "archive": "java_tools-v12.1.zip",
+ "sha256": "0504b2ea80179652121bf50f38c5c4c4bfdf9a4b357f0338bcbc60f1d43ad17f",
"urls": [
- "https://mirror.bazel.build/bazel_java_tools/releases/java/v11.1/java_tools-v11.1.zip",
- "https://github.com/bazelbuild/java_tools/releases/download/java_v11.1/java_tools-v11.1.zip",
+ "https://github.com/davido/java_tools/releases/download/v12.1/java_tools-v12.1.zip",
],
"used_in": [
"additional_distfiles",After activating all new EP checks from release: 2.5.0 [1], I could confirm that new bug patterns are flagged by EP, e.g.: $ bazel build :release
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
WARNING: Option 'java_toolchain' is deprecated
INFO: Invocation ID: e48a6477-47e9-434f-ad01-09339f56ae68
DEBUG: /home/davido/.cache/bazel/_bazel_davido/5c01f4f713b675540b8b424c5c647f63/external/bazel_toolchains/rules/rbe_repo/version_check.bzl:59:14:
Current running Bazel is not a release version and one was not defined explicitly in rbe_autoconfig target. Falling back to '4.0.0'
INFO: Analyzed target //:release (1324 packages loaded, 46697 targets configured).
INFO: Found 1 target...
ERROR: /home/davido/projects/gerrit2/java/com/google/gerrit/extensions/BUILD:29:13: Building java/com/google/gerrit/extensions/libapi.jar (346 source files) and running annotation processors (AutoAnnotationProcessor, AutoValueProcessor, AutoOneOfProcessor) failed: (Exit 1): java failed: error executing command external/remotejdk11_linux/bin/java '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED' ... (remaining 14 argument(s) skipped)
java/com/google/gerrit/extensions/common/FileInfo.java:37: error: [ObjectEqualsForPrimitives] Avoid unnecessary boxing by using plain == for primitive types.
&& Objects.equals(sizeDelta, fileInfo.sizeDelta)
^
(see https://errorprone.info/bugpattern/ObjectEqualsForPrimitives)
Did you mean '&& (sizeDelta == fileInfo.sizeDelta)'?
java/com/google/gerrit/extensions/common/FileInfo.java:38: error: [ObjectEqualsForPrimitives] Avoid unnecessary boxing by using plain == for primitive types.
&& Objects.equals(size, fileInfo.size);
^
(see https://errorprone.info/bugpattern/ObjectEqualsForPrimitives)
Did you mean '&& (size == fileInfo.size);'?
Target //:release failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 61.787s, Critical Path: 24.53s
INFO: 29 processes: 6 internal, 14 linux-sandbox, 9 worker.
FAILED: Build did NOT complete successfullyI even fixed some of them already: [2]. [1] https://gerrit-review.googlesource.com/c/gerrit/+/296780 |
To test Java 16 toolchain, I conducted new version of java_tools. It turns out, that the new EP 2.5.1 is more stricter. Particularly, In the past there were concerns that unconditional upgrade of EP would mean disruptive change, so that I could imagine that other Bazel users would also complain. [1] https://bugs.chromium.org/p/gerrit/issues/detail?id=14302 |
|
I don't think that should have been enabled as an error by default, I'll look at turning it off. It's of course possible to disable with |
+1.
That's unfortunate, as my Java 16 Toolchain support PR is waiting to be reviewed/approved and released in |
I believe you can add them to default_java_toolchain.bzl. It is used everywhere. |
Thanks. I couldn't remember if that would be a problem for |
|
@cushon, there are more downstream failures due to errorprone upgrade. I made java_tools rc1; didn't release it yet. Here are downstream tests: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/1994#8ab8848c-ea09-4d5a-a7ca-344b0cc30860 |
|
Looking into Kythe breakage it seems that the EP upgrade is actually broken with NCFE: |
|
Yea, while 2.6.0 includes this CL: google/error-prone@d1b93d4, where checker framework version was bumped to 3.11.0, the upgrade CL: 22c8fc3 missed to upgrade the checker framework to 3.11.0. It's still Hm... just noticed, that this PR, that was actually abandoned, did update |
Thanks. Meantime, I filed this issue: #13346, may be update the commit message in your PR? Moreover, comparing this PR to the merged PRs, other transitive dependencies were missed to be upgraded as well. I compared pom.xml in EP with third-party directory in Bazel: auto/auto-service-1.0-rc7.jar Not sure it's required, though. |
Done
Upgrading these too is probably a good idea, but I suspect there weren't any breaking changes that matter for Error Prone, especially at runtime, since both of those are annotation processors. |
#13016 (comment) Closes #13348. Signed-off-by: Philipp Wollermann <philwo@google.com>
@comius I think there's just the one breakage in We can also disable that check globally, LMK if you have a preference. |
This is perfect! Thank @cushon. |
Also update transitive dependencies.
Change-Id: I7b35948d1c6c7ba7f1856405a7d4f2d0be76b9af