1427790 - Bootstrap for android installs latest java (9 now on mac). Should install java 8 to keep android SDK compatibility

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Pierre Bertran]

Attachment: firefox-java-patch.diff

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171206182557

Steps to reproduce:

Bootstrapping "Firefox for android" on macOS


Actual results:

It installed java 9, which is incompatible with the android SDK


Expected results:

It should have installed Java 8
This can be fixed by adding an 8 there: https://dxr.mozilla.org/mozilla-central/source/python/mozboot/mozboot/osx.py#353

I join a patch that does just that. Not sure about the patch format thought as I never contributed myself yet (it's just a plain hg diff).

Comment 1

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8939570 [details] [diff] [review]
firefox-java-patch.diff

Review of attachment 8939570 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for this!

Comment 2

[Nick Alexander :nalexander [he/him]]

This needs check-in, but might also need a good commit message.  Sheriff, could you try:

Bug 1427790 - Specify Java version 8 in Mac OS X mobile/android bootstrap. r=nalexander

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a89b4f4ff4d
Ensure that Java8 is bootstrapped on Mac to maintain Android SDK compatibility. r=nalexander

Comment 4

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/5a89b4f4ff4d

Comment 5

[Nick Alexander :nalexander [he/him]]

Looks like we got this wrong -- it's supposed to be

brew cask install caskroom/versions/java8

and we're doing

brew cask install java8

This requires us to partially roll back https://hg.mozilla.org/mozilla-central/rev/4f056c7650c7, since the cask business has changed yet again.

Comment 6

[Nick Alexander :nalexander [he/him]]

Clearing NI; just wanted mkaply to see this ticket.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Hey - I just hit this when attempting to run ./mach boostrap for Fennec Artifact builds:

Please choose the version of Firefox you want to build:
1. Firefox for Desktop Artifact Mode
2. Firefox for Desktop
3. Firefox for Android Artifact Mode
4. Firefox for Android
Your choice: 3

Looks like you have Homebrew installed. We will install all required packages via Homebrew.


We are now installing all required packages via Homebrew. You will see a lot of
output as packages are built.

Error: Cask 'java8' is unavailable: No Cask with this name exists.
Error running mach:

    ['bootstrap']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

CalledProcessError: Command '[u'/usr/local/bin/brew', u'cask', u'install', u'java8']' returned non-zero exit status 1

  File "/Users/mikeconley/Projects/mozilla-central/python/mozboot/mozboot/mach_commands.py", line 32, in bootstrap
    bootstrapper.bootstrap()
  File "/Users/mikeconley/Projects/mozilla-central/python/mozboot/mozboot/bootstrap.py", line 297, in bootstrap
    getattr(self.instance, 'install_%s_packages' % application)()
  File "/Users/mikeconley/Projects/mozilla-central/python/mozboot/mozboot/osx.py", line 206, in install_mobile_android_artifact_mode_packages
    self.package_manager)(artifact_mode=True)
  File "/Users/mikeconley/Projects/mozilla-central/python/mozboot/mozboot/osx.py", line 357, in ensure_homebrew_mobile_android_packages
    installed = self._ensure_homebrew_casks(casks)
  File "/Users/mikeconley/Projects/mozilla-central/python/mozboot/mozboot/osx.py", line 315, in _ensure_homebrew_casks
    return self._ensure_homebrew_packages(casks, extra_brew_args=['cask'])
  File "/Users/mikeconley/Projects/mozilla-central/python/mozboot/mozboot/osx.py", line 309, in _ensure_homebrew_packages
    subprocess.check_call(cmd + ['install', package])
  File "/usr/local/Cellar/python/2.7.13_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)


I suspect we need to correct the error highlighted in comment 5 of this bug. Would you be able to provide a new patch, Pierre?

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Note that I was able to work around the issue by installing java8 manually via

brew cask install caskroom/versions/java8

(from comment 5)

Comment 10

[Pierre Bertran]

(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews and needinfos from comment #8)
> Would you be able to provide a new patch, Pierre?

Yes I can change the patch to make it look for "caskroom/versions/java8"

But I guess the problem with my patch is the need for this line, which have been removed in the patch mentionned by nalexander in comment 5 :
 https://hg.mozilla.org/mozilla-central/rev/4f056c7650c7#l1.67

So I'm not sure why this has been removed. Maybe restore this and let 'java8' alone could be a better approach.

With your advice on this, I'll make a new patch.

Comment 12

[Tom Ritter [:tjr]]

I hit Alex's Homebrew issue, and I changed it from java8 to java and it seems like it's working for me. Datapoint++

Comment 13

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1427790 - Bootstrap caskroom/versions/java8 for macOS+brew+mobile/android.

Two things have changed.  One, Brew's java package became Java 9,
which doesn't work for building on Android.  Two, Brew's cask system
also changed, requiring some small updates.

In order to actually use the install java toolchain, we need to use
the --with-java-bin-path configure option, which required some small
tweaks to the suggested mozconfigs.

Review commit: https://reviewboard.mozilla.org/r/219162/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/219162/

Comment 14

[Nathan Froyd [:froydnj]]

Comment on attachment 8949844 [details]
Bug 1427790 - Bootstrap caskroom/versions/java8 for macOS+brew+mobile/android.

https://reviewboard.mozilla.org/r/219162/#review224932

::: python/mozboot/mozboot/osx.py:322
(Diff revision 1)
> +        # Ensure we've tapped the caskroom.  This is idempotent, so no need to
> +        # avoid repeat invocation.

I admire the cleverness of Homebrew for all their beer-related terminology; perhaps we should clarify what "tapped the caskroom" is here for people who aren't intimately familiar with Homebrew terminology?

Comment 15

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8949844 [details]
Bug 1427790 - Bootstrap caskroom/versions/java8 for macOS+brew+mobile/android.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/219162/diff/1-2/

Comment 16

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8949844 [details]
Bug 1427790 - Bootstrap caskroom/versions/java8 for macOS+brew+mobile/android.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/219162/diff/2-3/

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/31fc795bb254
Bootstrap caskroom/versions/java8 for macOS+brew+mobile/android. r=froydnj

Comment 18

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/31fc795bb254

Comment 20

[Nick Alexander :nalexander [he/him]]

Clearing NI since I believe I this is now addressed.