Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2666783004:
Android: Update docs to say Java 8 is required (Closed)
DescriptionAndroid: Update docs and install-build-deps-android.sh to require java8
We don't strictly require java8 yet, but will soon.
BUG=662467
Review-Url: https://codereview.chromium.org/2666783004
Cr-Commit-Position: refs/heads/master@{#455459}
Committed: https://chromium.googlesource.com/chromium/src/+/8638ef810c10cee09e467b2ecd678341e12fc892
Patch Set 1 #Patch Set 2 : Android: Update docs and install-build-deps-android.sh to require java8 #
Total comments: 9
Patch Set 3 : rework error message for java #Patch Set 4 : Android: Update docs and install-build-deps-android.sh to require java8 #
Total comments: 1
Patch Set 5 : rebase #
Total comments: 1
Depends on Patchset: Messages
Total messages: 25 (13 generated)
Description was changed from ========== Android: Update docs to say Java 8 is required This isn't true yet, but will be true soon enough. BUG=662467 ========== to ========== Android: Update docs and install-build-deps-android.sh to require java8 We don't strictly require java8 yet, but will soon. BUG=662467 ==========
agrieve@chromium.org changed reviewers: + jbudorick@chromium.org
The CQ bit was checked by agrieve@chromium.org to run a CQ dry run
On 2017/01/31 21:44:20, agrieve (mostly OOO till 31st) wrote: > The CQ bit was checked by mailto:agrieve@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... File build/install-build-deps-android.sh (right): https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... build/install-build-deps-android.sh:24: $1 -version 2>&1 | grep '1\.8' > /dev/null grep -q ? https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... build/install-build-deps-android.sh:57: sudo apt-get -y install openjdk-8-jre openjdk-8-jdk This will just fail on 14.04 or earlier. https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... build/install-build-deps-android.sh:60: # Assume java8 still not reporting being there means update-java-alternatives Doing this before any of the bots switch & when such a switch isn't imminent seems unwise. https://codereview.chromium.org/2666783004/diff/20001/docs/android_build_inst... File docs/android_build_instructions.md (left): https://codereview.chromium.org/2666783004/diff/20001/docs/android_build_inst... docs/android_build_instructions.md:117: Make also sure that OpenJDK 1.7 is selected as default: I think this section should still be there, just w/ 1.8 referenced instead of 1.7.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... File build/install-build-deps-android.sh (right): https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... build/install-build-deps-android.sh:24: $1 -version 2>&1 | grep '1\.8' > /dev/null On 2017/01/31 21:51:35, jbudorick wrote: > grep -q ? Done. https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... build/install-build-deps-android.sh:57: sudo apt-get -y install openjdk-8-jre openjdk-8-jdk On 2017/01/31 21:51:34, jbudorick wrote: > This will just fail on 14.04 or earlier. Expanded the error message. This also made me realize there's a -e at the top of the file, which I've moved to its own command to be more noticeable. https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... build/install-build-deps-android.sh:60: # Assume java8 still not reporting being there means update-java-alternatives On 2017/01/31 21:51:35, jbudorick wrote: > Doing this before any of the bots switch & when such a switch isn't imminent > seems unwise. Folded into an error message. Given that this isn't run by any bots (I don't think it is anyways...), I figured it'd be fine to update. Happy to sit on it as well though until the bot rollout if further along. https://codereview.chromium.org/2666783004/diff/20001/docs/android_build_inst... File docs/android_build_instructions.md (left): https://codereview.chromium.org/2666783004/diff/20001/docs/android_build_inst... docs/android_build_instructions.md:117: Make also sure that OpenJDK 1.7 is selected as default: On 2017/01/31 21:51:35, jbudorick wrote: > I think this section should still be there, just w/ 1.8 referenced instead of > 1.7. We instruct devs to run build/install-build-deps-android.sh two steps above. So long as the script checks for java8, I think it'd be more valuable to make our docs shorter.
lgtm but hold until we deploy java8 more widely on the bots. https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... File build/install-build-deps-android.sh (right): https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... build/install-build-deps-android.sh:60: # Assume java8 still not reporting being there means update-java-alternatives On 2017/02/01 02:39:21, agrieve wrote: > On 2017/01/31 21:51:35, jbudorick wrote: > > Doing this before any of the bots switch & when such a switch isn't imminent > > seems unwise. > > Folded into an error message. Given that this isn't run by any bots (I don't > think it is anyways...), I figured it'd be fine to update. Happy to sit on it as > well though until the bot rollout if further along. It isn't run on the bots, but I'd prefer that we update this around the same time that we update chromium.android + chromium.linux. https://codereview.chromium.org/2666783004/diff/60001/build/install-build-dep... File build/install-build-deps-android.sh (right): https://codereview.chromium.org/2666783004/diff/60001/build/install-build-dep... build/install-build-deps-android.sh:75: echo " sudo update-java-alternatives -s java-1.8.0-openjdk-amd64" ah, ok, with this in the error text, I'm ok with removing the references to update-alternatives from the markdown.
On 2017/02/01 16:30:39, jbudorick wrote: > lgtm but hold until we deploy java8 more widely on the bots. > > https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... > File build/install-build-deps-android.sh (right): > > https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... > build/install-build-deps-android.sh:60: # Assume java8 still not reporting being > there means update-java-alternatives > On 2017/02/01 02:39:21, agrieve wrote: > > On 2017/01/31 21:51:35, jbudorick wrote: > > > Doing this before any of the bots switch & when such a switch isn't imminent > > > seems unwise. > > > > Folded into an error message. Given that this isn't run by any bots (I don't > > think it is anyways...), I figured it'd be fine to update. Happy to sit on it > as > > well though until the bot rollout if further along. > > It isn't run on the bots, but I'd prefer that we update this around the same > time that we update chromium.android + chromium.linux. > > https://codereview.chromium.org/2666783004/diff/60001/build/install-build-dep... > File build/install-build-deps-android.sh (right): > > https://codereview.chromium.org/2666783004/diff/60001/build/install-build-dep... > build/install-build-deps-android.sh:75: echo " sudo update-java-alternatives > -s java-1.8.0-openjdk-amd64" > ah, ok, with this in the error text, I'm ok with removing the references to > update-alternatives from the markdown. I'm thinking this is good to go now. wdyt?
On 2017/03/08 01:46:48, agrieve wrote: > On 2017/02/01 16:30:39, jbudorick wrote: > > lgtm but hold until we deploy java8 more widely on the bots. > > > > > https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... > > File build/install-build-deps-android.sh (right): > > > > > https://codereview.chromium.org/2666783004/diff/20001/build/install-build-dep... > > build/install-build-deps-android.sh:60: # Assume java8 still not reporting > being > > there means update-java-alternatives > > On 2017/02/01 02:39:21, agrieve wrote: > > > On 2017/01/31 21:51:35, jbudorick wrote: > > > > Doing this before any of the bots switch & when such a switch isn't > imminent > > > > seems unwise. > > > > > > Folded into an error message. Given that this isn't run by any bots (I don't > > > think it is anyways...), I figured it'd be fine to update. Happy to sit on > it > > as > > > well though until the bot rollout if further along. > > > > It isn't run on the bots, but I'd prefer that we update this around the same > > time that we update chromium.android + chromium.linux. > > > > > https://codereview.chromium.org/2666783004/diff/60001/build/install-build-dep... > > File build/install-build-deps-android.sh (right): > > > > > https://codereview.chromium.org/2666783004/diff/60001/build/install-build-dep... > > build/install-build-deps-android.sh:75: echo " sudo > update-java-alternatives > > -s java-1.8.0-openjdk-amd64" > > ah, ok, with this in the error text, I'm ok with removing the references to > > update-alternatives from the markdown. > > I'm thinking this is good to go now. wdyt? Agreed. lgtm still
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by agrieve@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jbudorick@chromium.org Link to the patchset: https://codereview.chromium.org/2666783004/#ps80001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1488986995620260,
"parent_rev": "ecaade2ce813c97d168576d035f8610c642947eb", "commit_rev":
"8638ef810c10cee09e467b2ecd678341e12fc892"}
Message was sent while issue was closed.
Description was changed from ========== Android: Update docs and install-build-deps-android.sh to require java8 We don't strictly require java8 yet, but will soon. BUG=662467 ========== to ========== Android: Update docs and install-build-deps-android.sh to require java8 We don't strictly require java8 yet, but will soon. BUG=662467 Review-Url: https://codereview.chromium.org/2666783004 Cr-Commit-Position: refs/heads/master@{#455459} Committed: https://chromium.googlesource.com/chromium/src/+/8638ef810c10cee09e467b2ecd67... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/8638ef810c10cee09e467b2ecd67...
Message was sent while issue was closed.
friedman@google.com changed reviewers: + friedman@google.com
Message was sent while issue was closed.
https://codereview.chromium.org/2666783004/diff/80001/build/install-build-dep... File build/install-build-deps-android.sh (right): https://codereview.chromium.org/2666783004/diff/80001/build/install-build-dep... build/install-build-deps-android.sh:57: sudo apt-get -y install openjdk-8-jre openjdk-8-jdk You can't do this on Trusty... java8 doesn't exist there. You have to get the debs from a later install and manually install them, which is what we have puppet doing in our environment.
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/2858063002/ by friedman@google.com. The reason for reverting is: You can't do this on Trusty... java8 doesn't exist there. You have to get the debs from a later install and manually install them, which is what we have puppet doing in our environment.. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
