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

On 10/7/2022 11:28 AM, Michael Albinus wrote:
> Now your ert test passes for me. Changed connection-local.el added.

Thanks, this makes connection-local variables a lot clearer to me. Since 
this seems pretty tricky to get right for people who don't know much 
about connection-local variables, maybe it would make sense to add some 
helper functions for anyone who wants to do something similar?

I attached an updated version of my connection-local.el that tries to 
pull out the additions you made into some helpers. What do you think?

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

;;; Possible enhancements to connection-local variables:

(defvar connection-local-criteria nil
  "The current connection-local criteria, or nil.
This is set while executing the body of
`with-connection-local-application-variables'.")

(defun connection-local-profile-name ()
  "Get a connection-local profile name.

This allows `connection-local-setq' to use this profile name when
setting variables connection-locally. In theory, a user of these
functions could locally override this function if they wanted to
change the naming scheme."
  (when connection-local-criteria
    (intern (concat
             "auto-connection-local-profile/"
             (symbol-name (plist-get connection-local-criteria :application))
             "/" (or (file-remote-p default-directory) "local")))))

(defmacro with-connection-local-application-variables (application &rest body)
  "Apply connection-local variables for APPLICATION in `default-directory'.
Execute BODY, and unwind connection-local variables.

This is just `with-connection-local-variables', plus the ability
to set an application."
  (declare (debug t) (indent 1))
  `(with-connection-local-application-variables-1
    ,application (lambda () ,@body)))

(defun with-connection-local-application-variables-1 (application body-fun)
  "Apply connection-local variables for APPLICATION in `default-directory'.
Call BODY-FUN with no args, and then unwind connection-local variables."
  (if (file-remote-p default-directory)
      (let ((enable-connection-local-variables t)
            (connection-local-criteria
             (connection-local-criteria-for-default-directory application))
            (old-buffer-local-variables (buffer-local-variables))
	    connection-local-variables-alist)
	(hack-connection-local-variables-apply connection-local-criteria)
	(unwind-protect
            (funcall body-fun)
	  ;; Cleanup.
	  (dolist (variable connection-local-variables-alist)
	    (let ((elt (assq (car variable) old-buffer-local-variables)))
	      (if elt
		  (set (make-local-variable (car elt)) (cdr elt))
		(kill-local-variable (car variable)))))))
    ;; No connection-local variables to apply.
    (funcall body-fun)))

(defmacro connection-local-setq (&rest pairs)
  "Set variables in PAIRS connection-locally.
If there's no connection-local profile to use, just set the
variables as normal.

\(fn [VARIABLE VALUE]...)"
  (let ((set-expr nil)
        (profile-vars nil))
    (while pairs
      (unless (symbolp (car pairs))
        (error "Attempting to set a non-symbol: %s" (car pairs)))
      (push `(set ',(car pairs) ,(cadr pairs)) set-expr)
      (push `(cons ',(car pairs) ,(car pairs)) profile-vars)
      (setq pairs (cddr pairs)))
    `(prog1
         ,(macroexp-progn (nreverse set-expr))
       (when-let ((profile-name (connection-local-profile-name)))
         (connection-local-set-profile-variables
          profile-name
          (list ,@(nreverse profile-vars)))
         (connection-local-set-profiles
          connection-local-criteria
          profile-name)))))

;;; Eshell code:

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

;; 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."
  (with-connection-local-application-variables 'eshell
    (or eshell-path-env-list
        ;; If not already cached, get the path from `exec-path',
        ;; removing the last element, which is `exec-directory'.
        (connection-local-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'."
  (with-connection-local-application-variables 'eshell
    (connection-local-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))))))

;;; Eshell tests:

(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