Enhance launcher #8408

matthiasblaesing · 2025-04-08T18:02:23Z

Enhance windows launcher to use PATH and JAVA_HOME environment variables as potential JDK homes
Enhance IDE launcher to support substitution of variables in most configuration options

…les as potential JDK homes

neilcsmith-net · 2025-04-09T09:52:44Z

Thanks for looking at this. Generally (not tested yet) looks good. Have you tested with the platform launcher? IIRC this extends the IDE launcher Need to make sure everything is still working as expected there, and that it's picking up the new basedir changes.

When this is ready for merging, please add an additional commit mirroring the one at 272ebc2 to update the various versions to 101.3.0.0, 101300 or 101,3,0,0. Happy to help get this through release vote once merged if you want me to?

mbien · 2025-04-09T12:29:07Z

smoke test worked:

installed temurin with default options (will set path but not java_home), replaced launcher and NB started
now I set JAVA_HOME to something else, started and it picked it up
tested ${BASEDIR} by adding it via -J-Dfoo=${BASEDIR} (in netbeans.config) and checking log for correct variable injection

looking through the code:
The variable substitution runs while the config is scanned line-by-line, this has the consequence that properties have to be defined in the right order, otherwise things will be missed. Setting -J-Dmyhome=${netbeans_jdkhome} does only work if netbeans_jdkhome is moved before netbeans_default_options. This is likely no problem in practice as long we are aware of this.

neilcsmith-net · 2025-04-09T13:23:01Z

Assuming Windows platform launcher is picking up the change correctly, incidentally, this will need the platform Unix launcher altering too - https://github.com/apache/netbeans/blob/master/harness/apisupport.harness/release/launchers/app.sh#L36 That will need the same $basedir calculation adding I guess rather than relying on ../

The sooner we can consolidate launchers in one place the better! (hint, hint, @mbien 😄 )

The variable substitution runs while the config is scanned line-by-line, this has the consequence that properties have to be defined in the right order, otherwise things will be missed. Setting -J-Dmyhome=${netbeans_jdkhome} does only work if netbeans_jdkhome is moved before netbeans_default_options. This is likely no problem in practice as long we are aware of this.

Given that the Unix launcher sources the conf as a shell script, that's likely the right behaviour. I assume we don't want to support adding rm -rf / in the Windows conf as well though. 😆

matthiasblaesing · 2025-04-09T15:53:42Z

looking through the code: The variable substitution runs while the config is scanned line-by-line, this has the consequence that properties have to be defined in the right order, otherwise things will be missed. Setting -J-Dmyhome=${netbeans_jdkhome} does only work if netbeans_jdkhome is moved before netbeans_default_options.

The list of supported variables is:

DEFAULT_USERDIR_ROOT
DEFAULT_CACHEDIR_ROOT
HOME
BASEDIR

everything else is undefined behavior (this wording makes me like C 😁 ) and unsupported.

Assuming Windows platform launcher is picking up the change correctly, incidentally, this will need the platform Unix launcher altering too - https://github.com/apache/netbeans/blob/master/harness/apisupport.harness/release/launchers/app.sh#L36 That will need the same $basedir calculation adding I guess rather than relying on ../

The platform launcher will not pickup this. The variable substitution described above is in the IDE launcher.

mbien · 2025-04-09T15:57:32Z

The list of supported variables is:

yes but you have to add "in that order" ;)

matthiasblaesing · 2025-04-09T15:59:35Z

The list of supported variables is:

yes but you have to add "in that order" ;)

If you mean, that one of these values holds a variable itself, then yes, we are well in "undefined behavior" territory.

matthiasblaesing · 2025-04-09T16:03:15Z

Thanks for looking at this. Generally (not tested yet) looks good. Have you tested with the platform launcher? IIRC this extends the IDE launcher Need to make sure everything is still working as expected there, and that it's picking up the new basedir changes.

arg f*** now I understand what you mean. The chain is harness launcher -> ide launcher -> platform launcher for windows and harness launcher -> platform launcher for the rest. One of the moments where I want to have a serious talk with the original developers....

mbien · 2025-04-09T16:07:27Z

maybe I didn't describe it well enough

example:

netbeans_default_options= -J-Dmyhome=${netbeans_jdkhome} ...
netbeans_jdkhome=asdf

this won't insert asdf into netbeans_default_options since it hasn't read that far yet. But as I said this is likely no problem, we just have to be aware of this.

swapping the properties in the config file fixes it:

netbeans_jdkhome=asdf
netbeans_default_options= -J-Dmyhome=${netbeans_jdkhome} ...

neilcsmith-net · 2025-04-09T16:18:37Z

@mbien try that in a shell script. Does it work there?

matthiasblaesing · 2025-04-09T16:21:14Z

Please stop discussion undefined behavior here. Any variable apart from four explicitly mentioned variables is undefined behavior and replacements more than one level deep are undefined behavior. Lets keep this focused on fixing a single problem.

mbien · 2025-04-09T16:26:09Z

and replacements more than one level deep are undefined behavior.

puts on undefined behavior list

matthiasblaesing · 2025-04-09T18:38:15Z

I don't have a platform application, so I only stared at the launcher code, put harness configuration and launcher configuration side by side and aligned the relevant parts. So my theory would be, that now platform also works.

It would be great if a platform user could test this. The new commit will be squashed into the substitution commit (second one).

neilcsmith-net

Many thanks @matthiasblaesing ! 😄 Bunch of testing done.

Tested the app.sh launcher on Ubuntu with a a platform application - all seems to work well and pick up new token.

Tested the Windows launcher from https://github.com/apache/netbeans/actions/runs/14364396031 in a platform application. Working to set jdkhome with BASEDIR. Also added netbeans-javaagent.jar from build on #8361 to the platform application with config and debug option - registered and transformed the clipboard as far as I can tell (my machine has never replicated the actual bug but logging to console verified).

Slightly ugly paths in the logs, as suspected at #8361 (comment) but as Windows generally seems OK with / nothing seems to actually break.

All in all, great job on this and the agent! ❤️

If you can squash as needed, then add a separate commit with the versions as requested, I think we should get this PR in ASAP and through the necessary vote to allow integration before freeze. Give me a shout if you'd rather not handle the vote admin.

…most configuration options - Added $BASEDIR to supported list of variables (represents base of NetBeans/Application installation) - Allow usage of $DEFAULT_USERDIR_ROOT, $DEFAULT_CACHEDIR_ROOT, $HOME and $BASEDIR in configuration options: - netbeans_default_userdir / default_userdir - netbeans_default_cachedir / default_cachedir - netbeans_default_options / default_options - netbeans_jdkhome / jdkhome

matthiasblaesing · 2025-04-10T18:46:49Z

@neilcsmith-net thank you very much for testing and review. I squashed and added the version bump commit. Would you mind having a final look? If ok, I'll merge and the prepare the vote threat. Thanks for willing to help with the vote, I'd like to do it myself, so that not there are more people here, that did "it".

neilcsmith-net · 2025-04-10T20:08:34Z

Looks good from a glance at diff on my phone. It looks like you found one missing from last time too.

Artefacts will then be version 3-GIT_SHORT_HASH.

Great that you'll run vote. More people knowing processes the better. Just aware I've been pushing for changes and pushing to get this in before we branch!

matthiasblaesing added the Platform [ci] enable platform tests (platform/*) label Apr 8, 2025

matthiasblaesing added this to the NB26 milestone Apr 8, 2025

Enhance windows launcher to use PATH and JAVA_HOME environment variab…

59c700a

…les as potential JDK homes

matthiasblaesing force-pushed the enhance_launcher branch from 0d68d23 to b941014 Compare April 8, 2025 19:03

mbien mentioned this pull request Apr 9, 2025

Correctly find and use OpenJDK #8336

Closed

matthiasblaesing mentioned this pull request Apr 9, 2025

Fix flaky clipboard on Windows #8361

Merged

neilcsmith-net approved these changes Apr 10, 2025

View reviewed changes

matthiasblaesing added 2 commits April 10, 2025 20:30

Bump launcher version to 3.0.0

d30154c

matthiasblaesing force-pushed the enhance_launcher branch from 3459c82 to d30154c Compare April 10, 2025 18:43

matthiasblaesing merged commit 282bbc0 into apache:master Apr 10, 2025
35 checks passed

matthiasblaesing deleted the enhance_launcher branch April 13, 2025 18:30

thurka mentioned this pull request Aug 6, 2025

Honor the JAVA_HOME environment variable oracle/visualvm#623

Open

neilcsmith-net mentioned this pull request Aug 23, 2025

Make sure netbeans in embedded terminal means itself #8756

Merged

Enhance launcher #8408

Enhance launcher #8408

Uh oh!

Conversation

matthiasblaesing commented Apr 8, 2025

Uh oh!

neilcsmith-net commented Apr 9, 2025

Uh oh!

mbien commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neilcsmith-net commented Apr 9, 2025

Uh oh!

matthiasblaesing commented Apr 9, 2025

Uh oh!

mbien commented Apr 9, 2025

Uh oh!

matthiasblaesing commented Apr 9, 2025

Uh oh!

matthiasblaesing commented Apr 9, 2025

Uh oh!

mbien commented Apr 9, 2025

Uh oh!

neilcsmith-net commented Apr 9, 2025

Uh oh!

matthiasblaesing commented Apr 9, 2025

Uh oh!

mbien commented Apr 9, 2025

Uh oh!

matthiasblaesing commented Apr 9, 2025

Uh oh!

neilcsmith-net left a comment

Choose a reason for hiding this comment

Uh oh!

matthiasblaesing commented Apr 10, 2025

Uh oh!

neilcsmith-net commented Apr 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mbien commented Apr 9, 2025 •

edited

Loading