From 5b915a21708332d1b25bd78d3742ae90d93473b2 Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Thu, 15 Sep 2022 12:24:37 -0700 Subject: [PATCH 6/7] Improve handling of $PATH in Eshell for remote directories * lisp/eshell/esh-util.el (eshell-path-env, eshell-parse-colon-path): Make obsolete. (eshell-path-env-list): New variable. (eshell-connection-default-profile): New connection-local profile. (eshell-get-path): Reimplement using 'eshell-path-env-list'. (eshell-set-path): New function. * lisp/eshell/esh-var.el (eshell-variable-aliases-list): Add entry for $PATH. (eshell-var-initialize): Add 'eshell-path-env-list' to 'eshell-subcommand-bindings'. * lisp/eshell/esh-ext.el (eshell-search-path): Use 'file-name-concat' instead of 'concat'. (eshell/addpath): Use 'eshell-get-path' and 'eshell-set-path'. * lisp/net/tramp-integration.el: Only apply Eshell hooks when 'eshell-path-env-list' is unbound. * test/lisp/eshell/esh-var-tests.el (esh-var-test/path-var/local-directory) (esh-var-test/path-var/remote-directory, esh-var-test/path-var/set) (esh-var-test/path-var/set-locally) (esh-var-test/path-var-preserve-across-hosts): New tests. * test/lisp/eshell/esh-ext-tests.el: New file. * test/lisp/eshell/eshell-tests-helpers.el (with-temp-eshell): Set 'eshell-last-dir-ring-file-name' to nil. (eshell-tests-remote-accessible-p, eshell-last-input) (eshell-last-output): New functions. (eshell-match-output, eshell-match-output--explainer): Use 'eshell-last-input' and 'eshell-last-output'. * doc/misc/eshell.texi (Variables): Document $PATH. * etc/NEWS: Announce this change (bug#57556). --- doc/misc/eshell.texi | 10 ++++ etc/NEWS | 5 ++ lisp/eshell/esh-ext.el | 23 ++++--- lisp/eshell/esh-util.el | 53 +++++++++++++++-- lisp/eshell/esh-var.el | 12 +++- lisp/net/tramp-integration.el | 21 +++---- test/lisp/eshell/esh-ext-tests.el | 76 ++++++++++++++++++++++++ test/lisp/eshell/esh-var-tests.el | 60 +++++++++++++++++++ test/lisp/eshell/eshell-tests-helpers.el | 32 +++++++--- 9 files changed, 255 insertions(+), 37 deletions(-) create mode 100644 test/lisp/eshell/esh-ext-tests.el diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi index 21c1671a21..d518eafd72 100644 --- a/doc/misc/eshell.texi +++ b/doc/misc/eshell.texi @@ -942,6 +942,16 @@ Variables directory ring via subscripting, e.g.@: @samp{$-[1]} refers to the working directory @emph{before} the previous one. +@vindex $PATH +@item $PATH +This specifies the directories to search for executable programs. Its +value is a string, separated by @code{":"} for Unix and GNU systems, +and @code{";"} for MS systems. This variable is connection-aware, so +whenever you change the current directory to a different host +(@pxref{Remote Files, , , emacs, The GNU Emacs Manual}), +the value will automatically update to reflect the search path on that +host. + @vindex $_ @item $_ This refers to the last argument of the last command. With a diff --git a/etc/NEWS b/etc/NEWS index 72b2331b81..871060148d 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -356,6 +356,11 @@ previous 'C-x ='. ** Eshell +*** Eshell's PATH is now derived from 'exec-path'. +For consistency with remote connections, Eshell now uses 'exec-path' +to determine the execution path on the local system, instead of using +the PATH environment variable directly. + --- *** 'source' and '.' no longer accept the '--help' option. This is for compatibility with the shell versions of these commands, diff --git a/lisp/eshell/esh-ext.el b/lisp/eshell/esh-ext.el index 98902fc6f2..d513d750d9 100644 --- a/lisp/eshell/esh-ext.el +++ b/lisp/eshell/esh-ext.el @@ -77,7 +77,7 @@ eshell-search-path (let ((list (eshell-get-path)) suffixes n1 n2 file) (while list - (setq n1 (concat (car list) name)) + (setq n1 (file-name-concat (car list) name)) (setq suffixes eshell-binary-suffixes) (while suffixes (setq n2 (concat n1 (car suffixes))) @@ -239,17 +239,16 @@ eshell/addpath (?h "help" nil nil "display this usage message") :usage "[-b] PATH Adds the given PATH to $PATH.") - (if args - (progn - (setq eshell-path-env (getenv "PATH") - args (mapconcat #'identity args path-separator) - eshell-path-env - (if prepend - (concat args path-separator eshell-path-env) - (concat eshell-path-env path-separator args))) - (setenv "PATH" eshell-path-env)) - (dolist (dir (parse-colon-path (getenv "PATH"))) - (eshell-printn dir))))) + (let ((path (eshell-get-path t))) + (if args + (progn + (setq path (if prepend + (append args path) + (append path args))) + (eshell-set-path path) + (string-join path (path-separator))) + (dolist (dir path) + (eshell-printn dir)))))) (put 'eshell/addpath 'eshell-no-numeric-conversions t) (put 'eshell/addpath 'eshell-filename-arguments t) diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el index 9258ca5e40..55983b1feb 100644 --- a/lisp/eshell/esh-util.el +++ b/lisp/eshell/esh-util.el @@ -249,17 +249,58 @@ eshell-path-env It might be different from \(getenv \"PATH\"), when `default-directory' points to a remote host.") -(defun eshell-get-path () +(make-obsolete-variable 'eshell-path-env 'eshell-get-path "29.1") + +(defvar-local eshell-path-env-list nil) + +(connection-local-set-profile-variables + 'eshell-connection-default-profile + '((eshell-path-env-list . nil))) + +(connection-local-set-profiles + '(:application eshell) + 'eshell-connection-default-profile) + +(defun eshell-get-path (&optional local-part) "Return $PATH as a list. -Add the current directory on MS-Windows." - (eshell-parse-colon-path - (if (eshell-under-windows-p) - (concat "." path-separator eshell-path-env) - eshell-path-env))) +If LOCAL-PART is non-nil, only return the local part of the path. +Otherwise, return the full, possibly-remote path. + +On MS-Windows, add the current directory as the first directory +in the path." + (with-connection-local-application-variables 'eshell + (let ((remote (file-remote-p default-directory)) + (path + (or eshell-path-env-list + ;; If not already cached, get the path from + ;; `exec-path', removing the last element, which is + ;; `exec-directory'. + (setq-connection-local eshell-path-env-list + (butlast (exec-path)))))) + (when (and (eshell-under-windows-p) + (not remote)) + (push "." path)) + (if (and remote (not local-part)) + (mapcar (lambda (x) (file-name-concat remote x)) path) + 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 + (setq-connection-local + 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)))))) (defun eshell-parse-colon-path (path-env) "Split string with `parse-colon-path'. Prepend remote identification of `default-directory', if any." + (declare (obsolete nil "29.1")) (let ((remote (file-remote-p default-directory))) (if remote (mapcar diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el index caf143e1a1..57ea42f493 100644 --- a/lisp/eshell/esh-var.el +++ b/lisp/eshell/esh-var.el @@ -156,7 +156,14 @@ eshell-variable-aliases-list ("LINES" ,(lambda () (window-body-height nil 'remap)) t t) ("INSIDE_EMACS" eshell-inside-emacs t) - ;; for eshell-cmd.el + ;; for esh-ext.el + ("PATH" (,(lambda () (string-join (eshell-get-path t) (path-separator))) + . ,(lambda (_ value) + (eshell-set-path value) + value)) + t t) + + ;; for esh-cmd.el ("_" ,(lambda (indices quoted) (if (not indices) (car (last eshell-last-arguments)) @@ -249,7 +256,8 @@ eshell-var-initialize (setq-local eshell-subcommand-bindings (append '((process-environment (eshell-copy-environment)) - (eshell-variable-aliases-list eshell-variable-aliases-list)) + (eshell-variable-aliases-list eshell-variable-aliases-list) + (eshell-path-env-list eshell-path-env-list)) eshell-subcommand-bindings)) (setq-local eshell-special-chars-inside-quoting diff --git a/lisp/net/tramp-integration.el b/lisp/net/tramp-integration.el index 35c0636b1c..4be019edd9 100644 --- a/lisp/net/tramp-integration.el +++ b/lisp/net/tramp-integration.el @@ -136,16 +136,17 @@ tramp-eshell-directory-change (getenv "PATH")))) (with-eval-after-load 'esh-util - (add-hook 'eshell-mode-hook - #'tramp-eshell-directory-change) - (add-hook 'eshell-directory-change-hook - #'tramp-eshell-directory-change) - (add-hook 'tramp-integration-unload-hook - (lambda () - (remove-hook 'eshell-mode-hook - #'tramp-eshell-directory-change) - (remove-hook 'eshell-directory-change-hook - #'tramp-eshell-directory-change)))) + (unless (boundp 'eshell-path-env-list) + (add-hook 'eshell-mode-hook + #'tramp-eshell-directory-change) + (add-hook 'eshell-directory-change-hook + #'tramp-eshell-directory-change) + (add-hook 'tramp-integration-unload-hook + (lambda () + (remove-hook 'eshell-mode-hook + #'tramp-eshell-directory-change) + (remove-hook 'eshell-directory-change-hook + #'tramp-eshell-directory-change))))) ;;; Integration of recentf.el: diff --git a/test/lisp/eshell/esh-ext-tests.el b/test/lisp/eshell/esh-ext-tests.el new file mode 100644 index 0000000000..54191e9409 --- /dev/null +++ b/test/lisp/eshell/esh-ext-tests.el @@ -0,0 +1,76 @@ +;;; esh-ext-tests.el --- esh-ext test suite -*- lexical-binding:t -*- + +;; Copyright (C) 2022 Free Software Foundation, Inc. + +;; This file is part of GNU Emacs. + +;; GNU Emacs is free software: you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; GNU Emacs is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with GNU Emacs. If not, see . + +;;; Commentary: + +;; Tests for Eshell's external command handling. + +;;; Code: + +(require 'ert) +(require 'esh-mode) +(require 'esh-ext) +(require 'eshell) + +(require 'eshell-tests-helpers + (expand-file-name "eshell-tests-helpers" + (file-name-directory (or load-file-name + default-directory)))) + +;;; Tests: + +(ert-deftest esh-ext-test/addpath/end () + "Test that \"addpath\" adds paths to the end of $PATH." + (with-temp-eshell + (let ((eshell-path-env-list '("/some/path" "/other/path")) + (expected-path (string-join '("/some/path" "/other/path" "/new/path" + "/new/path2") + (path-separator)))) + (eshell-match-command-output "addpath /new/path /new/path2" + (concat expected-path "\n")) + (eshell-match-command-output "echo $PATH" + (concat expected-path "\n"))))) + +(ert-deftest esh-ext-test/addpath/begin () + "Test that \"addpath -b\" adds paths to the beginning of $PATH." + (with-temp-eshell + (let ((eshell-path-env-list '("/some/path" "/other/path")) + (expected-path (string-join '("/new/path" "/new/path2" "/some/path" + "/other/path") + (path-separator)))) + (eshell-match-command-output "addpath -b /new/path /new/path2" + (concat expected-path "\n")) + (eshell-match-command-output "echo $PATH" + (concat expected-path "\n"))))) + +(ert-deftest esh-ext-test/addpath/set-locally () + "Test adding to the path temporarily in a subcommand." + (let* ((eshell-path-env-list '("/some/path" "/other/path")) + (original-path (string-join eshell-path-env-list (path-separator))) + (local-path (string-join (append eshell-path-env-list '("/new/path")) + (path-separator)))) + (with-temp-eshell + (eshell-match-command-output + "{ addpath /new/path; env }" + (format "PATH=%s\n" (regexp-quote local-path))) + ;; After the last command, the previous $PATH value should be restored. + (eshell-match-command-output "echo $PATH" + (concat original-path "\n"))))) + +;; esh-ext-tests.el ends here diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el index a7ac52ed24..31b01c5605 100644 --- a/test/lisp/eshell/esh-var-tests.el +++ b/test/lisp/eshell/esh-var-tests.el @@ -23,6 +23,7 @@ ;;; Code: +(require 'tramp) (require 'ert) (require 'esh-mode) (require 'esh-var) @@ -610,6 +611,65 @@ esh-var-test/inside-emacs-var-split-indices (eshell-match-command-output "echo $INSIDE_EMACS[, 1]" "eshell"))) +(ert-deftest esh-var-test/path-var/local-directory () + "Test using $PATH in a local directory." + (let ((expected-path (string-join (eshell-get-path t) (path-separator)))) + (with-temp-eshell + (eshell-match-command-output "echo $PATH" (regexp-quote expected-path))))) + +(ert-deftest esh-var-test/path-var/remote-directory () + "Test using $PATH in a remote directory." + (skip-unless (eshell-tests-remote-accessible-p)) + (let* ((default-directory ert-remote-temporary-file-directory) + (expected-path (string-join (eshell-get-path t) (path-separator)))) + (with-temp-eshell + (eshell-match-command-output "echo $PATH" (regexp-quote expected-path))))) + +(ert-deftest esh-var-test/path-var/set () + "Test setting $PATH." + (let* ((path-to-set-list '("/some/path" "/other/path")) + (path-to-set (string-join path-to-set-list (path-separator)))) + (with-temp-eshell + (eshell-match-command-output (concat "set PATH " path-to-set) + (concat path-to-set "\n")) + (eshell-match-command-output "echo $PATH" (concat path-to-set "\n")) + (should (equal (eshell-get-path) path-to-set-list))))) + +(ert-deftest esh-var-test/path-var/set-locally () + "Test setting $PATH temporarily for a single command." + (let* ((path-to-set-list '("/some/path" "/other/path")) + (path-to-set (string-join path-to-set-list (path-separator)))) + (with-temp-eshell + (eshell-match-command-output (concat "set PATH " path-to-set) + (concat path-to-set "\n")) + (eshell-match-command-output "PATH=/local/path env" + "PATH=/local/path\n") + ;; After the last command, the previous $PATH value should be restored. + (eshell-match-command-output "echo $PATH" (concat path-to-set "\n")) + (should (equal (eshell-get-path) path-to-set-list))))) + +(ert-deftest esh-var-test/path-var/preserve-across-hosts () + "Test that $PATH can be set independently on multiple hosts." + (let ((local-directory default-directory) + local-path remote-path) + (with-temp-eshell + ;; Set the $PATH on localhost. + (eshell-insert-command "set PATH /local/path") + (setq local-path (eshell-last-output)) + ;; `cd' to a remote host and set the $PATH there too. + (eshell-insert-command + (format "cd %s" ert-remote-temporary-file-directory)) + (eshell-insert-command "set PATH /remote/path") + (setq remote-path (eshell-last-output)) + ;; Return to localhost and check that $PATH is the value we set + ;; originally. + (eshell-insert-command (format "cd %s" local-directory)) + (eshell-match-command-output "echo $PATH" (regexp-quote local-path)) + ;; ... and do the same for the remote host. + (eshell-insert-command + (format "cd %s" ert-remote-temporary-file-directory)) + (eshell-match-command-output "echo $PATH" (regexp-quote remote-path))))) + (ert-deftest esh-var-test/last-status-var-lisp-command () "Test using the \"last exit status\" ($?) variable with a Lisp command" (with-temp-eshell diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el index e713e162ad..1d9674070c 100644 --- a/test/lisp/eshell/eshell-tests-helpers.el +++ b/test/lisp/eshell/eshell-tests-helpers.el @@ -31,11 +31,22 @@ (require 'eshell) (defvar eshell-history-file-name nil) +(defvar eshell-last-dir-ring-file-name nil) (defvar eshell-test--max-subprocess-time 5 "The maximum amount of time to wait for a subprocess to finish, in seconds. See `eshell-wait-for-subprocess'.") +(defun eshell-tests-remote-accessible-p () + "Return if a test involving remote files can proceed. +If using this function, be sure to load `tramp' near the +beginning of the test file." + (ignore-errors + (and + (file-remote-p ert-remote-temporary-file-directory) + (file-directory-p ert-remote-temporary-file-directory) + (file-writable-p ert-remote-temporary-file-directory)))) + (defmacro with-temp-eshell (&rest body) "Evaluate BODY in a temporary Eshell buffer." `(save-current-buffer @@ -44,6 +55,7 @@ with-temp-eshell ;; back on $HISTFILE. (process-environment (cons "HISTFILE" process-environment)) (eshell-history-file-name nil) + (eshell-last-dir-ring-file-name nil) (eshell-buffer (eshell t))) (unwind-protect (with-current-buffer eshell-buffer @@ -83,19 +95,25 @@ eshell-insert-command (insert-and-inherit command) (funcall (or func 'eshell-send-input))) +(defun eshell-last-input () + "Return the input of the last Eshell command." + (buffer-substring-no-properties + eshell-last-input-start eshell-last-input-end)) + +(defun eshell-last-output () + "Return the output of the last Eshell command." + (buffer-substring-no-properties + (eshell-beginning-of-output) (eshell-end-of-output))) + (defun eshell-match-output (regexp) "Test whether the output of the last command matches REGEXP." - (string-match-p - regexp (buffer-substring-no-properties - (eshell-beginning-of-output) (eshell-end-of-output)))) + (string-match-p regexp (eshell-last-output))) (defun eshell-match-output--explainer (regexp) "Explain the result of `eshell-match-output'." `(mismatched-output - (command ,(buffer-substring-no-properties - eshell-last-input-start eshell-last-input-end)) - (output ,(buffer-substring-no-properties - (eshell-beginning-of-output) (eshell-end-of-output))) + (command ,(eshell-last-input)) + (output ,(eshell-last-output)) (regexp ,regexp))) (put 'eshell-match-output 'ert-explainer #'eshell-match-output--explainer) -- 2.25.1