From 88d3713beb8310eb1ab45dde8aa767f14489affe Mon Sep 17 00:00:00 2001 From: Michael Albinus Date: Thu, 20 Dec 2018 11:07:15 +0100 Subject: [PATCH] Fix Bug#33781 * lisp/net/tramp-sh.el (tramp-set-remote-path): Use a temporary file for setting $PATH, if it exceeds PATH_MAX on the remote system. (tramp-send-command-and-read): Ignore errors if NOERROR. (Bug#33781) * test/lisp/net/tramp-tests.el (tramp-test34-remote-path): New test. --- lisp/net/tramp-sh.el | 25 +++++++------- test/lisp/net/tramp-tests.el | 66 ++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 13 deletions(-) diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el index 14ae2cb51b4..2959422a5da 100644 --- a/lisp/net/tramp-sh.el +++ b/lisp/net/tramp-sh.el @@ -3885,22 +3885,21 @@ This function expects to be in the right *tramp* buffer." I.e., for each directory in `tramp-remote-path', it is tested whether it exists and if so, it is added to the environment variable PATH." - (let ((path (mapconcat 'identity (tramp-get-remote-path vec) ":")) + (let ((command + (format "PATH=%s; export PATH" + (mapconcat 'identity (tramp-get-remote-path vec) ":"))) (path-max (with-tramp-connection-property vec "path-max" (tramp-send-command-and-read vec "getconf PATH_MAX /"))) - index) + tmpfile) (tramp-message vec 5 "Setting $PATH environment variable") - (unless (< (length path) path-max) - (setq index path-max) - (while (not (string-equal (substring path (1- index) index) ":")) - (setq index (1- index))) - ;; FIXME: Is this sufficient? Or shall we raise an error? - (tramp-message - vec 2 "$PATH environment variable is too long. Ignoring \"%s\"" - (substring path index)) - (setq path (substring path 0 (1- index)))) - (tramp-send-command vec (format "PATH=%s; export PATH" path)))) + (if (< (length command) path-max) + (tramp-send-command vec command) + ;; Use a temporary file. + (setq tmpfile (tramp-make-tramp-temp-file vec)) + (write-region command nil tmpfile) + (tramp-send-command vec (format ". %s" tmpfile)) + (delete-file tmpfile)))) ;; ------------------------------------------------------------ ;; -- Communication with external shell -- @@ -5066,7 +5065,7 @@ If MARKER is a regexp, read the output after that string. In case there is no valid Lisp expression and NOERROR is nil, it raises an error." (when (if noerror - (tramp-send-command-and-check vec command) + (ignore-errors (tramp-send-command-and-check vec command)) (tramp-barf-unless-okay vec command "`%s' returns with error" command)) (with-current-buffer (tramp-get-connection-buffer vec) diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el index f3ad8edf839..a485b8d8a70 100644 --- a/test/lisp/net/tramp-tests.el +++ b/test/lisp/net/tramp-tests.el @@ -4187,6 +4187,72 @@ This tests also `make-symbolic-link', `file-truename' and `add-name-to-file'." ;; Cleanup. (ignore-errors (delete-file tmp-name))))) +;; This test is inspired by Bug#33781. +;; `exec-path' was introduced in Emacs 27.1. `executable-find' has +;; changed the number of parameters, so we use `apply' for older +;; Emacsen. +(ert-deftest tramp-test34-remote-path () + "Check loooong `tramp-remote-path'." + (skip-unless (tramp--test-enabled)) + (skip-unless (tramp--test-sh-p)) + ;; Since Emacs 27.1. + (skip-unless (fboundp 'exec-path)) + + (let* ((tmp-name (tramp--test-make-temp-name)) + (default-directory tramp-test-temporary-file-directory) + (orig-exec-path (exec-path)) + (tramp-remote-path tramp-remote-path) + (orig-tramp-remote-path tramp-remote-path)) + (unwind-protect + (progn + ;; Non existing directories are removed. + (setq tramp-remote-path + (cons (file-remote-p tmp-name 'localname) tramp-remote-path)) + (tramp-cleanup-connection + (tramp-dissect-file-name tramp-test-temporary-file-directory) + 'keep-debug 'keep-password) + (should (equal (with-no-warnings (exec-path)) orig-exec-path)) + (setq tramp-remote-path orig-tramp-remote-path) + + ;; Double entries are removed. + (setq tramp-remote-path (append '("/" "/") tramp-remote-path)) + (tramp-cleanup-connection + (tramp-dissect-file-name tramp-test-temporary-file-directory) + 'keep-debug 'keep-password) + (should + (equal (with-no-warnings (exec-path)) (cons "/" orig-exec-path))) + (setq tramp-remote-path orig-tramp-remote-path) + + ;; We make a super long `tramp-remote-path'. + (make-directory tmp-name) + (should (file-directory-p tmp-name)) + (while (< (length (mapconcat 'identity orig-exec-path ":")) 5000) + (let ((dir (make-temp-file (file-name-as-directory tmp-name) 'dir))) + (should (file-directory-p dir)) + (setq tramp-remote-path + (cons (file-remote-p dir 'localname) tramp-remote-path) + orig-exec-path + (cons (file-remote-p dir 'localname) orig-exec-path)))) + (tramp-cleanup-connection + (tramp-dissect-file-name tramp-test-temporary-file-directory) + 'keep-debug 'keep-password) + (should (equal (with-no-warnings (exec-path)) orig-exec-path)) + (should + (string-equal + ;; Ignore trailing newline. + (substring (shell-command-to-string "echo $PATH") nil -1) + ;; The last element of `exec-path' is `exec-directory'. + (mapconcat 'identity (butlast orig-exec-path) ":"))) + ;; The shell "sh" shall always exist. + (should (apply 'executable-find '("sh" remote)))) + + ;; Cleanup. + (tramp-cleanup-connection + (tramp-dissect-file-name tramp-test-temporary-file-directory) + 'keep-debug 'keep-password) + (setq tramp-remote-path orig-tramp-remote-path) + (ignore-errors (delete-directory tmp-name 'recursive))))) + (ert-deftest tramp-test35-vc-registered () "Check `vc-registered'." :tags '(:expensive-test) -- 2.39.5