Closed Bug 1528314 Opened 8 months ago Closed 4 months ago

"mach bootstrap" on macos suggests non-existing path in --with-java-bin-path=/Library/Java/Home/bin

Categories

(Firefox Build System :: Bootstrap Configuration, defect)

Desktop
macOS
defect
Not set

Tracking

(firefox69 fixed)

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: robwu, Assigned: glandium)

Details

Attachments

(2 files)

When I run mach bootstrap and choose 3 (GeckoView/Firefox for Android Artifact Mode), it does detect the right Java,

Your version of Java (/usr/bin/java) is at least 1.8 (1.8).

And suggests the following mozconfig (I stripped the end that is not relevant):

>>>
# Build GeckoView/Firefox for Android Artifact Mode:
ac_add_options --enable-application=mobile/android
ac_add_options --target=arm-linux-androideabi
ac_add_options --enable-artifact-builds

# With the following java:
ac_add_options --with-java-bin-path="/Library/Java/Home/bin"

This configuration does not allow me to build Fennec, as I get the following error:

 0:04.30 checking for java... not found
 0:04.30 ERROR: The program java was not found.  Set $JAVA_HOME to your Java SDK directory or use '--with-java-bin-path={java-bin-dir}'
 0:04.34 *** Fix above errors and then restart with               "./mach build"
 0:04.35 make: *** [configure] Error 1

I do not have a /Library/Java/Home/bin directory (it seems to have been removed from the java8 formula some time ago - https://github.com/Homebrew/homebrew-cask-versions/pull/6949), but I do have /Library/Java/JavaVirtualMachines/jdk1.8.0_202.jdk/Contents/Home/bin (from Homebrew).

The path in --with-java-bin-path=/Library/Java/Home/bin seems to be hardcoded at https://searchfox.org/mozilla-central/rev/38035ee92463c9e9fbca729ac7e66476ac7eb27a/python/mozboot/mozboot/osx.py#394
Since the Java binary is not always at that location, the bootstrap script should not hard-code it, but use a different method to locate the installation path:

/usr/libexec/java_home returns the Java path (e.g. /Library/Java/JavaVirtualMachines/jdk1.8.0_202.jdk/Contents/Home) if found, and otherwise has exit code 1 (if no Java has been found). You can even try to look for a specific version with -v, e.g. /usr/libexec/java_home -v1.8

Am I the only one who observed this issue, or have you received reports from others as well?

What do you think of my proposed fix at the bottom of comment 0?

Flags: needinfo?(nalexander)

(In reply to Rob Wu [:robwu] from comment #1)

Am I the only one who observed this issue, or have you received reports from others as well?

I have not. If java8 is on the path, you don't need the stanza at all, so I don't know how many people even set it.

What do you think of my proposed fix at the bottom of comment 0?

Fine by me! Anything to make this more robust.

Flags: needinfo?(nalexander)

The path that bootstrap outputs has changed, so the bug as originally filed doesn't apply anymore, but I'm going to hijack it to avoid bootstrap making developers add --with-java-bin-path at all, by making configure find the right one with /usr/libexec/java_home (found independently ; I found this bug when I was about to file a bug for this work that I already have the patches for)

Assignee: nobody → mh+mozilla

Configure should just be able to find the right one. If it doesn't, that
should be fixed in configure rather than with suggestions in bootstrap.

Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/91919cdf9159
Make configure find an appropriate java via /usr/libexec/java_home on macOS. r=nalexander
https://hg.mozilla.org/integration/autoland/rev/4a544cd4b035
Remove --with-java-bin-path mozconfig entry from bootstrap suggestions. r=nalexander
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(mh+mozilla)
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/a4e82d6fdd5b
Make configure find an appropriate java via /usr/libexec/java_home on macOS. r=nalexander
https://hg.mozilla.org/integration/autoland/rev/9a10f1cef191
Remove --with-java-bin-path mozconfig entry from bootstrap suggestions. r=nalexander
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.