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

From: Jim Porter <[email-masked]>
Date: Sat, 1 Oct 2022 15:02:04 -0700

On 10/1/2022 1:25 PM, Michael Albinus wrote:
> Jim Porter <[email-masked]> writes:
> 
> I didn't install your patches, but I gave them a cursory review.

Thanks for taking a look.

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

Yeah, it took me quite a while to realize that Eshell's meaning of 
#'symbol is different from Elisp's. I recently added some documentation 
in the Eshell manual to (hopefully) clarify the potential confusion.

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

I'll take a look at doing that. As I understand it, connection-local 
variables are cleared if the associated connection gets cleaned up, 
right? I wonder if that would be the right thing to do. For example, if 
I cd into a remote host in Eshell, then update Eshell's $PATH for that 
host, then clean up the connection, should the $PATH be reset to the 
default for that host? I'm really not sure...

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

I wanted to say, "Instead of using the variable 'eshell-path-env', call 
the function 'eshell-get-path'." Maybe that's not the right way to 
indicate that though.

>> +(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?

Good point. I'd forgotten to update that when copying that bit out of 
'eshell-parse-colon-path'.