Package name not recognized when opening standalone java files #1766

snjeza · 2021-05-10T17:58:46Z

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

rgrunber

Overall, change looks good. It looks like this adjusts the root path correctly now prior to passing it to loading the invisible project.

...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectImporter.java

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java

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

snjeza · 2021-05-11T21:41:34Z

@rgrunber @jdneo I have updated the PR.

rgrunber · 2021-05-12T04:02:22Z

...eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandler.java

+			while (!BaseInitHandler.isWorkspaceInitialized()) {
+				try {
+					Thread.sleep(100);
+				} catch (InterruptedException e) {
+					// ignore and keep waiting
+				}
+			}
+			try {
+				Job.getJobManager().join(SyntaxInitHandler.JAVA_LS_INITIALIZATION_JOBS, null);
+			} catch (OperationCanceledException | InterruptedException e) {
+				logException(e.getMessage(), e);
+			}


Does this support the usage of triggerFiles or is this some separate issue discovered ? I think we should avoid having requests from the client waiting on each other.

Sometimes VS Code calls BuildWorkspaceHandler.buildWorkspace(boolean, IProgressMonitor) before initializing a workspace.
@rgrunber you have try the following:

clean the Java LS workspace

create a vscode-java vsix with and without this code

install the vsix

unzip wrong-packagename

cd wrong-packagename code .

https://github.com/snjeza/vscode-test/raw/master/java-0.79.3.vsix includes this PR.

@rgrunber @jdneo See redhat-developer/vscode-java#1941 (comment)

jdneo · 2021-05-12T04:34:55Z

Tried to build the jar and played for a while. Encountered some problems:

I got an error

c:\Users\sheche\AppData\Roaming\Code\User\workspaceStorage\b3281020046c073d2668dc7168c1ed4f\redhat.java\jdt_ws: Error: EBUSY: resource busy or locked, unlink 'c:\Users\sheche\AppData\Roaming\Code\User\workspaceStorage\b3281020046c073d2668dc7168c1ed4f\redhat.java\jdt_ws\.metadata\.plugins\org.eclipse.m2e.logback.configuration\0.log'

When I tried to clean the workspace and restart VS Code. (might be a different issue)

There are still some possibility seeing The declared package "mypackage" does not match the expected package "" if keeping clean and restart for several times.

To repro this, you can first set the setting

"java.jdt.ls.vmargs": "-XX:+UseParallelGC -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -Dsun.zip.disableMemoryMapping=true -Xmx1G -Xms100m -agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005,quiet=y"

to the project to open the debug port, and then attach Java Debugger to that project.

Meanwhile, set a breakpoint at InvisibleProjectImporter.java@line 339:

if (project == null || !project.isAccessible()) {
    return "";  // Set breakpoint here
}

And then keep doing clean and restart. You may hit the breakpoint after some tries.

Since the package name is returned as an empty string, thus the result to infer the source directory is wrong because of the empty package name.

It seems that when we clean the workspace, it will also remove the default project, which will make the code fall into the if block that returns an empty package name

jdneo · 2021-05-12T05:28:42Z

After some deep investigation, there are some more findings.

The problem can be divided into the following small pieces.

The declared package "mypackage" does not match the expected package ""

The repro steps is what I mentioned above, I guess it's caused by removing the default project when clean the workspace storage.

The file is not on source path after reload.

This is caused by here:

String invisibleProjectOutputPath = getString(configuration, JAVA_PROJECT_OUTPUT_PATH_KEY, "");

The output path is set to empty string if it's not in the configuration body sent from the client side. But it's initial value is null at first. This difference will invoke preferencesChange() in InvisibleProjectPreferenceChangeListener.

Changing the default value to null should fix this one:

String invisibleProjectOutputPath = getString(configuration, JAVA_PROJECT_OUTPUT_PATH_KEY, null);

About persist the triggerFiles

Suppose a user has been set the source path setting before, and then he remove that entry in his settings.json. In this case, preferencesChange() in InvisibleProjectPreferenceChangeListener will be invoked with the new sourcePath equals null.

In that case we originally think the user should be aware of that he can update the source path and the remove action indicates something as no source paths. However, If we persist the triggerFiles, then we can re-guess the source path as we first import the invisible project, which I think is a better approach.

snjeza · 2021-05-12T16:10:13Z

When I tried to clean the workspace and restart VS Code. (might be a different issue)

Related issue - #1010

The repro steps is what I mentioned above, I guess it's caused by removing the default project when clean the workspace storage.

@jdneo Could you reproduce the issue with https://github.com/snjeza/vscode-test/raw/master/java-0.79.3.vsix

rgrunber · 2021-05-12T21:31:44Z

Ok so (1) has a separate bug if we wish to address it, which just leaves (2) and my complaint about having BuildWorkspaceHandler.buildWorkspace(..) wait on initialization.

With the change by @snjeza (whether I use java-0.79.3.vsix or build the latest PR) , I still see the ".. package does not match expected package .." after a few clean/restarts. If I set the invisibleProjectOutputPath initialization in Preferences so it defaults to null, I still see the same issue :|

I also see this in the logs when the diagnostic is emitted :

[Error - 5:13:17 PM] May 12, 2021, 5:13:16 PM Failed to build workspace.
jdt.ls-java-project does not exist
Java Model Exception: Java Model Status [jdt.ls-java-project does not exist]
	at org.eclipse.jdt.internal.core.JavaElement.newJavaModelException(JavaElement.java:583)
	at org.eclipse.jdt.internal.core.Openable.generateInfos(Openable.java:254)
	at org.eclipse.jdt.internal.core.JavaElement.openWhenClosed(JavaElement.java:596)
	at org.eclipse.jdt.internal.core.JavaElement.getElementInfo(JavaElement.java:326)
	at org.eclipse.jdt.internal.core.JavaElement.getElementInfo(JavaElement.java:312)
	at org.eclipse.jdt.internal.core.JavaElement.getChildren(JavaElement.java:267)
	at org.eclipse.jdt.internal.core.JavaProject.getPackageFragmentRoots(JavaProject.java:2322)
	at org.eclipse.jdt.internal.core.JavaProject.getPackageFragments(JavaProject.java:2346)
	at org.eclipse.jdt.ls.core.internal.managers.ProjectsManager.cleanupResources(ProjectsManager.java:212)
	at org.eclipse.jdt.ls.core.internal.handlers.BuildWorkspaceHandler.buildWorkspace(BuildWorkspaceHandler.java:67)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.lambda$26(JDTLanguageServer.java:834)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.lambda$51(JDTLanguageServer.java:1006)
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:642)
	at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:295)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1016)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1665)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1598)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

For testing I was using a simple Java file with package yourpackage.mypackage; located under src/yourpackage/mypackage from the folder being imported.

One thing I noticed, is if the error happens after a "Clean Java Language Server Workspace", then if I just "Reload Window", it will load things correctly without errors. Monitoring the contents of .classpath after a workspace clean, I see that it first sets the "src" entry to <classpathentry kind="src" path="_/src/mypackage/yourpackage"/> before it gets changed to <classpathentry kind="src" path="_/src"/>, but the error still shows up. To me this seems to imply validation is done on the previous (invalid source entry). This probably explains why a simple restart fixes things.

snjeza · 2021-05-13T02:53:48Z

@rgrunber @jdneo Could you check https://github.com/snjeza/vscode-test/raw/master/java-0.79.4.vsix ?

jdneo · 2021-05-13T08:37:46Z

0.79.4 works well on my windows machine. Also share some findings from my side.

Seems the root cause is that when we handle the initialized notification from client side:

try {
    Job.getJobManager().join(InitHandler.JAVA_LS_INITIALIZATION_JOBS, null);
} catch (OperationCanceledException | InterruptedException e) {
    logException(e.getMessage(), e);
}

we use the above code to wait the initialize job finished. Unfortunately, it does not take effect for some reason. (I suspect it's because a new scheduled job is not set to running status immediately?). Thus, we failed to block the dispatch thread here, then the server starts to handle with the following client request even it's actually not ready.

Changing to JobHelpers.waitForInitializeJobs(); works on my side. In waitForInitializeJobs() it directly get the initialization jobs no matter what status it is, I guess that's why it works.

// It is very interesting since it seems that this issue is there for a long time. 🤔

Anyway, I also upload a fix: #1767. @rgrunber @snjeza Feel free to take a look. And you can just close it if @snjeza has already done the job.

testforstephen · 2021-05-13T09:08:39Z

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/PreferenceManager.java

@@ -68,6 +70,7 @@
 	private ListenerList<IPreferencesChangeListener> preferencesChangeListeners;
 	private IEclipsePreferences eclipsePrefs;
 	private static Map<String, Template> templates = new LinkedHashMap<>();
+	private static Collection<IPath> triggerFiles;


We should not persist the trigger files. They should be used only once. And only use them to calculate a default source path when initializing a new invisible project.

If the user modifies the source paths in a subsequent action, such as changing the workspace setting java.project.sourcePaths, we should always follow the user's action to update the source paths.

This means that if the user sets sourcePaths to null, we should just clean up all the source paths. There is no need to fall back and use trigger files to calculate the source paths again. Since the user knows the settings to change the source paths, he/she can change them back if he/she wants.

testforstephen · 2021-05-13T09:10:21Z

.../src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectPreferenceChangeListener.java

-					IPath outputPath = InvisibleProjectImporter.getOutputPath(javaProject, newPreferences.getInvisibleProjectOutputPath(), true /*isUpdate*/);
-					IClasspathEntry[] classpathEntries = InvisibleProjectImporter.resolveClassPathEntries(javaProject, sourcePaths, excludingPaths, outputPath);
-					javaProject.setRawClasspath(classpathEntries, outputPath, new NullProgressMonitor());
+					InvisibleProjectImporter.updateSourcePaths(javaProject);


This will always use the trigger files to update source paths, regardless of the sourcePaths preference. It's a breaking change.

Only if sourcePaths preference is explicitly changed, then need to update source paths. Otherwise, just update project output path directly via javaProject.setOutputLocation.

See @jdneo's PR https://github.com/eclipse/eclipse.jdt.ls/pull/1767/files

snjeza · 2021-05-13T14:55:11Z

See #1767

rgrunber · 2021-05-13T15:26:27Z

Thanks, we can have a look there. FWIW, your change (java-0.79.4.vsix ) does seem to work for me now and from what I can tell, modifying java.project.sourcePaths is respected, but if there's a solution that avoid touching triggerFiles / waiting on LSP calls, we should go with that.

snjeza requested review from gorkem, fbricon, rgrunber, jdneo and testforstephen May 10, 2021 17:58

rgrunber requested changes May 10, 2021

View reviewed changes

...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectImporter.java Outdated Show resolved Hide resolved

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java Outdated Show resolved Hide resolved

Package name not recognized when opening standalone java files

3ff0a08

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

snjeza force-pushed the issue-1764 branch from 0155210 to 3ff0a08 Compare May 11, 2021 21:40

rgrunber reviewed May 12, 2021

View reviewed changes

snjeza mentioned this pull request May 12, 2021

Code Workspace Classpath is corrupted after restart redhat-developer/vscode-java#1941

Closed

snjeza added the in progress label May 12, 2021

jdneo mentioned this pull request May 13, 2021

Fix package name not recognized when opening standalone java files #1767

Merged

testforstephen reviewed May 13, 2021

View reviewed changes

snjeza closed this May 13, 2021

snjeza mentioned this pull request Jul 16, 2021

jdtls freezes when importing maven project #1816

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Package name not recognized when opening standalone java files #1766

Package name not recognized when opening standalone java files #1766

snjeza commented May 10, 2021

rgrunber left a comment

snjeza commented May 11, 2021

rgrunber May 12, 2021 •

edited

Loading

snjeza May 12, 2021

snjeza May 12, 2021

jdneo commented May 12, 2021

jdneo commented May 12, 2021 •

edited

Loading

snjeza commented May 12, 2021

rgrunber commented May 12, 2021 •

edited

Loading

snjeza commented May 13, 2021

jdneo commented May 13, 2021

testforstephen May 13, 2021

testforstephen May 13, 2021

snjeza commented May 13, 2021

rgrunber commented May 13, 2021

Package name not recognized when opening standalone java files #1766

Package name not recognized when opening standalone java files #1766

Conversation

snjeza commented May 10, 2021

rgrunber left a comment

Choose a reason for hiding this comment

snjeza commented May 11, 2021

rgrunber May 12, 2021 • edited Loading

Choose a reason for hiding this comment

snjeza May 12, 2021

Choose a reason for hiding this comment

snjeza May 12, 2021

Choose a reason for hiding this comment

jdneo commented May 12, 2021

jdneo commented May 12, 2021 • edited Loading

The declared package "mypackage" does not match the expected package ""

The file is not on source path after reload.

About persist the triggerFiles

snjeza commented May 12, 2021

rgrunber commented May 12, 2021 • edited Loading

snjeza commented May 13, 2021

jdneo commented May 13, 2021

testforstephen May 13, 2021

Choose a reason for hiding this comment

testforstephen May 13, 2021

Choose a reason for hiding this comment

snjeza commented May 13, 2021

rgrunber commented May 13, 2021

rgrunber May 12, 2021 •

edited

Loading

jdneo commented May 12, 2021 •

edited

Loading

rgrunber commented May 12, 2021 •

edited

Loading