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

Flymake doesn't start properly when using eglot over Tramp #670

Closed
jimporter opened this issue Apr 16, 2021 · 8 comments
Closed

Flymake doesn't start properly when using eglot over Tramp #670

jimporter opened this issue Apr 16, 2021 · 8 comments

Comments

@jimporter
Copy link
Contributor

jimporter commented Apr 16, 2021

When opening a file over Tramp, Flymake fails to highlight errors, and once you make a change to the file, the Flymake status in the modeline will be stuck at "Wait" forever. To test this, I used the following C++ file:

#include <vector>

int main() {
  std::vector<int> v;
  v.

If I open this file locally and activate eglot via M-x eglot, I see Flymake errors on lines 3 (at the {) and 5 (at the .), which is what I'd expect. If I open this file over Tramp (with C-x C-f /ssh:localhost:~/path/to/file.cpp RET) and activate eglot, I don't see Flymake errors. However, I am able to use LSP completion as normal, e.g. by putting the point at the end of the file and hitting C-M-i; all the member functions for std::vector show up as expected.

I'm not sure if there's a Flymake-specific buffer that I should provide to help diagnose this (I've never really used Flymake until now), so just let me know if you need any other info.

Note: this seems similar to #632, but I only see a problem when editing a file over Tramp.


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

[client-request] (id:1) Fri Apr 16 14:05:28 2021:
(:jsonrpc "2.0" :id 1 :method "initialize" :params
	  (:processId nil :rootPath "/home/jim/test/" :rootUri "file:///home/jim/test" :initializationOptions #s(hash-table size 65 test eql rehash-size 1.5 rehash-threshold 0.8125 data
															    ())
		      :capabilities
		      (:workspace
		       (:applyEdit t :executeCommand
				   (:dynamicRegistration :json-false)
				   :workspaceEdit
				   (:documentChanges :json-false)
				   :didChangeWatchedFiles
				   (:dynamicRegistration t)
				   :symbol
				   (:dynamicRegistration :json-false)
				   :configuration t)
		       :textDocument
		       (:synchronization
			(:dynamicRegistration :json-false :willSave t :willSaveWaitUntil t :didSave t)
			:completion
			(:dynamicRegistration :json-false :completionItem
					      (:snippetSupport :json-false)
					      :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)
			:declaration
			(:dynamicRegistration :json-false)
			:implementation
			(:dynamicRegistration :json-false)
			:typeDefinition
			(:dynamicRegistration :json-false)
			: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))
		       :experimental #s(hash-table size 65 test eql rehash-size 1.5 rehash-threshold 0.8125 data
						   ()))))
[server-reply] (id:1) Fri Apr 16 14:05:28 2021:
(:jsonrpc "2.0" :id 1 :result
	  (:capabilities
	   (:textDocumentSync
	    (:openClose t :change 2 :willSave :json-false :willSaveWaitUntil :json-false :save
			(:includeText :json-false))
	    :hoverProvider t :completionProvider
	    (:resolveProvider :json-false :triggerCharacters
			      ["." ":" ">" "#" "<" "\"" "/"])
	    :signatureHelpProvider
	    (:triggerCharacters
	     ["(" ","])
	    :declarationProvider t :definitionProvider t :implementationProvider t :typeDefinitionProvider t :referencesProvider t :documentHighlightProvider t :documentSymbolProvider t :workspaceSymbolProvider t :codeActionProvider
	    (:codeActionKinds
	     ["quickfix"])
	    :codeLensProvider
	    (:resolveProvider :json-false)
	    :documentFormattingProvider t :documentRangeFormattingProvider t :documentOnTypeFormattingProvider
	    (:firstTriggerCharacter "}" :moreTriggerCharacter
				    [])
	    :renameProvider t :documentLinkProvider
	    (:resolveProvider t)
	    :foldingRangeProvider t :executeCommandProvider
	    (:commands
	     ["ccls.xref"])
	    :workspace
	    (:workspaceFolders
	     (:supported t :changeNotifications t)))))
[client-notification] Fri Apr 16 14:05:28 2021:
(:jsonrpc "2.0" :method "initialized" :params #s(hash-table size 65 test eql rehash-size 1.5 rehash-threshold 0.8125 data
							    ()))
[client-notification] Fri Apr 16 14:05:28 2021:
(:jsonrpc "2.0" :method "textDocument/didOpen" :params
	  (:textDocument
	   (:uri "file:///home/jim/test/test.cpp" :version 0 :languageId "c++" :text "#include <vector>\n\nint main() {\n  std::vector<int> v;\n  v.\n")))
[client-notification] Fri Apr 16 14:05:28 2021:
(:jsonrpc "2.0" :method "workspace/didChangeConfiguration" :params
	  (:settings nil))
[server-request] (id:0) Fri Apr 16 14:05:28 2021:
(:jsonrpc "2.0" :method "client/registerCapability" :id 0 :params
	  (:registrations
	   [(:id "didChangeWatchedFiles" :method "workspace/didChangeWatchedFiles" :registerOptions
		 (:watchers
		  [(:globPattern "**/*")]))]))
[client-reply] (id:0) Fri Apr 16 14:05:29 2021:
(:jsonrpc "2.0" :id 0 :result nil)
[server-notification] Fri Apr 16 14:05:29 2021:
(:jsonrpc "2.0" :method "$ccls/publishSkippedRanges" :params
	  (:uri "file:///home/jim/test/test.cpp" :skippedRanges
		[]))
[server-notification] Fri Apr 16 14:05:29 2021:
(:jsonrpc "2.0" :method "$ccls/publishSemanticHighlight" :params
	  (:uri "file:///home/jim/test/test.cpp" :symbols
		[(:id 0 :parentKind 12 :kind 13 :storage 0 :ranges
		      [(:L 51 :R 52)]
		      :lsRanges
		      [])
		 (:id 1 :parentKind 0 :kind 3 :storage 0 :ranges
		      [(:L 34 :R 37)]
		      :lsRanges
		      [])
		 (:id 0 :parentKind 1 :kind 12 :storage 0 :ranges
		      [(:L 23 :R 27)]
		      :lsRanges
		      [])
		 (:id 0 :parentKind 0 :kind 5 :storage 0 :ranges
		      [(:L 39 :R 45)]
		      :lsRanges
		      [])]))
[server-notification] Fri Apr 16 14:05:29 2021:
(:jsonrpc "2.0" :method "$ccls/publishSemanticHighlight" :params
	  (:uri "file:///home/jim/test/test.cpp" :symbols
		[(:id 0 :parentKind 12 :kind 13 :storage 0 :ranges
		      [(:L 51 :R 52)]
		      :lsRanges
		      [])
		 (:id 1 :parentKind 0 :kind 3 :storage 0 :ranges
		      [(:L 34 :R 37)]
		      :lsRanges
		      [])
		 (:id 0 :parentKind 1 :kind 12 :storage 0 :ranges
		      [(:L 23 :R 27)]
		      :lsRanges
		      [])
		 (:id 0 :parentKind 0 :kind 5 :storage 0 :ranges
		      [(:L 39 :R 45)]
		      :lsRanges
		      [])]))
[server-notification] Fri Apr 16 14:05:29 2021:
(:jsonrpc "2.0" :method "textDocument/publishDiagnostics" :params
	  (:uri "file:///home/jim/test/test.cpp" :diagnostics
		[(:range
		  (:start
		   (:line 4 :character 4)
		   :end
		   (:line 4 :character 4))
		  :severity 1 :code 4 :source "ccls" :message "expected unqualified-id" :relatedInformation
		  [])
		 (:range
		  (:start
		   (:line 4 :character 4)
		   :end
		   (:line 4 :character 4))
		  :severity 1 :code 4 :source "ccls" :message "expected '}'\n\ntest.cpp:3:12: note: to match this '{'" :relatedInformation
		  [])
		 (:range
		  (:start
		   (:line 2 :character 11)
		   :end
		   (:line 2 :character 12))
		  :severity 3 :code 0 :source "ccls" :message "to match this '{'\n\ntest.cpp:5:5: error: expected '}'" :relatedInformation
		  [])]))
[internal] Fri Apr 16 14:05:29 2021:
(:message "Diagnostics received for unvisited (file:///home/jim/test/test.cpp)")
[client-request] (id:2) Fri Apr 16 14:05:31 2021:
(:jsonrpc "2.0" :id 2 :method "textDocument/signatureHelp" :params
	  (:textDocument
	   (:uri "file:///home/jim/test/test.cpp")
	   :position
	   (:line 4 :character 4)))
[client-request] (id:3) Fri Apr 16 14:05:31 2021:
(:jsonrpc "2.0" :id 3 :method "textDocument/hover" :params
	  (:textDocument
	   (:uri "file:///home/jim/test/test.cpp")
	   :position
	   (:line 4 :character 4)))
[client-request] (id:4) Fri Apr 16 14:05:31 2021:
(:jsonrpc "2.0" :id 4 :method "textDocument/documentHighlight" :params
	  (:textDocument
	   (:uri "file:///home/jim/test/test.cpp")
	   :position
	   (:line 4 :character 4)))
[server-reply] (id:3) Fri Apr 16 14:05:31 2021:
(:jsonrpc "2.0" :id 3 :result
	  (:contents
	   []))
[server-reply] (id:4) Fri Apr 16 14:05:31 2021:
(:jsonrpc "2.0" :id 4 :result
	  [])
[server-reply] (id:2) Fri Apr 16 14:05:31 2021:
(:jsonrpc "2.0" :id 2 :result
	  (:signatures
	   []
	   :activeSignature 0 :activeParameter 0))
[client-request] (id:5) Fri Apr 16 14:05:57 2021:
(:jsonrpc "2.0" :id 5 :method "textDocument/signatureHelp" :params
	  (:textDocument
	   (:uri "file:///home/jim/test/test.cpp")
	   :position
	   (:line 5 :character 0)))
[client-request] (id:6) Fri Apr 16 14:05:57 2021:
(:jsonrpc "2.0" :id 6 :method "textDocument/hover" :params
	  (:textDocument
	   (:uri "file:///home/jim/test/test.cpp")
	   :position
	   (:line 5 :character 0)))
[client-request] (id:7) Fri Apr 16 14:05:57 2021:
(:jsonrpc "2.0" :id 7 :method "textDocument/documentHighlight" :params
	  (:textDocument
	   (:uri "file:///home/jim/test/test.cpp")
	   :position
	   (:line 5 :character 0)))
[server-reply] (id:5) Fri Apr 16 14:05:57 2021:
(:jsonrpc "2.0" :id 5 :result
	  (:signatures
	   []
	   :activeSignature 0 :activeParameter 0))
[server-reply] (id:6) Fri Apr 16 14:05:57 2021:
(:jsonrpc "2.0" :id 6 :result
	  (:contents
	   []))
[server-reply] (id:7) Fri Apr 16 14:05:57 2021:
(:jsonrpc "2.0" :id 7 :result
	  [])
[client-request] (id:8) Fri Apr 16 14:06:02 2021:
(:jsonrpc "2.0" :id 8 :method "textDocument/signatureHelp" :params
	  (:textDocument
	   (:uri "file:///home/jim/test/test.cpp")
	   :position
	   (:line 5 :character 0)))
[client-request] (id:9) Fri Apr 16 14:06:02 2021:
(:jsonrpc "2.0" :id 9 :method "textDocument/hover" :params
	  (:textDocument
	   (:uri "file:///home/jim/test/test.cpp")
	   :position
	   (:line 5 :character 0)))
[client-request] (id:10) Fri Apr 16 14:06:02 2021:
(:jsonrpc "2.0" :id 10 :method "textDocument/documentHighlight" :params
	  (:textDocument
	   (:uri "file:///home/jim/test/test.cpp")
	   :position
	   (:line 5 :character 0)))
[server-reply] (id:8) Fri Apr 16 14:06:02 2021:
(:jsonrpc "2.0" :id 8 :result
	  (:signatures
	   []
	   :activeSignature 0 :activeParameter 0))
[server-reply] (id:9) Fri Apr 16 14:06:02 2021:
(:jsonrpc "2.0" :id 9 :result
	  (:contents
	   []))
[server-reply] (id:10) Fri Apr 16 14:06:02 2021:
(:jsonrpc "2.0" :id 10 :result
	  [])

Minimal configuration (mandatory)

# Type this in a shell to start an Emacs with Eglot configured
$ emacs
(require 'package)
(add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/") t)
(package-install "tramp") ; Get Tramp (2.5.0.3) from GNU ELPA (not 100% sure if this is required)
(package-install "eglot") ; Get eglot + deps from MELPA
@joaotavora
Copy link
Owner

joaotavora commented Apr 17, 2021

Type this in a shell to start an Emacs with Eglot configured
$ emacs

If you don't provide -Q or -q this is going to load your configuration.

But since you seem to understand how to call programs from the shell, can you try with? It would be much better if

$ /path/to/a/certain/version/of/emacs -Q -f package-initialize -L /path/to/git-cloned/eglot -l eglot.el 

Regardless, I think I have enough to reproduce your issue when I find some time. Thanks!

@jimporter
Copy link
Contributor Author

jimporter commented Apr 17, 2021

If you don't provide -Q or -q this is going to load your configuration.

I commented out everything in my .emacs.d/init.el to prevent that. I've retested things using the invocation above with emacs -Q ..., using a fresh Git clone of this repo, and I get the same issue as described above.


I think I was just overthinking the instructions in the bug template. Since I'm testing on Emacs 27, eglot depends on the newer GNU ELPA versions of several core packages, including Flymake. I was worried that this would result in things breaking in unexpected ways. I had had issues originally when I installed eglot via M-x list-packages and it used the old, built-in version of package.el until I restarted Emacs, so I tried to avoid that problem when reproducing this issue by keeping my installed ELPA packages around (after carefully verifying that the versions were correct).

It turns out I didn't need to go through all that for this bug, but it could be relevant when other people try to report bugs on older versions of Emacs.

@jimporter
Copy link
Contributor Author

jimporter commented Apr 20, 2021

After some debugging, I think I've found where the issue is:

eglot/eglot.el

Line 1652 in fc221c8

(if-let ((buffer (find-buffer-visiting (eglot--uri-to-path uri))))

This takes a URI from the LSP server, converts it into a regular path, and then tries to find a buffer for it. However, (current-buffer) at this point is a temp buffer, so (eglot-current-server) (called from inside eglot--uri-to-path) is nil. Thus, eglot--uri-to-path doesn't add the remote Tramp prefix, and eglot ends up thinking we got diagnostics for a file not being visited.

Luckily, we know what the current server is, since the snippet above is in eglot-handle-notification, a method on the server. If eglot--uri-to-path took an optional parameter for the server, then this issue would be fixed.

I can write a patch for this, but I'm still waiting for copyright assignment paperwork for some other Emacs changes. A patch for this is probably small enough to not count as "legally significant", but I have a couple of merged Tramp patches that put me at the limit. I'm not sure if it would be against the rules for me to supply a patch here (at least not until my copyright paperwork is done), so I've erred on the side of caution and avoided writing a patch.

@joaotavora
Copy link
Owner

This takes a URI from the LSP server, converts it into a regular path, and then tries to find a buffer for it. However, (current-buffer) at this point is a temp buffer, so (eglot-current-server) (called from inside eglot--uri-to-path) is nil. Thus, eglot--uri-to-path doesn't add the remote Tramp prefix, and eglot ends up thinking we got diagnostics for a file not being visited.

Nice. Good analysis. So I guess this should either be called from the correct buffer, or eglot--cached-server should be set. I'll have a look, thanks for the analysis.

@joaotavora
Copy link
Owner

joaotavora commented Apr 20, 2021

Can you try this patch?

diff --git a/eglot.el b/eglot.el
index 20f5995..f926709 100644
--- a/eglot.el
+++ b/eglot.el
@@ -945,7 +945,8 @@ This docstring appeases checkdoc, that's all."
                                   (format "*%s stderr*" readable-name))
                          :file-handler t)))))))
          (spread (lambda (fn) (lambda (server method params)
-                                (apply fn server method (append params nil)))))
+                                (let ((eglot--cached-server server))
+                                 (apply fn server method (append params nil))))))
          (server
           (apply
            #'make-instance class

@jimporter
Copy link
Contributor Author

That also works! Flymake errors show up correctly and editing the file correctly updates the Flymake annotations.

However, I do get another unusual problem as a result of this: occasionally, if I make successive edits to the file, eventually I get this error message: tramp-send-string: peculiar error: "Forbidden reentrant call of Tramp". It seems that sometimes this is just a temporary hiccup and subsequent commands work fine, but other times I have to restart eglot. I haven't had a chance to fully understand what's going on here, but my guess is that eglot is starting a Tramp operation from an async function while there's already a Tramp operation in progress. There was recently a discussion on tramp-devel about this too: https://lists.gnu.org/archive/html/tramp-devel/2021-04/msg00034.html.

@albinus, do you have any thoughts on the "Forbidden reentrant call of Tramp" error?

I'll try to provide more details here as I narrow down what's actually happening.

@joaotavora
Copy link
Owner

joaotavora commented Apr 20, 2021

@jimporter nice, so I'll fix this issue, but please open another, different issue for this seemingly completely different problem.

@jimporter
Copy link
Contributor Author

jimporter commented Apr 20, 2021

@joaotavora I installed the latest revision from MELPA on an MS Windows system and it all looks good, with one weird issue I don't understand: the automatically-compiled eglot.elc didn't work for me and I had to manually byte-compile the file. Once I did that, everything was ok. I tried removing my previous installation of eglot and reinstalling it, but that didn't help. Only manually byte-compiling eglot.el again fixed it.

I'm not sure what's causing this (it's all magic to me), but I figured I'd mention it. Maybe there's something eglot can do about this, or maybe it's just a bug in Emacs (or package.el specifically)...

Edit: Hmm, I notice that the Travis CI build for eglot failed with an error pointing to the newly-added code: https://travis-ci.org/github/joaotavora/eglot/jobs/767765262#L513

joaotavora added a commit that referenced this issue Apr 23, 2021
Per #670.

Otherwise the dynamic binding of it in in eglot--connect won't work.

* eglot.el (eglot--cached-server): Move up.
bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 18, 2022
…ion handlers

* eglot.el (eglot--connect): Ensure `eglot--cached-server` bound
when calling notification/request methods.
bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 18, 2022
Per joaotavora/eglot#670.

Otherwise the dynamic binding of it in in eglot--connect won't work.

* eglot.el (eglot--cached-server): Move up.
bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 19, 2022
…ion handlers

* eglot.el (eglot--connect): Ensure `eglot--cached-server` bound
when calling notification/request methods.
bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 19, 2022
Per joaotavora/eglot#670.

Otherwise the dynamic binding of it in in eglot--connect won't work.

* eglot.el (eglot--cached-server): Move up.
bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 19, 2022
* eglot.el (eglot--connect): Ensure `eglot--cached-server` bound
when calling notification/request methods.

#670: joaotavora/eglot#670
bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 19, 2022
Per #670.

Otherwise the dynamic binding of it in in eglot--connect won't work.

* eglot.el (eglot--cached-server): Move up.

#673: joaotavora/eglot#673
#670: joaotavora/eglot#670
jollaitbot pushed a commit to sailfishos-mirror/emacs that referenced this issue Oct 12, 2022
* eglot.el (eglot--connect): Ensure `eglot--cached-server` bound
when calling notification/request methods.

GitHub-reference: fix joaotavora/eglot#670
jollaitbot pushed a commit to sailfishos-mirror/emacs that referenced this issue Oct 12, 2022
Per joaotavora/eglot#670.

Otherwise the dynamic binding of it in in eglot--connect won't work.

* eglot.el (eglot--cached-server): Move up.

GitHub-reference: fix joaotavora/eglot#673
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

No branches or pull requests

2 participants