Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java LS with vscode-pde rebuilds workspace for every restart #1961

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

snjeza
Copy link
Contributor

@snjeza snjeza commented Dec 6, 2021

Fixes #1960

Both, VS Code PDE and Java LS set default JVMs.
See

If those JVMs are different, Java LS will update all projects.
The issue can't be reproduced when starting VS Code with JDK 11 and without java.configuration.runtimes.

The PR adds the java.ls.doNotSetDefaultJVM property. Java LS won't set a default JVM if it is started with -Djava.ls.doNotSetDefaultJVM=true.

Test vsix: https://github.com/snjeza/vscode-test/raw/master/java-1.2.4.vsix
Test project: https://github.com/eclipse/eclipse.jdt.ls
Test vmargs:

"java.jdt.ls.vmargs": "-Djava.ls.doNotSetDefaultJVM=true <YOUR_VMARGS>",

Signed-off-by: Snjezana Peco [email protected]

@testforstephen
Copy link
Contributor

If those JVMs are different, Java LS will update all projects.
The issue can't be reproduced when starting VS Code with JDK 11 and without java.configuration.runtimes.

The reason is probably from the following code. It will bring some unnecessary project update event if java.configuration.runtimes explicitly sets a different JDK as default.

https://github.com/eclipse/eclipse.jdt.ls/blob/791f30ed63f9960d5ad7dc2e9ba3b9e203cf2b9a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JVMConfigurator.java#L306-L308

Reproduce steps:

  • set java.home is set JDK 11
  • add a JDK 17 to java.configuration.runtimes, and set its default to true.
  • reopen vscode on a Java project (e.g. eclipse.jdt.ls), you will see the log like "!MESSAGE defaultVMInstallChanged -> force update of org.eclipse.jdt.ls.core".

@snjeza
Copy link
Contributor Author

snjeza commented Dec 10, 2021

The reason is probably from the following code. It will bring some unnecessary project update event if java.configuration.runtimes explicitly sets a different JDK as default.

Right!
The -Djava.ls.doNotSetDefaultJVM=true fixes it.

snjeza added a commit to snjeza/vscode-java that referenced this pull request Apr 29, 2022
snjeza added a commit to snjeza/vscode-test that referenced this pull request Apr 29, 2022
@rgrunber
Copy link
Contributor

rgrunber commented Apr 30, 2022

How would this work on the client side ? Would the client need to set -Djava.ls.doNotSetDefaultJVM=true if they don't detect a setting for java.configuration.runtimes where default: true is used ? Is there any way to fix this on the language server side by addressing the TODO comment ?

@snjeza
Copy link
Contributor Author

snjeza commented Apr 30, 2022

How would this work on the client side ? Would the client need to set -Djava.ls.doNotSetDefaultJVM=true if they don't detect a setting for java.configuration.runtimes where default: true is used ? Is there any way to fix this on the language server side by addressing the TODO comment ?

It could be done in the Eclipse PDE support extension.

@rgrunber
Copy link
Contributor

@testforstephen , can this be used to avoid having vscode-pde rebuilding the workspace unnecessarily ? How would vscode-pde set this property, since it only contributes to JDT-LS via bundles, and at that point it's probably too late to set the system property.

@testforstephen
Copy link
Contributor

@testforstephen , can this be used to avoid having vscode-pde rebuilding the workspace unnecessarily ? How would vscode-pde set this property, since it only contributes to JDT-LS via bundles, and at that point it's probably too late to set the system property.

Yes, vscode-pde doesn't have a good way to specify such property. We need a better approach that does not depends on system property to fix this.

@rgrunber
Copy link
Contributor

Ok, so then we can't merge this PR in the current form.

What we really need is a nice way for JDT-LS to detect whether a bundle extension wants to handle setting the runtime, and if so, simply avoid setting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Java LS with the vscode-pde extension rebuilds workspace for every restart
3 participants