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 Guava to 28.1 #10394

Closed
wants to merge 2 commits into from
Closed

Conversation

tetromino
Copy link
Contributor

@tetromino tetromino commented Dec 10, 2019

To allow us to start using Duration with com.google.common.util.concurrent.

@tetromino tetromino force-pushed the guava-28.1 branch 2 times, most recently from d6b5815 to f2ae768 Compare December 10, 2019 01:47
@tetromino
Copy link
Contributor Author

tetromino commented Dec 17, 2019

@ahumesky and @jin - it looks like updating to modern Guava will require updating or patching third_party/android_common/com.android.tools_sdk-common_25.0.0.jar to use CharMatcher.javaLetter() instead of JAVA_LETTER (a deprecated beta API that got removed from Guava a year ago).

See https://storage.googleapis.com/bazel-untrusted-buildkite-artifacts/1a91facd-e9c1-4636-ac2f-828632639479/src/test/java/com/google/devtools/build/android/AndroidDataWriterTest/attempt_1.log

What would you recommend as the least bad way to update the jar? Or maybe we can just patch it?

@iirina iirina requested a review from ahumesky December 18, 2019 14:00
@jin
Copy link
Member

jin commented Dec 20, 2019

(as discussed offline, the simplest way to patch this)

@jin jin added the team-Android Issues for Android team label Dec 20, 2019
@aiuto
Copy link
Contributor

aiuto commented May 11, 2020

@tetromino Are you moving forward with this?

@davido
Copy link
Contributor

davido commented May 19, 2020

FTR: Upcoming Error Prone release 2.3.5 is also switched to using recent Guava release, so that this PR is also failing the Andoird tests and waiting for Guava upgrade in Bazel.

Interestingly non Andoird projects can be built and all tests are passing with PR.

@meisterT
Copy link
Member

meisterT commented Jul 2, 2020

@tetromino ping

@tetromino
Copy link
Contributor Author

I'm working on this at the moment. Sorry for the delay. (In other news, rebuilding old android_common jars is unnecessarily difficult.)

@tetromino tetromino force-pushed the guava-28.1 branch 3 times, most recently from 0bba344 to 1b2411a Compare July 16, 2020 03:34
tetromino added a commit to tetromino/bazel that referenced this pull request Jul 16, 2020
As a followup to the bundled Guava update in bazelbuild#10394

Due to historical repo/testing infrastructure setup, we could not update
non-third_party/* sources in the same PR as third_party/* jars.

This will be followed up by another PR removing old Guava 25.1 jars.

RELNOTES: None.
@tetromino
Copy link
Contributor Author

tetromino commented Jul 16, 2020

@jin @ahumesky - ready for review (buildkite/bazel-bazel-github-presubmit/rbe-ubuntu-16-dot-04-openjdk-8 is timing out for reasons unrelated to this PR: the //src/main/java/com/google/devtools/build/lib:gen_skylarklibrary genrule - required by //src/test/shell/bazel:bazel_docgen_test - has been repeatedly timing out in RBE for many people for the past 2 days; see internal bug b/161446091).

The follow-up update to scripts/bootstrap/compile.sh is in tetromino@7750ea9 (will make a PR from it once this one is committed).

@tetromino
Copy link
Contributor Author

@jin @ahumesky - ping for review? (See comments above about spurious rbe-ubuntu-16-dot-04-openjdk-8 failure.)

@aiuto
Copy link
Contributor

aiuto commented Aug 4, 2020

Friendly review ping

… modern Guava

com.android.tools_sdk-common_25.0.0.jar (and the stripped variant
derived from it) were built, afaict, against Guava version ~ 17.
In the years since, Guava removed several symbols used by the sdk-common
jar, which thus blocks updating Bazel to a more modern Guava version.

To fix, we have to patch the jar (and it stripped variant), which
involved a small exercize in software archaeology; the exact steps are
documented in third_party/android_common/README.md.

The patched jars should work with Guava 25.1 - 29.0.

RELNOTES: None.
To allow us to start using Duration with com.google.common.util.concurrent;
note that com.google.common.util.concurrent.internal.InternalFutureFailureAccess -
which we do use - has been split out into a separate jar by Guava upstream.

Due to historical repo/testing infrastructure setup, we cannot update
third_party jars in the same PR as non-third_party sources. We therefore
have to keep the old Guava 25.1 jars to allow scripts/bootstrap/compile.sh
(which explicitly references version 25.1) to continue to run, and to allow
//src/test/shell/bazel:bazel_bootstrap_distfile_test to continue to pass.

As an immediate followup to this PR, we will have to

1. update compile.sh to use Guava 29.0 (and the new failureaccess jar); and
2. remove Guava 25.1 jars.

RELNOTES: None.
@tetromino
Copy link
Contributor Author

Rebased since the rbe-ubuntu-16-dot-04-openjdk-8 buildkite failure was worked around by 1a528c8

@tetromino
Copy link
Contributor Author

Ping for review? @jin @ahumesky

Copy link
Member

@jin jin left a comment

Choose a reason for hiding this comment

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

Thanks, and sorry again for the delay.

@meteorcloudy
Copy link
Member

Sorry for the delay, I'm merge this PR now!

@bazel-io bazel-io closed this in ae230d2 Sep 14, 2020
@philwo
Copy link
Member

philwo commented Sep 14, 2020

Thank you, Yun! And sorry @ahumesky @tetromino, usually we don't need that long.. 😞 I'll check our processes for this and will see how we can ensure that this doesn't happen again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Android Issues for Android team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants