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.

diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 14ae2cb51b4f6c680097e8ce2b34f1a727d89812..2959422a5da331e04b0bd184ce2b3f12c1908ef1 100644 (file)
--- 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 f3ad8edf8395cc47b04106d195503fdbd235795a..a485b8d8a707f8359ddf737e7b80c44bc8cb4c1d 100644 (file)
--- 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)