Skip to content

Enhance platform launcher to honour JAVA_HOME on macos#9425

Open
sid-srini wants to merge 1 commit into
apache:masterfrom
sid-srini:launcher-java_home-macos-parity-with-unix
Open

Enhance platform launcher to honour JAVA_HOME on macos#9425
sid-srini wants to merge 1 commit into
apache:masterfrom
sid-srini:launcher-java_home-macos-parity-with-unix

Conversation

@sid-srini
Copy link
Copy Markdown
Contributor

@sid-srini sid-srini commented Jun 3, 2026

  • The nbexec platform launcher, used by ide launcher (netbeans) and the harness launcher (app.sh), even on macos, should honour JAVA_HOME and PATH-based JDKs when jdkhome is unspecified.

  • This fix brings parity with unix (PR Honor the JAVA_HOME environment variable in nbexec script #8725) and windows (PR Enhance launcher #8408) for the nbexec launcher's behaviour on MacOS.

  • Additionally, since netbeans is no longer supported on JDK 8, the Apple-specific options targeting JDK 8 as a fallback are removed.

  • Finally, the Apple /usr/libexec/java_home tool no longer supports a suffix '+' in the version string to indicate a minimum supported version, and only uses the --version filter as an exact match.

    • Thus, the --version argument has been dropped.
    • The current behaviour of the tool is to return the latest version jvm available in /Library/Java/JavaVirtualMachines/ (or similar Apple-specific locations only) as the default.
    • The PATH based fallback is left as the last one since the java_home tool provides the correctly resolved JDK home instead of the Apple java launcher utilities, /usr/bin/javac and /usr/bin/java, which are expected to always be on the PATH.

^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

  • are all your own work, and you have the right to contribute them.
  • are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

  1. Was this PR correctly labeled, did the right tests run? When did they run?
  2. Is this PR squashed?
  3. Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
  4. Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

@mbien mbien added Platform [ci] enable platform tests (platform/*) os:macos ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Jun 3, 2026
@apache apache locked and limited conversation to collaborators Jun 3, 2026
@apache apache unlocked this conversation Jun 3, 2026
Copy link
Copy Markdown
Member

@neilcsmith-net neilcsmith-net left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking at this. It's a good change, although I think the now identical check for JAVA_HOME should be extracted from the case statements, and ideally take precedence.

I still think this should also set the value of JAVA_HOME to match whatever jdkhome gets set to.

Will need to be tested with the NBPackage Swift launcher, but should be OK.

Comment on lines +157 to +159
# check if JAVA_HOME is empty string
if [ ! -z "$JAVA_HOME" ]; then
jdkhome="${JAVA_HOME}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be tempted to move this statement before the check for sdkman, and remove from inside both case options, which would also allow removing an if/else level. I think JAVA_HOME should take precedence over sdkman, but could go after it if others disagree. Also can remove the comment, or change to "use JAVA_HOME if set"?

@sid-srini
Copy link
Copy Markdown
Contributor Author

Thanks a lot @neilcsmith-net for your review and suggestions. I've incorporated the feedback on lifting the JAVA_HOME check outside the OS-specific block, as well as, making it take precedence over sdkman.

I have not exported JAVA_HOME with the value set for jdkhome mainly so that the change may be made uniformly for windows and *nixes in a separate PR, depending on overall consensus.

Personally, I am not entirely convinced if the launcher should change the value of a well-known env var, not specific to the NetBeans platform, for subsequent usage from the IDE. For example, it may cause a mismatch in the user's expectations on their code run from the IDE vs. a terminal. This may be even more stark with the IDE distribution which contains a (private) jdkhome bundled within itself.

@neilcsmith-net
Copy link
Copy Markdown
Member

Thanks, looks good to me!

Personally, I am not entirely convinced if the launcher should change the value of a well-known env var, not specific to the NetBeans platform, for subsequent usage from the IDE. For example, it may cause a mismatch in the user's expectations on their code run from the IDE vs. a terminal. This may be even more stark with the IDE distribution which contains a (private) jdkhome bundled within itself.

Yes, that's fair enough - keep as a separate issue for further thought. The IDE distributions with JDK for Linux do actually set JAVA_HOME to the same as jdkhome if it's not already set. I added that because it's quite useful when using the terminal in the IDE. I just realised the macOS launcher doesn't while considering how it interacts with your change.

@mbien mbien added this to the NB31 milestone Jun 3, 2026
@sid-srini
Copy link
Copy Markdown
Contributor Author

Yes, that's fair enough - keep as a separate issue for further thought. The IDE distributions with JDK for Linux do actually set JAVA_HOME to the same as jdkhome if it's not already set. I added that because it's quite useful when using the terminal in the IDE. I just realised the macOS launcher doesn't while considering how it interacts with your change.

Understood. I agree that setting JAVA_HOME to jdkhome when it is not already set, seems like a good improvement without much of a down-side. I'll create a separate PR later, including all the 3 platforms for this.
Thanks @neilcsmith-net.

- The nbexec platform launcher, used by ide launcher (netbeans) and
  the harness launcher (app.sh), even on macos, should honour JAVA_HOME
  and PATH-based JDKs when jdkhome is unspecified.
- This brings parity with unix (PR 8725) and windows (PR 8408) for the
  nbexec launcher's behaviour on MacOS.
- Made JAVA_HOME take precedence over sdkman.
    - Thanks neilcsmith-net for this suggestion

- Additionally, since netbeans is no longer supported on JDK 8, the
  Apple-specific options targeting JDK 8 as a fallback are removed.

- Finally, the Apple /usr/libexec/java_home tool no longer supports a
  suffix '+' in the version string to indicate a minimum supported
  version, and only uses the filter as an exact match.
    - Thus, the version argument has been dropped.
    - The current behaviour of the tool is to return the latest version
      jvm available in /Library/Java/JavaVirtualMachines/ (or similar
      Apple-specific locations only).
    - The PATH based fallback is left as the last one since the
      java_home tool provides the correctly resolved JDK home instead of
      the Apple java launcher utilities, /usr/bin/javac and
      /usr/bin/java, which are expected to always be on the PATH.
@sid-srini sid-srini force-pushed the launcher-java_home-macos-parity-with-unix branch from 6702063 to 1bc174c Compare June 4, 2026 09:22
@sid-srini
Copy link
Copy Markdown
Contributor Author

I've squashed the commits after review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) os:macos Platform [ci] enable platform tests (platform/*)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants