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

Update error prone version to 2.3.4 #11271

Closed

Conversation

davido
Copy link
Contributor

@davido davido commented Apr 30, 2020

Fixes #11272 (first attempt that was reverted: #9286).

@davido
Copy link
Contributor Author

davido commented Apr 30, 2020

We are trying to activate additional error prone checks in Gerrit build: [1]. For this purpose I've conducted a new release for java_tools with new error prone version and published it here: [2]. Finally in this CL: [3] we are trying to switch using custom java_tools with bumped error prone version.

While doing this, we are running into multiple issues: [4] and: [5].

[5] is particularly annoying, because I have to specify java_toolchain to switch to custom java_tools release. But given that java_tools distribution contains native code, we cannot just add yet another fancy option to the WORKSPACE/.bazelrc file, and can only document to use different options on different platforms:

To build on Linux please do this:

  $ bazel build \
    --java_toolchain=@remote_java_tools_linux//:toolchain \
    --host_java_toolchain=@remote_java_tools_linux//:toolchain \
    :release

To build on Mac Os X please do that:

  $ bazel build \
    --java_toolchain=@remote_java_tools_darwin//:toolchain \
    --host_java_toolchain=@remote_java_tools_darwin//:toolchain \
    :release

[1] https://gerrit-review.googlesource.com/c/gerrit/+/235098
[2] https://github.com/davido/java_tools/releases/tag/javac11-v11.0
[3] https://gerrit-review.googlesource.com/c/gerrit/+/265493/4
[4] #8268
[5] #10031

//CC @cushon @meisterT.

davido added a commit to davido/rules_kotlin that referenced this pull request May 1, 2020
…nerated code

This is a reproducer to the error reported by new error prone version
2.3.4.

Note that due to this breakage the last attempt to upgrade Error Prone
in Bazel was reverted: [1], [2].

Test Plan:

On Linux run:

  $ bazel build \
      --java_toolchain=@remote_java_tools_linux//:toolchain
      --host_java_toolchain=@remote_java_tools_linux//:toolchain
      src/main/kotlin/...

Note, that this should be executed on Linux platform, because I have not
built custom java_tool with this CL included: [3] on Mac Os X yet.

[1] bazelbuild/bazel#10023
[2] bazelbuild/bazel@d990746
[3] bazelbuild/bazel#11271
@meisterT meisterT requested a review from meteorcloudy May 4, 2020 07:21
@meisterT
Copy link
Member

meisterT commented May 4, 2020

Yun, can you please have a look how this ties into your Debian efforts?

Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This library is not in Debian anyway. And it's better to depend on a formal version instead of a snapshot.

@davido
Copy link
Contributor Author

davido commented May 4, 2020

@meteorcloudy @meisterT Thanks for the feedback. Note, though, that this PR also adds new dependency to Bazel: Ben-Manes caffeine library: [1]. By chance I know, that Gerrit is also relying on the same version: 2.7.0, so that it should be already imported in Google's Monorepo.

But is there any special procedure/process to include new third party dependency in Bazel?

[1] https://github.com/ben-manes/caffeine

@meteorcloudy
Copy link
Member

Oh.. I see. Thanks for the notice. I checked the caffeine library is not in Debian. So this will add more difficulties for the effort of getting Bazel into Debian. Check #9408

Is it because the new error prone version requires this library? Besides, the checked in version is 2.8.0 in Google.

@meisterT
Copy link
Member

meisterT commented May 4, 2020

David, how large is the new dependency?

@davido
Copy link
Contributor Author

davido commented May 4, 2020

Is it because the new error prone version requires this library?

Sure. The dependency was added in EP tree in this CL: [1] and was updated to 2.7.0 in this CL: [2].

David, how large is the new dependency?

It's 846706 bytes:

$ wget https://repo1.maven.org/maven2/com/github/ben-manes/caffeine/caffeine/2.7.0/caffeine-2.7.0.jar
--2020-05-04 10:50:49--  https://repo1.maven.org/maven2/com/github/ben-manes/caffeine/caffeine/2.7.0/caffeine-2.7.0.jar
Connecting to 16.175.95.22:8080... connected.
Proxy request sent, awaiting response... 200 OK
Length: 846706 (827K) [application/java-archive]
Saving to: ‘caffeine-2.7.0.jar’

caffeine-2.7.0.jar                       100%[==================================================================================>] 826,86K  --.-KB/s    in 0,1s

2020-05-04 10:50:49 (5,45 MB/s) - ‘caffeine-2.7.0.jar’ saved [846706/846706]

[1] google/error-prone@bfb1789
[2] google/error-prone@f0428b2

@davido
Copy link
Contributor Author

davido commented May 4, 2020

Besides, the checked in version is 2.8.0 in Google.

@meteorcloudy Thanks for checking!

I have sent this PR: [1] to sync Caffeine version in EP tree to 2.8.0.

UPDATE Let's wait, if my PR: [1] is approved in EP, I will update this PR and bump Caffeine library to 2.8.0.

[1] google/error-prone#1602

@davido
Copy link
Contributor Author

davido commented May 14, 2020

@meteorcloudy

Besides, the checked in version is 2.8.0 in Google.

My PR: [1] was merged in EP, so that I will adapt this PR correspondingly.

[1] google/error-prone#1602

@davido davido force-pushed the bump_error_prone_version_to_2.3.4 branch from af99d21 to 9734be0 Compare May 14, 2020 06:05
@davido
Copy link
Contributor Author

davido commented May 14, 2020

My PR: [1] was merged in EP, so that I will adapt this PR correspondingly.

Done.

Fixes bazelbuild#11272 (first attempt that was reverted: bazelbuild#9286).
@davido davido force-pushed the bump_error_prone_version_to_2.3.4 branch from 9734be0 to cbfdeed Compare May 17, 2020 21:56
@davido
Copy link
Contributor Author

davido commented May 29, 2020

Superseded by #11426.

@davido davido closed this May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update error prone version to 2.4.0
4 participants