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

[Michael Albinus <michael.albinus@gmx.de>]

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

Jim Porter <jporterbugs@gmail.com> writes:

Hi Jim,

Thanks your the example file, it is a pleasure to work on something like this!

> 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.

Well, your example code has some problems:

- connection-local-criteria-for-default-directory returns nil for a
  local default directory. In order to handle this case, I have added
  eshell-connection-local-criteria-for-default-directory.

- You cannot use the same profile for different criteria, unless you
  intend the same settings. Therefore, I have added
  eshell-connection-local-profile.

- If you change settings in eshell-get-path or eshell-set-path, you must
  tell this to the connection-local variables machinery. I've extended
  both functions accordingly.

Now your ert test passes for me. Changed connection-local.el added.

Best regards, Michael.


[-- Attachment #2: connection-local.el --]
[-- Type: text/plain, Size: 3743 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)

(defsubst eshell-connection-local-criteria-for-default-directory ()
  (or (connection-local-criteria-for-default-directory 'eshell)
      '(:application eshell :machine local)))

(defsubst eshell-connection-local-profile ()
  (intern (concat "eshell-connection-local-profile-"
		  (or (file-remote-p default-directory) "local"))))

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

(connection-local-set-profiles
 '(:application 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
     (eshell-connection-local-criteria-for-default-directory))
    (prog1
	(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))))
      ;; Set connection-local-variable.
      (connection-local-set-profile-variables
       (eshell-connection-local-profile)
       `((eshell-path-env-list . ,eshell-path-env-list)))
      (connection-local-set-profiles
       (eshell-connection-local-criteria-for-default-directory)
       (eshell-connection-local-profile)))))

(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
     (eshell-connection-local-criteria-for-default-directory))
    (prog1
	(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))))
      ;; Set connection-local-variable.
      (connection-local-set-profile-variables
       (eshell-connection-local-profile)
       `((eshell-path-env-list . ,eshell-path-env-list)))
      (connection-local-set-profiles
       (eshell-connection-local-criteria-for-default-directory)
       (eshell-connection-local-profile)))))

(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