Make sure netbeans in embedded terminal means itself

[jtulach]

• since I am working with VSCode I admire its integration with its embedded shell
• I'd like NetBeans terminal integration to be as good
• one thing I like a lot is ability to open a file from the terminal

• e.g. typing netbeans pom.xml in the embedded terminal would open the file in the IDE the terminal is embedded into
• I had such a setup locally for years, but:
  • it doesn't work with other than default --userdir well
  • not everyone is capable to set it up
• thus I've created this PR to provide such a support for everyone

[jtulach]

• it was always clear to me I need to set an environment variable to represent the value of --userdir
• otherwise the CLIHandler won't be able to connect to the same NetBeans process
• originally I thought I will have to make changes here, but ...

[jtulach]

• Store the value of --userdir to a system variable
• so that child processes can pick it up

[jtulach]

• any objections against this change?
• if no, then TODO:
  • document the new environment variable
  • modify netbeans.exe to behave the same
• let's have a time for inception review now
• I plan to get this change into NetBeans 28

[neilcsmith-net]

I'm in favour of the general principle - in fact I use this feature with the DEB package quite a bit, and alternative userdir support there would be great.

I have a few reservations about the implementation. Our packaging tool works by wrapping the launcher in various ways to meet different OS requirements. Some will already have netbeans in the path. We need to ensure that this works for standalone and installed usage. And I think we need to ensure JAVA_HOME is set in the parent to be picked up in the child process.

[jtulach]

• modify netbeans.exe to behave the same

• do I understand correctly I need to make the change in the Propagate NETBEANS_USERDIR environment variable netbeans-native-launchers#12 repository?

[neilcsmith-net]

No, the Windows launcher code is back in the main repository. It is built by the workflow at https://github.com/apache/netbeans/blob/master/.github/workflows/native-binary-build-launcher.yml When ready, the version numbers will need updating, binaries built from the workflow, and a release vote done - see #8408 and https://lists.apache.org/thread/491tvp6o44nj9wpmcpbdtcmt8rn0d9h8

EDIT : I'm currently -1 to the PATH modification with prepend. Looking at it I'm not sure we can make that work everywhere it needs to. It will break with some of our packages by bypassing wrapper scripts already in the path. Append is somewhat better (-0.5! 😄 ) but will not work in the cases mentioned earlier. I still think environment variables would be preferable.

Have pinged @matthiasblaesing and @mbien for thoughts. Matthias also ran the last Windows launcher update and vote.

[mbien]

I am somewhat mixed on this. On one hand I think that if a terminal behaves differently in the IDE it is essentially a bug (that is also why I think that the launcher should probably not export GDK_SCALE on linux). On the other hand I can see that netbeans could be seen as special kind of program and the utility for it for users who run NB from the zip (I am one of them, but I do have nb already as alias without adding anything to the path, see 4th paragraph).

I am a bit worried about the can of worms this opens (why is javac, mvn, ant etc not in path too etc) and that it is set by default without opt-in or out.

Maybe the solution is to store it in an env var and add a text field to the terminal settings where users can configure path extensions in the sense of PATH:NB_LAUNCHER_PATH. (might have to be hidden on non-unix based systems since cygwin+windows might produce interesting results here and it could have NB_DEFAULT_JDK etc too in future)

I am wondering if PATH is even the right vehicle for this. Letting the NB terminal run alias nb=NB_LAUNCHER_PATH on start might have the smallest risk for side effects and cover the intended usecase.

EDIT : I'm currently -1 to the PATH modification with prepend (..)

I agree, prepend would break existing user setups too, so if at all, it would have to be append IMO.

(slightly related to this proposal would be: path selection -> right click -> open in NB or System for output and terminal as quality of life improvement)

[neilcsmith-net]

I am a bit worried about the can of worms this opens (why is javac, mvn, ant etc not in path too etc) and that it is set by default without opt-in or out.

+1

I am wondering if PATH is even the right vehicle for this. Letting the NB terminal run alias nb=NB_LAUNCHER_PATH on start might have the smallest risk for side effects and cover the intended usecase.

I hadn't thought about setting an alias based on the environment variable, but that's a good idea. I think, set up something like NB_BASEDIR and NB_LAUNCHER in the launchers, and the rest can be addressed elsewhere in the codebase over time. It might actually be good to have a customizable script in the userdir that is invoked similarly to ~/.profile and could be used to set up aliases. There's a few other setup things done - here iirc - https://github.com/apache/netbeans/blob/master/ide/dlight.terminal/src/org/netbeans/modules/dlight/terminal/action/TerminalSupportImpl.java#L269

But in the shorter term, the environment variable could still be invoked directly or used to set an alias manually.

Having basedir in there too could allow for pointing aliases at some of those other things you mentioned, should people wish to.

The launcher variable would need to be kept separate to the basedir, and only set if not already configured, so the various package scripts can use it to route through themselves.

and it could have NB_DEFAULT_JDK etc too in future)

Maybe. Some of the packages set JAVA_HOME here, and I think that could be good in the launchers. That does count as a situation where the terminal in the IDE may behave differently to the system mind you (though probably rightly IMO).

[jtulach]

Thank you for your review:

• let's only append to PATH - that's gentle - as originally proposed in ed6a3f7
• have an opt-out (e13fa29) to be safe if anything goes wrong
• KISS: more complex architectures only if something really gets broken

I'll wrestle with the .exe and update docs (done in 66734c7) now.

[jtulach]

I tested these changes with following program:

package execenv;

public class ExecEnv {
    public static void main(String[] args) throws Exception {
        System.out.println("Env: " + System.getenv("PATH"));
        var p = new ProcessBuilder("netbeans", "S:\\readme.txt").start();
        var r = p.waitFor();
        System.out.println("Res: " + r);
    }
}

when executed on NetBeans 27 it fails with:

run:
Env: C:\Program Files\OpenSSH\;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;C:\WINDOWS\System32\OpenSSH\;C:\Program Files\Git\cmd;C:\Users\jst\AppData\Local\nvm;C:\nvm4w\nodejs;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit\;C:\Users\jst\.cargo\bin;C:\Users\jst\AppData\Local\Microsoft\WindowsApps;C:\Users\jst\AppData\Local\nvm;C:\nvm4w\nodejs;C:\Program Files\Git\bin;S:\windows\bin;S:\windows\bin\graalvm-community-openjdk-24.0.1+9.1\bin\;S:\windows\bin\sbt-1.7.2\bin\;
Exception in thread "main" java.io.IOException: Cannot run program "netbeans": CreateProcess error=2, The system cannot find the file specified
	at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1110)
	at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1044)
	at execenv.ExecEnv.main(ExecEnv.java:6)
Caused by: java.io.IOException: CreateProcess error=2, The system cannot find the file specified
	at java.base/java.lang.ProcessImpl.create(Native Method)
	at java.base/java.lang.ProcessImpl.<init>(ProcessImpl.java:485)
	at java.base/java.lang.ProcessImpl.start(ProcessImpl.java:146)
	at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1076)
	... 2 more
C:\Users\jst\AppData\Local\NetBeans\Cache\27\executor-snippets\run.xml:111: The following error occurred while executing this line:
C:\Users\jst\AppData\Local\NetBeans\Cache\27\executor-snippets\run.xml:68: Java returned: 1
BUILD FAILED (total time: 1 second)

With changes in this PR it properly opens the file in the IDE executing the script. Setting NETBEANS_USERDIR=IGNORE opts-out and falls back to previous behavior.

[jtulach]

the Windows launcher code is back in the main repository. It is built by the workflow at https://github.com/apache/netbeans/blob/master/.github/workflows/native-binary-build-launcher.yml When ready, the version numbers will need updating, binaries built from the workflow, and a release vote done - see #8408 and https://lists.apache.org/thread/491tvp6o44nj9wpmcpbdtcmt8rn0d9h8

• the changes were introduced by Enhance launcher #8408
• then there was a subsequent PR integrating the voted launchers: Fix flaky clipboard on Windows #8361
• we do have the binaries built as well but (of course) not yet from the merge commit

When ready, the version numbers will need updating

• my understanding is that the only remaining change in this PR is to update version numbers
  • just like netbeans.exe.manifest
  • just like netbeans64.exe.manifest
  • just like version.h
  • Done in 756370d
• then we integrate this PR and once we have a merge commit
• I start a vote about the launcher binaries and proceed with another PR to integrate them

Final steps in this PR:

• squash commits into one
• merge

[matthiasblaesing]

In general looks sane. I have left two inline comments.

The usability on windows is IMHO not good. I you run netbeans under windows you get an error, because the script launcher is invoked as it is a 100% match:

You need to know, that you have to invoke the exe launcher, which (for me surprisingly works even though I have a 64bit JDK):

[jtulach]

The usability on windows is IMHO not good. I you run netbeans under windows you get an error, because the script launcher is invoked as it is a 100% match:
You need to know, that you have to invoke the exe launcher, which

• I do not have cygwin installed, so I didn't run into the problem myself
• This problem is general - e.g. bin/netbeans doesn't work in Cygwin
• it's just exposed by my modification of PATH
• the best fix is to detect Cygwin and exec $basedir/netbeans.exe $*, isn't it?
• here is the Cygwin change: b0a20a2

(for me surprisingly works even though I have a 64bit JDK)

• if I remember correctly there was some work on detecting 32/64 bit OS and invoking the right version
• maybe it is this change: emilianbold/netbeans-releases@b386843#diff-8e5bfb87c8f91779308ed39f01e2484c0eb2bfbb1a1bdcf34eeef36130717c62R196

[neilcsmith-net]

Personally, I don't see a reason to rush this to get it into NB28, which with the Windows launcher changes will be a hard push to do before freeze. It would be better to get this in early for NB29 for proper testing.

The Windows changes here probably affect platform applications, but the Unix changes don't. That should probably be looked at to ensure aligned behaviour.

The workflows shouldn't be showing waiting for approval - there's something awry with your account or the commit info here.

[jtulach]

Personally, I don't see a reason to rush this to get it into NB28, which with the Windows launcher changes will be a hard push to do before freeze. It would be better to get this in early for NB29 for proper testing.

• The original plan aimed "to get this change into NetBeans 28". But plan is a living document. Let's wait for NB29.

The Windows changes here probably affect platform applications,

• Oh, no:

• netbeans/harness/apisupport.harness/windows-launcher-src/applauncher.h

  Line 30 in 2e80f29

  #include "../../../nb/ide.launcher/windows/nblauncher.h"

• who's architected such a crappy design?
• in my architect's view, applauncher should only depend on nbexec, but not the IDE's launcher!
• the fact that it is not the case, deserves some code changes

[neilcsmith-net]

Thanks!

• who's architected such a crappy design?

• in my architect's view, applauncher should only depend on nbexec, but not the IDE's launcher!

• the fact that it is not the case, deserves some code changes

Matthias made similar comments on the PR I linked to above - #8408 (comment)

Short term, it's possibly better to align the behaviour on this for the platform as well anyway? ie. update the platform shell launcher rather than further changes in the Windows launcher. Could be just as useful for platform applications.

Longer term if you're considering re-architecting, for info, there was some discussion on the NB Slack channel a while ago about the possibility of using a Java file launcher instead that could run either as single Java source or be native compiled. That's tangential to this PR obviously.