-
Notifications
You must be signed in to change notification settings - Fork 4k
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
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 successfully I 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 bazel/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/VanillaJavaBuilder.java Line 179 in 7461f44
|
@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