New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 693079 link

Starred by 3 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Feature

Blocking:
issue 921422



Sign in to add a comment

Manage java versions via DEPS

Project Member Reported by jbudorick@chromium.org, Feb 16 2017

Issue description

Java updates are very difficult to handle consistently at the moment. We should switch to DEPS-based management of the java tools needed to build chromium.

Additionally, relying on a system javac can cause inconsistent binary size ( bug 921422 ), and even inconsistent build failures (bug 921945).

As a mitigation, we are now using //third_party/errorprone for all compiles. Having javac available as well would improve build speeds.
 
Components: Infra>Client>Chrome
Project Member

Comment 2 by sheriffbot@chromium.org, Jul 18

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Recharge-Cold
Status: Available (was: Untriaged)
I'm guessing it's still relevant, but probably not urgent enough (?)

Is there any other context for this bug? Like docs, links to code, specific problems with the current approach, level of urgency? (so someone not intimately familiar with the system can have a starting point).
Definitely still relevant, but P2 is likely appropriate.

Context: we want our builds to be hermetic, which is why we download many many other tools and toolchains as part of a typical gclient sync (for android, that includes clang, all of the android sdk build tools, etc).
Blocking: 921422
Cc: wnwen@chromium.org
Diffing javac versions are causing inconsistent build results on perf bots. Setting this as a blocking bug, although it's also fixable by updating the version of java on those bots.

Coincidentally, I also just has a CL make it through CQ but fail on some waterfall bots due to a javac warning (bug 921945). Only way I can think of for this to happen is differing javac versions.

One theory as to why I've run into two problems recently and none for a long time before, is that we recently switched from building with errorprone -> javac in order to improve compile times. I think this had a side-effect of making our java builds depend on system javac rather than a consistent compiler provided by errorprone. I think we should try reverting this change and see if it fixes the sawtooth in the blocking bug.
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28543d8fe6777cfa45a11cf7e028a4b86f89b9f0

commit 28543d8fe6777cfa45a11cf7e028a4b86f89b9f0
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed Jan 16 00:36:53 2019

Revert "Android: Parallelize Error Prone"

This reverts commit 3cc62d6500b11c4ac271832766140fc9000e0331.

Reason for revert: Our bots don't have a consistent javac version

Original change's description:
> Android: Parallelize Error Prone
>
> Run two compile_java steps in parallel, one without errorprone (~20s) to
> unblock subsequent build steps (desugar, bytecode rewriting, dexing),
> and one with errorprone (if enabled) to check the build (~40s).
>
> This saves approximately 19 seconds for the overall incremental build of
> chrome_java.
>
> Bug: 906803
> Change-Id: I4dee29c246bbc65791088dcc5e38d1030310c466
> Reviewed-on: https://chromium-review.googlesource.com/c/1346992
> Commit-Queue: Peter Wen <wnwen@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#610477}

Bug: 906803, 693079,  921422 
Change-Id: I3a09313563dd702c6fff4f351602d6f80bbf883e
Reviewed-on: https://chromium-review.googlesource.com/c/1412273
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622934}
[modify] https://crrev.com/28543d8fe6777cfa45a11cf7e028a4b86f89b9f0/build/android/gyp/javac.py
[modify] https://crrev.com/28543d8fe6777cfa45a11cf7e028a4b86f89b9f0/build/config/android/internal_rules.gni

I'll see if we can use what's recommended here to use the same javac as errorprone even when running it in parallel: https://errorprone.info/docs/installation#java-8
Of course, it'd temporary until we manage our own java versions via DEPS (this bug).

Comment 9 by agrieve@chromium.org, Jan 17 (6 days ago)

Great idea to use errorprone always!

The revert helped quite a bit, but third_party code disables errorprone, and the remaining sawtooth is entirely from code in third_party.

I'll reland the revert and then look at using errorprone in the javac case.

Also found that even though the .class files are the same now for chrome_java.jar, the META-INF/MANIFEST.MF differs, because that records the name of the compiler where the "jar" tool is from. We should just stop using "jar" in favour of zipping via python.
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 17 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fd9e2835636c89c2e177fbbf640f464b59e1f27b

commit fd9e2835636c89c2e177fbbf640f464b59e1f27b
Author: Peter Wen <wnwen@chromium.org>
Date: Thu Jan 17 04:17:11 2019

Reland "Android: Parallelize Error Prone"

Original CL: https://crrev.com/c/1346992

This is a simple reland as the revert was speculative and did not remove
perf bot issues.

Bug: 906803, 693079,  921422 
Change-Id: Iaddeaf90a40c261775885a1ef37d4320a2e39cd4
Reviewed-on: https://chromium-review.googlesource.com/c/1414875
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623572}
[modify] https://crrev.com/fd9e2835636c89c2e177fbbf640f464b59e1f27b/build/android/gyp/javac.py
[modify] https://crrev.com/fd9e2835636c89c2e177fbbf640f464b59e1f27b/build/config/android/internal_rules.gni

Comment 11 by wnwen@chromium.org, Jan 17 (6 days ago)

Agreed with not using jar anymore, python zipping is both faster and easier to understand/improve when it is a performance bottleneck.

Comment 12 by wnwen@chromium.org, Jan 17 (6 days ago)

Regarding using errorprone in the javac case, the structure of errorprone has changed dramatically (for us), since they no longer bundle everything together in an error_prone_ant.jar.

As you can see here, there is no 2.3.2 anymore: https://repo1.maven.org/maven2/com/google/errorprone/error_prone_ant

According to the docs we seem to need to download a couple more dependencies if we continue to download .jar files into third_party/errorprone/lib.
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 17 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7efc8d8255a7c809caba578c08204dbbb784ccf

commit a7efc8d8255a7c809caba578c08204dbbb784ccf
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu Jan 17 16:50:09 2019

Android: Create .jar files via Python rather than using "jar" tool

Makes builds more hermetic. The "jar" tools was adding a META-INF/MANIFEST.MF
containing the version of the jar tool (differs by machine).

Bug: 693079
Change-Id: I8a8bcaf64c47c47661e16de2a717c2de3320de96
Reviewed-on: https://chromium-review.googlesource.com/c/1416556
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623719}
[modify] https://crrev.com/a7efc8d8255a7c809caba578c08204dbbb784ccf/build/android/gyp/jar.py
[modify] https://crrev.com/a7efc8d8255a7c809caba578c08204dbbb784ccf/components/cronet/tools/api_static_checks.py
[modify] https://crrev.com/a7efc8d8255a7c809caba578c08204dbbb784ccf/components/cronet/tools/update_api.py

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 17 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21f79dc8a0c822a4e5bb676bd04e22b58974ab69

commit 21f79dc8a0c822a4e5bb676bd04e22b58974ab69
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu Jan 17 23:25:24 2019

Android: Use errorprone with checks disabled rather than javac

javac version changes based the host machine, and varying versions is
currently causing a sawtooth build size for MonochromePublic.apk

Bug: 693079,  921422 
Change-Id: I0b7a921bce86f0d88c56f29f47886d60578807f2
Reviewed-on: https://chromium-review.googlesource.com/c/1418898
Reviewed-by: Peter Wen <wnwen@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623903}
[modify] https://crrev.com/21f79dc8a0c822a4e5bb676bd04e22b58974ab69/build/android/gyp/javac.py
[modify] https://crrev.com/21f79dc8a0c822a4e5bb676bd04e22b58974ab69/build/config/android/internal_rules.gni

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 17 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/89794a7a6cd8e6a84d6f3c1df7184815ec09fe36

commit 89794a7a6cd8e6a84d6f3c1df7184815ec09fe36
Author: Thomas Anderson <thomasanderson@chromium.org>
Date: Thu Jan 17 23:31:26 2019

Revert "Android: Use errorprone with checks disabled rather than javac"

This reverts commit 21f79dc8a0c822a4e5bb676bd04e22b58974ab69.

Reason for revert: breaks build on android-rel: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/android-rel/7111

Original change's description:
> Android: Use errorprone with checks disabled rather than javac
> 
> javac version changes based the host machine, and varying versions is
> currently causing a sawtooth build size for MonochromePublic.apk
> 
> Bug: 693079,  921422 
> Change-Id: I0b7a921bce86f0d88c56f29f47886d60578807f2
> Reviewed-on: https://chromium-review.googlesource.com/c/1418898
> Reviewed-by: Peter Wen <wnwen@chromium.org>
> Commit-Queue: Peter Wen <wnwen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#623903}

TBR=wnwen@chromium.org,agrieve@chromium.org

Change-Id: I586133a80c3c2e13834541a210277891242e2a34
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 693079,  921422 
Reviewed-on: https://chromium-review.googlesource.com/c/1419301
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623905}
[modify] https://crrev.com/89794a7a6cd8e6a84d6f3c1df7184815ec09fe36/build/android/gyp/javac.py
[modify] https://crrev.com/89794a7a6cd8e6a84d6f3c1df7184815ec09fe36/build/config/android/internal_rules.gni

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 18 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/999d2d0bbcb798d24840308486f8c1adc223a4c8

commit 999d2d0bbcb798d24840308486f8c1adc223a4c8
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Jan 18 02:56:07 2019

Reland "Android: Use errorprone with checks disabled rather than javac"

javac version changes based the host machine, and varying versions is
currently causing a sawtooth build size for MonochromePublic.apk

Reverted in: 89794a7a6cd8e6a84d6f3c1df7184815ec09fe36.

Reason for reland: Fixed missing dep.

TBR=wnwen@chromium.org

Change-Id: I9244fd4324b2f545d2b12ce1396e47ced2c812b1
Bug: 693079,  921422 
Reviewed-on: https://chromium-review.googlesource.com/c/1419202
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623974}
[modify] https://crrev.com/999d2d0bbcb798d24840308486f8c1adc223a4c8/build/android/gyp/javac.py
[modify] https://crrev.com/999d2d0bbcb798d24840308486f8c1adc223a4c8/build/config/android/internal_rules.gni

Comment 17 by agrieve@chromium.org, Jan 18 (5 days ago)

Description: Show this description
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 18 (5 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb8cc32f17b408726cee347ff838d13f805bbff7

commit eb8cc32f17b408726cee347ff838d13f805bbff7
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Jan 18 15:38:40 2019

Android: Always compile with errorprone (even for debug)

Turns out differing system javac versions not only result in binary size
variations, but also cause compiles to fail sometimes.

Bug: 693079
Change-Id: I76b7d7348dff46be35e6fa996457bb2d9ffc3401
Reviewed-on: https://chromium-review.googlesource.com/c/1420307
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Auto-Submit: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624135}
[modify] https://crrev.com/eb8cc32f17b408726cee347ff838d13f805bbff7/build/config/android/internal_rules.gni

Sign in to add a comment