Pass data directory to jdtls (#2191)

[manuel-uberti]

trafficstars

This PR should take care of https://github.com/eclipse/eclipse.jdt.ls/issues/2191 and improve the interaction with Eglot (see https://github.com/joaotavora/eglot/issues/1008). Or at least, it's a start. :)

If I am missing something, please do let me know. I'm not a Python expert, so I may be setting -data in the wrong way here.

[eclipse-ls-bot]

Can one of the admins verify this patch?

[fbricon]

I don't think hard-coding the data directory will work, as you'll certainly run into some catastrophic issues if multiple jdtls processes try to write into the same directory. A proper fix should

• allow -data args to be passed to jdtls.py
• if no -data is passed, then build a repeatable data path out of the current directory. eg. tempdir/jdtls-sha1(currdir). That'd allow 2 instances to reuse caches for their current working directory.

[joaotavora]

The first bullet is already done, if I understand correctly, and the second bullet looks easy enough. I could help implement it, if you explain further, with an example of two concurrent jdtls processes, what this sha-ing does to prevent problems. What exactly should the name of the temporary cache/data directory depend on?

[rgrunber]

In python, there should be some library that gives you the system's temporary directory. Maybe https://docs.python.org/3/library/tempfile.html ?

So something like path.join(tempfile.gettempdir(), path.basename(os.getcwd())) .

[joaotavora]

So something like path.join(tempfile.gettempdir(), path.basename(os.getcwd())) .

This solution sounds perfectly fine and much better than what is available now.

I can't resist noting something however: if I read correctly, this will make a directory to hold the cache that "multiple jdtls processes" will try to write to simultaneously, IF these multiple processes target the same Java project. Not sure if this doesn't violate @fbricon 's concern. Also, while this approach is anatomically different from placing a .cache diretory inside the project directory -- functionally it seems to be exactly the same. Maybe @fbricon believes it is best to use something that is more unique to each process like https://docs.python.org/3/library/tempfile.html#tempfile.TemporaryDirectory. But then the cache will not be reusable across different jtdls sessions.

[fbricon]

Indeed, this won't prevent multiple processes working on the same directory to collide, but it's a compromise between performance and safety. Making it safer will prevent reusing cache, as you guessed it. But if anyone is looking to work on the same project from multiple processes at the same time, I feel like they're looking for trouble.

[joaotavora]

Indeed, this won't prevent multiple processes working on the same directory to collide, but it's a compromise between performance and safety. Making it safer will prevent reusing cache, as you guessed it. But if anyone is looking to work on the same project from multiple processes at the same time, I feel like they're looking for trouble.

In that case, I think -- unless I'm missing something very obvious -- that the solution right now being proposed in this very PR by @manuel-uberti , the one with a single line of code provides exactly the same compromise. And it's probably only a couple of green-button clicks away.

[mfussenegger]

For nvim-jdtls I recommend setting -data to a directory outside of the project folder because it can lead to problems.

E.g. if I set -data .jdtls-cache as done in this PR, I run into the following error:

PM Synchronize project crate failed due to an error while importing the root project.
org.eclipse.buildship.core.internal.GradlePluginsRuntimeException: org.eclipse.core.internal.resources.ResourceException: Invalid project description.
org.eclipse.buildship.core.internal.ImportRootProjectException: org.eclipse.buildship.core.internal.GradlePluginsRuntimeException: org.eclipse.core.internal.resources.ResourceException: Invalid project description.
	at org.eclipse.buildship.core.internal.workspace.ImportRootProjectOperation.run(ImportRootProjectOperation.java:53)
	at org.eclipse.buildship.core.internal.DefaultGradleBuild$SynchronizeOperation.runInToolingApi(DefaultGradleBuild.java:225)
	at org.eclipse.buildship.core.internal.operation.DefaultToolingApiOperationManager$WorkspaceRunnableAdapter.run(DefaultToolingApiOperationManager.java:58)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:2380)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:2405)
	at org.eclipse.buildship.core.internal.operation.DefaultToolingApiOperationManager.run(DefaultToolingApiOperationManager.java:39)
	at org.eclipse.buildship.core.internal.DefaultGradleBuild$SynchronizeOperation.run(DefaultGradleBuild.java:192)
	at org.eclipse.buildship.core.internal.DefaultGradleBuild.synchronize(DefaultGradleBuild.java:100)
	at org.eclipse.buildship.core.internal.DefaultGradleBuild.synchronize(DefaultGradleBuild.java:86)
	at org.eclipse.jdt.ls.core.internal.managers.GradleProjectImporter.startSynchronization(GradleProjectImporter.java:407)
	at org.eclipse.jdt.ls.core.internal.managers.GradleProjectImporter.importDir(GradleProjectImporter.java:295)
	at org.eclipse.jdt.ls.core.internal.managers.GradleProjectImporter.importToWorkspace(GradleProjectImporter.java:211)
	at org.eclipse.jdt.ls.core.internal.managers.ProjectsManager.importProjects(ProjectsManager.java:149)
	at org.eclipse.jdt.ls.core.internal.managers.ProjectsManager.initializeProjects(ProjectsManager.java:111)
	at org.eclipse.jdt.ls.core.internal.handlers.InitHandler$1.runInWorkspace(InitHandler.java:244)
	at org.eclipse.core.internal.resources.InternalWorkspaceJob.run(InternalWorkspaceJob.java:43)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:63)
Caused by: org.eclipse.buildship.core.internal.GradlePluginsRuntimeException: org.eclipse.core.internal.resources.ResourceException: Invalid project description.
	at org.eclipse.buildship.core.internal.workspace.DefaultWorkspaceOperations.createProject(DefaultWorkspaceOperations.java:128)
	at org.eclipse.buildship.core.internal.workspace.ImportRootProjectOperation.importRootProject(ImportRootProjectOperation.java:95)
	at org.eclipse.buildship.core.internal.workspace.ImportRootProjectOperation$1.run(ImportRootProjectOperation.java:68)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:2380)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:2400)
	at org.eclipse.buildship.core.internal.workspace.ImportRootProjectOperation.runInWorkspace(ImportRootProjectOperation.java:58)
	at org.eclipse.buildship.core.internal.workspace.ImportRootProjectOperation.run(ImportRootProjectOperation.java:51)
	... 16 more
Caused by: org.eclipse.core.internal.resources.ResourceException: Invalid project description.
	at org.eclipse.core.internal.resources.Project.checkDescription(Project.java:178)
	at org.eclipse.core.internal.resources.Project.assertCreateRequirements(Project.java:62)
	at org.eclipse.core.internal.resources.Project.create(Project.java:272)
	at org.eclipse.core.internal.resources.Project.create(Project.java:258)
	at org.eclipse.buildship.core.internal.workspace.DefaultWorkspaceOperations.createProject(DefaultWorkspaceOperations.java:113)
	... 22 more

And diagnostics report: is a non-project file, only syntax errors are reported

[joaotavora]

Seems not obvious at all where that error is coming from or exactly what thing "is a non-project-file".

Can't one have a hidden directory with whatever things in a Java project? Maybe not. Personally, I don't use Java (:pray: ) and I'd just like my LSP client's users a way to start up their projects, so if a somewhat complicated out-of-project scheme is the way to go, then I say go for it.

[manuel-uberti]

@mfussenegger Where does -data usually point at for you?

If it needs to be outside of one's Java projects, then an OS-agnostic default for -data is needed, maybe? (Just thinking out loud.)

[rgrunber]

@mfussenegger Where does -data usually point at for you?

If it needs to be outside of one's Java projects, then an OS-agnostic default for -data is needed, maybe? (Just thinking out loud.)

So something like path.join(tempfile.gettempdir(), path.basename(os.getcwd())) .

That's the purpose of "tmpdir". It would be something defined by the OS. I guess the problem with a default of .jdtls-cache is some people may try to start the script within a project folder, which seems to be an issue when importing the project.

[rgrunber]

Are people able to verify this works for them ?

[manuel-uberti]

Are people able to verify this works for them ?

Yes, and it works for me with the following setup:

• GNU Emacs 29.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.33, cairo version 1.16.0) of 2022-09-26
• Eglot 1.8.0.20220919.151933

This is what I did:

• I created a temp file in /tmp/test-java/Test.java
• I hit M-x eglot
• Then, for instance, I got code completion working as expected (using Emacs built-in completion-at-point)

[rgrunber]

The initial ECA complaint was because you need to create an Eclipse account and sign the ECA. However since I think I've replaced all lines in the initial PR, there shouldn't be any issue. I've listed you as a co-author since your initial PR would have solved the issue, and helped generate enough interest to get this resolved.

[manuel-uberti]

The initial ECA complaint was because you need to create an Eclipse account and sign the ECA. However since I think I've replaced all lines in the initial PR, there shouldn't be any issue. I've listed you as a co-author since your initial PR would have solved the issue, and helped generate enough interest to get this resolved.

Fantastic, thank you. And thanks a lot for the fix.