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.

Sign up for GitHub

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

Jump to bottom

Fix build on Windows #991

Closed
wants to merge 1 commit into from
Closed

Fix build on Windows #991

wants to merge 1 commit into from

Conversation

snjeza
Copy link
Contributor

@snjeza snjeza commented Apr 12, 2019

@snjeza snjeza requested review from gorkem, fbricon and yaohaizh April 12, 2019 03:52
@snjeza snjeza mentioned this pull request Apr 12, 2019
Merged
yaohaizh
yaohaizh reviewed Apr 12, 2019
View reviewed changes
org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java Outdated
return new Object();
});
}

private void disconnectFromWorkspace() {
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

This will called twice and log two duplicate logs?

yaohaizh
yaohaizh reviewed Apr 12, 2019
View reviewed changes
org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java Outdated
@@ -304,27 +305,35 @@ private CodeActionOptions getCodeActionOptions() {
@Override
public CompletableFuture<Object> shutdown() {
logInfo(">> shutdown");
disconnectFromWorkspace();
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious why this is not put in the computeAsync body?

@snjeza
Copy link
Contributor Author

snjeza commented Apr 12, 2019 •
edited
Loading

This will called twice and log two duplicate logs?

We have to save a workspace in both, the exit and shutdown method because ParentProcessWatcher calls only the exit method. If you kill VS Code, ParentProcessWatcher will stop Java LS server and call disconnectFromWorkspace only once.

Just curious why this is not put in the computeAsync body?

Saving a workspace is a priority task.

@fbricon
Copy link
Contributor

fbricon commented Apr 12, 2019 •
edited
Loading 

  Just curious why this is not put in the computeAsync body?

Saving a workspace is a priority task.

computeAsync still seems more "correct", after reading the comments on microsoft/vscode#35196 (comment)

@snjeza snjeza force-pushed the issue-793 branch 3 times, most recently from 125ff6a to f3bf055 Compare April 12, 2019 23:48
@snjeza
Copy link
Contributor Author

snjeza commented Apr 12, 2019 •
edited
Loading

@fbricon @yaohaizh I have updated the PR and https://raw.githubusercontent.com/snjeza/vscode-test/master/java-0.43.1.vsix

@snjeza snjeza changed the title Fix build on Windows [WIP] Fix build on Windows Apr 21, 2019
@snjeza snjeza changed the title [WIP] Fix build on Windows Fix build on Windows Apr 23, 2019
@snjeza
Copy link
Contributor Author

snjeza commented Apr 23, 2019

You can also test with the following property on Windows:

"java.jdt.ls.vmargs": "-DwatchParentProcess=10 -noverify -Xmx2G -XX:+UseG1GC -XX:+UseStringDeduplication",

'-DwatchParentProcess=10' is default on Mac/Linux.

@fbricon
Copy link
Contributor

fbricon commented May 2, 2019

Closing the spring-boot workspace leads to some project state being corrupted, failing to build on restart

@snjeza snjeza closed this May 2, 2019
@snjeza
Fix build on Windows
e22644c 
Signed-off-by: Snjezana Peco <[email protected]>
@snjeza snjeza reopened this May 2, 2019
@snjeza
Copy link
Contributor Author

snjeza commented May 3, 2019 •
edited
Loading

Closing the spring-boot workspace leads to some project state being corrupted, failing to build on restart

I can't reproduce the issue.
I suppose you closed the workspace while it was building. If so, you probably faced #1010

@snjeza snjeza mentioned this pull request Aug 5, 2019
Closed
@snjeza
Copy link
Contributor Author

snjeza commented Mar 8, 2021

The issue has been fixed within #1661

@snjeza snjeza closed this Mar 8, 2021
@snjeza snjeza mentioned this pull request Nov 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yaohaizh yaohaizh yaohaizh left review comments

@gorkem gorkem Awaiting requested review from gorkem

@fbricon fbricon Awaiting requested review from fbricon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@snjeza @fbricon @yaohaizh