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: 1556 bytes --]

On 10/2/2022 1:48 AM, Michael Albinus wrote:
> Yes. with-connection-local-variables is designed to provide those
> variables inside the BODY only. And you have used nil as CRITERIA in
> connection-local-set-profiles, which means you get the same variables
> for every kind of default-directory, which means you don't use
> connection-local variables at all :-)
> 
> What you need is a permanent setting of variables. Something like
> 
> (connection-local-set-profiles
>   (connection-local-criteria-for-default-directory 'eshell)
>   'eshell-connection-local-profile)
> 
> (let ((enable-connection-local-variables t)
>        connection-local-variables-alist) ;; I'm not sure this is needed.
>    (hack-connection-local-variables-apply
>     (connection-local-criteria-for-default-directory 'eshell))
>    ;; The body.
>    ...)

Hmm, I've tried this a few different ways, and I haven't been able to 
get it to work the way it should. Maybe I'm just missing something?

Attached is a minimal test case I've extracted to show the issue I'm 
having. It seems the problem is that, while I can update the path in 
'eshell-set-path' with no problem, when I call 'eshell-get-path' again, 
'hack-connection-local-variables-apply' resets 'eshell-path-env-list' to 
nil, so the modified path is lost.

Do you have any ideas about what I'm doing wrong? Or maybe 
connection-local variables aren't supposed to be used this way. All the 
documentation I see on them involves setting variables to constant 
values, not updating them in-place over the life of a program.

[-- Attachment #2: connection-local.el --]
[-- Type: text/plain, Size: 2779 bytes --]

;;; -*- lexical-binding:t -*-

;; Run these tests with:
;;  emacs -Q --batch -l ~/etc/emacs/connection-local.el \
;;        --eval '(ert-run-tests-batch-and-exit t)'

(require 'tramp)
(require 'ert)
(require 'ert-x)

(defvar-local eshell-path-env-list nil)

(connection-local-set-profile-variables
 'eshell-connection-local-profile
 '((eshell-path-env-list . nil)))

(connection-local-set-profiles
 (connection-local-criteria-for-default-directory 'eshell)
 'eshell-connection-local-profile)

(defun eshell-get-path ()
  "Return $PATH as a list."
  (let ((enable-connection-local-variables t)
        connection-local-variables-alist) ;; I'm not sure this is needed.
    (hack-connection-local-variables-apply
     (connection-local-criteria-for-default-directory 'eshell))
    (or eshell-path-env-list
        ;; If not already cached, get the path from `exec-path',
        ;; removing the last element, which is `exec-directory'.
        (setq eshell-path-env-list (butlast (exec-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'."
  (let ((enable-connection-local-variables t)
        connection-local-variables-alist) ;; I'm not sure this is needed.
    (hack-connection-local-variables-apply
     (connection-local-criteria-for-default-directory 'eshell))
    (setq 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))))))

(ert-deftest esh-var-test/path-var/preserve-across-hosts ()
  "Test that $PATH can be set independently on multiple hosts."
  ;; Test the initial value of the local $PATH.
  (should (equal (eshell-get-path) (butlast (exec-path))))

  ;; Set the local $PATH and make sure it retains the value we set.
  (should (equal (eshell-set-path "/local/path") '("/local/path")))
  (should (equal (eshell-get-path) '("/local/path")))      ; FAIL

  (let ((default-directory ert-remote-temporary-file-directory))
    ;; Test the initial value of the remote $PATH.
    (should (equal (eshell-get-path) (butlast (exec-path))))

    ;; Set the remote $PATH and make sure it retains the value we set.
    (should (equal (eshell-set-path "/remote/path") '("/remote/path")))
    (should (equal (eshell-get-path) '("/remote/path"))))  ; FAIL

  ;; Make sure we get the local $PATH we set above.
  (should (equal (eshell-get-path) '("/local/path")))      ; FAIL

  ;; Make sure we get the remote $PATH we set above.
  (let ((default-directory ert-remote-temporary-file-directory))
    (should (equal (eshell-get-path) '("/remote/path"))))) ; FAIL