From dd19cc3aa16ccc441a8a2bfcdeb3005a6eef2543 Mon Sep 17 00:00:00 2001 From: Michael Albinus Date: Mon, 4 Nov 2019 17:34:31 +0100 Subject: [PATCH] Improve Tramp error handling * lisp/net/tramp.el (tramp-set-syntax): Add missing argument. (tramp-signal-hook-function): Make it more robust. (tramp-handle-directory-files): * lisp/net/tramp-adb.el (tramp-adb-handle-directory-files-and-attributes) (tramp-adb-handle-copy-file, tramp-adb-handle-rename-file): * lisp/net/tramp-gvfs.el (tramp-gvfs-do-copy-or-rename-file): * lisp/net/tramp-rclone.el (tramp-rclone-do-copy-or-rename-file) (tramp-rclone-handle-directory-files): * lisp/net/tramp-sh.el (tramp-sh-handle-directory-files-and-attributes) (tramp-sh-handle-copy-directory, tramp-do-copy-or-rename-file): * lisp/net/tramp-smb.el (tramp-smb-handle-copy-directory) (tramp-smb-handle-copy-file, tramp-smb-handle-directory-files) (tramp-smb-handle-rename-file): * lisp/net/tramp-sudoedit.el (tramp-sudoedit-do-copy-or-rename-file): Improve error handling. * test/lisp/net/tramp-tests.el (tramp-test11-copy-file) (tramp-test12-rename-file, tramp-test14-delete-directory) (tramp-test15-copy-directory, tramp-test16-directory-files) (tramp-test19-directory-files-and-attributes): Extend tests. --- lisp/net/tramp-adb.el | 12 ++++++++++ lisp/net/tramp-gvfs.el | 4 ++++ lisp/net/tramp-rclone.el | 8 +++++++ lisp/net/tramp-sh.el | 11 +++++++++ lisp/net/tramp-smb.el | 19 ++++++++++++++++ lisp/net/tramp-sudoedit.el | 4 ++++ lisp/net/tramp.el | 15 +++++++++--- test/lisp/net/tramp-tests.el | 44 ++++++++++++++++++++++++++---------- 8 files changed, 102 insertions(+), 15 deletions(-) diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el index e1706bebe6e..e3098190e2b 100644 --- a/lisp/net/tramp-adb.el +++ b/lisp/net/tramp-adb.el @@ -383,6 +383,10 @@ ARGUMENTS to pass to the OPERATION." (defun tramp-adb-handle-directory-files-and-attributes (directory &optional full match nosort id-format) "Like `directory-files-and-attributes' for Tramp files." + (unless (file-exists-p directory) + (tramp-error + (tramp-dissect-file-name directory) tramp-file-missing + "No such file or directory" directory)) (when (file-directory-p directory) (with-parsed-tramp-file-name (expand-file-name directory) nil (copy-tree @@ -706,6 +710,10 @@ PRESERVE-UID-GID and PRESERVE-EXTENDED-ATTRIBUTES are completely ignored." (let ((t1 (tramp-tramp-file-p filename)) (t2 (tramp-tramp-file-p newname))) (with-parsed-tramp-file-name (if t1 filename newname) nil + (unless (file-exists-p filename) + (tramp-error + v tramp-file-missing + "Copying file" "No such file or directory" filename)) (when (and (not ok-if-already-exists) (file-exists-p newname)) (tramp-error v 'file-already-exists newname)) (when (and (file-directory-p newname) @@ -784,6 +792,10 @@ PRESERVE-UID-GID and PRESERVE-EXTENDED-ATTRIBUTES are completely ignored." (let ((t1 (tramp-tramp-file-p filename)) (t2 (tramp-tramp-file-p newname))) (with-parsed-tramp-file-name (if t1 filename newname) nil + (unless (file-exists-p filename) + (tramp-error + v tramp-file-missing + "Renaming file" "No such file or directory" filename)) (when (and (not ok-if-already-exists) (file-exists-p newname)) (tramp-error v 'file-already-exists newname)) (when (and (file-directory-p newname) diff --git a/lisp/net/tramp-gvfs.el b/lisp/net/tramp-gvfs.el index c08c7194cc7..6f5cade4c63 100644 --- a/lisp/net/tramp-gvfs.el +++ b/lisp/net/tramp-gvfs.el @@ -765,6 +765,10 @@ file names." (msg-operation (if (eq op 'copy) "Copying" "Renaming"))) (with-parsed-tramp-file-name (if t1 filename newname) nil + (unless (file-exists-p filename) + (tramp-error + v tramp-file-missing + "%s file" msg-operation "No such file or directory" filename)) (when (and (not ok-if-already-exists) (file-exists-p newname)) (tramp-error v 'file-already-exists newname)) (when (and (file-directory-p newname) diff --git a/lisp/net/tramp-rclone.el b/lisp/net/tramp-rclone.el index 1f0c7eadbc5..2b3799ef008 100644 --- a/lisp/net/tramp-rclone.el +++ b/lisp/net/tramp-rclone.el @@ -213,6 +213,10 @@ file names." (msg-operation (if (eq op 'copy) "Copying" "Renaming"))) (with-parsed-tramp-file-name (if t1 filename newname) nil + (unless (file-exists-p filename) + (tramp-error + v tramp-file-missing + "%s file" msg-operation "No such file or directory" filename)) (when (and (not ok-if-already-exists) (file-exists-p newname)) (tramp-error v 'file-already-exists newname)) (when (and (file-directory-p newname) @@ -298,6 +302,10 @@ file names." (defun tramp-rclone-handle-directory-files (directory &optional full match nosort) "Like `directory-files' for Tramp files." + (unless (file-exists-p directory) + (tramp-error + (tramp-dissect-file-name directory) tramp-file-missing + "No such file or directory" directory)) (when (file-directory-p directory) (setq directory (file-name-as-directory (expand-file-name directory))) (with-parsed-tramp-file-name directory nil diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el index 3c80c583099..be531ed3192 100644 --- a/lisp/net/tramp-sh.el +++ b/lisp/net/tramp-sh.el @@ -1713,6 +1713,10 @@ of." (directory &optional full match nosort id-format) "Like `directory-files-and-attributes' for Tramp files." (unless id-format (setq id-format 'integer)) + (unless (file-exists-p directory) + (tramp-error + (tramp-dissect-file-name directory) tramp-file-missing + "No such file or directory" directory)) (when (file-directory-p directory) (setq directory (expand-file-name directory)) (let* ((temp @@ -1923,6 +1927,10 @@ tramp-sh-handle-file-name-all-completions: internal error accessing `%s': `%s'" (let ((t1 (tramp-tramp-file-p dirname)) (t2 (tramp-tramp-file-p newname))) (with-parsed-tramp-file-name (if t1 dirname newname) nil + (unless (file-exists-p dirname) + (tramp-error + v tramp-file-missing + "Copying directory" "No such file or directory" dirname)) (if (and (not copy-contents) (tramp-get-method-parameter v 'tramp-copy-recursive) ;; When DIRNAME and NEWNAME are remote, they must have @@ -2011,6 +2019,9 @@ file names." (apply #'file-extended-attributes (list filename))))) (with-parsed-tramp-file-name (if t1 filename newname) nil + (unless (file-exists-p filename) + (tramp-error + v tramp-file-missing "No such file or directory" filename)) (when (and (not ok-if-already-exists) (file-exists-p newname)) (tramp-error v 'file-already-exists newname)) (when (and (file-directory-p newname) diff --git a/lisp/net/tramp-smb.el b/lisp/net/tramp-smb.el index 5e52b26e7c6..f87d4becfe0 100644 --- a/lisp/net/tramp-smb.el +++ b/lisp/net/tramp-smb.el @@ -415,6 +415,10 @@ pass to the OPERATION." (with-parsed-tramp-file-name (if t1 dirname newname) nil (with-tramp-progress-reporter v 0 (format "Copying %s to %s" dirname newname) + (unless (file-exists-p dirname) + (tramp-error + v tramp-file-missing + "Copying directory" "No such file or directory" dirname)) (when (and (file-directory-p newname) (not (tramp-compat-directory-name-p newname))) (tramp-error v 'file-already-exists newname)) @@ -570,6 +574,13 @@ PRESERVE-UID-GID and PRESERVE-EXTENDED-ATTRIBUTES are completely ignored." (if (file-directory-p filename) (copy-directory filename newname keep-date 'parents 'copy-contents) + (unless (file-exists-p filename) + (tramp-error + (tramp-dissect-file-name + (if (tramp-tramp-file-p filename) filename newname)) + tramp-file-missing + "Copying file" "No such file or directory" filename)) + (let ((tmpfile (file-local-copy filename))) (if tmpfile ;; Remote filename. @@ -669,6 +680,10 @@ PRESERVE-UID-GID and PRESERVE-EXTENDED-ATTRIBUTES are completely ignored." (defun tramp-smb-handle-directory-files (directory &optional full match nosort) "Like `directory-files' for Tramp files." + (unless (file-exists-p directory) + (tramp-error + (tramp-dissect-file-name directory) tramp-file-missing + "No such file or directory" directory)) (let ((result (mapcar #'directory-file-name (file-name-all-completions "" directory)))) ;; Discriminate with regexp. @@ -1333,6 +1348,10 @@ component is used as the target of the symlink." (with-parsed-tramp-file-name (if (tramp-tramp-file-p filename) filename newname) nil + (unless (file-exists-p filename) + (tramp-error + v tramp-file-missing + "Renaming file" "No such file or directory" filename)) (when (and (not ok-if-already-exists) (file-exists-p newname)) (tramp-error v 'file-already-exists newname)) (when (and (file-directory-p newname) diff --git a/lisp/net/tramp-sudoedit.el b/lisp/net/tramp-sudoedit.el index bfc9b3bdc3a..2d9d7ff7892 100644 --- a/lisp/net/tramp-sudoedit.el +++ b/lisp/net/tramp-sudoedit.el @@ -241,6 +241,10 @@ absolute file names." (msg-operation (if (eq op 'copy) "Copying" "Renaming"))) (with-parsed-tramp-file-name (if t1 filename newname) nil + (unless (file-exists-p filename) + (tramp-error + v tramp-file-missing + "%s file" msg-operation "No such file or directory" filename)) (when (and (not ok-if-already-exists) (file-exists-p newname)) (tramp-error v 'file-already-exists newname)) (when (and (file-directory-p newname) diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el index 21b6f0070f7..88ff36d98ea 100644 --- a/lisp/net/tramp.el +++ b/lisp/net/tramp.el @@ -719,7 +719,7 @@ Used in user option `tramp-syntax'. There are further variables to be set, depending on VALUE." ;; Check allowed values. (unless (memq value (tramp-syntax-values)) - (tramp-user-error "Wrong `tramp-syntax' %s" value)) + (tramp-user-error nil "Wrong `tramp-syntax' %s" value)) ;; Cleanup existing buffers. (unless (eq (symbol-value symbol) value) (tramp-cleanup-all-buffers)) @@ -1889,8 +1889,13 @@ the resulting error message." ;; This function provides traces in case of errors not triggered by ;; Tramp functions. (defun tramp-signal-hook-function (error-symbol data) - "Funtion to be called via `signal-hook-function'." - (tramp-error (car tramp-current-connection) error-symbol "%s" data)) + "Function to be called via `signal-hook-function'." + ;; `custom-initialize-*' functions provoke `void-variable' errors. + ;; We don't want to see them in the backtrace. + (unless (eq error-symbol 'void-variable) + (tramp-error + (car tramp-current-connection) error-symbol + "%s" (mapconcat (lambda (x) (format "%s" x)) data " ")))) (defmacro with-parsed-tramp-file-name (filename var &rest body) "Parse a Tramp filename and make components available in the body. @@ -3025,6 +3030,10 @@ User is always nil." (defun tramp-handle-directory-files (directory &optional full match nosort) "Like `directory-files' for Tramp files." + (unless (file-exists-p directory) + (tramp-error + (tramp-dissect-file-name directory) tramp-file-missing + "No such file or directory" directory)) (when (file-directory-p directory) (setq directory (file-name-as-directory (expand-file-name directory))) (let ((temp (nreverse (file-name-all-completions "" directory))) diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el index baebae17e1f..ec9cda0bbdd 100644 --- a/test/lisp/net/tramp-tests.el +++ b/test/lisp/net/tramp-tests.el @@ -2370,6 +2370,9 @@ This checks also `file-name-as-directory', `file-name-directory', ;; Copy simple file. (unwind-protect (progn + (should-error + (copy-file source target) + :type tramp-file-missing) (write-region "foo" nil source) (should (file-exists-p source)) (copy-file source target) @@ -2482,6 +2485,9 @@ This checks also `file-name-as-directory', `file-name-directory', ;; Rename simple file. (unwind-protect (progn + (should-error + (rename-file source target) + :type tramp-file-missing) (write-region "foo" nil source) (should (file-exists-p source)) (rename-file source target) @@ -2605,20 +2611,25 @@ This tests also `file-directory-p' and `file-accessible-directory-p'." (skip-unless (tramp--test-enabled)) (dolist (quoted (if (tramp--test-expensive-test) '(nil t) '(nil))) - (let ((tmp-name (tramp--test-make-temp-name nil quoted))) + (let* ((tmp-name1 (tramp--test-make-temp-name nil quoted)) + (tmp-name2 (expand-file-name "foo" tmp-name1))) ;; Delete empty directory. - (make-directory tmp-name) - (should (file-directory-p tmp-name)) - (delete-directory tmp-name) - (should-not (file-directory-p tmp-name)) + (make-directory tmp-name1) + (should (file-directory-p tmp-name1)) + (delete-directory tmp-name1) + (should-not (file-directory-p tmp-name1)) ;; Delete non-empty directory. - (make-directory tmp-name) - (should (file-directory-p tmp-name)) - (write-region "foo" nil (expand-file-name "bla" tmp-name)) - (should (file-exists-p (expand-file-name "bla" tmp-name))) - (should-error (delete-directory tmp-name) :type 'file-error) - (delete-directory tmp-name 'recursive) - (should-not (file-directory-p tmp-name))))) + (make-directory tmp-name1) + (should (file-directory-p tmp-name1)) + (write-region "foo" nil (expand-file-name "bla" tmp-name1)) + (should (file-exists-p (expand-file-name "bla" tmp-name1))) + (make-directory tmp-name2) + (should (file-directory-p tmp-name2)) + (write-region "foo" nil (expand-file-name "bla" tmp-name2)) + (should (file-exists-p (expand-file-name "bla" tmp-name2))) + (should-error (delete-directory tmp-name1) :type 'file-error) + (delete-directory tmp-name1 'recursive) + (should-not (file-directory-p tmp-name1))))) (ert-deftest tramp-test15-copy-directory () "Check `copy-directory'." @@ -2636,6 +2647,9 @@ This tests also `file-directory-p' and `file-accessible-directory-p'." ;; Copy complete directory. (unwind-protect (progn + (should-error + (copy-directory tmp-name1 tmp-name2) + :type tramp-file-missing) ;; Copy empty directory. (make-directory tmp-name1) (write-region "foo" nil tmp-name4) @@ -2696,6 +2710,9 @@ This tests also `file-directory-p' and `file-accessible-directory-p'." (tmp-name3 (expand-file-name "foo" tmp-name1))) (unwind-protect (progn + (should-error + (directory-files tmp-name1) + :type tramp-file-missing) (make-directory tmp-name1) (write-region "foo" nil tmp-name2) (write-region "bla" nil tmp-name3) @@ -3174,6 +3191,9 @@ They might differ only in time attributes or directory size." attr) (unwind-protect (progn + (should-error + (directory-files-and-attributes tmp-name1) + :type tramp-file-missing) (make-directory tmp-name1) (should (file-directory-p tmp-name1)) (setq tramp--test-start-time -- 2.39.5