New way of handling jdtls fails to set up required workspace directory.

[ShimmerFairy]

• Server used: jdtls
• Emacs version: 28.1
• Operating system: Gentoo Linux
• Eglot version: 20220726.1405
• Eglot installation method: By way of typing M-x package-list-packages, searching for the eglot package, typing i over it, and then x to complete the install procedure.
• Using Doom: No

LSP transcript - M-x eglot-events-buffer (mandatory unless Emacs inoperable)

[client-request] (id:1) Fri Aug 12 20:29:28 2022:
(:jsonrpc "2.0" :id 1 :method "initialize" :params
          (:processId 1123945 :rootPath "[REDACTED]" :rootUri "[REDACTED]" :initializationOptions #s(hash-table size 1 test eql rehash-size 1.5 rehash-threshold 0.8125 data
                                                                                                                                                  ())
                      :capabilities
                      (:workspace
                       (:applyEdit t :executeCommand
                                   (:dynamicRegistration :json-false)
                                   :workspaceEdit
                                   (:documentChanges t)
                                   :didChangeWatchedFiles
                                   (:dynamicRegistration t)
                                   :symbol
                                   (:dynamicRegistration :json-false)
                                   :configuration t :workspaceFolders t)
                       :textDocument
                       (:synchronization
                        (:dynamicRegistration :json-false :willSave t :willSaveWaitUntil t :didSave t)
                        :completion
                        (:dynamicRegistration :json-false :completionItem
                                              (:snippetSupport :json-false :deprecatedSupport t :tagSupport
                                                               (:valueSet
                                                                [1]))
                                              :contextSupport t)
                        :hover
                        (:dynamicRegistration :json-false :contentFormat
                                              ["markdown" "plaintext"])
                        :signatureHelp
                        (:dynamicRegistration :json-false :signatureInformation
                                              (:parameterInformation
                                               (:labelOffsetSupport t)
                                               :activeParameterSupport t))
                        :references
                        (:dynamicRegistration :json-false)
                        :definition
                        (:dynamicRegistration :json-false :linkSupport t)
                        :declaration
                        (:dynamicRegistration :json-false :linkSupport t)
                        :implementation
                        (:dynamicRegistration :json-false :linkSupport t)
                        :typeDefinition
                        (:dynamicRegistration :json-false :linkSupport t)
                        :documentSymbol
                        (:dynamicRegistration :json-false :hierarchicalDocumentSymbolSupport t :symbolKind
                                              (:valueSet
                                               [1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26]))
                        :documentHighlight
                        (:dynamicRegistration :json-false)
                        :codeAction
                        (:dynamicRegistration :json-false :codeActionLiteralSupport
                                              (:codeActionKind
                                               (:valueSet
                                                ["quickfix" "refactor" "refactor.extract" "refactor.inline" "refactor.rewrite" "source" "source.organizeImports"]))
                                              :isPreferredSupport t)
                        :formatting
                        (:dynamicRegistration :json-false)
                        :rangeFormatting
                        (:dynamicRegistration :json-false)
                        :rename
                        (:dynamicRegistration :json-false)
                        :publishDiagnostics
                        (:relatedInformation :json-false :codeDescriptionSupport :json-false :tagSupport
                                             (:valueSet
                                              [1 2])))
                       :experimental #s(hash-table size 1 test eql rehash-size 1.5 rehash-threshold 0.8125 data
                                                   ()))
                      :workspaceFolders
                      [(:uri "[REDACTED]" :name "[REDACTED]")]))
[stderr] OpenJDK 64-Bit Server VM warning: Options -Xverify:none and -noverify were deprecated in JDK 13 and will likely be removed in a future release.
[stderr] OpenJDK 64-Bit Server VM warning: Options -Xverify:none and -noverify were deprecated in JDK 13 and will likely be removed in a future release.
[stderr] WARNING: Using incubator modules: jdk.incubator.foreign, jdk.incubator.vector
[stderr] WARNING: Using incubator modules: jdk.incubator.foreign, jdk.incubator.vector
[internal] Fri Aug 12 20:29:28 2022:
(:message "Connection state changed" :change "exited abnormally with code 13\n")

----------b---y---e---b---y---e----------

Backtrace (mandatory, unless no error message seen or heard):

Debugger entered--Lisp error: (error "[eglot] -1: Server died")
  signal(error ("[eglot] -1: Server died"))
  error("[eglot] %s" "-1: Server died")
  eglot--error("-1: Server died")
  #f(compiled-function (jsonrpc-lambda-elem22) #<bytecode -0x1fc60213c2d5bf89>)((:code -1 :message "Server died"))
  #f(compiled-function (id triplet) #<bytecode 0xf1ce29bfd5be44d>)(1 (#f(compiled-function (jsonrpc-lambda-elem21) #<bytecode -0x1c97129554ea8997>) #f(compiled-function (jsonrpc-lambda-elem22) #<bytecode -0x1fc60213c2d5bf89>) [nil 25334 61887 501427 nil #f(compiled-function () #<bytecode 0x1c0aa9887147945d>) nil nil 470000 nil]))
  maphash(#f(compiled-function (id triplet) #<bytecode 0xf1ce29bfd5be44d>) #<hash-table eql 1/65 0x156576fc6c03>)
  jsonrpc--process-sentinel(#<process EGLOT (as-git/java-mode)> "exited abnormally with code 13\n")

Minimum Reproducible Example (mandatory)

• Get whatever is the latest copy of jdtls from https://download.eclipse.org/jdtls/snapshots/?d ,
• Open emacs as a graphical application
• Open any old .java file
• Type M-x eglot
• Enter in the path to the jdtls binary (since I do not have it in my PATH)
• Watch it fail

A little investigation showed me that the issue is seemingly with eglot no longer passing the -data argument to jdtls the way it used to, a problem that was noted in the comments of #868 months ago. This is backed up by my own quick experiments with jdtls on the commandline, where omitting -data generates the same error log I get from trying to run the server via eglot (and providing -data, in contrast, keeps it from immediately terminating).

For anyone else with this issue, the "Restore previous JDT interface" code in #868 is a suitable workaround until this regression is fixed.

[joaotavora]

Thanks, i understand the problem now, but this is miles off Eglot's responsibility, so "won't fix". -data is a path to a temporary directory to hold cache related to the current project, a cache which is never written to or read by Eglot, so why on earth should eglot choose its location? Ask jdtls devs to improve their script instead, or do it yourself in python, doesn't seem particularly hard. See https://github.com/joaotavora/eglot/discussions/868#discussioncomment-2551838

[ShimmerFairy]

Eglot should handle it for two very simple reasons:

1. It used to handle it, so clearly there was a point in time where this was seen as "Eglot's responsibility". I'd like an explanation as to what has changed to make it no longer its responsibility. I can also see from eglot.el that you're willing to supply required arguments to other language servers (e.g. (ruby-mode . ("solargraph" "socket" "--port" :autoport))), so I really don't understand what makes -data different.
2. It is an argument that is required for jdtls to function, so if you want Eglot to be functional for java code it has to provide the argument. The only justification I could see for forcing the user to deal with it is to let them pick where to put the cache, but I can't imagine that's something anybody would really need to do (it's just a cache, after all).

"Just do it in python" is not a solution, since that would require manually fixing the python code every time you wanted to update jdtls. It also requires having knowledge of python, which I don't.

I agree that jdtls shouldn't be forcing you to pick a place to put the cache, especially if other language servers don't do this. jdtls definitely needs to be improved here. However, until it's fixed, it's eglot's job to handle it, especially if it purports to support java code out of the box.

I suppose I'm just confused because jdtls, at one point, required much more special-case handling to invoke (as the replacement code in #868 can attest to). And yet now providing one required argument is suddenly too much for Eglot to do itself? I just don't see how a wall of elisp code was more acceptable than this.

There are really only two acceptable solutions to this:

1. Add back the -data argument, just like eglot used to provide.
2. Remove (java-mode . ("jdtls")) until such time that jdtls has been improved to work without arguments. No point in having it if people are required to provide their own java-mode entry via init.el anyway.

[joaotavora]

so I really don't understand what makes -data different.

Because it's something that the server can decide for itself, it doesn't need essential Eglot's input.

[joaotavora]

if it purports to support java code out of the box.

It doesn't, it purports to support LSP period, giving you ample, documented ways to start servers that speak that protocol.And some or rather most,servers are intelligent enough to guess good defaults, while giving you liberty to override them, a liberty that Eglot doesn't interfere with. Seems like jdtls isn't smart enough yet, though it's much better than it used to be. No drama, just ask to make its python code better, and in the meantime configure eglot-server-programs to work around its limitation, as evidently you're able to. But clearly the lines of code you desire readded don't belong in eglot.el, so they're not going to go there.

[ShimmerFairy]

Because it's something that the server can decide for itself, it doesn't need essential Eglot's input.

I can understand that the -data argument is quite different from the arguments other LSPs require you to provide, but I object to the idea that it's not "essential Eglot input". Clearly, jdtls considers it essential input, and thus it is.

if it purports to support java code out of the box.

It doesn't,

By including code for what to do when invoking eglot in java-mode buffers, you do purport to support java out of the box. What (java-mode . ("jdtls")) means is that you've worked out how to invoke an LSP for Java code (at least, Java code being worked on through java-mode), and have decided to set that up for eglot users so they don't have to do it themselves. What I'm saying is that this setup doesn't work, so either modify it to add the argument, or remove it and force users to add a working setup themselves. Whichever way you prefer, the current halfway state of "we know about jdtls but won't invoke it in a way that works" is just wrong.

My point is, if you want people working on Java code to have eglot work without any extra effort (beyond getting the LSP software itself, of course), then the java-mode entry should be modified to provide a default workspace directory. If anyone needs to set up their own directory, they can be directed to override the entry in eglot-server-programs, just like every other language server someone needs to set up custom arguments for.

If instead you want to require people to set up their own -data argument for jdtls, that's fine. But since that means everybody using it would have to override the default java-mode entry in eglot-server-programs, there's no point in having the default entry in the first place. I say it's better to signal your lack of support as soon as possible, to cut down on user confusion and frustration.

But clearly the lines of code you desire readded don't belong in eglot.el, so they're not going to go there.

You misunderstand, I don't want the mess of old code to be put back in, I only meant to point out that it works as a workaround for this regression. What I want is for

(java-mode . ("jdtls"))

to be changed into something like

(java-mode . ("jdtls" "-data" (expand-file-name (md5 (project-root (eglot--current-project)))
                                (locate-user-emacs-file
                                 "eglot-eclipse-jdt-cache"))))

to restore the old behavior. (I can't say if this code would actually work, it's just to illustrate the point.) The only reason why I went for the mess of old code in my workaround is because the way eglot used to do it worked for me, so it was faster for me to use that snippet of code than to work out how to pass the required argument in a way shown above.

All that being said, my quick investigation of the eclipse side of things last night seemed to indicate that -data is more an artifact of how all Eclipse software works in general, rather than a specific design choice on jdtls's part. So hopefully there's room to convince them to have a default workspace directory, since I feel like that's not often something an LSP user needs to control. (Certainly not in my case, I couldn't care less where the generated cruft ends up.) If they can't be convinced, however, then it would fall back on eglot to either provide working out-of-the-box support, or to stop pretending it does.

[joaotavora]

"we know about jdtls but won't invoke it in a way that works" is just wrong.

Please don't attribute these ill intentions to us. There's no ill-will or stubbornness here: the servers that Eglot suggests in eglot-server-programs are just suggestions. They maybe have a million bugs that Eglot is not going fix or to work around.

Here's an alternative: spend your energy in a issue report to the authors of jdtls. You're barking up the wrong tree here.

What I want is for [foo] to be changed into [bla]

Then change it in your config!

As far as I know, @manuel-uberti (and the CI!) tested minimally with jdtls. Maybe something else changed in the Eclipse/jdtls in the meantime. Either way, it's not Eglot's responsibility. Read Manuel's post, it clearly says how to pass additional args to jdtls if you need them or even get back the old config if you have trouble.

Eglot is a lightweight user-facing frontend that is NOT a substitute for downloading, configuring or troubleshooting LSP servers. That is because I have a life. But I do try to make it simpler for people to configure the servers they use.

If this is not what you want from Eglot, sorry. You have another alternative: if I understand correctly, lsp-mode does provide -- at the cost of many a line of code and many dedicated maintainers -- the kind of "turnkey", all-in-one service that you may be looking for!

[manuel-uberti]

If I add the following to my init.el:

(with-eval-after-load 'eglot
  (let ((workspace
         (expand-file-name (md5 (project-root (eglot--current-project)))
                           (locate-user-emacs-file
                            "eglot-eclipse-jdt-cache"))))
    (add-to-list 'eglot-server-programs
                 `(java-mode "jdtls" "-data" ,workspace))))

M-x eglot in, say, /tmp/Test.java works fine with the latest jdtls (the one released on 2022-08-11 14:25) and openjdk version "18.0.2-ea" 2022-07-19.

Should we add this to 868?

[joaotavora]

Should we add this to https://github.com/joaotavora/eglot/discussions/868?

Yes, good idea. Please do that.

Given it's so simple, it could also be added to in eglot-server-programs, though maybe named "cache" or something. Though maybe not such a good idea or only if we remember to remove once some default is added to jdtls itself. Has someone brought this up with jdtls devs already? If so, paste then issue number here.

[manuel-uberti]

I added the instructions for -data to #868 for now.

[manuel-uberti]

Has someone brought this up with jdtls devs already? If so, paste then issue number here.

https://github.com/eclipse/eclipse.jdt.ls/issues/2191

[manuel-uberti]

@joaotavora Since the fix appears to be small and we have the above issue to keep track of the problem, would it make sense to patch Eglot while we wait for the proper fix on the jdtls front?

[joaotavora]

OK, please show a patch, but if it's anything more complicated than:

diff --git a/eglot.el b/eglot.el
index 2e332c470f..8614652681 100644
--- a/eglot.el
+++ b/eglot.el
@@ -173,7 +173,7 @@ language-server/bin/php-language-server.php"))
                                 (go-mode . ("gopls"))
                                 ((R-mode ess-r-mode) . ("R" "--slave" "-e"
                                                         "languageserver::run()"))
-                                (java-mode . ("jdtls"))
+                                (java-mode . ("jdtls" "-data" ".jdtls-cache"))
                                 (dart-mode . ("dart" "language-server"
                                               "--client-id" "emacs.eglot-dart"))
                                 (elixir-mode . ("language_server.sh"))

Then I think it's better to leave it to the user to configure, or wait for jtdls to fix the problem.

[manuel-uberti]

I see your point, and I guess the fix would have to be a bit more complicated than that, because it'd have to take into account a per-project cache directory (i.e. (md5 (project-root (eglot--current-project)))). Unless it's not really necessary, but I'm not sure about this.

Anyway, it's few lines of code for the user to add while we wait for jdtls to provide the correct behavior, and we are documenting how to do it in #868. We can leave it like this.

[joaotavora]

Unless it's not really necessary, but I'm not sure about this.

It would be nice if someone could make sure. My expectation is that passing a relative path to jdtls 's -data argument will make a directory relative to the current path, which is the project directory. So it would, by definition, becomes per-project. But I don't have jdtls to experiment.

[manuel-uberti]

It would be nice if someone could make sure. My expectation is that passing a relative path to jdtls 's -data argument will make a directory relative to the current path, which is the project directory. So it would, by definition, becomes per-project. But I don't have jdtls to experiment.

This is what I quickly did:

• I added this to my init.el

  (with-eval-after-load 'eglot
  (add-to-list 'eglot-server-programs `(java-mode "jdtls" "-data" ".jdtls-cache")))

• I created a /tmp/java-project directory
• I created a /tmp/java-project/Test.java file
• I hit M-x eglot
• I now see a /tmp/java-project/.jdtls-cache directory
• I shut down Eglot and killed the /tmp/java-project/Test.java buffer
• I created a /tmp/java-project2 directory
• I created a /tmp/java-project2/Test2.java file
• I hit M-x eglot
• I now see a /tmp/java-project2/.jdtls-cache directory

I guess you're right, then. The directory passed to -data is relative to the current path.

[joaotavora]

The directory passed to -data is relative to the current path.

Meno male, means jdtls isn't insane after all. Good.

This btw is how clangd and ccls (IIRC) handles caching (it just calls the directory .cache though). A PR to the jdtls probably makes sense, but the way they're ignoring the bug report doesn't bode too well.

[manuel-uberti]

A PR to the jdtls probably makes sense, but the way they're ignoring the bug report doesn't bode too well.

That is why I was wondering if it makes sense for Eglot to act on this, to be honest. :)

[joaotavora]

Maybe they'll pay more attention if we submit a simple PR. And in the meantime, feel free to PR to eglot.el along those lines and maybe simplify the recipe in #868

[manuel-uberti]

Unfortunately, we need someone more used to Python than myself to tackle what's required here: https://github.com/eclipse/eclipse.jdt.ls/pull/2207#issuecomment-1233124919 :(

[joaotavora]

They bring up the problem of concurrent jdtls accessing the same project simultaneously, though I'm not sure I understand the problem. If the concern is justified, then that's one more reason Eglot shouldn't be doing any decision here.

[manuel-uberti]

I see. To me it sounds like we can close https://github.com/joaotavora/eglot/pull/1026 then, because it really feels like the "burden" of a proper configuration is more on jdtls than Eglot.

I am going to simplify the recipe in #868 with your suggestion. At least users can workaround this problem for now.

[joaotavora]

Just to confirm that this is not a regression in Eglot. At some point jdtls worked fine without a -data directory: https://github.com/eclipse/eclipse.jdt.ls/issues/2191#issuecomment-1245437160

[campisano]

It would be nice if someone could make sure. My expectation is that passing a relative path to jdtls 's -data argument will make a directory relative to the current path, which is the project directory. So it would, by definition, becomes per-project. But I don't have jdtls to experiment.

About that, there is a definition in the README of jtdls project: Choose a value for -data: An absolute path to your data directory. eclipse.jdt.ls stores workspace specific information in it. This should be unique per workspace/project.

I'm looking about that because if I set "-data" ".jdtls-cache", the language server starts but does not find language definitions (No definitions found for: LSP identifier at point), and if I set "-data" ,(expand-file-name (md5 (project-root (project-current t)))(locate-user-emacs-file "jdtls-cache")) it works well but it is evaluated at startup time I think, and I got an error when I'm opening a file without be part of a project (probably is a issue of my own config).

However, there is a way to configure eglot-server-programs to evaluate a function only when the eglot is starting for a project?

[phikal]

However, there is a way to configure eglot-server-programs to evaluate a function only when the eglot is starting for a project?

Wouldn't a function in eglot-server-programs do that?

(with-eval-after-load 'eglot
  (defun eglot-jdtls-with-data (_interactive project)
     (let ((cache (file-name-concat
           (xdg-cache-home) "jdtls-cache"
           (sha1 (project-root project)))))
       (list  "jdtls" "-data" cache)))
  (add-to-list 'eglot-server-programs `((java-mode java-ts-mode) . ,#'eglot-jdtls-with-data)))

Can't test it right now, but it is based on @manuel-uberti's snippet above.

Update: Tested the above.

[joaotavora]

Can't test it right now,

It's not exactly that syntax, but yes, it's a a sound idea. I don't have time to fix it. YOu shouldn't use eglot--current-project though, the function is passed the problem as the second argument in recent builds. And the first argument says whether the Eglot invocation was interactive or not.

Of course having good defaults in jdtls trumps all this. Most servers can be told where to cache things, but don't need to