bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[jeberger]

From https://github.com/joaotavora/eglot/issues/1175

# Steps to reproduce:

- On the remote machine:
    - Install a language server in a non-standard path (e.g. `rust-analyzer`, which installs in a subdirectory under `$HOME` and not `/usr/bin`)
    - Make sure that the server is in the path for a shell other than `sh` (e.g. set the path in `.zprofile` for `zsh`).
- On the local machine:
    - Open a file remotely through TRAMP
    - Set `shell-file-name` to the shell you're using on the remote (e.g. `(setq shell-file-name "zsh")`)
    - <key>M-x</key> `eglot`.

# Expected result:

Eglot should start

# Actual result:

Eglot asks `Enter program to execute (or <host>:<port>):`, then when I type `rust-analyzer` it fails with message `[jsonrpc] Server exited with status 127`. Looking at `eglot-events-buffer`, it is clear that eglot tried to use `sh` to start the server.

# Partial fix

The server starts correctly if I change function `eglot--cmd` to use `(or shell-file-name "sh")` instead of plain `"sh"`, e.g.:

```lisp
(defun eglot--cmd (contact)
  "Helper for `eglot--connect'."
  (if (file-remote-p default-directory)
      ;; TODO: this seems like a bug, although it’s everywhere. For
      ;; some reason, for remote connections only, over a pipe, we
      ;; need to turn off line buffering on the tty.
      ;;
      ;; Not only does this seem like there should be a better way,
      ;; but it almost certainly doesn’t work on non-unix systems.
      (list (or shell-file-name "sh") "-c"
            (string-join (cons "stty raw > /dev/null;"
                               (mapcar #'shell-quote-argument contact))
                         " "))
    contact))
```

It still fails to autodetect the server and asks for the program to execute though.

----

In GNU Emacs 27.2 (build 1, x86_64-w64-mingw32)
 of 2021-03-26 built on CIRROCUMULUS
Repository revision: deef5efafb70f4b171265b896505b92b6eef24e6
Repository branch: HEAD
Windowing system distributor 'Microsoft Corp.', version 10.0.19042
System Description: Microsoft Windows 10 Enterprise (v10.0.2009.19042.2604)

Recent messages:
Cleaning up the recentf list...done (0 removed)
Quit
Cleaning up the recentf list...done (0 removed)
Timer #[257 "\303\x01!\205!\0r\211q\210\b\203\x1d\0	\203\x1d\0\304\305\306\307	$\210\310\311!\210\312\211\x12)\207" [flymake-mode flymake-no-changes-timeout flymake-timer buffer-live-p flymake--log-1 :debug flymake "starting syntax check after idle for %s seconds" flymake-start t nil] 6 "

(fn BUFFER)"] ran for 00:00:17 from 08:15:39 to 08:15:56
Couldn’t find local shell prompt for C:/ProgramData/chocolatey/lib/Emacs/tools/emacs/libexec/emacs/27.2/x86_64-w64-mingw32/cmdproxy.exe
Cleaning up the recentf list...done (0 removed)
Timer flycheck--handle-idle-trigger ran for 00:00:49 from 08:15:38 to 08:16:27
Auto-saving...

Configured using:
 'configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY W32NOTIFY ACL GNUTLS LIBXML2
HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS MODULES THREADS JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: FRA
  locale-coding-system: cp1252

Major mode: Rust

Minor modes in effect:
  flymake-mode: t
  flycheck-mode: t
  desktop-save-mode: t
  rainbow-delimiters-mode: t
  which-function-mode: t
  shell-dirtrack-mode: t
  recentf-mode: t
  global-linum-mode: t
  linum-mode: t
  global-hl-line-mode: t
  global-leerzeichen-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  ws-butler-global-mode: t
  ws-butler-mode: t
  global-whitespace-mode: t
  tabbar-mwheel-mode: t
  tabbar-mode: t
  global-semanticdb-minor-mode: t
  global-semantic-idle-scheduler-mode: t
  global-semantic-idle-local-symbol-highlight-mode: t
  global-semantic-idle-summary-mode: t
  global-semantic-highlight-func-mode: t
  semantic-mode: t
  phi-rectangle-mode: t
  ivy-mode: t
  highlight-doxygen-global-mode: t
  global-dash-fontify-mode: t
  global-company-mode: t
  company-mode: t
  bar-cursor-mode: t
  override-global-mode: t
  show-paren-mode: t
  delete-selection-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
d:/home/.emacs.d/elpa/jsonrpc-1.0.16/jsonrpc hides c:/ProgramData/chocolatey/lib/Emacs/tools/emacs/share/emacs/27.2/lisp/jsonrpc
d:/home/.emacs.d/elpa/xref-1.6.2/xref hides c:/ProgramData/chocolatey/lib/Emacs/tools/emacs/share/emacs/27.2/lisp/progmodes/xref
d:/home/.emacs.d/elpa/project-0.9.8/project hides c:/ProgramData/chocolatey/lib/Emacs/tools/emacs/share/emacs/27.2/lisp/progmodes/project
d:/home/.emacs.d/elpa/flymake-1.2.2/flymake hides c:/ProgramData/chocolatey/lib/Emacs/tools/emacs/share/emacs/27.2/lisp/progmodes/flymake
d:/home/.emacs.d/packages/mpuz hides c:/ProgramData/chocolatey/lib/Emacs/tools/emacs/share/emacs/27.2/lisp/play/mpuz
d:/home/.emacs.d/elpa/seq-2.23/seq hides c:/ProgramData/chocolatey/lib/Emacs/tools/emacs/share/emacs/27.2/lisp/emacs-lisp/seq
d:/home/.emacs.d/elpa/eldoc-1.13.0/eldoc hides c:/ProgramData/chocolatey/lib/Emacs/tools/emacs/share/emacs/27.2/lisp/emacs-lisp/eldoc

Features:
(shadow sort mail-extr emacsbug message rmc rfc822 mml mml-sec epa
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail tramp-cmds counsel-gtags minimal-session-saver
css-mode sgml-mode eww mm-url gnus nnheader gnus-util rmail
rmail-loaddefs rfc2047 rfc2045 ietf-drums mail-utils mm-util mail-prsvr
url-queue url url-proxy url-privacy url-expand url-methods url-history
mailcap shr text-property-search url-cookie url-domsuf puny svg xml dom
ffap eieio-opt semantic/sb speedbar sb-image dframe eglot
external-completion array filenotify jsonrpc ert ewoc debug backtrace
flymake-proc flymake warnings url-util cl-print help-fns radix-tree
web-mode bitbake mmm-mode mmm-univ mmm-class mmm-region mmm-auto
mmm-vars mmm-utils mmm-compat markdown-mode yaml-mode python sh-script
smie executable semantic/bovine/make semantic/bovine/make-by make-mode
toml-mode conf-mode racer f etags fileloop generator add-log
semantic/tag-file semantic/imenu semantic/db-file data-debug cedet-files
semantic/bovine/c semantic/decorate/include semantic/decorate/mode
semantic/decorate semantic/bovine/c-by semantic/lex-spp
semantic/bovine/gcc semantic/dep semantic/bovine semantic/analyze/refs
semantic/db-find semantic/db-ref hideif hideshow jb-prog-licences
jb-prog-font align jb-cc-mode cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs flycheck-rust flycheck
rust-utils rust-mode rust-rustfmt rust-playpen rust-compile rust-cargo
tramp-cache vc-dispatcher vc-hg diff-mode desktop frameset thingatpt
counsel xdg xref project dired dired-loaddefs swiper matlab matlab-scan
matlab-syntax matlab-compat pulse rainbow-delimiters windmove s server
ibuf-macs which-func imenu tramp-sh tramp tramp-loaddefs trampver
tramp-integration files-x tramp-compat shell pcomplete parse-time
iso8601 time-date format-spec recentf tree-widget linum hl-line
leerzeichen yasnippet ws-butler cus-edit wid-edit whitespace tabbar
smart-mode-line-light-theme smart-mode-line rich-minority
semantic/db-mode semantic/idle semantic/analyze semantic/sort
semantic/scope semantic/analyze/fcn semantic/db eieio-base
semantic/format ezimage semantic/tag-ls semantic/find semantic/ctxt
semantic/util-modes semantic/util semantic pp semantic/tag semantic/lex
semantic/fw mode-local find-func cedet phi-rectangle rect kconfig-mode
derived ivy-hydra hydra lv ivy ivy-faces ivy-overlay colir color flx avy
ht highlight-doxygen noutline outline dash company-oddmuse company-files
company-capf company-cmake company-clang company-semantic company-css
company-nxml company-bbdb company-keywords company-gtags
company-template company-dabbrev-code company-dabbrev company pcase
bar-cursor peg cl-extra help-mode advice use-package use-package-ensure
use-package-delight use-package-diminish use-package-bind-key bind-key
easy-mmode use-package-core finder-inf edmacro kmacro compile comint
ansi-color ring paren avoid delsel cus-start cus-load epg epg-config
gnu-elpa-keyring-update rx info package easymenu browse-url url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp disp-table
term/w32-win w32-win w32-vars term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads w32notify w32
lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 1041436 370370)
 (symbols 48 44596 7)
 (strings 32 219890 55700)
 (string-bytes 1 6131935)
 (vectors 16 115782)
 (vector-slots 8 2654593 430756)
 (floats 8 347 768)
 (intervals 56 50094 1190)
 (buffers 1000 168))

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Eli Zaretskii]

> Date: Fri, 24 Feb 2023 08:39:45 +0100 (CET)
> From: jeberger@free.fr
> 
> >From https://github.com/joaotavora/eglot/issues/1175
> 
> # Steps to reproduce:
> 
> - On the remote machine:
>     - Install a language server in a non-standard path (e.g. `rust-analyzer`, which installs in a subdirectory under `$HOME` and not `/usr/bin`)
>     - Make sure that the server is in the path for a shell other than `sh` (e.g. set the path in `.zprofile` for `zsh`).
> - On the local machine:
>     - Open a file remotely through TRAMP
>     - Set `shell-file-name` to the shell you're using on the remote (e.g. `(setq shell-file-name "zsh")`)
>     - <key>M-x</key> `eglot`.
> 
> # Expected result:
> 
> Eglot should start
> 
> # Actual result:
> 
> Eglot asks `Enter program to execute (or <host>:<port>):`, then when I type `rust-analyzer` it fails with message `[jsonrpc] Server exited with status 127`. Looking at `eglot-events-buffer`, it is clear that eglot tried to use `sh` to start the server.
> 
> # Partial fix
> 
> The server starts correctly if I change function `eglot--cmd` to use `(or shell-file-name "sh")` instead of plain `"sh"`, e.g.:

I don't think this is the correct fix.  The literal "sh -c" is used in
Eglot only for remote server invocations, and I presume this was done
to avoid the problems with running Eglot on MS-Windows.  Using
shell-file-name is incorrect for this use case because shell-file-name
gives you the name of the _local_ shell.  I think we have better ways
of dealing with this issue' I've CC'ed Michael in the hope that he
will tell how to solve this properly.

Thanks.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[jeberger]

> De: "Eli Zaretskii" <eliz@gnu.org>
> À: jeberger@free.fr, "Michael Albinus" <michael.albinus@gmx.de>
> Cc: 61748@debbugs.gnu.org
> Envoyé: Vendredi 24 Février 2023 09:15:52
> Objet: Re: bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file
>
>> Date: Fri, 24 Feb 2023 08:39:45 +0100 (CET)
>> From: jeberger@free.fr
>> 
>> # Partial fix
>> 
>> The server starts correctly if I change function `eglot--cmd` to use `(or shell-file-name "sh")` instead of plain `"sh"`, e.g.:
>
> I don't think this is the correct fix.  The literal "sh -c" is used in
> Eglot only for remote server invocations, and I presume this was done
> to avoid the problems with running Eglot on MS-Windows.  Using
> shell-file-name is incorrect for this use case because shell-file-name
> gives you the name of the _local_ shell.  I think we have better ways
> of dealing with this issue' I've CC'ed Michael in the hope that he
> will tell how to solve this properly.
>
> Thanks.

According to https://www.gnu.org/software/tramp/#Running-shell_002dcommand-on-a-remote-host
`shell-file-name` is also used for _remote_ command invocation and set to `/bin/sh` by default.
It can be configured using connection local variables. From the same source, the fix
should also use `(or shell-command-switch "-c")` instead of plain `"-c"`.

Cordially.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

jeberger@free.fr writes:

Hi,

>>> # Partial fix
>>>
>>> The server starts correctly if I change function `eglot--cmd` to use `(or shell-file-name "sh")` instead of plain `"sh"`, e.g.:
>>
>> I don't think this is the correct fix.  The literal "sh -c" is used in
>> Eglot only for remote server invocations, and I presume this was done
>> to avoid the problems with running Eglot on MS-Windows.  Using
>> shell-file-name is incorrect for this use case because shell-file-name
>> gives you the name of the _local_ shell.  I think we have better ways
>> of dealing with this issue' I've CC'ed Michael in the hope that he
>> will tell how to solve this properly.
>>
>> Thanks.
>
> According to https://www.gnu.org/software/tramp/#Running-shell_002dcommand-on-a-remote-host
> `shell-file-name` is also used for _remote_ command invocation and set to `/bin/sh` by default.
> It can be configured using connection local variables. From the same source, the fix
> should also use `(or shell-command-switch "-c")` instead of plain `"-c"`.

Indeed. Since Tramp 2.4 (Emacs 27), Tramp provides both
`shell-file-name` and `shell-command-switch' as connection-local
variables. Per default, the former is "/bin/sh" (or "/system/bin/sh" on
Android devices).

When it is not known, whether a given remote buffer has expanded
connection-local variables, the usage pattern might be

(with-connection-local-variables shell-file-name)

> Cordially.

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

Hello,

I'm Eglot maintainer and, as of very recently, in analyzing bug#61350,
also an Eglot-over-Tramp user.  So I can now reproduce this bug.

I have the `jdtls` script installed in `~/bin/jdtls` of my remote host user's
HOME, and I too suffer from this bug.  On the local host, I use
zsh and shell-file-name is "/bin/zsh", although in Tramp-accessed
buffers it is "/bin/sh".

Bizarrely, this only happens sometimes.  When it works, it works perfectly
and, as expected, the remote PATH of the remote host user is consulted.

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

After a bit more investigation, I'm not sure this is a bug after all.
It might be.  My symptoms were the same as jeberger's but
my setup is slightly different (I'm not using zsh on the remote).

At least in my case, I was able to make Eglot/Tramp work successfully
by adding 'tramp-own-remote-path' to the 'tramp-remote-path' list.
If I do that, everything works as expected.

In my case I was using simply a /bin/sh on the remote server
and the user had a ~/bin directory that her ~/.profile adds to PATH.
But without the 'tramp-own-remote-path' fix, the shell used by Tramp
is not interactive and thus won't consider ~/.profile at all.

It's a bit unfortunate that in such a simple case, there is this hoop
to jump over.  But since at least it can be jumped over, I will add this
to the Eglot manual section on Tramp-related troubleshooting.

João

On Mon, Feb 27, 2023 at 11:07 AM João Távora <joaotavora@gmail.com> wrote:
>
> Hello,
>
> I'm Eglot maintainer and, as of very recently, in analyzing bug#61350,
> also an Eglot-over-Tramp user.  So I can now reproduce this bug.
>
> I have the `jdtls` script installed in `~/bin/jdtls` of my remote host user's
> HOME, and I too suffer from this bug.  On the local host, I use
> zsh and shell-file-name is "/bin/zsh", although in Tramp-accessed
> buffers it is "/bin/sh".
>
> Bizarrely, this only happens sometimes.  When it works, it works perfectly
> and, as expected, the remote PATH of the remote host user is consulted.
>
> João



-- 
João Távora

bug#61748: [informatique] Re: bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Jérôme M. Berger]

[-- Attachment #1.1: Type: text/plain, Size: 1365 bytes --]

On 2/27/23 1:05 PM, João Távora wrote:
> After a bit more investigation, I'm not sure this is a bug after all.
> It might be.  My symptoms were the same as jeberger's but
> my setup is slightly different (I'm not using zsh on the remote).
>
> At least in my case, I was able to make Eglot/Tramp work successfully
> by adding 'tramp-own-remote-path' to the 'tramp-remote-path' list.
> If I do that, everything works as expected.
>
> In my case I was using simply a /bin/sh on the remote server
> and the user had a ~/bin directory that her ~/.profile adds to PATH.
> But without the 'tramp-own-remote-path' fix, the shell used by Tramp
> is not interactive and thus won't consider ~/.profile at all.
>
> It's a bit unfortunate that in such a simple case, there is this hoop
> to jump over.  But since at least it can be jumped over, I will add this
> to the Eglot manual section on Tramp-related troubleshooting.

     I already had `tramp-own-remote-path` in my `tramp-remote-path`. 
But I don't use `~/.profile` to setup my path so it doesn't help for 
Eglot. Note that Eglot is the only tool which has this issue: others 
(e.g. `counsel-rg`) work fine over Tramp and pick up the path from `zsh` 
correctly.

     Jérôme

-- 
mailto:jeberger@free.fr
http://jeberger.free.fr
Diaspora*: https://framasphere.org/u/jeberger



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 203 bytes --]

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

> At least in my case, I was able to make Eglot/Tramp work successfully
> by adding 'tramp-own-remote-path' to the 'tramp-remote-path' list.
> If I do that, everything works as expected.
>
> In my case I was using simply a /bin/sh on the remote server
> and the user had a ~/bin directory that her ~/.profile adds to PATH.
> But without the 'tramp-own-remote-path' fix, the shell used by Tramp
> is not interactive and thus won't consider ~/.profile at all.

Why don't you add (the expanded) ~/bin to tramp-remote-path?

> João

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

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

On Mon, Feb 27, 2023, 19:57 Michael Albinus <michael.albinus@gmx.de> wrote:

>
> > In my case I was using simply a /bin/sh on the remote server
> > and the user had a ~/bin directory that her ~/.profile adds to PATH.
> > But without the 'tramp-own-remote-path' fix, the shell used by Tramp
> > is not interactive and thus won't consider ~/.profile at all.
>
> Why don't you add (the expanded) ~/bin to tramp-remote-path?
>

I did, but it did not work reliably, Michael. Only sometimes.

I will try again though.

João

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

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

>     Why don't you add (the expanded) ~/bin to tramp-remote-path?
>
> I did, but it did not work reliably, Michael. Only sometimes.
>
> I will try again though.

Show me your settings, we'll fix it together.

> João

Best regards, Michael.

bug#61748: [informatique] Re: bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

Jérôme M. Berger <jeberger@free.fr> writes:

> On 2/27/23 1:05 PM, João Távora wrote:

>     I already had `tramp-own-remote-path` in my
> `tramp-remote-path`. But I don't use `~/.profile` to setup my path so
> it doesn't help for Eglot. Note that Eglot is the only tool which has
> this issue: others (e.g. `counsel-rg`) work fine over Tramp and pick
> up the path from `zsh` correctly.

I don't know what counsel is or how it deals with this issue, but feel
free to share any relevant code here.  Even though "make it work" hacks
abound, Eglot promotes separation of responsibilities.  Being is core is
great for figuring out exactly where fixes should go.  Eglot's business
is LSP.  TRAMP accesses remote files.  Eglot knows close to nothing
about TRAMP or how it does its file access job.  After all the 'T' in
tramp stands for "transparent".

Not ruling out changes to Eglot, but it look like what is needed here
for things to work out-of-the-box (or nearly so) is a way to tell Emacs
something like: "Whevever I'm working on files of this remote server,
find executables in the remote PATH.  Calculate PATH to be the same as
if I had just logged in interactively as the user I'm accessing this
file with."  Whether this can be a good default or something you set in
your configuration is not up to me, but often what works for you does
not work for someone else.

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

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

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

> João Távora <joaotavora@gmail.com> writes:
>
> Hi João,
>
>>     Why don't you add (the expanded) ~/bin to tramp-remote-path?
>>
>> I did, but it did not work reliably, Michael. Only sometimes.
>>
>> I will try again though.
>
> Show me your settings, we'll fix it together.

I use the attached dockerfile that I used for the reproduction of
bug#61350.  It installs a Java LSP server and has a Java project.  I
build and run this Docker container with:


[-- Attachment #2: dockerfile --]
[-- Type: text/plain, Size: 1215 bytes --]

FROM ubuntu:latest
RUN apt update && apt install  openssh-server sudo openjdk-17-jdk openjdk-17-jre git wget -y
# Create a user “sshuser” and group “sshgroup”
RUN groupadd sshgroup && useradd -ms /bin/bash -g sshgroup sshuser
# Create sshuser directory in home
RUN mkdir -p /home/sshuser/.ssh
# Copy the ssh public key in the authorized_keys file. The idkey.pub below is a public key file you get from ssh-keygen. They are under ~/.ssh directory by default.
COPY id_rsa.pub /home/sshuser/.ssh/authorized_keys
# change ownership of the key file. 
RUN chown sshuser:sshgroup /home/sshuser/.ssh/authorized_keys && chmod 600 /home/sshuser/.ssh/authorized_keys
# Start SSH service
RUN service ssh start
# Expose docker port 22
EXPOSE 22

USER sshuser
WORKDIR /home/sshuser
RUN git clone --depth 1 --no-tags --single-branch -b eglot-tramp-freeze-repro https://github.com/thkoch2001/yacy_search_server
RUN wget https://download.eclipse.org/jdtls/milestones/1.19.0/jdt-language-server-1.19.0-202301171536.tar.gz
RUN mkdir jdtls
RUN tar xvfz jdt-language-server-1.19.0-202301171536.tar.gz -C jdtls
RUN mkdir bin
RUN ln -sf ~/jdtls/bin/jdtls ~/bin/jdtls

USER root
CMD ["/usr/sbin/sshd","-D"]

[-- Attachment #3: Type: text/plain, Size: 1778 bytes --]


  docker build -t sshubuntu .
  docker run -d -p 2022:22 sshubuntu

In a directory where I have the attached dockerfile and a id_rsa.pub
which is the public key to my default ssh key.  

After this ssh access works with the user 'sshuser' on localhost port
2022 mapped to the server's 22.  The LSP server executable 'jdtls' is
linked to this user's $HOME/bin, or /home/sshuser/bin.

The default shell for user 'sshuser' is '/bin/bash' (though Tramp seems
to access it with '/bin/sh').  The local shell is '/bin/zsh'.

In the local machine I launch Emacs -Q sessions in slightly different
ways.  This fails:

    export REMOTE_FILE=/home/sshuser/yacy_search_server/source/net/yacy/yacy.java
    ~/Source/Emacs/emacs/src/emacs -Q                                                 \
       /ssh:sshuser@localhost#2022:$REMOTE_FILE                                       \
       --eval '(add-to-list (quote tramp-remote-path) "/home/sshuser/bin")'           \
       -f eglot

Even if it worked, I would be suboptimal.  I think I would like to avoid
the hardcoding of "/home/sshuser/bin" in my config.  This also fails.

    ~/Source/Emacs/emacs/src/emacs -Q                                                 \
       /ssh:sshuser@localhost#2022:$REMOTE_FILE                                       \
       --eval '(add-to-list (quote tramp-remote-path) "~/bin")'                       \
       -f eglot

And the following works.  Eglot is started correctly.

    ~/Source/Emacs/emacs/src/emacs -Q                                                 \
       /ssh:sshuser@localhost#2022:$REMOTE_FILE                                       \
       --eval '(add-to-list (quote tramp-remote-path) (quote tramp-own-remote-path))' \
       -f eglot

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

>     ~/Source/Emacs/emacs/src/emacs -Q                                                 \
>        /ssh:sshuser@localhost#2022:$REMOTE_FILE                                       \
>        --eval '(add-to-list (quote tramp-remote-path) "~/bin")'                       \
>        -f eglot

You must set tramp-remote-path before accessing the remote server. Try this:

     ~/Source/Emacs/emacs/src/emacs -Q -l tramp                                        \
        --eval '(add-to-list (quote tramp-remote-path) "~/bin")'                       \
        /ssh:sshuser@localhost#2022:$REMOTE_FILE                                       \
        -f eglot

> João

Best regards, Michael.

bug#61748: [informatique] Re: bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[jeberger]

> João Távora wrote:
>
> I don't know what counsel is or how it deals with this issue, but feel
> free to share any relevant code here.  Even though "make it work" hacks
> abound, Eglot promotes separation of responsibilities.  Being is core is
> great for figuring out exactly where fixes should go.  Eglot's business
> is LSP.  TRAMP accesses remote files.  Eglot knows close to nothing
> about TRAMP or how it does its file access job.  After all the 'T' in
> tramp stands for "transparent".
> 
I don't know much about counsel's internals (or Eglot's for that matter),
but this comment in the counsel-gtags code seems very relevant:

```
`process-lines' does not support Tramp because it uses `call-process'.  Using
`process-file' makes Tramp support auto-magical."
```
https://github.com/emacsmirror/counsel-gtags/blob/1d52eaeffeb60266434d4f7416a108ca058fde91/counsel-gtags.el#L337-L338

From a quick grep it looks like Eglot uses `make-process`, which probably
suffers from the same limitations as `call-process` when it comes to Tramp
support.

Jérôme

bug#61748: [informatique] Re: bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

jeberger@free.fr writes:

Hi Jérôme,

> From a quick grep it looks like Eglot uses `make-process`, which probably
> suffers from the same limitations as `call-process` when it comes to Tramp
> support.

Since Emacs 27, make-process supports the keyword :file-handler. If set
non-nil, remote processes are supported.

> Jérôme

Best regards, Michael.

bug#61748: [informatique] Re: bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

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

On Tue, Feb 28, 2023, 10:14 Michael Albinus <michael.albinus@gmx.de> wrote:

> jeberger@free.fr writes:
>
> Hi Jérôme,
>
> > From a quick grep it looks like Eglot uses `make-process`, which probably
> > suffers from the same limitations as `call-process` when it comes to
> Tramp
> > support.
>
> Since Emacs 27, make-process supports the keyword :file-handler. If set
> non-nil, remote processes are supported.
>

Yep. And that's exactly what Eglot uses.

So I'd say this comment you find in counsel, Jerôme, is not the culprit.

João

>

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

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

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

> João Távora <joaotavora@gmail.com> writes:
>
> Hi João,
>
>>     ~/Source/Emacs/emacs/src/emacs -Q                                                 \
>>        /ssh:sshuser@localhost#2022:$REMOTE_FILE                                       \
>>        --eval '(add-to-list (quote tramp-remote-path) "~/bin")'                       \
>>        -f eglot
>
> You must set tramp-remote-path before accessing the remote server. Try this:
>
>      ~/Source/Emacs/emacs/src/emacs -Q -l tramp                                        \
>         --eval '(add-to-list (quote tramp-remote-path) "~/bin")'                       \
>         /ssh:sshuser@localhost#2022:$REMOTE_FILE                                       \
>         -f eglot

This works, and this would explain the instability I saw earlier.
Thanks.  I wonder if it fixes Jerôme's problem, too.

But IMHO this is a grim pain point for Eglot users expecting things to
"just work".  After all, there I am in the other side thread singing the
praises of Tramp's "transparency", and now it seems that the user be
aware of this Tramp implementation detail -- the early binding of
tramp-remote-path.

Surely we can fix this in Tramp, so that it either considers this
variable's value just-in-time, or detects if there were changes to it
and reacts accordingly.  Can we not?

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

> But IMHO this is a grim pain point for Eglot users expecting things to
> "just work".  After all, there I am in the other side thread singing the
> praises of Tramp's "transparency", and now it seems that the user be
> aware of this Tramp implementation detail -- the early binding of
> tramp-remote-path.
>
> Surely we can fix this in Tramp, so that it either considers this
> variable's value just-in-time, or detects if there were changes to it
> and reacts accordingly.  Can we not?

Patches welcome.

> João

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

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

>> Surely we can fix this in Tramp, so that it either considers this
>> variable's value just-in-time, or detects if there were changes to it
>> and reacts accordingly.  Can we not?
>
> Patches welcome.

Great.  I'd love to help, but I'm not familiar with how Tramp uses this
variable to "know" or set a remote path so that e.g. executable-find
finds it.

Do you think it requires re-starting the Tramp connection for that file?
Or can we just tell the current connection: "hey, refresh your remote
PATH, because the variable has changed?"

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

>>> Surely we can fix this in Tramp, so that it either considers this
>>> variable's value just-in-time, or detects if there were changes to it
>>> and reacts accordingly.  Can we not?
>>
>> Patches welcome.
>
> Great.  I'd love to help, but I'm not familiar with how Tramp uses this
> variable to "know" or set a remote path so that e.g. executable-find
> finds it.
>
> Do you think it requires re-starting the Tramp connection for that file?
> Or can we just tell the current connection: "hey, refresh your remote
> PATH, because the variable has changed?"

Anything goes. But I don't see why this is needed. A proper Tramp
configuration in time is all what you need. People are familiar with
this for decades.

> João

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Tue, Feb 28, 2023 at 1:05 PM Michael Albinus <michael.albinus@gmx.de> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> Hi João,
>
> >>> Surely we can fix this in Tramp, so that it either considers this
> >>> variable's value just-in-time, or detects if there were changes to it
> >>> and reacts accordingly.  Can we not?
> >> Patches welcome.
> > Great.  I'd love to help, but I'm not familiar with how Tramp uses this
> > variable to "know" or set a remote path so that e.g. executable-find
> > finds it.
> >
> > Do you think it requires re-starting the Tramp connection for that file?
> > Or can we just tell the current connection: "hey, refresh your remote
> > PATH, because the variable has changed?"
> Anything goes. But I don't see why this is needed. A proper Tramp
> configuration in time is all what you need. People are familiar with
> this for decades.

Michael, it's become evident that people are using Tramp without
realizing that they are using Tramp.  I think this is great, and
you should be quite proud.  The "transparent" works! ... mostly
If  it's not quite perfect let's improve on it.  This is one of those points.

In this and in the other and yet in the other decade , I've
always appreciated when I don't need to restart my restart my
Emacs (ugh) because I worry about when variable values I'm
experimenting with are read.  This is a Lisp machine after all :-)

At the very least, the documentation of tramp-remote-path should
state that the variable doesn't take immediate effect and that
something must be done (is it really restarting Emacs, or is there
some M-x tramp-cleanup-* thing that has to be done?)

So, if "anything goes", is such a minimal docstring patch
acceptable?

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[jeberger]

João Távora <joaotavora@gmail.com> writes:
> Michael Albinus <michael.albinus@gmx.de> writes:
> 
> > You must set tramp-remote-path before accessing the remote server.
> > Try this:
> >
> >      ~/Source/Emacs/emacs/src/emacs -Q -l tramp
> >                                             \
> >         --eval '(add-to-list (quote tramp-remote-path) "~/bin")'
> >                               \
> >         /ssh:sshuser@localhost#2022:$REMOTE_FILE
> >                                               \
> >         -f eglot
> 
> This works, and this would explain the instability I saw earlier.
> Thanks.  I wonder if it fixes Jerôme's problem, too.
> 
No I've already got it setup as part of my config before I ever access
Tramp, which seems only logical: first configure the environment, then
try to use it.

Just to be clear, which problem are we talking about here? There were
two related problems in my report:

1. When told that the language server is `rust-analyzer`, Eglot fails
   to launch it. This is caused by the hardcoded `"sh" "-c"` in
   `eglot--cmd` and is fixed by using `(or shell-file-name "sh")
   (or shell-command-switch "-c")`, possibly with an added
   `with-connection-local-variables` as per Michael's mail.
   Or maybe by removing the whole `eglot--cmd` function since from
   the comment it looks like it only exists to call `stty raw` for
   remote files, and Eglot is the only package that tries to do that.
   This can only be fixed in Eglot since that's where it's hardcoded.

2. Eglot doesn't detect `rust-analyzer` automatically. This could be
   related to the fact that `rust-analyzer` is in a non-standard path
   or to some mix-up between the local and remote environments (e.g.
   if Eglot tries to detect the server on the local PC even though it
   would need the remote one), or something else altogether. It is not
   fixed by the abovementioned changes. 
   I have no idea how Eglot tries to determine which server it should
   use, and so no idea where to start looking for a solution.

Jérôme

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

> At the very least, the documentation of tramp-remote-path should
> state that the variable doesn't take immediate effect and that
> something must be done (is it really restarting Emacs, or is there
> some M-x tramp-cleanup-* thing that has to be done?)
>
> So, if "anything goes", is such a minimal docstring patch
> acceptable?

Sure. RTFM :-)

--8<---------------cut here---------------start------------->8---
   When remote search paths are changed, local TRAMP caches must be
recomputed.  To force TRAMP to recompute afresh, call ‘M-x
tramp-cleanup-this-connection <RET>’ or friends (*note Cleanup remote
connections::).
--8<---------------cut here---------------end--------------->8---

See also the whole discussion at (info "(tramp) Remote programs")

I'm open for proposals to improve the text.

> João

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Tue, Feb 28, 2023 at 2:10 PM Michael Albinus <michael.albinus@gmx.de> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> Hi João,
>
> > At the very least, the documentation of tramp-remote-path should
> > state that the variable doesn't take immediate effect and that
> > something must be done (is it really restarting Emacs, or is there
> > some M-x tramp-cleanup-* thing that has to be done?)
> >
> > So, if "anything goes", is such a minimal docstring patch
> > acceptable?
>
> Sure. RTFM :-)

I didn't complain about the manual.  I complained about the
variable docstring, which is what people read when changing a
variable's value.  If you don't think there's a shortcoming
there feel free to let me know and I won't waste my time.  I
read your "patches welcome" as meaning that you welcomed
patches to fix said shortcoming.  If you don't welcome
patches, say "patches not welcome" instead.

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Tue, Feb 28, 2023 at 1:59 PM <jeberger@free.fr> wrote:

> > This works, and this would explain the instability I saw earlier.
> > Thanks.  I wonder if it fixes Jerôme's problem, too.
> No I've already got it setup as part of my config before I ever access
> Tramp, which seems only logical: first configure the environment, then
> try to use it.

It may seem logical to you but since you didn't state this in your
original recipe, it's only logical that I wondered.

For me, a change to a variable should take effect immediately,
no reboots or restarts.  If you're satisfied with less, suit yourself
(maybe you're on M$ windows?).

> Just to be clear, which problem are we talking about here?

The one that I described fully with a full reproduction recipe,
a few messages ago.  It seemed very similar to your problem, so I
didn't think it was worth a separate bug report.

> There were two related problems in my report:
>
> 1. When told that the language server is `rust-analyzer`, Eglot fails
>    to launch it. This is caused by the hardcoded `"sh" "-c"` in
>    `eglot--cmd` and is fixed by using `(or shell-file-name "sh")
>    (or shell-command-switch "-c")`, possibly with an added
>    `with-connection-local-variables` as per Michael's mail.
>    Or maybe by removing the whole `eglot--cmd` function since from
>    the comment it looks like it only exists to call `stty raw` for
>    remote files, and Eglot is the only package that tries to do that.
>    This can only be fixed in Eglot since that's where it's hardcoded.

Patches welcome.  Patches that remove code and don't break anything
even more welcome.  The patch you sent is insufficient, by your own
admission.  And Eli didn't like it either.

> 2. Eglot doesn't detect `rust-analyzer` automatically. This could be
>    related to the fact that `rust-analyzer` is in a non-standard path
>    or to some mix-up between the local and remote environments (e.g.
>    if Eglot tries to detect the server on the local PC even though it
>    would need the remote one), or something else altogether. It is not
>    fixed by the abovementioned changes.
>    I have no idea how Eglot tries to determine which server it should
>    use, and so no idea where to start looking for a solution.

Eglot uses (executable-find "rust-analyzer" t).  The second argument
asks to consider the remote path, but sometimes that path, which is
configured by tramp-remote-path isn't considered immediately. Michael
clarified this and I expressed the opinion that it is shortcoming
of Tramp.

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

> I didn't complain about the manual.  I complained about the
> variable docstring, which is what people read when changing a
> variable's value.  If you don't think there's a shortcoming
> there feel free to let me know and I won't waste my time.  I
> read your "patches welcome" as meaning that you welcomed
> patches to fix said shortcoming.  If you don't welcome
> patches, say "patches not welcome" instead.

"Patches welcome" means "send patches" :-)

> João

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

> For me, a change to a variable should take effect immediately,
> no reboots or restarts.  If you're satisfied with less, suit yourself
> (maybe you're on M$ windows?).

For Tramp, you shall take into account that it uses caches heavily. A
variable change, w/o propagating to the cache, has no effect.

> João

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[jeberger]

João Távora <joaotavora <at> gmail.com> writes:
> On Tue, Feb 28, 2023 at 1:59 PM <jeberger@free.fr> wrote:
> 
> > > This works, and this would explain the instability I saw earlier.
> > > Thanks.  I wonder if it fixes Jerôme's problem, too.
> > No I've already got it setup as part of my config before I ever
> > access
> > Tramp, which seems only logical: first configure the environment,
> > then
> > try to use it.
> 
> It may seem logical to you but since you didn't state this in your
> original recipe, it's only logical that I wondered.
> 
> For me, a change to a variable should take effect immediately,
> no reboots or restarts.  If you're satisfied with less, suit yourself
> (maybe you're on M$ windows?).
> 
My main platform is Linux, I only use W$ at work because I have to. But
on either platform, I don't expect changes to the environment to be 
picked by child processes or connections that were started before the
change.

> > 1. When told that the language server is `rust-analyzer`, Eglot
> > fails
> Patches welcome.  Patches that remove code and don't break anything
> even more welcome.  The patch you sent is insufficient, by your own
> admission.  And Eli didn't like it either.
> 
Here you go:

==============================8<------------------------------
--- a/eglot.el  2023-02-28 15:38:01.161316788 +0100
+++ b/eglot.el  2023-02-28 15:38:57.445653435 +0100
@@ -1156,10 +1156,11 @@
       ;;
       ;; Not only does this seem like there should be a better way,
       ;; but it almost certainly doesn’t work on non-unix systems.
-      (list "sh" "-c"
-            (string-join (cons "stty raw > /dev/null;"
-                               (mapcar #'shell-quote-argument contact))
-             " "))
+      (with-connection-local-variables
+       (list shell-file-name shell-command-switch
+             (string-join (cons "stty raw > /dev/null;"
+                                (mapcar #'shell-quote-argument contact))
+                          " ")))
     contact))

 (defvar-local eglot--cached-server nil
------------------------------>8==============================

Note that:
- I said it was insufficient because it only fixed point 1 not point 2,
  but that doesn't mean it's not worthwhile (see below for point 2).
- Eli didn't like it due to a doubt about the behaviour of
  `shell-file-name` for remote connections, but Michael has confirmed
  that this use is correct.

> > 2. Eglot doesn't detect `rust-analyzer` automatically. 
> 
> Eglot uses (executable-find "rust-analyzer" t).  The second argument
> asks to consider the remote path, but sometimes that path, which is
> configured by tramp-remote-path isn't considered immediately. Michael
> clarified this and I expressed the opinion that it is shortcoming
> of Tramp.
> 
Effectively, `(executable-find "rust-analyzer" t)` doesn't find it. I'll
try to find out why, but this point should indeed be fixed in
`executable-find` rather than Eglot.

Jérôme

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Tue, Feb 28, 2023 at 2:49 PM Michael Albinus <michael.albinus@gmx.de> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> Hi João,
>
> > For me, a change to a variable should take effect immediately,
> > no reboots or restarts.  If you're satisfied with less, suit yourself
> > (maybe you're on M$ windows?).
>
> For Tramp, you shall take into account that it uses caches heavily. A
> variable change, w/o propagating to the cache, has no effect.

Indeed, as the adage goes, cache invalidation is a hard problem of
computer science, we deal with it every day.  But it's a bit unfair to expect
the user to solve it.

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Tue, Feb 28, 2023 at 2:49 PM <jeberger@free.fr> wrote:

> > For me, a change to a variable should take effect immediately,
> > no reboots or restarts.  If you're satisfied with less, suit yourself
> > (maybe you're on M$ windows?).
> >
> My main platform is Linux, I only use W$ at work because I have to.

Heyy, I know the feeling ;-)

> But on either platform, I don't expect changes to the environment to be
> picked by child processes or connections that were started before the
> change.

Hmmm.  I do.  In Eglot, I certainly strive to allow this.
eglot-managed-mode-hook is an exception, but a self-documenting one.
I don't know of many more exceptions (there could be).

And especially in Tramp, I would expect to, because those processes
are IMHO implementation details that an end-user shouldn't concern
herself with.  In Eglot there's a user-visible concept of
"Eglot-management session".  In Tramp, unless you start looking
at funny named buffers in your buffer list, there isn't this
visibility of the connection to the user (and IMO there shouldn't be).

> > > 1. When told that the language server is `rust-analyzer`, Eglot
> > > fails
> > Patches welcome.  Patches that remove code and don't break anything
> > even more welcome.  The patch you sent is insufficient, by your own
> > admission.  And Eli didn't like it either.
> >
> Here you go:
>
> ==============================8<------------------------------
> --- a/eglot.el  2023-02-28 15:38:01.161316788 +0100
> +++ b/eglot.el  2023-02-28 15:38:57.445653435 +0100
> @@ -1156,10 +1156,11 @@
>        ;;
>        ;; Not only does this seem like there should be a better way,
>        ;; but it almost certainly doesn’t work on non-unix systems.
> -      (list "sh" "-c"
> -            (string-join (cons "stty raw > /dev/null;"
> -                               (mapcar #'shell-quote-argument contact))
> -             " "))
> +      (with-connection-local-variables
> +       (list shell-file-name shell-command-switch
> +             (string-join (cons "stty raw > /dev/null;"
> +                                (mapcar #'shell-quote-argument contact))
> +                          " ")))
>      contact))
>
>  (defvar-local eglot--cached-server nil
> ------------------------------>8==============================
>
> Note that:
> - I said it was insufficient because it only fixed point 1 not point 2,
>   but that doesn't mean it's not worthwhile (see below for point 2).

OK thanks.  I'm OK with that patch, but I'd rather wait until you
also figure out point 2.  That's because -- I think -- if you can get
executable-find to find the right program in the firsdt place.  Then
we don't need to worry what shell we use to launch the language
server remotely. All things considered, launching the standard "sh"
should be more correct than launching the user's pimped-out custom
shell.

I was even more interested in your idea of removing the need for
that stty in eglot--cmd, which I have to admit I don't understand.
So if you want to try your hand at this, go ahead, too.

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

jeberger@free.fr writes:

Hi Jérôme,

> Effectively, `(executable-find "rust-analyzer" t)` doesn't find it.

Your current buffer is a remote one when you call it, right?

> I'll try to find out why, but this point should indeed be fixed in
> `executable-find` rather than Eglot.

Pls set tramp-verbose to 10, rerun the test, and show me the Tramp debug
buffer. Something like

--8<---------------cut here---------------start------------->8---
# emacs -Q -l tramp --eval '(setq tramp-verbose 10)' --eval '(add-to-list (quote tramp-remote-path) ...)' /ssh:user@host: --eval '(executable-find "rust-analyzer" t)'
--8<---------------cut here---------------end--------------->8---

> Jérôme

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Tue, Feb 28, 2023 at 2:45 PM Michael Albinus <michael.albinus@gmx.de> wrote:
> > > Sure.  RTFM :-)
> > I didn't complain about the manual.  I complained about the
> > variable docstring, which is what people read when changing a
> > variable's value.  If you don't think there's a shortcoming
> "Patches welcome" means "send patches" :-)

Here you go :-)

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index eaddc36b54a..d841bbf60e6 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -1372,7 +1372,9 @@ tramp-remote-path

 `Private Directories' are the settings of the $PATH environment,
 as given in your `~/.profile'.  This entry is represented in
-the list by the special value `tramp-own-remote-path'."
+the list by the special value `tramp-own-remote-path'.
+
+Also RFTM."
   :group 'tramp
   :type '(repeat (choice
    (const :tag "Default Directories" tramp-default-remote-path)

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

>> > For me, a change to a variable should take effect immediately,
>> > no reboots or restarts.  If you're satisfied with less, suit yourself
>> > (maybe you're on M$ windows?).
>>
>> For Tramp, you shall take into account that it uses caches heavily. A
>> variable change, w/o propagating to the cache, has no effect.
>
> Indeed, as the adage goes, cache invalidation is a hard problem of
> computer science, we deal with it every day.  But it's a bit unfair to expect
> the user to solve it.

I don't expect a user to solve it. And a user shouldn't expect that
changing a variable is sufficient. There's more to consider.

> João

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Tue, Feb 28, 2023 at 3:15 PM Michael Albinus <michael.albinus@gmx.de> wrote:

> > Indeed, as the adage goes, cache invalidation is a hard problem of
> > computer science, we deal with it every day.  But it's a bit unfair to expect
> > the user to solve it.
>
> I don't expect a user to solve it. And a user shouldn't expect that
> changing a variable is sufficient.

I respectfully but diametrically disagree here.

Here's an untested patch that could solve this.  It can probably
be made smarter by only invalidating the cache when a change
to the variable is detected.

João

diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index ec8437176db..eae00923395 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -5578,11 +5578,7 @@ tramp-get-remote-path
     ;; Expand connection-local variables.
     (tramp-set-connection-local-variables vec)
     (with-tramp-connection-property
- ;; When `tramp-own-remote-path' is in `tramp-remote-path', we
- ;; cache the result for the session only.  Otherwise, the
- ;; result is cached persistently.
- (if (memq 'tramp-own-remote-path tramp-remote-path)
-     (tramp-get-process vec) vec)
+ (tramp-get-process vec)
  "remote-path"
       (let* ((remote-path (copy-tree tramp-remote-path))
       (elt1 (memq 'tramp-default-remote-path remote-path))

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

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

João Távora <joaotavora@gmail.com> writes:

Hi João,

>> "Patches welcome" means "send patches" :-)
>
> Here you go :-)
>
> diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
> index eaddc36b54a..d841bbf60e6 100644
> --- a/lisp/net/tramp.el
> +++ b/lisp/net/tramp.el
> @@ -1372,7 +1372,9 @@ tramp-remote-path
>
>  `Private Directories' are the settings of the $PATH environment,
>  as given in your `~/.profile'.  This entry is represented in
> -the list by the special value `tramp-own-remote-path'."
> +the list by the special value `tramp-own-remote-path'.
> +
> +Also RFTM."
>    :group 'tramp
>    :type '(repeat (choice
>     (const :tag "Default Directories" tramp-default-remote-path)

Ah, you mean


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

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index eaddc36b54a..b24bd33de82 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -1372,7 +1372,9 @@ tramp-remote-path

 `Private Directories' are the settings of the $PATH environment,
 as given in your `~/.profile'.  This entry is represented in
-the list by the special value `tramp-own-remote-path'."
+the list by the special value `tramp-own-remote-path'.
+
+For a full discussion, see Info node `(tramp) Remote programs'."
   :group 'tramp
   :type '(repeat (choice
 		  (const :tag "Default Directories" tramp-default-remote-path)

[-- Attachment #3: Type: text/plain, Size: 56 bytes --]


Pushed to the emacs-29 branch.

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

>> I don't expect a user to solve it. And a user shouldn't expect that
>> changing a variable is sufficient.
>
> I respectfully but diametrically disagree here.
>
> Here's an untested patch that could solve this.  It can probably
> be made smarter by only invalidating the cache when a change
> to the variable is detected.

Hmm. It will delay startup of remote processes, if tramp-remote-path
doesn't change for different processes. Perhaps it is negligable, but at
least for slow connections people will be hit by this.

I'll play with it. Perhaps there is a smarter version of it, as you say.

> João
>
> diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
> index ec8437176db..eae00923395 100644
> --- a/lisp/net/tramp-sh.el
> +++ b/lisp/net/tramp-sh.el
> @@ -5578,11 +5578,7 @@ tramp-get-remote-path
>      ;; Expand connection-local variables.
>      (tramp-set-connection-local-variables vec)
>      (with-tramp-connection-property
> - ;; When `tramp-own-remote-path' is in `tramp-remote-path', we
> - ;; cache the result for the session only.  Otherwise, the
> - ;; result is cached persistently.
> - (if (memq 'tramp-own-remote-path tramp-remote-path)
> -     (tramp-get-process vec) vec)
> + (tramp-get-process vec)
>   "remote-path"
>        (let* ((remote-path (copy-tree tramp-remote-path))
>        (elt1 (memq 'tramp-default-remote-path remote-path))

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[jeberger]

Michael Albinus wrote:
> jeberger@free.fr writes:
> 
> Hi Jérôme,
> 
> > Effectively, `(executable-find "rust-analyzer" t)` doesn't find it.
> 
> Your current buffer is a remote one when you call it, right?
> 
> > I'll try to find out why, but this point should indeed be fixed in
> > `executable-find` rather than Eglot.
> 
> Pls set tramp-verbose to 10, rerun the test, and show me the Tramp
> debug
> buffer. Something like
> 
> --8<---------------cut here---------------start------------->8---
> # emacs -Q -l tramp --eval '(setq tramp-verbose 10)' --eval
> '(add-to-list (quote tramp-remote-path) ...)' /ssh:user@host: --eval
> '(executable-find "rust-analyzer" t)'
> --8<---------------cut here---------------end--------------->8---
> 
> > Jérôme
> 
Yes, it's with a remote buffer. I also noticed that `(exec-path)` is
wrong: it only contains the standard paths ("/bin" "/usr/bin" "/sbin"
"/usr/sbin" "/usr/local/bin" "/usr/local/sbin") plus the path of the
current file. It's missing the paths from the remote environment and
the paths I've added manually to `tramp-remote-path`.

However `(shell-command "echo $PATH")` prints the correct path, and
`(shell-command "rust-analyzer --version")` works.




I tried with a minimalist emacs:

```
emacs -Q -l tramp \
    --eval '(setq tramp-verbose 10)' \
    --eval "(setq tramp-remote-path '(tramp-own-remote-path tramp-default-remote-path))" \
    --eval '(setq shell-file-name "zsh")' \
    /plink:user@host: \
    --eval '(message (exec-path))'
```

But Tramp ignores `shell-file-name` and uses `/bin/sh` instead. E.g.
in the Tramp debug buffer, I see lines like this:

```
16:53:16.188134 tramp-send-command (6) # /bin/sh -l -c 'echo 1e814d9777358b9e0b5ab89ad5422ddb \"$PATH\"' 2>/dev/null; echo tramp_exit_status $?
```

and when I call `shell-command`, it's clear that it's using `sh` too
even though `C-h v` shows that `shell-file-name` is correctly set to
`zsh`.

Jérôme

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Tue, Feb 28, 2023 at 3:50 PM Michael Albinus <michael.albinus@gmx.de> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> Hi João,
>
> >> I don't expect a user to solve it. And a user shouldn't expect that
> >> changing a variable is sufficient.
> >
> > I respectfully but diametrically disagree here.
> >
> > Here's an untested patch that could solve this.  It can probably
> > be made smarter by only invalidating the cache when a change
> > to the variable is detected.
>
> Hmm. It will delay startup of remote processes, if tramp-remote-path
> doesn't change for different processes. Perhaps it is negligable, but at
> least for slow connections people will be hit by this.
>
> I'll play with it. Perhaps there is a smarter version of it, as you say.

Here's another still untested but smarter patch.  Caches fully
on until user messes with tramp-remote-path, else immediately
flushed.

diff --git a/doc/misc/tramp.texi b/doc/misc/tramp.texi
index 56436d32970..335d718b139 100644
--- a/doc/misc/tramp.texi
+++ b/doc/misc/tramp.texi
@@ -2316,10 +2316,7 @@ Remote programs
 @end lisp

 When remote search paths are changed, local @value{tramp} caches must
-be recomputed.  To force @value{tramp} to recompute afresh, call
-@kbd{M-x tramp-cleanup-this-connection @key{RET}} or friends
-(@pxref{Cleanup remote connections}).
-
+be recomputed, but not by the user, so don't worry about this.

 @node Remote shell setup
 @section Remote shell setup hints
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index ec8437176db..ef618855584 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -5571,10 +5571,18 @@ tramp-check-remote-uname
   "Check whether REGEXP matches the connection property \"uname\"."
   (string-match-p regexp (tramp-get-connection-property vec "uname" "")))

+(defvar tramp-last-used-remote-path nil)
+
 (defun tramp-get-remote-path (vec)
   "Compile list of remote directories for PATH.
 Nonexistent directories are removed from spec."
   (with-current-buffer (tramp-get-connection-buffer vec)
+    (when (not (equal tramp-last-used-remote-path tramp-remote-path))
+      ;; If user has tweaked `tramp-remote-path', flush any caches
+      ;; bug#61748.
+      (dolist (v (list vec (tramp-get-process vec)))
+        (tramp-flush-connection-property v "remote-path")))
+      (setq tramp-last-used-remote-path tramp-remote-path))
     ;; Expand connection-local variables.
     (tramp-set-connection-local-variables vec)
     (with-tramp-connection-property

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[jeberger]

Huh, I just noticed that Emacs doesn't use my configured value for 
`tramp-remote-path`. I've set it through `customize` and it has the
correct value in `custom.el` but `C-h v` shows that it has been
reset to another value that doesn't include `tramp-own-remote-path`.

It was fine when I first set it up (in 2016 according to the `hg`
log) but it no longer is :(

The issue is not in my config, I can reproduce it with:

```
emacs -Q --eval "(custom-set-variables '(tramp-remote-path (tramp-own-remote-path tramp-default-remote-path)))" -l tramp
```

Then `C-h v` or `(customize-variable 'tramp-remote-path)` shows
that it has been reset to:

```
(tramp-default-remote-path "/bin" "/usr/bin" "/sbin" "/usr/sbin" "/usr/local/bin" "/usr/local/sbin" "/local/bin" "/local/freeware/bin" "/local/gnu/bin" "/usr/freeware/bin" "/usr/pkg/bin" "/usr/contrib/bin" "/opt/bin" "/opt/sbin" "/opt/local/bin")
```

Other Tramp customizations (e.g. `'(tramp-verbose 10)`) work fine.

Regards,
Jérôme

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Eli Zaretskii]

> Cc: jeberger@free.fr, 61748@debbugs.gnu.org
> From: João Távora <joaotavora@gmail.com>
> Date: Tue, 28 Feb 2023 14:24:01 +0000
> 
> On Tue, Feb 28, 2023 at 2:10 PM Michael Albinus <michael.albinus@gmx.de> wrote:
> >
> > João Távora <joaotavora@gmail.com> writes:
> >
> > Hi João,
> >
> > > At the very least, the documentation of tramp-remote-path should
> > > state that the variable doesn't take immediate effect and that
> > > something must be done (is it really restarting Emacs, or is there
> > > some M-x tramp-cleanup-* thing that has to be done?)
> > >
> > > So, if "anything goes", is such a minimal docstring patch
> > > acceptable?
> >
> > Sure. RTFM :-)
> 
> I didn't complain about the manual.  I complained about the
> variable docstring, which is what people read when changing a
> variable's value.  If you don't think there's a shortcoming
> there feel free to let me know and I won't waste my time.  I
> read your "patches welcome" as meaning that you welcomed
> patches to fix said shortcoming.  If you don't welcome
> patches, say "patches not welcome" instead.

Please be less belligerent, João.  Michael doesn't deserve this
unfriendly attitude.  Even if you disagree with his judgment.

Thanks in advance.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Tue, Feb 28, 2023 at 4:43 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > > Sure. RTFM :-)
> >
> Please be less belligerent, João.  Michael doesn't deserve this
> unfriendly attitude.  Even if you disagree with his judgment.

Well, I'm the one who was asked to go RTFM.  Twice.

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Tue, Feb 28, 2023 at 4:41 PM <jeberger@free.fr> wrote:
>
> Huh, I just noticed that Emacs doesn't use my configured value for
> `tramp-remote-path`. I've set it through `customize` and it has the
> correct value in `custom.el` but `C-h v` shows that it has been
> reset to another value that doesn't include `tramp-own-remote-path`.

Does this mean the bug you experience goes away when
you add either tramp-own-remote-path or ~/bin to that
variable and make sure it is set correctly?

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Tue, 28 Feb 2023 16:49:12 +0000
> Cc: michael.albinus@gmx.de, jeberger@free.fr, 61748@debbugs.gnu.org
> 
> On Tue, Feb 28, 2023 at 4:43 PM Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > > > Sure. RTFM :-)
> > >
> > Please be less belligerent, João.  Michael doesn't deserve this
> > unfriendly attitude.  Even if you disagree with his judgment.
> 
> Well, I'm the one who was asked to go RTFM.  Twice.

With a smiley.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > > Please be less belligerent, João.  Michael doesn't deserve this
  > > > unfriendly attitude.  Even if you disagree with his judgment.
  > > 
  > > Well, I'm the one who was asked to go RTFM.  Twice.

If you feel someone spoken harshly to you, please do not respond by
being harsh.  That tends to create a cycle of retaliation that makes
things worse.  Please look for a way to respond without anger.

In this case, reportedly the other was not actually harsh to you; he
softened it with a smiley.  But if someone else really speaks to you
harshly, please don't retaliate with similar harshness.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[jeberger]

"João Távora" <joaotavora@gmail.com> wrote:
> On Tue, Feb 28, 2023 at 4:41 PM <jeberger@free.fr> wrote:
> >
> > Huh, I just noticed that Emacs doesn't use my configured value for
> > `tramp-remote-path`. I've set it through `customize` and it has the
> > correct value in `custom.el` but `C-h v` shows that it has been
> > reset to another value that doesn't include
> > `tramp-own-remote-path`.
> 
> Does this mean the bug you experience goes away when
> you add either tramp-own-remote-path or ~/bin to that
> variable and make sure it is set correctly?
> 
Adding `tramp-own-remote-path` and making sure that it is set correctly
fixes my point 2, but then `rust-analyzer` doesn't have the path set and
so fails to find the Rust toolchain. So my patch is still required to fix
that part.

Jérôme

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

jeberger@free.fr writes:

Hi Jérôme,

> Huh, I just noticed that Emacs doesn't use my configured value for 
> `tramp-remote-path`. I've set it through `customize` and it has the
> correct value in `custom.el` but `C-h v` shows that it has been
> reset to another value that doesn't include `tramp-own-remote-path`.
>
> It was fine when I first set it up (in 2016 according to the `hg`
> log) but it no longer is :(
>
> The issue is not in my config, I can reproduce it with:
>
> ```
> emacs -Q --eval "(custom-set-variables '(tramp-remote-path (tramp-own-remote-path tramp-default-remote-path)))" -l tramp
> ```
>
> Then `C-h v` or `(customize-variable 'tramp-remote-path)` shows
> that it has been reset to:
>
> ```
> (tramp-default-remote-path "/bin" "/usr/bin" "/sbin" "/usr/sbin" "/usr/local/bin" "/usr/local/sbin" "/local/bin" "/local/freeware/bin" "/local/gnu/bin" "/usr/freeware/bin" "/usr/pkg/bin" "/usr/contrib/bin" "/opt/bin" "/opt/sbin" "/opt/local/bin")
> ```
>
> Other Tramp customizations (e.g. `'(tramp-verbose 10)`) work fine.

Try

--8<---------------cut here---------------start------------->8---
# emacs -Q --eval "(custom-set-variables '(tramp-remote-path '(tramp-own-remote-path tramp-default-remote-path)))"
--8<---------------cut here---------------end--------------->8---

Note the additional apostrophe.

> Regards,
> Jérôme

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[jeberger]

"Michael Albinus" <michael.albinus@gmx.de> wrote:
> jeberger@free.fr writes:
> 
> ```
> > emacs -Q --eval "(custom-set-variables '(tramp-remote-path
> > (tramp-own-remote-path tramp-default-remote-path)))" -l tramp
> > ```
> 
> Try
> 
> --8<---------------cut here---------------start------------->8---
> # emacs -Q --eval "(custom-set-variables '(tramp-remote-path
> '(tramp-own-remote-path tramp-default-remote-path)))"
> --8<---------------cut here---------------end--------------->8---
> 
> Note the additional apostrophe.
> 
Hi Michael,

I tried both with or without the second apostrophe, same result.

Regards,
Jérôme

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

jeberger@free.fr writes:

> Hi Michael,

Hi Jérôme,

>> Try
>>
>> --8<---------------cut here---------------start------------->8---
>> # emacs -Q --eval "(custom-set-variables '(tramp-remote-path
>> '(tramp-own-remote-path tramp-default-remote-path)))"
>> --8<---------------cut here---------------end--------------->8---
>>
>> Note the additional apostrophe.
>>
>
> I tried both with or without the second apostrophe, same result.

You're right. I'm working with Emacs 30.0.50, and my example is fine
there. In Emacs 27, custom-set-variables requires that the respective
library is loaded already. So please try instead

--8<---------------cut here---------------start------------->8---
# emacs -Q -l tramp --eval "(custom-set-variables '(tramp-remote-path '(tramp-own-remote-path tramp-default-remote-path)))"
--8<---------------cut here---------------end--------------->8---

> Regards,
> Jérôme

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

> was not actually harsh to you; he softened it with a smiley.

A pig with lipstick is still a pig.

:-)

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[jeberger]

"Michael Albinus" <michael.albinus@gmx.de>wrote:
> You're right. I'm working with Emacs 30.0.50, and my example is fine
> there. In Emacs 27, custom-set-variables requires that the respective
> library is loaded already. So please try instead
> 
> --8<---------------cut here---------------start------------->8---
> # emacs -Q -l tramp --eval "(custom-set-variables '(tramp-remote-path
> '(tramp-own-remote-path tramp-default-remote-path)))"
> --8<---------------cut here---------------end--------------->8---
> 
No change. Note that AFAICT `tramp-remote-path` is the only custom
variable that is affected. I suspect that this is because there is
a `(defvar tramp-remote-path …)` in `tramp-loaddefs.el` in addition
to the `(defcustom tramp-remote-path …)` from `tramp-sh.el`.

This is with the builtin Tramp in Emacs 27. I just tried installing
Tramp 2.6.0.2 from Elpa and `tramp-remote-path` is set properly
(but trying to open a remote file fails with a missing symbol error,
probably due to an out-of-date dependency).

So the issue appears to be fixed in the latest Tramp, and for the time
being I can work around it using `eval-after-load` to force-set the
path, e.g. this works:

```
emacs -Q -l tramp --eval "(eval-after-load 'tramp '(setq tramp-remote-path '(tramp-own-remote-path tramp-default-remote-path)))"
```

Thanks,
Jérôme

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

jeberger@free.fr writes:

Hi Jérôme,

> This is with the builtin Tramp in Emacs 27. I just tried installing
> Tramp 2.6.0.2 from Elpa and `tramp-remote-path` is set properly
> (but trying to open a remote file fails with a missing symbol error,
> probably due to an out-of-date dependency).
>
> So the issue appears to be fixed in the latest Tramp, and for the time
> being I can work around it using `eval-after-load` to force-set the
> path, e.g. this works:
>
> ```
> emacs -Q -l tramp --eval "(eval-after-load 'tramp '(setq tramp-remote-path '(tramp-own-remote-path tramp-default-remote-path)))"
> ```

Thanks for the confirmation. I'm glad that Tramp 2.6.0.2 did help,
that's what Tramp in GNU ELPA is good for, isn't it?

> Thanks,
> Jérôme

Best regards.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Wed, 1 Mar 2023 09:31:57 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, jeberger@free.fr, michael.albinus@gmx.de, 
> 	61748@debbugs.gnu.org
> 
> > was not actually harsh to you; he softened it with a smiley.
> 
> A pig with lipstick is still a pig.
> 
> :-)

There's nothing wrong with pigs, per se.  Some people even have them
as pets which sleep with them in the same bed.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Wed, Mar 1, 2023 at 1:04 PM Eli Zaretskii <eliz@gnu.org> wrote:

>       61748@debbugs.gnu.org
> >
> > > was not actually harsh to you; he softened it with a smiley.
> >
> > A pig with lipstick is still a pig.
> >
> > :-)
>
> There's nothing wrong with pigs, per se.  Some people even have them
> as pets which sleep with them in the same bed.

Yes, but the lipstick makes a mess.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

>> >> I don't expect a user to solve it. And a user shouldn't expect that
>> >> changing a variable is sufficient.
>> >
>> > I respectfully but diametrically disagree here.
>> >
>> > Here's an untested patch that could solve this.  It can probably
>> > be made smarter by only invalidating the cache when a change
>> > to the variable is detected.
>>
>> Hmm. It will delay startup of remote processes, if tramp-remote-path
>> doesn't change for different processes. Perhaps it is negligable, but at
>> least for slow connections people will be hit by this.
>>
>> I'll play with it. Perhaps there is a smarter version of it, as you say.
>
> Here's another still untested but smarter patch.  Caches fully
> on until user messes with tramp-remote-path, else immediately
> flushed.

Just flushing the connection property "remote-path" is not sufficient I
believe. You still must cleanup the process. This is because the remote
PATH environment is set when starting the process.

And I fail to understand, why connection-local variables don't serve the
purpose. They are described wrt to the remote path in

--8<---------------cut here---------------start------------->8---
(info "(tramp) Remote programs")
--8<---------------cut here---------------end--------------->8---

If this isn't sufficient, we must improve this, instead of introducing
another mechanism. And remember, remote processes are not designed to be
as flexible as local processes, where you could simply call
(setenv "PATH" ...)

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

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

>>> I'll play with it. Perhaps there is a smarter version of it, as you say.
>>
>> Here's another still untested but smarter patch.  Caches fully
>> on until user messes with tramp-remote-path, else immediately
>> flushed.
>
> Just flushing the connection property "remote-path" is not sufficient I
> believe.

I've tested it and it works just fine on my end.  What problems did you
see.

Let me be clear about what i'm attempting to fix, using condensed Emacs
-Q recipes that illustrate what I do during actual sessions.

Before applying my patch, this prints "jdtls", correctly

    ~/Source/Emacs/emacs/src/emacs -Q --batch                \
    -l tramp                                                 \
    --eval '(add-to-list (quote tramp-remote-path) "~/bin")' \
    $REMOTE_FILE                                             \
    --eval '(message "%s" (executable-find "jdtls" t))'

which is fine, of course.  You happened to remember early enough that
the remote host you're going to connect to needs a ~/bin in
tramp-remote-path, so you set that early.

But if you don't remember at the right time, like here:

    ~/Source/Emacs/emacs/src/emacs -Q --batch $REMOTE_FILE   \
    -l tramp                                                 \
    --eval '(add-to-list (quote tramp-remote-path) "~/bin")' \
    --eval '(message "%s" (executable-find "jdtls" t))'

It prints nil.

I find this very confusing, and lost many hours to it.  I expect
'tramp-remote-path' to work like most other variables: set values with
M-:, custom-set-variable, etc.  Then verify the settings work, then
maybe put that in my init file.

After my patch, both invocations print "jdtls".  Can't see how it
doesn't improve things.  Is trivial to understand and has no performance
drawbacks.  It relieves users from keeping a mental model of Tramp's
caches.

> You still must cleanup the process. This is because the remote
> PATH environment is set when starting the process.

This is doesn't differ from local PATH vs local exec-path.  You add
things to exec-path that affect `executable-find`, but don't affect
PATH.

> And I fail to understand, why connection-local variables don't serve the
> purpose. They are described wrt to the remote path in
>
> (info "(tramp) Remote programs")

How am I to use "connection-local variables" here?  Reading the manual
it seems I add multiple setting to my configuration for every host I
connect to.  Like dir-local variables.  That's fine, but I don't know
those hosts and settings in advance.  And if I wanted to change my
configuration, setting tramp-remote-path early on in my .emacs _already_
works.  Which is exactly the confusing part.

> If this isn't sufficient, we must improve this, instead of introducing
> another mechanism.

I'm not proposing new mechanisms or changes to existing ones.  Simply to
invalidate a cache value when there are changes to the source of truth
whence said cached value came.  IME, this is pretty standard thing to
do.

> And remember, remote processes are not designed to be
> as flexible as local processes, where you could simply call
> (setenv "PATH" ...)

I never do that, and I don't want to do that anyway.  I just want
executable-find to not be tricked by some misinvalidated cache.

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[Michael Albinus]

João Távora <joaotavora@gmail.com> writes:

Hi João,

>> Just flushing the connection property "remote-path" is not sufficient I
>> believe.
>
> I've tested it and it works just fine on my end.  What problems did you
> see.

Tramp has documented how a remote path could be set differently for
several hosts. What you describe here does not solve the bug, it is
rather a change request.

I'd prefer to separate this request from the bug. Please write a new bug
report (wishlist severity), and we'll discuss it then. As time permits,
there are other Tramp bugs I need to fix first.

> João

Best regards, Michael.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Wed, Mar 1, 2023 at 8:09 AM <jeberger@free.fr> wrote:
> `tramp-own-remote-path` and making sure that it is set correctly
> fixes my point 2, but then `rust-analyzer` doesn't have the path set and
> so fails to find the Rust toolchain.

AFAIC is a different problem now. Earlier, the program couldn't be invoked
at all.

>  So my patch is still required to fix that part.

I've pushed a simpler patch to emacs-29.  Please test.

João

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Thu, Mar 2, 2023 at 11:40 AM Michael Albinus <michael.albinus@gmx.de> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> I'd prefer to separate this request from the bug. Please write a new bug
> report (wishlist severity), and we'll discuss it then. As time permits,
> there are other Tramp bugs I need to fix first.

Done. See bug#61926.  The patch is attached.

bug#61748: 27.2; Eglot should use shell-file-name when launching the language server for a remote file

[João Távora]

On Thu, Mar 2, 2023 at 1:28 PM João Távora <joaotavora@gmail.com> wrote:
>
> On Wed, Mar 1, 2023 at 8:09 AM <jeberger@free.fr> wrote:
> > `tramp-own-remote-path` and making sure that it is set correctly
> > fixes my point 2, but then `rust-analyzer` doesn't have the path set and
> > so fails to find the Rust toolchain.
>
> AFAIC is a different problem now. Earlier, the program couldn't be invoked
> at all.
>
> >  So my patch is still required to fix that part.
>
> I've pushed a simpler patch to emacs-29.  Please test.

I'm going to assume the lack of reply is because the problem
doesn't happen anymore.  It doesn't in my tests, but my remote
language server in the remote $HOME doesn't rely on PATH
to figure out the toolchain and thus doesn't suffer from this
vulnerability.

Closing.  If the problem persists, we can reopen.

João