Subject: bug#57556: 28.1; Eshell not finding executables in PATH when tramp-integration loaded

From: Jim Porter <[email-masked]>
Date: Fri, 14 Oct 2022 13:53:33 -0700

On 10/14/2022 5:27 AM, Michael Albinus wrote:
> Jim Porter <[email-masked]> writes:
> 
> Hi Jim,
> 
> All patches look OK to me (patch 0002 in the newer version). Just two
> minor nits:
> 
>> [6. text/plain; 0005-Allow-setting-the-values-of-variable-aliases-in-Eshe.patch]
>>
>> +You can set variables in a few different ways.  To set a Lisp
>> +variable, you can use the command @samp{setq @var{name} @var{value}},
>> +which works much like its Lisp counterpart.  To set an environment
>> +variable, use @samp{export @var{NAME}=@var{value}}. You can also use
>> +@samp{set @var{name} @var{value}}, which sets a Lisp variable if
>> +@var{name} is a symbol, or an environment variable if @var{name} is a
>> +string.  Finally, you can temporarily set environment variables for a
>> +single command with @samp{@var{NAME}=@var{value} @var{command}
>> +@dots{}}. This is equivalent to:
>> +
>> +@example
>> +@{
>> +  set @var{NAME} @var{value}
>> +  @var{command} @dots{}
>> +@}
>> +@end example
> 
> @var{} produces already capital letters, so you are more consistent with
> @var{name}.

My intent was to make that display as all-caps in the HTML documentation 
as well. In that excerpt, 'NAME' should always be an environment 
variable, so I used the capitalization conventions that env vars usually 
use. 'name', on the other hand, could be a Lisp variable or an env var.

I adjusted these docs a bit since they seemed unclear to me on a second 
reading (see attached), but kept the all-caps NAME for env vars. If you 
still think that's wrong, I'll change it to lower-case before merging.

>> [7. text/plain; 0006-Improve-handling-of-PATH-in-Eshell-for-remote-direct.patch]
>>
>> +@vindex $PATH
>> +@item $PATH
>> +This specifies the directories to search for executable programs as a
>> +string, separated by @code{":"} for Unix and GNU systems, and
>> +@code{";"} for MS systems.  This variable is connection-aware, so when
>> +the current directory on a remote host, it will automatically update
>> +to reflect the search path on that host.
> 
> "... when the current directory is on a remote host ..."
> 
> Perhaps you could also say, that when the current directory changes back
> to the local host, $PATH is also updated respectively.

I reworked this to make it clearer that the value is updated *every* 
time you 'cd' into a different host (including from one remote host to 
another) and also added a cross-reference to the Remote Files section of 
the Emacs manual.

From 28daaf0bd9890aa6d6df7fb03478358821aaa80e Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Sun, 25 Sep 2022 21:47:26 -0700
Subject: [PATCH 5/7] Allow setting the values of variable aliases in Eshell

This makes commands like "COLUMNS=40 some-command" work as expected.

* lisp/eshell/esh-cmd.el (eshell-subcommand-bindings): Remove
'process-environment' from here...

* lisp/eshell/esh-var.el (eshell-var-initialize): ... and add to here,
along with 'eshell-variable-aliases-list'.
(eshell-inside-emacs): Convert to a 'defvar-local' to make it settable
in a particular Eshell buffer.
(eshell-variable-aliases-list): Make $?, $$, and $* read-only and
update docstring.
(eshell-set-variable): New function...
(eshell-handle-local-variables, eshell/export, eshell/unset): ... use
it.
(eshell/set, pcomplete/eshell-mode/set): New functions.
(eshell-get-variable): Get the variable alias's getter function when
appropriate and use a safer method for checking function arity.

* test/lisp/eshell/esh-var-tests.el (esh-var-test/set/env-var)
(esh-var-test/set/symbol, esh-var-test/unset/env-var)
(esh-var-test/unset/symbol, esh-var-test/setq, esh-var-test/export)
(esh-var-test/local-variables, esh-var-test/alias/function)
(esh-var-test/alias/function-pair, esh-var-test/alias/string)
(esh-var-test/alias/string/prefer-lisp, esh-var-test/alias/symbol)
(esh-var-test/alias/symbol-pair, esh-var-test/alias/export)
(esh-var-test/alias/local-variables): New tests.

* doc/misc/eshell.texi (Built-ins): Add 'set' and update 'unset'
documentation.
(Variables): Expand documentation of how to get/set variables.
---
 doc/misc/eshell.texi              |  47 ++++++++--
 lisp/eshell/esh-cmd.el            |   4 +-
 lisp/eshell/esh-var.el            | 141 +++++++++++++++++++++--------
 test/lisp/eshell/esh-var-tests.el | 145 ++++++++++++++++++++++++++++++
 4 files changed, 291 insertions(+), 46 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 8036bbd83a..2deb6bdc20 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -694,10 +694,17 @@ Built-ins
 This command can be loaded as part of the eshell-xtra module, which is
 disabled by default.
 
+@item set
+@cmindex set
+Set variable values, using the function @code{set} like a command
+(@pxref{Setting Variables,,, elisp, GNU Emacs Lisp Reference Manual}).
+A variable name can be a symbol, in which case it refers to a Lisp
+variable, or a string, referring to an environment variable.
+
 @item setq
 @cmindex setq
-Set variable values, using the function @code{setq} like a command.
-@xref{Setting Variables,,, elisp, GNU Emacs Lisp Reference Manual}.
+Set variable values, using the function @code{setq} like a command
+(@pxref{Setting Variables,,, elisp, GNU Emacs Lisp Reference Manual}).
 
 @item source
 @cmindex source
@@ -743,7 +750,9 @@ Built-ins
 
 @item unset
 @cmindex unset
-Unset an environment variable.
+Unset one or more variables.  As with @command{set}, a variable name
+can be a symbol, in which case it refers to a Lisp variable, or a
+string, referring to an environment variable.
 
 @item wait
 @cmindex wait
@@ -881,12 +890,34 @@ Built-ins
 
 @node Variables
 @section Variables
-Since Eshell is just an Emacs @acronym{REPL}@footnote{
+@vindex eshell-prefer-lisp-variables
+Since Eshell is a combination of an Emacs @acronym{REPL}@footnote{
 Short for ``Read-Eval-Print Loop''.
-}
-, it does not have its own scope, and simply stores variables the same
-you would in an Elisp program.  Eshell provides a command version of
-@code{setq} for convenience.
+} and a command shell, it can refer to variables from two different
+sources: ordinary Emacs Lisp variables, as well as environment
+variables.  By default, when using a variable in Eshell, it will first
+look in the list of built-in variables, then in the list of
+environment variables, and finally in the list of Lisp variables.  If
+you would prefer to use Lisp variables over environment variables, you
+can set @code{eshell-prefer-lisp-variables} to @code{t}.
+
+You can set variables in a few different ways.  To set a Lisp
+variable, you can use the command @samp{setq @var{name} @var{value}},
+which works much like its Lisp counterpart (@pxref{Setting Variables,
+, , elisp, The Emacs Lisp Reference Manual}).  To set an environment
+variable, use @samp{export @var{NAME}=@var{value}}. You can also use
+@samp{set @var{variable} @var{value}}, which sets a Lisp variable if
+@var{variable}'s value is a symbol, or an environment variable if it's
+a string.  Finally, you can temporarily set environment variables for
+a single command with @samp{@var{NAME}=@var{value} @var{command}
+@dots{}}. This is equivalent to:
+
+@example
+@{
+  set @var{NAME} @var{value}
+  @var{command} @dots{}
+@}
+@end example
 
 @subsection Built-in variables
 Eshell knows a few built-in variables:
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 3f3a1616ee..c5ceb3ffd1 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -261,9 +261,9 @@ eshell-deferrable-commands
 (defcustom eshell-subcommand-bindings
   '((eshell-in-subcommand-p t)
     (eshell-in-pipeline-p nil)
-    (default-directory default-directory)
-    (process-environment (eshell-copy-environment)))
+    (default-directory default-directory))
   "A list of `let' bindings for subcommand environments."
+  :version "29.1"		       ; removed `process-environment'
   :type 'sexp
   :risky t)
 
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 3c09fc52fb..caf143e1a1 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -113,7 +113,7 @@
 (require 'pcomplete)
 (require 'ring)
 
-(defconst eshell-inside-emacs (format "%s,eshell" emacs-version)
+(defvar-local eshell-inside-emacs (format "%s,eshell" emacs-version)
   "Value for the `INSIDE_EMACS' environment variable.")
 
 (defgroup eshell-var nil
@@ -162,8 +162,8 @@ eshell-variable-aliases-list
 	        (car (last eshell-last-arguments))
 	      (eshell-apply-indices eshell-last-arguments
 				    indices quoted))))
-    ("?" eshell-last-command-status)
-    ("$" eshell-last-command-result)
+    ("?" (eshell-last-command-status . nil))
+    ("$" (eshell-last-command-result . nil))
 
     ;; for em-alias.el and em-script.el
     ("0" eshell-command-name)
@@ -176,7 +176,7 @@ eshell-variable-aliases-list
     ("7" ,(lambda () (nth 6 eshell-command-arguments)) nil t)
     ("8" ,(lambda () (nth 7 eshell-command-arguments)) nil t)
     ("9" ,(lambda () (nth 8 eshell-command-arguments)) nil t)
-    ("*" eshell-command-arguments))
+    ("*" (eshell-command-arguments . nil)))
   "This list provides aliasing for variable references.
 Each member is of the following form:
 
@@ -186,6 +186,11 @@ eshell-variable-aliases-list
 compute the string value that will be returned when the variable is
 accessed via the syntax `$NAME'.
 
+If VALUE is a cons (GET . SET), then variable references to NAME
+will use GET to get the value, and SET to set it.  GET and SET
+can be one of the forms described below.  If SET is nil, the
+variable is read-only.
+
 If VALUE is a function, its behavior depends on the value of
 SIMPLE-FUNCTION.  If SIMPLE-FUNCTION is nil, call VALUE with two
 arguments: the list of the indices that were used in the reference,
@@ -193,23 +198,30 @@ eshell-variable-aliases-list
 quoted with double quotes.  For example, if `NAME' were aliased
 to a function, a reference of `$NAME[10][20]' would result in that
 function being called with the arguments `((\"10\") (\"20\"))' and
-nil.
-If SIMPLE-FUNCTION is non-nil, call the function with no arguments
-and then pass its return value to `eshell-apply-indices'.
+nil.  If SIMPLE-FUNCTION is non-nil, call the function with no
+arguments and then pass its return value to `eshell-apply-indices'.
+
+When VALUE is a function, it's read-only by default.  To make it
+writeable, use the (GET . SET) form described above.  If SET is a
+function, it takes two arguments: a list of indices (currently
+always nil, but reserved for future enhancement), and the new
+value to set.
 
-If VALUE is a string, return the value for the variable with that
-name in the current environment.  If no variable with that name exists
-in the environment, but if a symbol with that same name exists and has
-a value bound to it, return that symbol's value instead.  You can
-prefer symbol values over environment values by setting the value
-of `eshell-prefer-lisp-variables' to t.
+If VALUE is a string, get/set the value for the variable with
+that name in the current environment.  When getting the value, if
+no variable with that name exists in the environment, but if a
+symbol with that same name exists and has a value bound to it,
+return that symbol's value instead.  You can prefer symbol values
+over environment values by setting the value of
+`eshell-prefer-lisp-variables' to t.
 
-If VALUE is a symbol, return the value bound to it.
+If VALUE is a symbol, get/set the value bound to it.
 
 If VALUE has any other type, signal an error.
 
 Additionally, if COPY-TO-ENVIRONMENT is non-nil, the alias should be
 copied (a.k.a. \"exported\") to the environment of created subprocesses."
+  :version "29.1"
   :type '(repeat (list string sexp
 		       (choice (const :tag "Copy to environment" t)
                                (const :tag "Use only in Eshell" nil))
@@ -234,6 +246,11 @@ eshell-var-initialize
   ;; changing a variable will affect all of Emacs.
   (unless eshell-modify-global-environment
     (setq-local process-environment (eshell-copy-environment)))
+  (setq-local eshell-subcommand-bindings
+              (append
+               '((process-environment (eshell-copy-environment))
+                 (eshell-variable-aliases-list eshell-variable-aliases-list))
+               eshell-subcommand-bindings))
 
   (setq-local eshell-special-chars-inside-quoting
        (append eshell-special-chars-inside-quoting '(?$)))
@@ -282,9 +299,9 @@ eshell-handle-local-variables
 	     (while (string-match setvar command)
 	       (nconc
 		l (list
-		   (list 'setenv (match-string 1 command)
-			 (match-string 2 command)
-			 (= (length (match-string 2 command)) 0))))
+                   (list 'eshell-set-variable
+                         (match-string 1 command)
+                         (match-string 2 command))))
 	       (setq command (eshell-stringify (car args))
 		     args (cdr args)))
 	     (cdr l))
@@ -328,12 +345,11 @@ eshell/define
 
 (defun eshell/export (&rest sets)
   "This alias allows the `export' command to act as bash users expect."
-  (while sets
-    (if (and (stringp (car sets))
-	     (string-match "^\\([^=]+\\)=\\(.*\\)" (car sets)))
-	(setenv (match-string 1 (car sets))
-		(match-string 2 (car sets))))
-    (setq sets (cdr sets))))
+  (dolist (set sets)
+    (when (and (stringp set)
+               (string-match "^\\([^=]+\\)=\\(.*\\)" set))
+      (eshell-set-variable (match-string 1 set)
+                           (match-string 2 set)))))
 
 (defun pcomplete/eshell-mode/export ()
   "Completion function for Eshell's `export'."
@@ -343,16 +359,28 @@ pcomplete/eshell-mode/export
 	    (eshell-envvar-names)))))
 
 (defun eshell/unset (&rest args)
-  "Unset an environment variable."
-  (while args
-    (if (stringp (car args))
-	(setenv (car args) nil t))
-    (setq args (cdr args))))
+  "Unset one or more variables.
+This is equivalent to calling `eshell/set' for all of ARGS with
+the values of nil for each."
+  (dolist (arg args)
+    (eshell-set-variable arg nil)))
 
 (defun pcomplete/eshell-mode/unset ()
   "Completion function for Eshell's `unset'."
   (while (pcomplete-here (eshell-envvar-names))))
 
+(defun eshell/set (&rest args)
+  "Allow command-ish use of `set'."
+  (let (last-value)
+    (while args
+      (setq last-value (eshell-set-variable (car args) (cadr args))
+            args (cddr args)))
+    last-value))
+
+(defun pcomplete/eshell-mode/set ()
+  "Completion function for Eshell's `set'."
+  (while (pcomplete-here (eshell-envvar-names))))
+
 (defun eshell/setq (&rest args)
   "Allow command-ish use of `setq'."
   (let (last-value)
@@ -566,18 +594,21 @@ eshell-get-variable
 If QUOTED is non-nil, this was invoked inside double-quotes."
   (if-let ((alias (assoc name eshell-variable-aliases-list)))
       (let ((target (nth 1 alias)))
+        (when (and (not (functionp target))
+                   (consp target))
+          (setq target (car target)))
         (cond
          ((functionp target)
           (if (nth 3 alias)
               (eshell-apply-indices (funcall target) indices quoted)
-            (condition-case nil
-	        (funcall target indices quoted)
-              (wrong-number-of-arguments
-               (display-warning
-                :warning (concat "Function for `eshell-variable-aliases-list' "
-                                 "entry should accept two arguments: INDICES "
-                                 "and QUOTED.'"))
-               (funcall target indices)))))
+            (let ((max-arity (cdr (func-arity target))))
+              (if (or (eq max-arity 'many) (>= max-arity 2))
+                  (funcall target indices quoted)
+                (display-warning
+                 :warning (concat "Function for `eshell-variable-aliases-list' "
+                                  "entry should accept two arguments: INDICES "
+                                  "and QUOTED.'"))
+                (funcall target indices)))))
          ((symbolp target)
           (eshell-apply-indices (symbol-value target) indices quoted))
          (t
@@ -594,6 +625,44 @@ eshell-get-variable
 	 (getenv name)))
      indices quoted)))
 
+(defun eshell-set-variable (name value)
+  "Set the variable named NAME to VALUE.
+NAME can be a string (in which case it refers to an environment
+variable or variable alias) or a symbol (in which case it refers
+to a Lisp variable)."
+  (if-let ((alias (assoc name eshell-variable-aliases-list)))
+      (let ((target (nth 1 alias)))
+        (cond
+         ((functionp target)
+          (setq target nil))
+         ((consp target)
+          (setq target (cdr target))))
+        (cond
+         ((functionp target)
+          (funcall target nil value))
+         ((null target)
+          (unless eshell-in-subcommand-p
+            (error "Variable `%s' is not settable" (eshell-stringify name)))
+          (push `(,name ,(lambda () value) t t)
+                eshell-variable-aliases-list)
+          value)
+         ;; Since getting a variable alias with a string target and
+         ;; `eshell-prefer-lisp-variables' non-nil gets the
+         ;; corresponding Lisp variable, make sure setting does the
+         ;; same.
+         ((and eshell-prefer-lisp-variables
+               (stringp target))
+          (eshell-set-variable (intern target) value))
+         (t
+          (eshell-set-variable target value))))
+    (cond
+     ((stringp name)
+      (setenv name value))
+     ((symbolp name)
+      (set name value))
+     (t
+      (error "Unknown variable `%s'" (eshell-stringify name))))))
+
 (defun eshell-apply-indices (value indices &optional quoted)
   "Apply to VALUE all of the given INDICES, returning the sub-result.
 The format of INDICES is:
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index ad695e45d7..a7ac52ed24 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -25,6 +25,7 @@
 
 (require 'ert)
 (require 'esh-mode)
+(require 'esh-var)
 (require 'eshell)
 
 (require 'eshell-tests-helpers
@@ -439,6 +440,150 @@ esh-var-test/quoted-interp-convert-cmd-split-indices
   (eshell-command-result-equal "echo \"${echo \\\"000 010 020\\\"}[0]\""
                                "000"))
 
+
+;; Variable-related commands
+
+(ert-deftest esh-var-test/set/env-var ()
+  "Test that `set' with a string variable name sets an environment variable."
+  (with-temp-eshell
+   (eshell-match-command-output "set VAR hello" "hello\n")
+   (should (equal (getenv "VAR") "hello")))
+  (should-not (equal (getenv "VAR") "hello")))
+
+(ert-deftest esh-var-test/set/symbol ()
+  "Test that `set' with a symbol variable name sets a Lisp variable."
+  (let (eshell-test-value)
+    (eshell-command-result-equal "set #'eshell-test-value hello"
+                                 "hello")
+    (should (equal eshell-test-value "hello"))))
+
+(ert-deftest esh-var-test/unset/env-var ()
+  "Test that `unset' with a string variable name unsets an env var."
+  (let ((process-environment (cons "VAR=value" process-environment)))
+    (with-temp-eshell
+     (eshell-match-command-output "unset VAR" "\\`\\'")
+     (should (equal (getenv "VAR") nil)))
+    (should (equal (getenv "VAR") "value"))))
+
+(ert-deftest esh-var-test/unset/symbol ()
+  "Test that `unset' with a symbol variable name unsets a Lisp variable."
+  (let ((eshell-test-value "value"))
+    (eshell-command-result-equal "unset #'eshell-test-value" nil)
+    (should (equal eshell-test-value nil))))
+
+(ert-deftest esh-var-test/setq ()
+  "Test that `setq' sets Lisp variables."
+  (let (eshell-test-value)
+    (eshell-command-result-equal "setq eshell-test-value hello"
+                                 "hello")
+    (should (equal eshell-test-value "hello"))))
+
+(ert-deftest esh-var-test/export ()
+  "Test that `export' sets environment variables."
+  (with-temp-eshell
+   (eshell-match-command-output "export VAR=hello" "\\`\\'")
+   (should (equal (getenv "VAR") "hello"))))
+
+(ert-deftest esh-var-test/local-variables ()
+  "Test that \"VAR=value command\" temporarily sets variables."
+  (with-temp-eshell
+   (push "VAR=value" process-environment)
+   (eshell-match-command-output "VAR=hello env" "VAR=hello\n")
+   (should (equal (getenv "VAR") "value"))))
+
+
+;; Variable aliases
+
+(ert-deftest esh-var-test/alias/function ()
+  "Test using a variable alias defined as a function."
+  (with-temp-eshell
+   (push `("ALIAS" ,(lambda () "value") nil t) eshell-variable-aliases-list)
+   (eshell-match-command-output "echo $ALIAS" "value\n")
+   (eshell-match-command-output "set ALIAS hello"
+                                "Variable `ALIAS' is not settable\n"
+                                nil t)))
+
+(ert-deftest esh-var-test/alias/function-pair ()
+  "Test using a variable alias defined as a pair of getter/setter functions."
+  (with-temp-eshell
+   (let ((eshell-test-value "value"))
+     (push `("ALIAS" (,(lambda () eshell-test-value)
+                      . (lambda (_ value)
+                          (setq eshell-test-value (upcase value))))
+             nil t)
+           eshell-variable-aliases-list)
+     (eshell-match-command-output "echo $ALIAS" "value\n")
+     (eshell-match-command-output "set ALIAS hello" "HELLO\n")
+     (should (equal eshell-test-value "HELLO")))))
+
+(ert-deftest esh-var-test/alias/string ()
+  "Test using a variable alias defined as a string.
+This should get/set the aliased environment variable."
+  (with-temp-eshell
+   (let ((eshell-test-value "lisp-value"))
+     (push "eshell-test-value=env-value" process-environment)
+     (push `("ALIAS" "eshell-test-value") eshell-variable-aliases-list)
+     (eshell-match-command-output "echo $ALIAS" "env-value\n")
+     (eshell-match-command-output "set ALIAS hello" "hello\n")
+     (should (equal (getenv "eshell-test-value") "hello"))
+     (should (equal eshell-test-value "lisp-value")))))
+
+(ert-deftest esh-var-test/alias/string/prefer-lisp ()
+  "Test using a variable alias defined as a string.
+This sets `eshell-prefer-lisp-variables' to t and should get/set
+the aliased Lisp variable."
+  (with-temp-eshell
+   (let ((eshell-test-value "lisp-value")
+         (eshell-prefer-lisp-variables t))
+     (push "eshell-test-value=env-value" process-environment)
+     (push `("ALIAS" "eshell-test-value") eshell-variable-aliases-list)
+     (eshell-match-command-output "echo $ALIAS" "lisp-value\n")
+     (eshell-match-command-output "set ALIAS hello" "hello\n")
+     (should (equal (car process-environment) "eshell-test-value=env-value"))
+     (should (equal eshell-test-value "hello")))))
+
+(ert-deftest esh-var-test/alias/symbol ()
+  "Test using a variable alias defined as a symbol.
+This should get/set the value bound to the symbol."
+  (with-temp-eshell
+   (let ((eshell-test-value "value"))
+     (push '("ALIAS" eshell-test-value) eshell-variable-aliases-list)
+     (eshell-match-command-output "echo $ALIAS" "value\n")
+     (eshell-match-command-output "set ALIAS hello" "hello\n")
+     (should (equal eshell-test-value "hello")))))
+
+(ert-deftest esh-var-test/alias/symbol-pair ()
+  "Test using a variable alias defined as a pair of symbols.
+This should get the value bound to the symbol, but fail to set
+it, since the setter is nil."
+  (with-temp-eshell
+   (let ((eshell-test-value "value"))
+     (push '("ALIAS" (eshell-test-value . nil)) eshell-variable-aliases-list)
+     (eshell-match-command-output "echo $ALIAS" "value\n")
+     (eshell-match-command-output "set ALIAS hello"
+                                "Variable `ALIAS' is not settable\n"
+                                nil t))))
+
+(ert-deftest esh-var-test/alias/export ()
+  "Test that `export' properly sets variable aliases."
+  (with-temp-eshell
+   (let ((eshell-test-value "value"))
+     (push `("ALIAS" (,(lambda () eshell-test-value)
+                      . (lambda (_ value) (setq eshell-test-value value)))
+             nil t)
+           eshell-variable-aliases-list)
+     (eshell-match-command-output "export ALIAS=hello" "\\`\\'")
+     (should (equal eshell-test-value "hello")))))
+
+(ert-deftest esh-var-test/alias/local-variables ()
+  "Test that \"VAR=value cmd\" temporarily sets read-only variable aliases."
+  (with-temp-eshell
+   (let ((eshell-test-value "value"))
+     (push `("ALIAS" ,(lambda () eshell-test-value) t t)
+           eshell-variable-aliases-list)
+     (eshell-match-command-output "ALIAS=hello env" "ALIAS=hello\n")
+     (should (equal eshell-test-value "value")))))
+
 
 ;; Built-in variables
 
-- 
2.25.1

From 717df5f25828c969c998de73de114c7adf546880 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Thu, 15 Sep 2022 12:24:37 -0700
Subject: [PATCH 6/7] Improve handling of $PATH in Eshell for remote
 directories

* lisp/eshell/esh-util.el (eshell-path-env, eshell-parse-colon-path):
Make obsolete.
(eshell-path-env-list): New variable.
(eshell-connection-default-profile): New connection-local profile.
(eshell-get-path): Reimplement using 'eshell-path-env-list'.
(eshell-set-path): New function.

* lisp/eshell/esh-var.el (eshell-variable-aliases-list): Add entry for
$PATH.
(eshell-var-initialize): Add 'eshell-path-env-list' to
'eshell-subcommand-bindings'.

* lisp/eshell/esh-ext.el (eshell-search-path): Use 'file-name-concat'
instead of 'concat'.
(eshell/addpath): Use 'eshell-get-path' and 'eshell-set-path'.

* lisp/net/tramp-integration.el: Only apply Eshell hooks when
'eshell-path-env-list' is unbound.

* test/lisp/eshell/esh-var-tests.el
(esh-var-test/path-var/local-directory)
(esh-var-test/path-var/remote-directory, esh-var-test/path-var/set)
(esh-var-test/path-var/set-locally)
(esh-var-test/path-var-preserve-across-hosts): New tests.

* test/lisp/eshell/esh-ext-tests.el: New file.

* test/lisp/eshell/eshell-tests-helpers.el
(with-temp-eshell): Set 'eshell-last-dir-ring-file-name' to nil.
(eshell-tests-remote-accessible-p, eshell-last-input)
(eshell-last-output): New functions.
(eshell-match-output, eshell-match-output--explainer): Use
'eshell-last-input' and 'eshell-last-output'.

* doc/misc/eshell.texi (Variables): Document $PATH.

* etc/NEWS: Announce this change (bug#57556).
---
 doc/misc/eshell.texi                     | 10 ++++
 etc/NEWS                                 |  5 ++
 lisp/eshell/esh-ext.el                   | 23 ++++---
 lisp/eshell/esh-util.el                  | 53 +++++++++++++++--
 lisp/eshell/esh-var.el                   | 12 +++-
 lisp/net/tramp-integration.el            | 21 +++----
 test/lisp/eshell/esh-ext-tests.el        | 76 ++++++++++++++++++++++++
 test/lisp/eshell/esh-var-tests.el        | 60 +++++++++++++++++++
 test/lisp/eshell/eshell-tests-helpers.el | 32 +++++++---
 9 files changed, 255 insertions(+), 37 deletions(-)
 create mode 100644 test/lisp/eshell/esh-ext-tests.el

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 2deb6bdc20..ad09e9120b 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -940,6 +940,16 @@ Variables
 directory ring via subscripting, e.g.@: @samp{$-[1]} refers to the
 working directory @emph{before} the previous one.
 
+@vindex $PATH
+@item $PATH
+This specifies the directories to search for executable programs.  Its
+value is a string, separated by @code{":"} for Unix and GNU systems,
+and @code{";"} for MS systems.  This variable is connection-aware, so
+whenever you change the current directory to a different host
+(@pxref{Remote Files, , , emacs, The GNU Emacs Manual}),
+the value will automatically update to reflect the search path on that
+host.
+
 @vindex $_
 @item $_
 This refers to the last argument of the last command.  With a
diff --git a/etc/NEWS b/etc/NEWS
index 72b2331b81..871060148d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -356,6 +356,11 @@ previous 'C-x ='.
 
 ** Eshell
 
+*** Eshell's PATH is now derived from 'exec-path'.
+For consistency with remote connections, Eshell now uses 'exec-path'
+to determine the execution path on the local system, instead of using
+the PATH environment variable directly.
+
 ---
 *** 'source' and '.' no longer accept the '--help' option.
 This is for compatibility with the shell versions of these commands,
diff --git a/lisp/eshell/esh-ext.el b/lisp/eshell/esh-ext.el
index 98902fc6f2..d513d750d9 100644
--- a/lisp/eshell/esh-ext.el
+++ b/lisp/eshell/esh-ext.el
@@ -77,7 +77,7 @@ eshell-search-path
     (let ((list (eshell-get-path))
 	  suffixes n1 n2 file)
       (while list
-	(setq n1 (concat (car list) name))
+	(setq n1 (file-name-concat (car list) name))
 	(setq suffixes eshell-binary-suffixes)
 	(while suffixes
 	  (setq n2 (concat n1 (car suffixes)))
@@ -239,17 +239,16 @@ eshell/addpath
      (?h "help" nil nil  "display this usage message")
      :usage "[-b] PATH
 Adds the given PATH to $PATH.")
-   (if args
-       (progn
-	 (setq eshell-path-env (getenv "PATH")
-	       args (mapconcat #'identity args path-separator)
-	       eshell-path-env
-	       (if prepend
-		   (concat args path-separator eshell-path-env)
-		 (concat eshell-path-env path-separator args)))
-	 (setenv "PATH" eshell-path-env))
-     (dolist (dir (parse-colon-path (getenv "PATH")))
-       (eshell-printn dir)))))
+   (let ((path (eshell-get-path t)))
+     (if args
+         (progn
+           (setq path (if prepend
+                          (append args path)
+                        (append path args)))
+           (eshell-set-path path)
+           (string-join path (path-separator)))
+       (dolist (dir path)
+         (eshell-printn dir))))))
 
 (put 'eshell/addpath 'eshell-no-numeric-conversions t)
 (put 'eshell/addpath 'eshell-filename-arguments t)
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 9258ca5e40..55983b1feb 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -249,17 +249,58 @@ eshell-path-env
 It might be different from \(getenv \"PATH\"), when
 `default-directory' points to a remote host.")
 
-(defun eshell-get-path ()
+(make-obsolete-variable 'eshell-path-env 'eshell-get-path "29.1")
+
+(defvar-local eshell-path-env-list nil)
+
+(connection-local-set-profile-variables
+ 'eshell-connection-default-profile
+ '((eshell-path-env-list . nil)))
+
+(connection-local-set-profiles
+ '(:application eshell)
+ 'eshell-connection-default-profile)
+
+(defun eshell-get-path (&optional local-part)
   "Return $PATH as a list.
-Add the current directory on MS-Windows."
-  (eshell-parse-colon-path
-   (if (eshell-under-windows-p)
-       (concat "." path-separator eshell-path-env)
-     eshell-path-env)))
+If LOCAL-PART is non-nil, only return the local part of the path.
+Otherwise, return the full, possibly-remote path.
+
+On MS-Windows, add the current directory as the first directory
+in the path."
+  (with-connection-local-application-variables 'eshell
+    (let ((remote (file-remote-p default-directory))
+          (path
+           (or eshell-path-env-list
+               ;; If not already cached, get the path from
+               ;; `exec-path', removing the last element, which is
+               ;; `exec-directory'.
+               (setq-connection-local eshell-path-env-list
+                                      (butlast (exec-path))))))
+      (when (and (eshell-under-windows-p)
+                 (not remote))
+        (push "." path))
+      (if (and remote (not local-part))
+          (mapcar (lambda (x) (file-name-concat remote x)) path)
+        path))))
+
+(defun eshell-set-path (path)
+  "Set the Eshell $PATH to PATH.
+PATH can be either a list of directories or a string of
+directories separated by `path-separator'."
+  (with-connection-local-application-variables 'eshell
+    (setq-connection-local
+     eshell-path-env-list
+     (if (listp path)
+	 path
+       ;; Don't use `parse-colon-path' here, since we don't want
+       ;; the additonal translations it does on each element.
+       (split-string path (path-separator))))))
 
 (defun eshell-parse-colon-path (path-env)
   "Split string with `parse-colon-path'.
 Prepend remote identification of `default-directory', if any."
+  (declare (obsolete nil "29.1"))
   (let ((remote (file-remote-p default-directory)))
     (if remote
 	(mapcar
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index caf143e1a1..57ea42f493 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -156,7 +156,14 @@ eshell-variable-aliases-list
     ("LINES" ,(lambda () (window-body-height nil 'remap)) t t)
     ("INSIDE_EMACS" eshell-inside-emacs t)
 
-    ;; for eshell-cmd.el
+    ;; for esh-ext.el
+    ("PATH" (,(lambda () (string-join (eshell-get-path t) (path-separator)))
+             . ,(lambda (_ value)
+                  (eshell-set-path value)
+                  value))
+     t t)
+
+    ;; for esh-cmd.el
     ("_" ,(lambda (indices quoted)
 	    (if (not indices)
 	        (car (last eshell-last-arguments))
@@ -249,7 +256,8 @@ eshell-var-initialize
   (setq-local eshell-subcommand-bindings
               (append
                '((process-environment (eshell-copy-environment))
-                 (eshell-variable-aliases-list eshell-variable-aliases-list))
+                 (eshell-variable-aliases-list eshell-variable-aliases-list)
+                 (eshell-path-env-list eshell-path-env-list))
                eshell-subcommand-bindings))
 
   (setq-local eshell-special-chars-inside-quoting
diff --git a/lisp/net/tramp-integration.el b/lisp/net/tramp-integration.el
index 35c0636b1c..4be019edd9 100644
--- a/lisp/net/tramp-integration.el
+++ b/lisp/net/tramp-integration.el
@@ -136,16 +136,17 @@ tramp-eshell-directory-change
           (getenv "PATH"))))
 
 (with-eval-after-load 'esh-util
-  (add-hook 'eshell-mode-hook
-	    #'tramp-eshell-directory-change)
-  (add-hook 'eshell-directory-change-hook
-	    #'tramp-eshell-directory-change)
-  (add-hook 'tramp-integration-unload-hook
-	    (lambda ()
-	      (remove-hook 'eshell-mode-hook
-			   #'tramp-eshell-directory-change)
-	      (remove-hook 'eshell-directory-change-hook
-			   #'tramp-eshell-directory-change))))
+  (unless (boundp 'eshell-path-env-list)
+    (add-hook 'eshell-mode-hook
+	      #'tramp-eshell-directory-change)
+    (add-hook 'eshell-directory-change-hook
+	      #'tramp-eshell-directory-change)
+    (add-hook 'tramp-integration-unload-hook
+	      (lambda ()
+	        (remove-hook 'eshell-mode-hook
+			     #'tramp-eshell-directory-change)
+	        (remove-hook 'eshell-directory-change-hook
+			     #'tramp-eshell-directory-change)))))
 
 ;;; Integration of recentf.el:
 
diff --git a/test/lisp/eshell/esh-ext-tests.el b/test/lisp/eshell/esh-ext-tests.el
new file mode 100644
index 0000000000..54191e9409
--- /dev/null
+++ b/test/lisp/eshell/esh-ext-tests.el
@@ -0,0 +1,76 @@
+;;; esh-ext-tests.el --- esh-ext test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for Eshell's external command handling.
+
+;;; Code:
+
+(require 'ert)
+(require 'esh-mode)
+(require 'esh-ext)
+(require 'eshell)
+
+(require 'eshell-tests-helpers
+         (expand-file-name "eshell-tests-helpers"
+                           (file-name-directory (or load-file-name
+                                                    default-directory))))
+
+;;; Tests:
+
+(ert-deftest esh-ext-test/addpath/end ()
+  "Test that \"addpath\" adds paths to the end of $PATH."
+  (with-temp-eshell
+   (let ((eshell-path-env-list '("/some/path" "/other/path"))
+         (expected-path (string-join '("/some/path" "/other/path" "/new/path"
+                                       "/new/path2")
+                                     (path-separator))))
+     (eshell-match-command-output "addpath /new/path /new/path2"
+                                  (concat expected-path "\n"))
+     (eshell-match-command-output "echo $PATH"
+                                  (concat expected-path "\n")))))
+
+(ert-deftest esh-ext-test/addpath/begin ()
+  "Test that \"addpath -b\" adds paths to the beginning of $PATH."
+  (with-temp-eshell
+   (let ((eshell-path-env-list '("/some/path" "/other/path"))
+         (expected-path (string-join '("/new/path" "/new/path2" "/some/path"
+                                       "/other/path")
+                                     (path-separator))))
+     (eshell-match-command-output "addpath -b /new/path /new/path2"
+                                  (concat expected-path "\n"))
+     (eshell-match-command-output "echo $PATH"
+                                  (concat expected-path "\n")))))
+
+(ert-deftest esh-ext-test/addpath/set-locally ()
+  "Test adding to the path temporarily in a subcommand."
+  (let* ((eshell-path-env-list '("/some/path" "/other/path"))
+         (original-path (string-join eshell-path-env-list (path-separator)))
+         (local-path (string-join (append eshell-path-env-list '("/new/path"))
+                                  (path-separator))))
+    (with-temp-eshell
+     (eshell-match-command-output
+      "{ addpath /new/path; env }"
+      (format "PATH=%s\n" (regexp-quote local-path)))
+     ;; After the last command, the previous $PATH value should be restored.
+     (eshell-match-command-output "echo $PATH"
+                                  (concat original-path "\n")))))
+
+;; esh-ext-tests.el ends here
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index a7ac52ed24..31b01c5605 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -23,6 +23,7 @@
 
 ;;; Code:
 
+(require 'tramp)
 (require 'ert)
 (require 'esh-mode)
 (require 'esh-var)
@@ -610,6 +611,65 @@ esh-var-test/inside-emacs-var-split-indices
    (eshell-match-command-output "echo $INSIDE_EMACS[, 1]"
                                 "eshell")))
 
+(ert-deftest esh-var-test/path-var/local-directory ()
+  "Test using $PATH in a local directory."
+  (let ((expected-path (string-join (eshell-get-path t) (path-separator))))
+    (with-temp-eshell
+     (eshell-match-command-output "echo $PATH" (regexp-quote expected-path)))))
+
+(ert-deftest esh-var-test/path-var/remote-directory ()
+  "Test using $PATH in a remote directory."
+  (skip-unless (eshell-tests-remote-accessible-p))
+  (let* ((default-directory ert-remote-temporary-file-directory)
+         (expected-path (string-join (eshell-get-path t) (path-separator))))
+    (with-temp-eshell
+     (eshell-match-command-output "echo $PATH" (regexp-quote expected-path)))))
+
+(ert-deftest esh-var-test/path-var/set ()
+  "Test setting $PATH."
+  (let* ((path-to-set-list '("/some/path" "/other/path"))
+         (path-to-set (string-join path-to-set-list (path-separator))))
+    (with-temp-eshell
+     (eshell-match-command-output (concat "set PATH " path-to-set)
+                                  (concat path-to-set "\n"))
+     (eshell-match-command-output "echo $PATH" (concat path-to-set "\n"))
+     (should (equal (eshell-get-path) path-to-set-list)))))
+
+(ert-deftest esh-var-test/path-var/set-locally ()
+  "Test setting $PATH temporarily for a single command."
+  (let* ((path-to-set-list '("/some/path" "/other/path"))
+         (path-to-set (string-join path-to-set-list (path-separator))))
+    (with-temp-eshell
+     (eshell-match-command-output (concat "set PATH " path-to-set)
+                                  (concat path-to-set "\n"))
+     (eshell-match-command-output "PATH=/local/path env"
+                                  "PATH=/local/path\n")
+     ;; After the last command, the previous $PATH value should be restored.
+     (eshell-match-command-output "echo $PATH" (concat path-to-set "\n"))
+     (should (equal (eshell-get-path) path-to-set-list)))))
+
+(ert-deftest esh-var-test/path-var/preserve-across-hosts ()
+  "Test that $PATH can be set independently on multiple hosts."
+  (let ((local-directory default-directory)
+        local-path remote-path)
+    (with-temp-eshell
+     ;; Set the $PATH on localhost.
+     (eshell-insert-command "set PATH /local/path")
+     (setq local-path (eshell-last-output))
+     ;; `cd' to a remote host and set the $PATH there too.
+     (eshell-insert-command
+      (format "cd %s" ert-remote-temporary-file-directory))
+     (eshell-insert-command "set PATH /remote/path")
+     (setq remote-path (eshell-last-output))
+     ;; Return to localhost and check that $PATH is the value we set
+     ;; originally.
+     (eshell-insert-command (format "cd %s" local-directory))
+     (eshell-match-command-output "echo $PATH" (regexp-quote local-path))
+     ;; ... and do the same for the remote host.
+     (eshell-insert-command
+      (format "cd %s" ert-remote-temporary-file-directory))
+     (eshell-match-command-output "echo $PATH" (regexp-quote remote-path)))))
+
 (ert-deftest esh-var-test/last-status-var-lisp-command ()
   "Test using the \"last exit status\" ($?) variable with a Lisp command"
   (with-temp-eshell
diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
index e713e162ad..1d9674070c 100644
--- a/test/lisp/eshell/eshell-tests-helpers.el
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -31,11 +31,22 @@
 (require 'eshell)
 
 (defvar eshell-history-file-name nil)
+(defvar eshell-last-dir-ring-file-name nil)
 
 (defvar eshell-test--max-subprocess-time 5
   "The maximum amount of time to wait for a subprocess to finish, in seconds.
 See `eshell-wait-for-subprocess'.")
 
+(defun eshell-tests-remote-accessible-p ()
+  "Return if a test involving remote files can proceed.
+If using this function, be sure to load `tramp' near the
+beginning of the test file."
+  (ignore-errors
+    (and
+     (file-remote-p ert-remote-temporary-file-directory)
+     (file-directory-p ert-remote-temporary-file-directory)
+     (file-writable-p ert-remote-temporary-file-directory))))
+
 (defmacro with-temp-eshell (&rest body)
   "Evaluate BODY in a temporary Eshell buffer."
   `(save-current-buffer
@@ -44,6 +55,7 @@ with-temp-eshell
               ;; back on $HISTFILE.
               (process-environment (cons "HISTFILE" process-environment))
               (eshell-history-file-name nil)
+              (eshell-last-dir-ring-file-name nil)
               (eshell-buffer (eshell t)))
          (unwind-protect
              (with-current-buffer eshell-buffer
@@ -83,19 +95,25 @@ eshell-insert-command
   (insert-and-inherit command)
   (funcall (or func 'eshell-send-input)))
 
+(defun eshell-last-input ()
+  "Return the input of the last Eshell command."
+  (buffer-substring-no-properties
+   eshell-last-input-start eshell-last-input-end))
+
+(defun eshell-last-output ()
+  "Return the output of the last Eshell command."
+  (buffer-substring-no-properties
+   (eshell-beginning-of-output) (eshell-end-of-output)))
+
 (defun eshell-match-output (regexp)
   "Test whether the output of the last command matches REGEXP."
-  (string-match-p
-    regexp (buffer-substring-no-properties
-            (eshell-beginning-of-output) (eshell-end-of-output))))
+  (string-match-p regexp (eshell-last-output)))
 
 (defun eshell-match-output--explainer (regexp)
   "Explain the result of `eshell-match-output'."
   `(mismatched-output
-    (command ,(buffer-substring-no-properties
-               eshell-last-input-start eshell-last-input-end))
-    (output ,(buffer-substring-no-properties
-              (eshell-beginning-of-output) (eshell-end-of-output)))
+    (command ,(eshell-last-input))
+    (output ,(eshell-last-output))
     (regexp ,regexp)))
 
 (put 'eshell-match-output 'ert-explainer #'eshell-match-output--explainer)
-- 
2.25.1