eglot and tramp

eglot and tramp

[Pedro Andres Aranda Gutierrez]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

Hi

I'm looking for some advice on using eglot in a remote TRAMP editing
session. I have a couple of scenarios where I'd need to access a VM and
edit Python code residing inside. Having a Python LSP that can help me in
this use case would be really great.

Since there is nothing in the eglot manual about this scenario, I'm asking
for enlightenment here

Thanks a ton, /PA

-- 
Fragen sind nicht da um beantwortet zu werden,
Fragen sind da um gestellt zu werden
Georg Kreisler

Headaches with a Juju log:
unit-basic-16: 09:17:36 WARNING juju.worker.uniter.operation we should run
a leader-deposed hook here, but we can't yet

[-- Attachment #2: Type: text/html, Size: 971 bytes --]

Re: eglot and tramp

[Brian Cully via Emacs development discussions.]

Pedro Andres Aranda Gutierrez <paaguti@gmail.com> writes:

> I'm looking for some advice on using eglot in a remote TRAMP editing
> session. I have a couple of scenarios where I'd need to access a VM and
> edit Python code residing inside. Having a Python LSP that can help me in
> this use case would be really great.
>
> Since there is nothing in the eglot manual about this scenario, I'm asking
> for enlightenment here

As long as the language server is installed on the remote, it should
Just Work™.

You may need to add ‘'tramp-own-remote-path’ to ‘tramp-remote-path’,
depending on where the LSP is installed, however.

-bjc

Re: eglot and tramp

[Michael Albinus]

Brian Cully via "Emacs development discussions." <emacs-devel@gnu.org>
writes:

Hi,

>> I'm looking for some advice on using eglot in a remote TRAMP editing
>> session. I have a couple of scenarios where I'd need to access a VM and
>> edit Python code residing inside. Having a Python LSP that can help me in
>> this use case would be really great.
>>
>> Since there is nothing in the eglot manual about this scenario, I'm asking
>> for enlightenment here
>
> As long as the language server is installed on the remote, it should
> Just Work™.
>
> You may need to add ‘'tramp-own-remote-path’ to ‘tramp-remote-path’,
> depending on where the LSP is installed, however.

It is on my TODO to check how well Tramp is integrated with eglot. Great
to hear that it seems to work flawless, but I believe there might be
some hints for performance improvement, like setting
"direct-async-process" connection property.

When time permits I'll go through this, and I might contribute some few
words to the eglot manual.

> -bjc

Best regards, Michael.

Re: eglot and tramp

[Brian Cully via Emacs development discussions.]

Michael Albinus <michael.albinus@gmx.de> writes:

> It is on my TODO to check how well Tramp is integrated with eglot. Great
> to hear that it seems to work flawless, but I believe there might be
> some hints for performance improvement, like setting
> "direct-async-process" connection property.

I wouldn't go so far as to say "flawlessly". It does often run into the
"forbidden re-entrant call to Tramp" problem, but that's not unique to
Eglot, and it doesn't seem to cause any problems other than an annoying
message periodically in the echo area.

Otherwise it works pretty well. I've been using it like this for years,
and am pretty happy with it.

-bjc

Re: eglot and tramp

[Michael Albinus]

Brian Cully <bjc@spork.org> writes:

Hi Brian,

>> It is on my TODO to check how well Tramp is integrated with eglot. Great
>> to hear that it seems to work flawless, but I believe there might be
>> some hints for performance improvement, like setting
>> "direct-async-process" connection property.
>
> I wouldn't go so far as to say "flawlessly". It does often run into the
> "forbidden re-entrant call to Tramp" problem, but that's not unique to
> Eglot, and it doesn't seem to cause any problems other than an annoying
> message periodically in the echo area.

Yes, this is an open problem. I hoped to fix it by using threads in
Tramp, but this has other problems.

> Otherwise it works pretty well. I've been using it like this for years,
> and am pretty happy with it.

Great! Since you are a regular eglot+Tramp user, could I ask you for a
test?

I assume you are using Tramp with ssh or scp. If you also don't use a
password (for example via public key authentication), please try the
following setting:

--8<---------------cut here---------------start------------->8---
(add-to-list 'tramp-connection-properties
             (list (regexp-quote "/ssh:user@host:")
                   "direct-async-process" t))
--8<---------------cut here---------------end--------------->8---

"/ssh:user@host:" must be adapted of course, and you must set this prior
opening a connection to this remote path. This should give you better
performance when starting the language server.

Does it work for you? Does it make a difference?

> -bjc

Best regards, Michael.

Re: eglot and tramp

[Brian Cully via Emacs development discussions.]

Michael Albinus <michael.albinus@gmx.de> writes:

> Great! Since you are a regular eglot+Tramp user, could I ask you for a
> test?
>
> I assume you are using Tramp with ssh or scp. If you also don't use a
> password (for example via public key authentication), please try the
> following setting:
>
> --8<---------------cut here---------------start------------->8---
> (add-to-list 'tramp-connection-properties
>              (list (regexp-quote "/ssh:user@host:")
>                    "direct-async-process" t))
> --8<---------------cut here---------------end--------------->8---
>
> "/ssh:user@host:" must be adapted of course, and you must set this prior
> opening a connection to this remote path. This should give you better
> performance when starting the language server.

I use eglot+Tramp with various containers (nspawn, docker, podman),
which don't support the ‘tramp-direct-async’ flag, so I'm not sure how
helpful this testing would be.

I know we've spoken about that flag before, and I came away from those
discussions with the impression that those methods should not support
direct-async, but I'm unsure why. I have only skimmed the relevant code,
but it seems like there shouldn't be a problem with supporting
direct-async on the container methods. Is there some danger I should be
aware of?

If it's safe enough, I'd be happy to flag support for direct-async in
the methods and do the above test.

WDYT?

-bjc

Re: eglot and tramp

[Michael Albinus]

Brian Cully <bjc@spork.org> writes:

Hi Brian,

>> Great! Since you are a regular eglot+Tramp user, could I ask you for a
>> test?
>>
>> I assume you are using Tramp with ssh or scp. If you also don't use a
>> password (for example via public key authentication), please try the
>> following setting:
>>
>> --8<---------------cut here---------------start------------->8---
>> (add-to-list 'tramp-connection-properties
>>              (list (regexp-quote "/ssh:user@host:")
>>                    "direct-async-process" t))
>> --8<---------------cut here---------------end--------------->8---
>>
>> "/ssh:user@host:" must be adapted of course, and you must set this prior
>> opening a connection to this remote path. This should give you better
>> performance when starting the language server.
>
> I use eglot+Tramp with various containers (nspawn, docker, podman),
> which don't support the ‘tramp-direct-async’ flag, so I'm not sure how
> helpful this testing would be.

Good point. I haven't checked docker, podman and friends yet whether
they do suuport direct async processes. Short check: they don't yet. I
will work on adding this feature in Tramp.

> If it's safe enough, I'd be happy to flag support for direct-async in
> the methods and do the above test.

I'll come back to you when Tramp is prepared.

> -bjc

Best regards, Michael.

Re: eglot and tramp

[Michael Albinus]

[-- Attachment #1: Type: text/plain, Size: 2285 bytes --]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Brian,

>>> Great! Since you are a regular eglot+Tramp user, could I ask you for a
>>> test?
>>>
>>> I assume you are using Tramp with ssh or scp. If you also don't use a
>>> password (for example via public key authentication), please try the
>>> following setting:
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> (add-to-list 'tramp-connection-properties
>>>              (list (regexp-quote "/ssh:user@host:")
>>>                    "direct-async-process" t))
>>> --8<---------------cut here---------------end--------------->8---
>>>
>>> "/ssh:user@host:" must be adapted of course, and you must set this prior
>>> opening a connection to this remote path. This should give you better
>>> performance when starting the language server.
>>
>> I use eglot+Tramp with various containers (nspawn, docker, podman),
>> which don't support the ‘tramp-direct-async’ flag, so I'm not sure how
>> helpful this testing would be.
>
> Good point. I haven't checked docker, podman and friends yet whether
> they do suuport direct async processes. Short check: they don't yet. I
> will work on adding this feature in Tramp.
>
>> If it's safe enough, I'd be happy to flag support for direct-async in
>> the methods and do the above test.
>
> I'll come back to you when Tramp is prepared.

I gave it a shot. Appended is a patch, which improves
tramp-handle-make-process in tramp.el. The method specifications in
tramp-container.el are extended by the line

--8<---------------cut here---------------start------------->8---
		(tramp-direct-async (,tramp-default-remote-shell "-c"))
--8<---------------cut here---------------end--------------->8---

Locally, I have tested it with the methods "docker" and "kubernetes", it
seems to work. I couldn't test with "podman", because I haven't such a
system just now.

For "nspawn" I cannot say anything, I don't use it. Perhaps it is
sufficient to add a similar line in its tramp-methods specification,
don't know. You as author might know it better. And if it works
sufficiently, we might also think about integrating it into
tramp-container.el. But this would be another story.

>> -bjc

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2232 bytes --]

diff --git a/lisp/tramp-container.el b/lisp/tramp-container.el
index 0879d6f1..7b942532 100644
--- a/lisp/tramp-container.el
+++ b/lisp/tramp-container.el
@@ -163,6 +163,7 @@ see its function help for a description of the format."
                                    ("-u" "%u")
                                    ("%h")
 			           ("%l")))
+		(tramp-direct-async (,tramp-default-remote-shell "-c"))
                 (tramp-remote-shell ,tramp-default-remote-shell)
                 (tramp-remote-shell-login ("-l"))
                 (tramp-remote-shell-args ("-i" "-c"))))
@@ -174,6 +175,7 @@ see its function help for a description of the format."
                                    ("-u" "%u")
                                    ("%h")
 			           ("%l")))
+		(tramp-direct-async (,tramp-default-remote-shell "-c"))
                 (tramp-remote-shell ,tramp-default-remote-shell)
                 (tramp-remote-shell-login ("-l"))
                 (tramp-remote-shell-args ("-i" "-c"))))
@@ -186,6 +188,7 @@ see its function help for a description of the format."
                                    ("--")
 			           ("%l")))
 		(tramp-config-check tramp-kubernetes--current-context-data)
+		(tramp-direct-async (,tramp-default-remote-shell "-c"))
                 (tramp-remote-shell ,tramp-default-remote-shell)
                 (tramp-remote-shell-login ("-l"))
                 (tramp-remote-shell-args ("-i" "-c"))))
diff --git a/lisp/tramp.el b/lisp/tramp.el
index f20a1fc3..003912f8 100644
--- a/lisp/tramp.el
+++ b/lisp/tramp.el
@@ -4836,7 +4836,14 @@ substitution.  SPEC-LIST is a list of char/value pairs used for
 	       (command
 	        (append
 		 `("cd" ,(tramp-shell-quote-argument localname) "&&" "(" "env")
-		 env `(,command ")"))))
+		 env `(,command ")")))
+		;; Add remote shell if needed.
+	       (command
+		(if (consp (tramp-get-method-parameter v 'tramp-direct-async))
+		    (append
+		     (tramp-get-method-parameter v 'tramp-direct-async)
+ 		     `(,(mapconcat #'identity command " ")))
+		  command)))

 	  ;; Check for `tramp-sh-file-name-handler', because something
 	  ;; is different between tramp-sh.el, and tramp-adb.el or

Re: eglot and tramp

[Brian Cully via Emacs development discussions.]

Michael Albinus <michael.albinus@gmx.de> writes:

> Locally, I have tested it with the methods "docker" and "kubernetes", it
> seems to work. I couldn't test with "podman", because I haven't such a
> system just now.

I've applied your patch, and will get back to you after I've used it for
a few days. So far, so good, though. Is there any behavior I should be
paying special attention to?

> For "nspawn" I cannot say anything, I don't use it. Perhaps it is
> sufficient to add a similar line in its tramp-methods specification,
> don't know. You as author might know it better. And if it works
> sufficiently, we might also think about integrating it into
> tramp-container.el. But this would be another story.

If nspawn-style containers are desirable in mainline Tramp, I can put
together a patch for you when I get some cycles.

-bjc

Re: eglot and tramp

[Michael Albinus]

Brian Cully <bjc@spork.org> writes:

Hi Brian,

>> Locally, I have tested it with the methods "docker" and "kubernetes", it
>> seems to work. I couldn't test with "podman", because I haven't such a
>> system just now.
>
> I've applied your patch, and will get back to you after I've used it for
> a few days. So far, so good, though. Is there any behavior I should be
> paying special attention to?

Hmm, does it show better performance when accessing an lsp server?

Otherwise, special restrictions for Tramp's direct async processes are
domented at (info "(tramp)Improving performance of asynchronous remote processes").

>> For "nspawn" I cannot say anything, I don't use it. Perhaps it is
>> sufficient to add a similar line in its tramp-methods specification,
>> don't know. You as author might know it better. And if it works
>> sufficiently, we might also think about integrating it into
>> tramp-container.el. But this would be another story.
>
> If nspawn-style containers are desirable in mainline Tramp, I can put
> together a patch for you when I get some cycles.

Yes, it would be welcome in tramp-container.el. Even if it doesn't
support direct-async (yet).

> -bjc

Best regards, Michael.

Re: eglot and tramp

[Brian Cully via Emacs development discussions.]

Michael Albinus <michael.albinus@gmx.de> writes:

> Hmm, does it show better performance when accessing an lsp server?

I haven't noticed any particular difference in speed. But I didn't find
eglot in a container to be noticeably slow before, either. My computer
is a beast, and container execution is basically native anyway.

>> If nspawn-style containers are desirable in mainline Tramp, I can put
>> together a patch for you when I get some cycles.
>
> Yes, it would be welcome in tramp-container.el. Even if it doesn't
> support direct-async (yet).

I'll add it to my list and let you know when it's ready.

-bjc

Re: eglot and tramp

[Michael Albinus]

Brian Cully <bjc@spork.org> writes:

Hi Brian,

> I haven't noticed any particular difference in speed. But I didn't find
> eglot in a container to be noticeably slow before, either. My computer
> is a beast, and container execution is basically native anyway.

Bad for testing :-)

>>> If nspawn-style containers are desirable in mainline Tramp, I can put
>>> together a patch for you when I get some cycles.
>>
>> Yes, it would be welcome in tramp-container.el. Even if it doesn't
>> support direct-async (yet).
>
> I'll add it to my list and let you know when it's ready.

Thanks!

> -bjc

Best regards, Michael.

Re: eglot and tramp

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Brian,

>>>> If nspawn-style containers are desirable in mainline Tramp, I can put
>>>> together a patch for you when I get some cycles.
>>>
>>> Yes, it would be welcome in tramp-container.el. Even if it doesn't
>>> support direct-async (yet).
>>
>> I'll add it to my list and let you know when it's ready.
>
> Thanks!

Well, that was more than a year ago ...

Tramp 2.7 will be released in a couple of weeks, somehow late in
June. I'd like to have nspawn integrated as native method.

Do aou still plan to integrate it into tramp-container.el? Or shall I do
it myself?

Note, that I'm also working on integrating systemd-run into Tramp (as
run0 method). Perhaps these efforts could be combined. User
authentication for systemd is still an open point, so we might work on
polkit integration into Emacs.

>> -bjc

Best regards, Michael.

Re: eglot and tramp

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Brian,

> Well, that was more than a year ago ...
>
> Tramp 2.7 will be released in a couple of weeks, somehow late in
> June. I'd like to have nspawn integrated as native method.
>
> Do aou still plan to integrate it into tramp-container.el? Or shall I do
> it myself?
>
> Note, that I'm also working on integrating systemd-run into Tramp (as
> run0 method). Perhaps these efforts could be combined. User
> authentication for systemd is still an open point, so we might work on
> polkit integration into Emacs.

Both methods have been added to Tramp now. If you have comments, please
let me know.

>>> -bjc

Best regards, Michael.

Re: eglot and tramp

[brian via Emacs development discussions.]

Michael Albinus <michael.albinus@gmx.de> writes:

> Both methods have been added to Tramp now. If you have comments, please
> let me know.

Nothing to add. Thanks for adding them to Emacs proper. Does anything
need to happen on Elpa?

-bjc

Tramp nspawn method (was: eglot and tramp)

[Michael Albinus]

brian <bjc@spork.org> writes:

Hi Brian,

>> Both methods have been added to Tramp now. If you have comments, please
>> let me know.
>
> Nothing to add. Thanks for adding them to Emacs proper. Does anything
> need to happen on Elpa?

Well, I've added a check in tramp-compat.el, whether tramp-nspawn is
loaded. It raises an error then.

I plan to release Tramp 2.7.1 on GNU ELPA end of June. Once it is there,
you could remove tramp-nsapwn.el from GNU ELPA.

> -bjc

Best regards, Michael.