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

RESOLVED FIXED in Firefox 59

Status

()

RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: p.bertran, Assigned: p.bertran)

Tracking

Firefox 58
Firefox 60
All
Android
Points:
---

Firefox Tracking Flags

(firefox59 fixed, firefox60 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

11 months ago
Created attachment 8939570 [details] [diff] [review]
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).
(Assignee)

Updated

11 months ago
OS: Unspecified → Android
Hardware: Unspecified → All
Attachment #8939570 - Attachment is patch: true
Attachment #8939570 - Attachment mime type: text/x-patch → text/plain
Comment on attachment 8939570 [details] [diff] [review]
firefox-java-patch.diff

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

Thanks for this!
Attachment #8939570 - Flags: review+
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
Assignee: nobody → p.bertran
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: checkin-needed

Comment 3

11 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a89b4f4ff4d
Ensure that Java8 is bootstrapped on Mac to maintain Android SDK compatibility. r=nalexander
Keywords: checkin-needed

Comment 4

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5a89b4f4ff4d
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
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.
Status: RESOLVED → REOPENED
Flags: needinfo?(mozilla)
Resolution: FIXED → ---
Clearing NI; just wanted mkaply to see this ticket.
Flags: needinfo?(mozilla)
See Also: → bug 1428887
Duplicate of this bug: 1428887
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?
Flags: needinfo?(p.bertran)
Note that I was able to work around the issue by installing java8 manually via

brew cask install caskroom/versions/java8

(from comment 5)
(Assignee)

Comment 10

11 months ago
(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.
Flags: needinfo?(p.bertran) → needinfo?(nalexander)
Duplicate of this bug: 1436730

Comment 12

10 months ago
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 hidden (mozreview-request)

Comment 14

10 months ago
mozreview-review
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?
Attachment #8949844 - Flags: review+
Attachment #8949844 - Flags: review?(core-build-config-reviews) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 17

10 months ago
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/31fc795bb254
Bootstrap caskroom/versions/java8 for macOS+brew+mobile/android. r=froydnj

Comment 18

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/31fc795bb254
Status: REOPENED → RESOLVED
Last Resolved: 11 months ago10 months ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: Firefox 59 → Firefox 60

Updated

10 months ago
Duplicate of this bug: 1430226
Clearing NI since I believe I this is now addressed.
Flags: needinfo?(nalexander)
You need to log in before you can comment on or make changes to this bug.