bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jordan Wilson]

Hi all,

I've found a bug in Eshell on MS-Windows on 29.1. This is a regression
from 28.2. When connected to a remote machine using putty's "plink",
using any commands on the remote machine (e.g. not built-in to eshell)
prints this error in the eshell buffer (details replaced):

plink -l username -ssh -t example.com " env 'TERM=dumb'
'PROMPT_COMMAND=' 'PS1=#$ ' /bin/sh -i " ** exit || exit
'plink' is not recognised as an internal or external command, operable
program or batch file.

and the following in the minibuffer:
Tramp failed to connect.  If this happens repeatedly, try
    "M-x tramp-cleanup-this-connection"

An keyboard input after this causes the eshell buffer to be killed, and the
"Tramp failed to connect..." message to also be inserted into the next buffer.

Recipe, starting from "emacs -Q":
1) open eshell
2) connect to a remote host using plink
- cd /plink:username@example.com:/home/username/
3) ./test.sh (executable script)
or...
3) *ls
or...
3) (any remote command not built-in into Emacs)

Below is the backtrace produced from this (using
`toggle-debug-on-error') again with details replaced.

Thanks,
Jordan.

Debugger entered--Lisp error: (file-error "Tramp failed to connect.  If this happens repeated...")
  signal(file-error ("Tramp failed to connect.  If this happens repeated..."))
  tramp-error(nil file-error "Tramp failed to connect.  If this happens repeated...")
  tramp-signal-hook-function(file-error ("Tramp failed to connect.  If this happens repeated..."))
  signal(file-error ("Tramp failed to connect.  If this happens repeated..."))
  tramp-maybe-open-connection((tramp-file-name #("plink" 0 5 (escaped t)) #("username" 0 5 (escaped t)) nil #("example.com" 0 18 (escaped t)) nil nil nil))
  tramp-send-command((tramp-file-name #("plink" 0 5 (escaped t)) #("username" 0 5 (escaped t)) nil #("example.com" 0 18 (escaped t)) nil nil nil) "echo $$ 2>/dev/null; echo tramp_exit_status $?")
  tramp-send-command-and-check((tramp-file-name #("plink" 0 5 (escaped t)) #("username" 0 5 (escaped t)) nil #("example.com" 0 18 (escaped t)) nil nil nil) "echo $$")
  tramp-barf-unless-okay((tramp-file-name #("plink" 0 5 (escaped t)) #("username" 0 5 (escaped t)) nil #("example.com" 0 18 (escaped t)) nil nil nil) "echo $$" "`%s' returns with error" "echo $$")
  tramp-send-command-and-read((tramp-file-name #("plink" 0 5 (escaped t)) #("username" 0 5 (escaped t)) nil #("example.com" 0 18 (escaped t)) nil nil nil) "echo $$")
  tramp-sh-handle-make-process(:name "test.sh" :buffer #<killed buffer> :command ("/home/username/test.sh") :filter eshell-output-filter :sentinel eshell-sentinel :connection-type nil :stderr nil :file-handler t)
  tramp-sh-file-name-handler(make-process :name "test.sh" :buffer #<killed buffer> :command ("/home/username/test.sh") :filter eshell-output-filter :sentinel eshell-sentinel :connection-type nil :stderr nil :file-handler t)
  apply(tramp-sh-file-name-handler make-process (:name "test.sh" :buffer #<killed buffer> :command ("/home/username/test.sh") :filter eshell-output-filter :sentinel eshell-sentinel :connection-type nil :stderr nil :file-handler t))
  tramp-file-name-handler(make-process :name "test.sh" :buffer #<killed buffer> :command ("/home/username/test.sh") :filter eshell-output-filter :sentinel eshell-sentinel :connection-type nil :stderr nil :file-handler t)
  eshell-gather-process-output("/plink:username@example.com:/home/username/test.s..." nil)
  eshell-external-command("./test.sh" nil)
  eshell-plain-command("./test.sh" nil)
  eshell-named-command("./test.sh")
  eval((eshell-named-command '"./test.sh"))
  eshell-do-eval((eshell-named-command '"./test.sh") nil)
  eshell-do-eval((prog1 (eshell-named-command '"./test.sh") (mapc #'funcall eshell-this-command-hook)) nil)
  (condition-case err (eshell-do-eval '(prog1 (eshell-named-command '"./test.sh") (mapc #'funcall eshell-this-command-hook)) nil) ((debug error) (mapc #'funcall eshell-this-command-hook) (eshell-errorn (error-message-string err)) (eshell-close-handles 1)))
  eval((condition-case err (eshell-do-eval '(prog1 (eshell-named-command '"./test.sh") (mapc #'funcall eshell-this-command-hook)) nil) ((debug error) (mapc #'funcall eshell-this-command-hook) (eshell-errorn (error-message-string err)) (eshell-close-handles 1))))
  eshell-do-eval((condition-case err (eshell-do-eval '(prog1 (eshell-named-command '"./test.sh") (mapc #'funcall eshell-this-command-hook)) nil) ((debug error) (mapc #'funcall eshell-this-command-hook) (eshell-errorn (error-message-string err)) (eshell-close-handles 1))) nil)
  #f(compiled-function () #<bytecode -0x12092870cc88ed70>)()
  funcall(#f(compiled-function () #<bytecode -0x12092870cc88ed70>))
  (let ((eshell-this-command-hook '(ignore))) (funcall '#f(compiled-function () #<bytecode -0x12092870cc88ed70>)))
  eval((let ((eshell-this-command-hook '(ignore))) (funcall '#f(compiled-function () #<bytecode -0x12092870cc88ed70>))))
  eshell-do-eval((let ((eshell-this-command-hook '(ignore))) (condition-case err (eshell-do-eval '(prog1 (eshell-named-command '"./test.sh") (mapc #'funcall eshell-this-command-hook)) nil) ((debug error) (mapc #'funcall eshell-this-command-hook) (eshell-errorn (error-message-string err)) (eshell-close-handles 1)))) nil)
  eshell-do-eval((progn (let ((eshell-this-command-hook '(ignore))) (condition-case err (eshell-do-eval '(prog1 (eshell-named-command ...) (mapc ... eshell-this-command-hook)) nil) ((debug error) (mapc #'funcall eshell-this-command-hook) (eshell-errorn (error-message-string err)) (eshell-close-handles 1))))) nil)
  (catch 'top-level (eshell-do-eval '(progn (let ((eshell-this-command-hook '...)) (condition-case err (eshell-do-eval '... nil) ((debug error) (mapc ... eshell-this-command-hook) (eshell-errorn ...) (eshell-close-handles 1))))) nil))
  eval((catch 'top-level (eshell-do-eval '(progn (let ((eshell-this-command-hook ...)) (condition-case err (eshell-do-eval ... nil) (... ... ... ...)))) nil)))
  eshell-do-eval((catch 'top-level (eshell-do-eval '(progn (let ((eshell-this-command-hook ...)) (condition-case err (eshell-do-eval ... nil) (... ... ... ...)))) nil)) nil)
  eshell-do-eval((progn 'nil (catch 'top-level (eshell-do-eval '(progn (let (...) (condition-case err ... ...))) nil)) (run-hooks 'eshell-post-command-hook)) nil)
  #f(compiled-function () #<bytecode -0x12092870cc88ed70>)()
  funcall(#f(compiled-function () #<bytecode -0x12092870cc88ed70>))
  (let ((eshell-current-handles '[nil (t . 1) (t . 1)]) (eshell-current-subjob-p 'nil)) (funcall '#f(compiled-function () #<bytecode -0x12092870cc88ed70>)))
  eval((let ((eshell-current-handles '[nil (t . 1) (t . 1)]) (eshell-current-subjob-p 'nil)) (funcall '#f(compiled-function () #<bytecode -0x12092870cc88ed70>))))
  eshell-do-eval((let ((eshell-current-handles '[nil (t . 1) (t . 1)]) eshell-current-subjob-p) (progn 'nil (catch 'top-level (eshell-do-eval '(progn (let ... ...)) nil)) (run-hooks 'eshell-post-command-hook))))
  eshell-resume-eval()
  eshell-eval-command((let ((eshell-current-handles '[nil (t . 1) (t . 1)]) eshell-current-subjob-p) (progn 'nil (catch 'top-level (eshell-do-eval '(progn (let ... ...)) nil)) (run-hooks 'eshell-post-command-hook))) "./test.sh ")
  eshell-send-input(nil)
  funcall-interactively(eshell-send-input nil)
  #<subr call-interactively>(eshell-send-input nil nil)
  call-interactively@ido-cr+-record-current-command(#<subr call-interactively> eshell-send-input nil nil)
  apply(call-interactively@ido-cr+-record-current-command #<subr call-interactively> (eshell-send-input nil nil))
  call-interactively(eshell-send-input nil nil)
  command-execute(eshell-send-input)


In GNU Emacs 29.1 (build 2, x86_64-w64-mingw32) of 2023-08-02 built on
 AVALON
Windowing system distributor 'Microsoft Corp.', version 10.0.19045
System Description: Microsoft Windows 10 Home (v10.0.2009.19045.3324)

Configured using:
 'configure --with-modules --without-dbus --with-native-compilation=aot
 --without-compress-install --with-tree-sitter CFLAGS=-O2'

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

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

Major mode: Messages

Minor modes in effect:
  immortal-scratch-mode: t
  emms-mode-line-mode: t
  emms-playing-time-display-mode: t
  emms-playing-time-mode: t
  shell-dirtrack-mode: t
  recentf-mode: t
  global-anzu-mode: t
  anzu-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  winner-mode: t
  delete-selection-mode: t
  cua-mode: t
  ido-everywhere: t
  pdf-occur-global-minor-mode: t
  windmove-mode: t
  server-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: (only . t)
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t


--
Jordan Wilson
    Sent from Gnus v5.13, GNU Emacs 29.1 on WINDOWS-NT

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Eli Zaretskii]

> From: Jordan Wilson <jordan.t.wilson@gmx.com>
> Date: Sat, 26 Aug 2023 15:11:53 +0100
> 
> I've found a bug in Eshell on MS-Windows on 29.1. This is a regression
> from 28.2. When connected to a remote machine using putty's "plink",
> using any commands on the remote machine (e.g. not built-in to eshell)
> prints this error in the eshell buffer (details replaced):
> 
> plink -l username -ssh -t example.com " env 'TERM=dumb'
> 'PROMPT_COMMAND=' 'PS1=#$ ' /bin/sh -i " ** exit || exit
> 'plink' is not recognised as an internal or external command, operable
> program or batch file.

Is plink on PATH on that Windows system?

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jordan Wilson]

(oops, sorry Eli, didn't wide reply)

On 2023-08-26 (Sat) at 17:42 (+0300), Eli Zaretskii <eliz@gnu.org> wrote:
> Is plink on PATH on that Windows system?

Yes, it's on the Windows' machine I'm connecting from. Eshell
successfully connects to the GNU/Linux machine using plink, but in the
cases described it produces the error. I can use eshell built-in
commands (like `ls' and `pwd') fine on the remote machine, but external
commands on it produces the error.

--
Jordan Wilson
    Sent from Gnus v5.13, GNU Emacs 29.1 on WINDOWS-NT

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Eli Zaretskii]

> From: Jordan Wilson <jordan.t.wilson@gmx.com>
> Cc: 65551@debbugs.gnu.org
> Date: Sat, 26 Aug 2023 15:48:07 +0100
> 
> (oops, sorry Eli, didn't wide reply)
> 
> On 2023-08-26 (Sat) at 17:42 (+0300), Eli Zaretskii <eliz@gnu.org> wrote:
> > Is plink on PATH on that Windows system?
> 
> Yes, it's on the Windows' machine I'm connecting from. Eshell
> successfully connects to the GNU/Linux machine using plink, but in the
> cases described it produces the error. I can use eshell built-in
> commands (like `ls' and `pwd') fine on the remote machine, but external
> commands on it produces the error.

Then I don't think I understand the backtrace you posted.  It seems to
indicate that Trump signals a "failed to connect" error, but the error
message you cited, i.e.

  'plink' is not recognised as an internal or external command, operable
  program or batch file.

is the message shown by the Windows cmd.exe shell, it is not a Tramp
error message.  So it looks like the backtrace does not really show
the relevant error?  Could you perhaps produce a backtrace that is
more relevant to the problem you are having?

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jim Porter]

On 8/26/2023 7:11 AM, Jordan Wilson wrote:
> I've found a bug in Eshell on MS-Windows on 29.1. This is a regression
> from 28.2. When connected to a remote machine using putty's "plink",
> using any commands on the remote machine (e.g. not built-in to eshell)
> prints this error in the eshell buffer (details replaced):

Thanks for reporting this. I can reproduce this issue. Does evaluating 
the following work?

   (setf (alist-get 'tramp-login-program
                    (alist-get "plink" tramp-methods nil nil #'equal))
         (list (concat "\"" (executable-find "plink") "\"")))

If so, I think I've identified the bug: in 
'eshell-gather-process-output', we set the 'process-environment' to 
Eshell's environment variables. In a remote directory, this includes the 
remote PATH. However, that confuses Tramp, which uses that remote PATH 
to look for the local "plink.exe".

Michael, what would be the best way to handle this? In Eshell, I want to 
be able to call 'make-process' to start a remote process using the local 
system's default 'process-environment', but for the remote process to 
see Eshell's modified environment variables. I see that there's 
'tramp-remote-process-environment', but I'm not sure that's the right 
thing for me to use. Do you have any ideas?

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jordan Wilson]

Hi Jim,

On 2023-08-26 (Sat) at 12:16 (-0700), Jim Porter <jporterbugs@gmail.com> wrote:
> Thanks for reporting this. I can reproduce this issue. Does evaluating the
> following work?
>
>   (setf (alist-get 'tramp-login-program
>                    (alist-get "plink" tramp-methods nil nil #'equal))
>         (list (concat "\"" (executable-find "plink") "\"")))
>
> If so, I think I've identified the bug: in 'eshell-gather-process-output', we
> set the 'process-environment' to Eshell's environment variables. In a remote
> directory, this includes the remote PATH. However, that confuses Tramp, which
> uses that remote PATH to look for the local "plink.exe".

I can confirm that snippet does resolve the problem.

--
Jordan Wilson
    Sent from Gnus v5.13, GNU Emacs 29.1 on WINDOWS-NT

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jim Porter]

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

On 8/26/2023 2:28 PM, Jordan Wilson wrote:
> Hi Jim,
> 
> On 2023-08-26 (Sat) at 12:16 (-0700), Jim Porter <jporterbugs@gmail.com> wrote:
>> Thanks for reporting this. I can reproduce this issue. Does evaluating the
>> following work?
>>
>>    (setf (alist-get 'tramp-login-program
>>                     (alist-get "plink" tramp-methods nil nil #'equal))
>>          (list (concat "\"" (executable-find "plink") "\"")))
>>
>> If so, I think I've identified the bug: in 'eshell-gather-process-output', we
>> set the 'process-environment' to Eshell's environment variables. In a remote
>> directory, this includes the remote PATH. However, that confuses Tramp, which
>> uses that remote PATH to look for the local "plink.exe".
> 
> I can confirm that snippet does resolve the problem.

Thanks for checking. That's probably the best way to get things working 
for now, but I'll make sure to get a fix for this in for Emacs 29.2. In 
the meantime, here's a regression test that we should be able to use to 
verify the fix once we have it.

[-- Attachment #2: remote-path-test.patch --]
[-- Type: text/plain, Size: 906 bytes --]

diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index 8e02fbb5497..7d0432dbe68 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -259,4 +259,19 @@ esh-proc-test/kill-pipeline-head
                      output-start (eshell-end-of-output))
                     "")))))
 
+\f
+;; Remote processes
+
+(ert-deftest esh-var-test/remote/remote-path ()
+  "Ensure that setting the remote PATH in Eshell doesn't interfere with Tramp.
+See bug#65551."
+  (skip-unless (and (eshell-tests-remote-accessible-p)
+                    (executable-find "echo")))
+  (let ((default-directory ert-remote-temporary-file-directory))
+    (with-temp-eshell
+     (eshell-insert-command "set PATH ''")
+     (eshell-match-command-output
+      (format "%s hello" (executable-find "echo" t))
+      "\\`hello\n"))))
+
 ;;; esh-proc-tests.el ends here

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Michael Albinus]

Jordan Wilson <jordan.t.wilson@gmx.com> writes:

> Hi all,

Hi Jordan & Jim,

> I've found a bug in Eshell on MS-Windows on 29.1. This is a regression
> from 28.2. When connected to a remote machine using putty's "plink",
> using any commands on the remote machine (e.g. not built-in to eshell)
> prints this error in the eshell buffer (details replaced):
>
> plink -l username -ssh -t example.com " env 'TERM=dumb'
> 'PROMPT_COMMAND=' 'PS1=#$ ' /bin/sh -i " ** exit || exit
> 'plink' is not recognised as an internal or external command, operable
> program or batch file.
>
> Recipe, starting from "emacs -Q":
> 1) open eshell
> 2) connect to a remote host using plink
> - cd /plink:username@example.com:/home/username/
> 3) ./test.sh (executable script)
> or...
> 3) *ls
> or...
> 3) (any remote command not built-in into Emacs)
>
> Below is the backtrace produced from this (using
> `toggle-debug-on-error') again with details replaced.

First I try to understand the scenario Eshell is using. If Eshell has
changed to a remote directory, and a command like "./test.sh" is called,
I would expect a synchronous process. But this doesn't seem to be the case:

>   tramp-file-name-handler(make-process :name "test.sh" :buffer #<killed buffer> :command ("/home/username/test.sh") :filter eshell-output-filter :sentinel eshell-sentinel :connection-type nil :stderr nil :file-handler t)
>   eshell-gather-process-output("/plink:username@example.com:/home/username/test.s..." nil)
>   eshell-external-command("./test.sh" nil)

`make-process' is called, an asynchronous process. Is this intended? Has
this been changed since Emacs 28?

Second, I've played a little bit on MS Windows. I haven't installed
Emacs 29 there, just an Emacs git master checkout. And plink is not in
the PATH of my MINGW64 bash shell, so I'm using sshx. But everything
works under this constellation:

--8<---------------cut here---------------start------------->8---
Welcome to the Emacs shell

~ $ *hostname
win10
~ $ *uname -sr
MINGW64_NT-10.0-19045 3.2.0-340.x86_64
~ $ cd /sshx:detlef.fritz.box:
/sshx:detlef.fritz.box:~ $ *hostname
detlef
/sshx:detlef.fritz.box:~ $ *uname -sr
Linux 6.2.0-27-generic
/sshx:detlef.fritz.box:~ $ which hostname
/sshx:detlef.fritz.box://usr/bin/hostname
/sshx:detlef.fritz.box:~ $ *which hostname
/usr/bin/hostname
/sshx:detlef.fritz.box:~ $
--8<---------------cut here---------------end--------------->8---

> Thanks,
> Jordan.

Best regards, Michael.

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Michael Albinus]

Jim Porter <jporterbugs@gmail.com> writes:

Hi Jim,

> Thanks for reporting this. I can reproduce this issue. Does evaluating
> the following work?
>
>   (setf (alist-get 'tramp-login-program
>                    (alist-get "plink" tramp-methods nil nil #'equal))
>         (list (concat "\"" (executable-find "plink") "\"")))

This workaround might help in the given situation, but it won't work in
general. The user could apply a multi-hop file name, with plink being
the method of another hop.

Best regards, Michael.

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jim Porter]

On 8/27/2023 10:50 AM, Michael Albinus wrote:
> First I try to understand the scenario Eshell is using. If Eshell has
> changed to a remote directory, and a command like "./test.sh" is called,
> I would expect a synchronous process. But this doesn't seem to be the case:
> 
>>    tramp-file-name-handler(make-process :name "test.sh" :buffer #<killed buffer> :command ("/home/username/test.sh") :filter eshell-output-filter :sentinel eshell-sentinel :connection-type nil :stderr nil :file-handler t)
>>    eshell-gather-process-output("/plink:username@example.com:/home/username/test.s..." nil)
>>    eshell-external-command("./test.sh" nil)
> 
> `make-process' is called, an asynchronous process. Is this intended? Has
> this been changed since Emacs 28?

Yeah, that's intended. A couple of things did change in Emacs 29, but 
Eshell has always used an asynchronous process here. The goal is for the 
process to be only partially synchronous: Eshell will wait until the 
process is done before proceeding (e.g. to emit the next prompt), but it 
also wants to be async so that you can do other things in Emacs while 
waiting for the process to complete.

> Second, I've played a little bit on MS Windows. I haven't installed
> Emacs 29 there, just an Emacs git master checkout. And plink is not in
> the PATH of my MINGW64 bash shell, so I'm using sshx. But everything
> works under this constellation:

I'm using sshx locally to test this, and can reproduce the issue. Maybe 
your MINGW64 configuration is smoothing over the bug for you. When I 
tested this, I started from the MS-Windows "cmd.exe" and ran "emacs -Q", 
with "ssh.exe" in my PATH.

In Emacs 28, this worked fine, but it regressed in 29. I believe this is 
due to commit cee1cbfd54375cdece23d4741ced6b0c7091f6d9, which changes 
how Eshell manages its PATH env var.

I think what Eshell would want here is to set its environment variables 
*only* for the subprocess it creates via 'make-process'. For local 
subprocesses, I don't think we need to do anything fancy, but for the 
remote case, we'd need a way to say "use the original 
process-environment for all the Tramp code, but use this new 
process-environment for the actual subprocess". Maybe there's already a 
way to do this, or maybe we need to add some new powers to 
'make-process' or something...

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Michael Albinus]

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

Jim Porter <jporterbugs@gmail.com> writes:

Hi Jim,

>> Second, I've played a little bit on MS Windows. I haven't installed
>> Emacs 29 there, just an Emacs git master checkout. And plink is not in
>> the PATH of my MINGW64 bash shell, so I'm using sshx. But everything
>> works under this constellation:
>
> I'm using sshx locally to test this, and can reproduce the
> issue. Maybe your MINGW64 configuration is smoothing over the bug for
> you. When I tested this, I started from the MS-Windows "cmd.exe" and
> ran "emacs -Q", with "ssh.exe" in my PATH.

Yes, with this scenario I see the problem as well.

> In Emacs 28, this worked fine, but it regressed in 29. I believe this
> is due to commit cee1cbfd54375cdece23d4741ced6b0c7091f6d9, which
> changes how Eshell manages its PATH env var.
>
> I think what Eshell would want here is to set its environment
> variables *only* for the subprocess it creates via 'make-process'. For
> local subprocesses, I don't think we need to do anything fancy, but
> for the remote case, we'd need a way to say "use the original
> process-environment for all the Tramp code, but use this new
> process-environment for the actual subprocess". Maybe there's already
> a way to do this, or maybe we need to add some new powers to
> 'make-process' or something...

AFAICS, the problem is in eshell-environment-variables, which sets PATH
to the remote path. There are good reasons that Tramp doesn't handle the
PATH environment variable, but uses its own tramp-remote-path variable.

I've applied the following sledge-hammer patch, which cures the problem
for me. But I'm pretty sure there are better ways in Eshell to fix this.

Best regards, Michael.


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

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 711c35f8527..4c5ce1a1a35 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -459,7 +459,7 @@ eshell-environment-variables
 environment, as specified in `eshell-variable-aliases-list'."
   (let ((process-environment (eshell-copy-environment)))
     (dolist (var-alias eshell-variable-aliases-list)
-      (if (nth 2 var-alias)
+      (if (and (not (string-equal (car var-alias) "PATH")) (nth 2 var-alias))
 	  (setenv (car var-alias)
 		  (eshell-stringify
 		   (or (eshell-get-variable (car var-alias)) "")))))

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jim Porter]

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

On 8/28/2023 3:27 AM, Michael Albinus wrote:
> I've applied the following sledge-hammer patch, which cures the problem
> for me. But I'm pretty sure there are better ways in Eshell to fix this.

How about something like this? It's a little less invasive (but no less 
of a hack).

I'll have to think about this more generally though, since I think 
Eshell should be a lot more precise about how it handles environment 
variables on remote hosts in general. Maybe we even want each host to 
have its own distinct set of env vars. That would probably be safer...

[-- Attachment #2: 0001-Fix-remote-path-setting-in-Eshell.patch --]
[-- Type: text/plain, Size: 3475 bytes --]

From a433028e00e5dbe851fcb2df5cce35a8a91e8cc5 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 27 Aug 2023 12:53:40 -0700
Subject: [PATCH] Fix remote path setting in Eshell

This ensures that we supply Tramp with the local PATH so that it can
do its job of starting the local "ssh", or whatever the method uses
(bug#65551).

* lisp/eshell/esh-proc.el (eshell-gather-process-output): Add special
handling for remote processes.

* test/lisp/eshell/esh-proc-tests.el
(esh-var-test/remote/remote-path): New test.
---
 lisp/eshell/esh-proc.el            | 14 +++++++++++++-
 test/lisp/eshell/esh-proc-tests.el | 15 +++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index fcd59ab9f37..5bd8446eeec 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -265,6 +265,8 @@ eshell-last-sync-output-start
   "A marker that tracks the beginning of output of the last subprocess.
 Used only on systems which do not support async subprocesses.")
 
+(defvar tramp-remote-path)
+
 (defun eshell-gather-process-output (command args)
   "Gather the output from COMMAND + ARGS."
   (require 'esh-var)
@@ -272,7 +274,8 @@ eshell-gather-process-output
   (unless (and (file-executable-p command)
 	       (file-regular-p (file-truename command)))
     (error "%s: not an executable file" command))
-  (let* ((delete-exited-processes
+  (let* ((real-path (getenv "PATH"))
+         (delete-exited-processes
 	  (if eshell-current-subjob-p
 	      eshell-delete-exited-processes
 	    delete-exited-processes))
@@ -280,6 +283,15 @@ eshell-gather-process-output
          (coding-system-for-read coding-system-for-read)
          (coding-system-for-write coding-system-for-write)
 	 proc stderr-proc decoding encoding changed)
+    ;; HACK: We want to supply our subprocess with the all the
+    ;; environment variables we've set in Eshell.  However, supplying
+    ;; a remote PATH this way can break Tramp, which needs the *local*
+    ;; PATH for calling "ssh", etc.  Instead, set the local path in
+    ;; our `process-environment' and pass the remote PATH via
+    ;; `tramp-remote-path'.
+    (when (file-remote-p default-directory)
+      (push (concat "PATH=" real-path) process-environment)
+      (setq tramp-remote-path (eshell-get-path)))
     ;; MS-Windows needs special setting of encoding/decoding, because
     ;; (a) non-ASCII text in command-line arguments needs to be
     ;; encoded in the system's codepage; and (b) because many Windows
diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index 8e02fbb5497..7d0432dbe68 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -259,4 +259,19 @@ esh-proc-test/kill-pipeline-head
                      output-start (eshell-end-of-output))
                     "")))))
 
+\f
+;; Remote processes
+
+(ert-deftest esh-var-test/remote/remote-path ()
+  "Ensure that setting the remote PATH in Eshell doesn't interfere with Tramp.
+See bug#65551."
+  (skip-unless (and (eshell-tests-remote-accessible-p)
+                    (executable-find "echo")))
+  (let ((default-directory ert-remote-temporary-file-directory))
+    (with-temp-eshell
+     (eshell-insert-command "set PATH ''")
+     (eshell-match-command-output
+      (format "%s hello" (executable-find "echo" t))
+      "\\`hello\n"))))
+
 ;;; esh-proc-tests.el ends here
-- 
2.25.1

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jim Porter]

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

On 8/28/2023 9:29 AM, Jim Porter wrote:
> On 8/28/2023 3:27 AM, Michael Albinus wrote:
>> I've applied the following sledge-hammer patch, which cures the problem
>> for me. But I'm pretty sure there are better ways in Eshell to fix this.
> 
> How about something like this? It's a little less invasive (but no less 
> of a hack).

Oops, something more like this (let-binding 'tramp-remote-path').

[-- Attachment #2: 0001-Fix-remote-path-setting-in-Eshell.patch --]
[-- Type: text/plain, Size: 3524 bytes --]

From 0025ae9429d97501c493653593beb88ec9ea0259 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 27 Aug 2023 12:53:40 -0700
Subject: [PATCH] Fix remote path setting in Eshell

This ensures that we supply Tramp with the local PATH so that it can
do its job of starting the local "ssh", or whatever the method uses
(bug#65551).

* lisp/eshell/esh-proc.el (eshell-gather-process-output): Add special
handling for remote processes.

* test/lisp/eshell/esh-proc-tests.el
(esh-var-test/remote/remote-path): New test.
---
 lisp/eshell/esh-proc.el            | 15 ++++++++++++++-
 test/lisp/eshell/esh-proc-tests.el | 15 +++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index fcd59ab9f37..49577201ddd 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -265,6 +265,8 @@ eshell-last-sync-output-start
   "A marker that tracks the beginning of output of the last subprocess.
 Used only on systems which do not support async subprocesses.")
 
+(defvar tramp-remote-path)
+
 (defun eshell-gather-process-output (command args)
   "Gather the output from COMMAND + ARGS."
   (require 'esh-var)
@@ -272,7 +274,9 @@ eshell-gather-process-output
   (unless (and (file-executable-p command)
 	       (file-regular-p (file-truename command)))
     (error "%s: not an executable file" command))
-  (let* ((delete-exited-processes
+  (let* ((real-path (getenv "PATH"))
+         (tramp-remote-path tramp-remote-path)
+         (delete-exited-processes
 	  (if eshell-current-subjob-p
 	      eshell-delete-exited-processes
 	    delete-exited-processes))
@@ -280,6 +284,15 @@ eshell-gather-process-output
          (coding-system-for-read coding-system-for-read)
          (coding-system-for-write coding-system-for-write)
 	 proc stderr-proc decoding encoding changed)
+    ;; HACK: We want to supply our subprocess with the all the
+    ;; environment variables we've set in Eshell.  However, supplying
+    ;; a remote PATH this way can break Tramp, which needs the *local*
+    ;; PATH for calling "ssh", etc.  Instead, set the local path in
+    ;; our `process-environment' and pass the remote PATH via
+    ;; `tramp-remote-path'.
+    (when (file-remote-p default-directory)
+      (push (concat "PATH=" real-path) process-environment)
+      (setq tramp-remote-path (eshell-get-path)))
     ;; MS-Windows needs special setting of encoding/decoding, because
     ;; (a) non-ASCII text in command-line arguments needs to be
     ;; encoded in the system's codepage; and (b) because many Windows
diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index 8e02fbb5497..7d0432dbe68 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -259,4 +259,19 @@ esh-proc-test/kill-pipeline-head
                      output-start (eshell-end-of-output))
                     "")))))
 
+\f
+;; Remote processes
+
+(ert-deftest esh-var-test/remote/remote-path ()
+  "Ensure that setting the remote PATH in Eshell doesn't interfere with Tramp.
+See bug#65551."
+  (skip-unless (and (eshell-tests-remote-accessible-p)
+                    (executable-find "echo")))
+  (let ((default-directory ert-remote-temporary-file-directory))
+    (with-temp-eshell
+     (eshell-insert-command "set PATH ''")
+     (eshell-match-command-output
+      (format "%s hello" (executable-find "echo" t))
+      "\\`hello\n"))))
+
 ;;; esh-proc-tests.el ends here
-- 
2.25.1

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jim Porter]

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

On 8/28/2023 9:47 AM, Jim Porter wrote:
> On 8/28/2023 9:29 AM, Jim Porter wrote:
>> On 8/28/2023 3:27 AM, Michael Albinus wrote:
>>> I've applied the following sledge-hammer patch, which cures the problem
>>> for me. But I'm pretty sure there are better ways in Eshell to fix this.
>>
>> How about something like this? It's a little less invasive (but no 
>> less of a hack).
> 
> Oops, something more like this (let-binding 'tramp-remote-path').

Uh... the third time's the charm I guess? (Sorry for the extra emails.)

[-- Attachment #2: 0001-Fix-remote-path-setting-in-Eshell.patch --]
[-- Type: text/plain, Size: 3543 bytes --]

From 3d220232ff64e1ec1dcc9ec2c0cfefe37cdde431 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 27 Aug 2023 12:53:40 -0700
Subject: [PATCH] Fix remote path setting in Eshell

This ensures that we supply Tramp with the local PATH so that it can
do its job of starting the local "ssh", or whatever the method uses
(bug#65551).

* lisp/eshell/esh-proc.el (eshell-gather-process-output): Add special
handling for remote processes.

* test/lisp/eshell/esh-proc-tests.el
(esh-var-test/remote/remote-path): New test.
---
 lisp/eshell/esh-proc.el            | 15 ++++++++++++++-
 test/lisp/eshell/esh-proc-tests.el | 15 +++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index fcd59ab9f37..fcb0b7d7fd3 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -265,6 +265,8 @@ eshell-last-sync-output-start
   "A marker that tracks the beginning of output of the last subprocess.
 Used only on systems which do not support async subprocesses.")
 
+(defvar tramp-remote-path)
+
 (defun eshell-gather-process-output (command args)
   "Gather the output from COMMAND + ARGS."
   (require 'esh-var)
@@ -272,7 +274,9 @@ eshell-gather-process-output
   (unless (and (file-executable-p command)
 	       (file-regular-p (file-truename command)))
     (error "%s: not an executable file" command))
-  (let* ((delete-exited-processes
+  (let* ((real-path (getenv "PATH"))
+         (tramp-remote-path (bound-and-true-p tramp-remote-path))
+         (delete-exited-processes
 	  (if eshell-current-subjob-p
 	      eshell-delete-exited-processes
 	    delete-exited-processes))
@@ -280,6 +284,15 @@ eshell-gather-process-output
          (coding-system-for-read coding-system-for-read)
          (coding-system-for-write coding-system-for-write)
 	 proc stderr-proc decoding encoding changed)
+    ;; HACK: We want to supply our subprocess with the all the
+    ;; environment variables we've set in Eshell.  However, supplying
+    ;; a remote PATH this way can break Tramp, which needs the *local*
+    ;; PATH for calling "ssh", etc.  Instead, set the local path in
+    ;; our `process-environment' and pass the remote PATH via
+    ;; `tramp-remote-path'.
+    (when (file-remote-p default-directory)
+      (push (concat "PATH=" real-path) process-environment)
+      (setq tramp-remote-path (eshell-get-path)))
     ;; MS-Windows needs special setting of encoding/decoding, because
     ;; (a) non-ASCII text in command-line arguments needs to be
     ;; encoded in the system's codepage; and (b) because many Windows
diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index 8e02fbb5497..7d0432dbe68 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -259,4 +259,19 @@ esh-proc-test/kill-pipeline-head
                      output-start (eshell-end-of-output))
                     "")))))
 
+\f
+;; Remote processes
+
+(ert-deftest esh-var-test/remote/remote-path ()
+  "Ensure that setting the remote PATH in Eshell doesn't interfere with Tramp.
+See bug#65551."
+  (skip-unless (and (eshell-tests-remote-accessible-p)
+                    (executable-find "echo")))
+  (let ((default-directory ert-remote-temporary-file-directory))
+    (with-temp-eshell
+     (eshell-insert-command "set PATH ''")
+     (eshell-match-command-output
+      (format "%s hello" (executable-find "echo" t))
+      "\\`hello\n"))))
+
 ;;; esh-proc-tests.el ends here
-- 
2.25.1

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Michael Albinus]

Jim Porter <jporterbugs@gmail.com> writes:

Hi Jim,

> On 8/28/2023 9:29 AM, Jim Porter wrote:
>> On 8/28/2023 3:27 AM, Michael Albinus wrote:
>>> I've applied the following sledge-hammer patch, which cures the problem
>>> for me. But I'm pretty sure there are better ways in Eshell to fix this.
>> How about something like this? It's a little less invasive (but no
>> less of a hack).
>
> Oops, something more like this (let-binding 'tramp-remote-path').

Might work. But well, it is Tramp. Everything is more complex.

tramp-remote-path is just a template, It is taken the very first time
you connect to a remote host in order to determine proper PATH
settings. The result is cached in the connection property "remote-path".

Every new process for that remote host (like returned by make-process)
checks first, whether this connection property exists, and uses it. Only
if the connection property doesn't exist, it is recomputed starting with
tramp-remote-path.

So usually, you don't have to do anything wrt PATH. That's why my
sledge-hammer patch works.

If a user changes PATH for a remote connection in eshell (I don't recall
how, but I'm sure it is possible in Eshell), you just have to change the
respective connection property "remote-path" in Tramp.
See (info "(tramp) Predefined connection information")

There is an exception: If a user has the symbol tramp-own-remote-path
in tramp-remote-path, the cached value is not used for a new process,
and PATH is recomputed based on tramp-remote-path.

Writing this, it sounds to me too complex. OTOH, no other package has
tried yet to play with the remote PATH. There are bug reports by users
who request a simplification (bug#61926, bug#62326). Perhaps it is time
to redesign the machinery in Tramp.

Best regards, Michael.

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Michael Albinus]

Jim Porter <jporterbugs@gmail.com> writes:

Hi Jim,

> Uh... the third time's the charm I guess? (Sorry for the extra emails.)

Take your time. Tomorrow, my grand-son will arrive, staying for 10 days
or so (remainder of school holidays). Be prepared that I'll respond slower :-)

Best regards, Michael.

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jim Porter]

On 8/28/2023 10:33 AM, Michael Albinus wrote:
> tramp-remote-path is just a template, It is taken the very first time
> you connect to a remote host in order to determine proper PATH
> settings. The result is cached in the connection property "remote-path".

Yeah, I believe this result is where we get the starting value for PATH 
for remote hosts. If we don't already have a cached PATH for the remote 
host in Eshell, we get it by calling '(exec-path)', which I'm guessing 
uses tramp-remote-path (at least indirectly).

> If a user changes PATH for a remote connection in eshell (I don't recall
> how, but I'm sure it is possible in Eshell), you just have to change the
> respective connection property "remote-path" in Tramp.
> See (info "(tramp) Predefined connection information")

Changing the PATH is pretty simple: just run "export PATH=blah" or "set 
PATH blah".

> There is an exception: If a user has the symbol tramp-own-remote-path
> in tramp-remote-path, the cached value is not used for a new process,
> and PATH is recomputed based on tramp-remote-path.

I think we want to handle this case too: for example, I use 
'tramp-own-remote-path' so that I can pick up my remote hosts' PATH 
values, but then I might want to change that value in Eshell later. If 
Eshell let-binds and sets 'tramp-remote-path' when starting the remote 
process, it can override the 'tramp-own-remote-path' setting 
temporarily, which I think is what we want. We'd lose the ability for 
Tramp to recompute the remote PATH on its own, but in the context of 
Eshell I think that should be ok.

> Writing this, it sounds to me too complex. OTOH, no other package has
> tried yet to play with the remote PATH. There are bug reports by users
> who request a simplification (bug#61926, bug#62326). Perhaps it is time
> to redesign the machinery in Tramp.

Yeah, this is pretty tricky code to work with on both sides (both 
Tramp/remote connections and Eshell). One option that might make sense 
is if we could pass the environment vars for the subprocess as a key in 
'make-process', like ':env'. That would let us make sure that the env 
vars are only used exactly where we want them.

For the Eshell side, I think this bug has shown that it's time to review 
how it handles environment variables in general for remote hosts. 
Another interesting bug is how we set PWD and OLDPWD to Tramp filenames 
on remote hosts. When running an external process on that host, we 
should probably use the local file name instead, since non-Emacs 
programs won't understand our remote file syntax.

I'm sure there are other bugs besides that too. Maybe the default 
behavior for env vars is that they should be connection-local (so remote 
hosts don't see your env vars), and then we can opt into passing certain 
special vars (like TERM) to all remote hosts. I'll think about this some 
more and file a bug when I have something more concrete.

bug#65551: 29.1; Eshell on MS-Windows using plink: 'plink' is not recognized as an internal or external command...

[Jim Porter]

Version: 29.2

On 8/28/2023 9:53 AM, Jim Porter wrote:
> On 8/28/2023 9:47 AM, Jim Porter wrote:
>> On 8/28/2023 9:29 AM, Jim Porter wrote:
>>> How about something like this? It's a little less invasive (but no 
>>> less of a hack).
>>
>> Oops, something more like this (let-binding 'tramp-remote-path').
> 
> Uh... the third time's the charm I guess? (Sorry for the extra emails.)

Merged this to the Emacs 29 branch as 2af092741e5. Let's keep this open 
to discuss a less-hacky solution for Emacs 30, though.