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

[Jim Porter <jporterbugs@gmail.com>]

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

On 10/15/2022 3:38 AM, Michael Albinus wrote:
> Use what is offered by texinfo. Say for example
> 
> --8<---------------cut here---------------start------------->8---
> the @env{PATH} environment variable
> --8<---------------cut here---------------end--------------->8---

Ah ha, got it.

> And in sample code, do not apply further formatting, but say it as you
> mean it
> 
> --8<---------------cut here---------------start------------->8---
> @samp{setq name value}
> @samp{export NAME=value}
> --8<---------------cut here---------------end--------------->8---

The Texinfo manual says I should wrap metasyntactic variables though[1]:

     For example,

         To delete file @var{filename},
         type @samp{rm @var{filename}}.

     produces

         To delete file filename, type ‘rm filename’.

     (Note that @var may appear inside @code, @samp, @file, etc.)


> Otherwise, it LGTM. Somewhere there is only one space after a dot
> (should be two spaces), but I didn't marked this during review, and now
> I'm too lazy to look for :-)

I think I found it. Fixed.

> I'd say just push it to the repo, and if there's something left to do we
> can stiil do it.

There's one significant problem I noticed with patch 0002 that we should 
probably fix first: 'setq-connection-local' clears all connection-local 
variables for the profile that were set elsewhere. That is, this:

     (with-connection-local-variables
      (setq-connection-local foo "foo")
      (setq-connection-local bar "bar"))

only sets 'bar' on the connection-local profile; the second 
'setq-connection-local' clears 'foo'. Attached is a fix for this (I'll 
fold it into patch 0002 before merging). I'm not sure if the new 
'connection-local-update-profile-variables' I added is 100% perfect, but 
I think it should work for more real-world situations.

[1] 
https://www.gnu.org/software/texinfo/manual/texinfo/html_node/_0040var.html

[-- Attachment #2: connection-local-update-profile-variables.diff --]
[-- Type: text/plain, Size: 4548 bytes --]

diff --git a/lisp/files-x.el b/lisp/files-x.el
index 665ae2ffa8..0640413ddd 100644
--- a/lisp/files-x.el
+++ b/lisp/files-x.el
@@ -706,6 +706,23 @@ connection-local-set-profile-variables
   (customize-set-variable
    'connection-local-profile-alist connection-local-profile-alist))
 
+;;;###autoload
+(defun connection-local-update-profile-variables (profile variables)
+  "Update the variable settings for PROFILE in-place.
+VARIABLES is a list that declares connection-local variables for
+the connection profile.  An element in VARIABLES is an alist
+whose elements are of the form (VAR . VALUE).
+
+Unlike `connection-local-set-profile-variables' (which see), this
+function preserves the values of any existing variable
+definitions that aren't listed in VARIABLES."
+  (when-let ((existing-variables
+              (nreverse (alist-get profile connection-local-profile-alist))))
+    (dolist (var variables)
+      (setf (alist-get (car var) existing-variables) (cdr var)))
+    (setq variables (nreverse existing-variables)))
+  (connection-local-set-profile-variables profile variables))
+
 (defun hack-connection-local-variables (criteria)
   "Read connection-local variables according to CRITERIA.
 Store the connection-local variables in buffer local
@@ -833,7 +850,7 @@ setq-connection-local
     `(prog1
          ,(macroexp-progn (nreverse set-expr))
        (when connection-local-profile-name-for-setq
-         (connection-local-set-profile-variables
+         (connection-local-update-profile-variables
           connection-local-profile-name-for-setq
           (list ,@(nreverse profile-vars)))
          (connection-local-set-profiles
diff --git a/test/lisp/files-x-tests.el b/test/lisp/files-x-tests.el
index 9499c951c5..274c49bb80 100644
--- a/test/lisp/files-x-tests.el
+++ b/test/lisp/files-x-tests.el
@@ -37,7 +37,8 @@ files-x-test--variables3
 (defconst files-x-test--variables4
   '((remote-null-device . "null")))
 (defconst files-x-test--variables5
-  '((remote-lazy-var . nil)))
+  '((remote-lazy-var . nil)
+    (remote-null-device . "/dev/null")))
 (defvar remote-null-device)
 (defvar remote-lazy-var nil)
 (put 'remote-shell-file-name 'safe-local-variable #'identity)
@@ -95,6 +96,28 @@ files-x-test-connection-local-set-profile-variables
       (connection-local-get-profile-variables 'remote-nullfile)
       files-x-test--variables4))))
 
+(ert-deftest files-x-test-connection-local-update-profile-variables ()
+  "Test updating connection-local profile variables."
+
+  ;; Declare (PROFILE VARIABLES) objects.
+  (let (connection-local-profile-alist connection-local-criteria-alist)
+    (connection-local-set-profile-variables
+     'remote-bash (copy-alist files-x-test--variables1))
+    (should
+     (equal
+      (connection-local-get-profile-variables 'remote-bash)
+      files-x-test--variables1))
+
+    ;; Updating overwrites only the values specified in this call, but
+    ;; retains all the other values from previous calls.
+    (connection-local-update-profile-variables
+     'remote-bash files-x-test--variables2)
+    (should
+     (equal
+      (connection-local-get-profile-variables 'remote-bash)
+      (cons (car files-x-test--variables2)
+            (cdr files-x-test--variables1))))))
+
 (ert-deftest files-x-test-connection-local-set-profiles ()
   "Test setting connection-local profiles."
 
@@ -402,14 +425,21 @@ files-x-test-setq-connection-local
 
       ;; Set the remote value and make sure it retains the value we set.
       (should (equal (files-x-test--set-lazy-var "there") "there"))
-      (should (equal (files-x-test--get-lazy-var) "there")))
+      (should (equal (files-x-test--get-lazy-var) "there"))
+      (with-connection-local-application-variables
+          (cadr files-x-test--application)
+        (setq-connection-local remote-null-device "null")))
 
     ;; Make sure we get the local value we set above.
     (should (equal (files-x-test--get-lazy-var) "here"))
+    (should-not (boundp 'remote-null-device))
 
-  ;; Make sure we get the remote value we set above.
-  (let ((default-directory "/method:host:"))
-    (should (equal (files-x-test--get-lazy-var) "there")))))
+    ;; Make sure we get the remote values we set above.
+    (let ((default-directory "/method:host:"))
+      (should (equal (files-x-test--get-lazy-var) "there"))
+      (with-connection-local-application-variables
+          (cadr files-x-test--application)
+        (should (equal remote-null-device "null"))))))
 
 (provide 'files-x-tests)
 ;;; files-x-tests.el ends here