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

[Michael Albinus]

Jim Porter <jporterbugs@gmail.com> writes:

Hi Jim,

I didn't install your patches, but I gave them a cursory review.

> Patch #3: Allow setting variable aliases
> ----------------------------------------
>
> Since the plan is to make $PATH into a variable alias so that Eshell
> can do the right thing when changing directories to a different host,
> I wanted to be sure users can *set* variable aliases so that updating
> $PATH will be easy. This adds the ability to do that, along with a new
> "set" command in Eshell. That lets you set either environment
> variables or Lisp variables (note that "#'" is just Eshell's way of
> spelling "'", since a single-quote is used for literal strings in
> Eshell):
>
>   set ENV_VAR value
>   set #'lisp-var value

Well, in Elisp the #'symbol read syntax is used for function names, see
(info "(elisp) Special Read Syntax")

So it is surprising to see it used for variable names.

> Patch #4: Make $PATH a variable alias
> ----------------------------------------
>
> This stores the $PATH in an alist indexed by host, similar to
> 'grep-host-defaults-alist'. For consistency, it now derives its value
> from '(exec-path)' everywhere (formerly, it used '(getenv "PATH") for
> local hosts and '(exec-path)' for Tramp).

Again, no possibility to use connection-local variables? You use them
already by calling (path-separator) ...

Personally I believe 'grep-host-defaults-alist' shall also be changed to
a connection-local mechanism, but likely, this would break too much code
in the wild.

> -(defun eshell-get-path ()
> +(make-obsolete-variable 'eshell-path-env 'eshell-get-path "29.1")

I guess you mean 'eshell-host-path-env' as CURRENT-NAME.

> +(defun eshell-get-path (&optional local-part)
> +  (let* ((remote (file-remote-p default-directory))
> +         (path (cdr (eshell-get-path-assq remote t))))
> +    (when (and (eshell-under-windows-p)
> +               (not remote))
> +      (push "." path))
> +    (if (and remote (not local-part))
> +        (mapcar (lambda (x) (concat remote x)) path)

Why not file-name-concat?

Otherwise, I'd say let's install the patch, and see how it goes. There
isn't too much time left until the feature freeze in November.

Best regards, Michael.