8269967: JavaFX should fail fast on macOS below minimum version

[kevinrushforth]

This PR implements a version check in the JavaFX runtime initialization code on macOS to ensure that the platform is running a version of macOS that is at or above the minimum version. If the platform is below the specified minimum, the JavaFX initialization code throws an exception.

The minimum version is passed from the mac.gradle file to the Mac glass code as a pair of build time constants, which are compared at runtime to the platform version.

Notes to reviewers:

• In case anyone is interested, the first commit has some debug logging which will print the minimum version and the detected platform version. I removed them with the second commit.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8269967: JavaFX should fail fast on macOS below minimum version

Reviewers

• Johan Vos (@johanvos - Reviewer)
• Ambarish Rapte (@arapte - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jfx pull/567/head:pull/567
$ git checkout pull/567

Update a local copy of the PR:
$ git checkout pull/567
$ git pull https://git.openjdk.java.net/jfx pull/567/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 567

View PR using the GUI difftool:
$ git pr show -t 567

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jfx/pull/567.diff

[kevinrushforth]

/reviewers 2

[bridgekeeper]

👋 Welcome back kcr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@kevinrushforth
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

[mlbridge]

Webrevs

• 01: Full - Incremental (4e24d454)
• 00: Full (6d4cdd61)

[johanvos]

I checked and tested this, and it does what is expected.
I wonder if it would be better to move the check to the JNI_OnLoad method though, as that will be invoked (slightly) before the initIDs.

[kevinrushforth]

Thanks for the suggestion. I tried it and found that it leads to a less clear exception message. JNI_OnLoad is called from System.loadLibrary, which turns any exception into an UnsatisfiedLinkError without using the message of the caught exception. So instead of a runtime exception with the message Unsupported macOS version, we would get something like this:

Caused by: java.lang.UnsatisfiedLinkError: no glass in java.library.path:
    /Users/kcr/Library/Java/Extensions:/Library/Java/Extensions:/Network/Library/Java/Extensions:
    /System/Library/Java/Extensions:/usr/lib/java:.

So I'd prefer to leave it in initIDs (although it seems best to move the check up so it's the first thing done in that method).

[kevinrushforth]

Alternatively, I could add a new JNI function to check the version, and call that right after the loadLibrary. That might be cleaner than doing it as part of initIDs

[johanvos]

Thanks for the suggestion. I tried it and found that it leads to a less clear exception message. JNI_OnLoad is called from System.loadLibrary, which turns any exception into an UnsatisfiedLinkError without using the message of the caught exception. So instead of a runtime exception with the message Unsupported macOS version, we would get something like this:

Caused by: java.lang.UnsatisfiedLinkError: no glass in java.library.path:
    /Users/kcr/Library/Java/Extensions:/Library/Java/Extensions:/Network/Library/Java/Extensions:
    /System/Library/Java/Extensions:/usr/lib/java:.

So I'd prefer to leave it in initIDs (although it seems best to move the check up so it's the first thing done in that method).

You are right, the UnsatisfiedLinkError is a bit misleading in this case.

[johanvos]

Alternatively, I could add a new JNI function to check the version, and call that right after the loadLibrary. That might be cleaner than doing it as part of initIDs

I think it's ok to keep it in the initIDs, as that is called immediately after the loadLibrary. A new JNI function seems a bit overkill -- it would have fit nicely in the JNI_OnLoad but I do agree that that is not the best approach, since it will convert the exception/error into an UnsatiffiedLinkError.
Hence, +1 on your current approach with doing it in initIDs.

[kevinrushforth]

Agreed. I decided to move the check up to be the first thing done in initIDs (it seems cleaner than having it in between other operations).

[openjdk]

@kevinrushforth This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8269967: JavaFX should fail fast on macOS below minimum version

Reviewed-by: jvos, arapte

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 1 new commit pushed to the master branch:

• 0c98d96: 8268718: [macos] Video stops, but audio continues to play when stopTime is reached

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[kevinrushforth]

/integrate

[openjdk]

Going to push as commit 00b353e.
Since your change was applied there has been 1 commit pushed to the master branch:

• 0c98d96: 8268718: [macos] Video stops, but audio continues to play when stopTime is reached

Your commit was automatically rebased without conflicts.

[openjdk]

@kevinrushforth Pushed as commit 00b353e.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.