[Emacs-diffs] emacs-26 b500e06: Fix Bug#28896

[Michael Albinus]

branch: emacs-26
commit b500e06f4d05dc094b79f8f2c063e070a5a3306e
Author: Michael Albinus <address@hidden>
Commit: Michael Albinus <address@hidden>

    Fix Bug#28896
    
    * lisp/net/tramp-adb.el (tramp-adb-handle-rename-file):
    * lisp/net/tramp-gvfs.el (tramp-gvfs-do-copy-or-rename-file):
    * lisp/net/tramp-sh.el (tramp-do-copy-or-rename-file): Handle FILENAME
    being a directory.  (Bug#28896)
    
    * test/lisp/net/tramp-tests.el (tramp-test11-copy-file)
    (tramp-test12-rename-file): Test also FILENAME being a directory.
---
 lisp/net/tramp-adb.el        |  69 ++++----
 lisp/net/tramp-gvfs.el       |   7 +-
 lisp/net/tramp-sh.el         | 174 ++++++++++----------
 test/lisp/net/tramp-tests.el | 382 +++++++++++++++++++++----------------------
 4 files changed, 322 insertions(+), 310 deletions(-)

diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index e75efcf..bf21db2 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -802,38 +802,43 @@ PRESERVE-UID-GID and PRESERVE-EXTENDED-ATTRIBUTES are 
completely ignored."
   (setq filename (expand-file-name filename)
        newname (expand-file-name newname))
 
-  (let ((t1 (tramp-tramp-file-p filename))
-       (t2 (tramp-tramp-file-p newname)))
-    (with-parsed-tramp-file-name (if t1 filename newname) nil
-      (with-tramp-progress-reporter
-         v 0 (format "Renaming %s to %s" filename newname)
-
-       (if (and t1 t2
-                (tramp-equal-remote filename newname)
-                (not (file-directory-p filename)))
-           (let ((l1 (file-remote-p filename 'localname))
-                 (l2 (file-remote-p newname 'localname)))
-             (when (and (not ok-if-already-exists)
-                        (file-exists-p newname))
-               (tramp-error v 'file-already-exists newname))
-             ;; We must also flush the cache of the directory, because
-             ;; `file-attributes' reads the values from there.
-             (tramp-flush-file-property v (file-name-directory l1))
-             (tramp-flush-file-property v l1)
-             (tramp-flush-file-property v (file-name-directory l2))
-             (tramp-flush-file-property v l2)
-             ;; Short track.
-             (tramp-adb-barf-unless-okay
-              v (format
-                 "mv -f %s %s"
-                 (tramp-shell-quote-argument l1)
-                 (tramp-shell-quote-argument l2))
-              "Error renaming %s to %s" filename newname))
-
-         ;; Rename by copy.
-         (copy-file
-          filename newname ok-if-already-exists 'keep-time 'preserve-uid-gid)
-         (delete-file filename))))))
+  (if (file-directory-p filename)
+      (progn
+       (copy-directory filename newname t t)
+       (delete-directory filename 'recursive))
+
+    (let ((t1 (tramp-tramp-file-p filename))
+         (t2 (tramp-tramp-file-p newname)))
+      (with-parsed-tramp-file-name (if t1 filename newname) nil
+       (with-tramp-progress-reporter
+           v 0 (format "Renaming %s to %s" filename newname)
+
+         (if (and t1 t2
+                  (tramp-equal-remote filename newname)
+                  (not (file-directory-p filename)))
+             (let ((l1 (file-remote-p filename 'localname))
+                   (l2 (file-remote-p newname 'localname)))
+               (when (and (not ok-if-already-exists)
+                          (file-exists-p newname))
+                 (tramp-error v 'file-already-exists newname))
+               ;; We must also flush the cache of the directory, because
+               ;; `file-attributes' reads the values from there.
+               (tramp-flush-file-property v (file-name-directory l1))
+               (tramp-flush-file-property v l1)
+               (tramp-flush-file-property v (file-name-directory l2))
+               (tramp-flush-file-property v l2)
+               ;; Short track.
+               (tramp-adb-barf-unless-okay
+                v (format
+                   "mv -f %s %s"
+                   (tramp-shell-quote-argument l1)
+                   (tramp-shell-quote-argument l2))
+                "Error renaming %s to %s" filename newname))
+
+           ;; Rename by copy.
+           (copy-file
+            filename newname ok-if-already-exists 'keep-time 'preserve-uid-gid)
+           (delete-file filename)))))))
 
 (defun tramp-adb-handle-process-file
   (program &optional infile destination display &rest args)
diff --git a/lisp/net/tramp-gvfs.el b/lisp/net/tramp-gvfs.el
index 2b29a1b..51d24cb 100644
--- a/lisp/net/tramp-gvfs.el
+++ b/lisp/net/tramp-gvfs.el
@@ -675,6 +675,11 @@ file names."
   (unless (memq op '(copy rename))
     (error "Unknown operation `%s', must be `copy' or `rename'" op))
 
+  (if (file-directory-p filename)
+      (progn
+       (copy-directory filename newname keep-date t)
+       (when (eq op 'rename) (delete-directory filename 'recursive)))
+
     (let ((t1 (tramp-tramp-file-p filename))
          (t2 (tramp-tramp-file-p newname))
          (equal-remote (tramp-equal-remote filename newname))
@@ -738,7 +743,7 @@ file names."
          (when t2
            (with-parsed-tramp-file-name newname nil
              (tramp-flush-file-property v (file-name-directory localname))
-             (tramp-flush-file-property v localname)))))))
+             (tramp-flush-file-property v localname))))))))
 
 (defun tramp-gvfs-handle-copy-file
   (filename newname &optional ok-if-already-exists keep-date
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index bdb7a13..52a6b8f 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -2039,96 +2039,102 @@ of `copy' and `rename'.  FILENAME and NEWNAME must be 
absolute
 file names."
   (unless (memq op '(copy rename))
     (error "Unknown operation `%s', must be `copy' or `rename'" op))
-  (let ((t1 (tramp-tramp-file-p filename))
-       (t2 (tramp-tramp-file-p newname))
-       (length (tramp-compat-file-attribute-size
-                (file-attributes (file-truename filename))))
-       (attributes (and preserve-extended-attributes
-                        (apply 'file-extended-attributes (list filename)))))
 
-    (with-parsed-tramp-file-name (if t1 filename newname) nil
-      (when (and (not ok-if-already-exists) (file-exists-p newname))
-       (tramp-error v 'file-already-exists newname))
-
-      (with-tramp-progress-reporter
-         v 0 (format "%s %s to %s"
-                     (if (eq op 'copy) "Copying" "Renaming")
-                     filename newname)
+  (if (file-directory-p filename)
+      (progn
+       (copy-directory filename newname keep-date t)
+       (when (eq op 'rename) (delete-directory filename 'recursive)))
 
-       (cond
-        ;; Both are Tramp files.
-        ((and t1 t2)
-         (with-parsed-tramp-file-name filename v1
-           (with-parsed-tramp-file-name newname v2
-             (cond
-              ;; Shortcut: if method, host, user are the same for
-              ;; both files, we invoke `cp' or `mv' on the remote
-              ;; host directly.
-              ((tramp-equal-remote filename newname)
-               (tramp-do-copy-or-rename-file-directly
-                op filename newname
-                ok-if-already-exists keep-date preserve-uid-gid))
-
-              ;; Try out-of-band operation.
-              ((and
-                (tramp-method-out-of-band-p v1 length)
-                (tramp-method-out-of-band-p v2 length))
-               (tramp-do-copy-or-rename-file-out-of-band
-                op filename newname keep-date))
-
-              ;; No shortcut was possible.  So we copy the file
-              ;; first.  If the operation was `rename', we go back
-              ;; and delete the original file (if the copy was
-              ;; successful).  The approach is simple-minded: we
-              ;; create a new buffer, insert the contents of the
-              ;; source file into it, then write out the buffer to
-              ;; the target file.  The advantage is that it doesn't
-              ;; matter which file name handlers are used for the
-              ;; source and target file.
-              (t
-               (tramp-do-copy-or-rename-file-via-buffer
-                op filename newname keep-date))))))
-
-        ;; One file is a Tramp file, the other one is local.
-        ((or t1 t2)
-         (cond
-          ;; Fast track on local machine.
-          ((tramp-local-host-p v)
-           (tramp-do-copy-or-rename-file-directly
-            op filename newname
-            ok-if-already-exists keep-date preserve-uid-gid))
+    (let ((t1 (tramp-tramp-file-p filename))
+         (t2 (tramp-tramp-file-p newname))
+         (length (tramp-compat-file-attribute-size
+                  (file-attributes (file-truename filename))))
+         (attributes (and preserve-extended-attributes
+                          (apply 'file-extended-attributes (list filename)))))
 
-          ;; If the Tramp file has an out-of-band method, the
-          ;; corresponding copy-program can be invoked.
-          ((tramp-method-out-of-band-p v length)
-           (tramp-do-copy-or-rename-file-out-of-band
-            op filename newname keep-date))
+      (with-parsed-tramp-file-name (if t1 filename newname) nil
+       (when (and (not ok-if-already-exists) (file-exists-p newname))
+         (tramp-error v 'file-already-exists newname))
 
-          ;; Use the inline method via a Tramp buffer.
-          (t (tramp-do-copy-or-rename-file-via-buffer
-              op filename newname keep-date))))
+       (with-tramp-progress-reporter
+           v 0 (format "%s %s to %s"
+                       (if (eq op 'copy) "Copying" "Renaming")
+                       filename newname)
 
-        (t
-         ;; One of them must be a Tramp file.
-         (error "Tramp implementation says this cannot happen")))
+         (cond
+          ;; Both are Tramp files.
+          ((and t1 t2)
+           (with-parsed-tramp-file-name filename v1
+             (with-parsed-tramp-file-name newname v2
+               (cond
+                ;; Shortcut: if method, host, user are the same for
+                ;; both files, we invoke `cp' or `mv' on the remote
+                ;; host directly.
+                ((tramp-equal-remote filename newname)
+                 (tramp-do-copy-or-rename-file-directly
+                  op filename newname
+                  ok-if-already-exists keep-date preserve-uid-gid))
+
+                ;; Try out-of-band operation.
+                ((and
+                  (tramp-method-out-of-band-p v1 length)
+                  (tramp-method-out-of-band-p v2 length))
+                 (tramp-do-copy-or-rename-file-out-of-band
+                  op filename newname keep-date))
+
+                ;; No shortcut was possible.  So we copy the file
+                ;; first.  If the operation was `rename', we go back
+                ;; and delete the original file (if the copy was
+                ;; successful).  The approach is simple-minded: we
+                ;; create a new buffer, insert the contents of the
+                ;; source file into it, then write out the buffer to
+                ;; the target file.  The advantage is that it doesn't
+                ;; matter which file name handlers are used for the
+                ;; source and target file.
+                (t
+                 (tramp-do-copy-or-rename-file-via-buffer
+                  op filename newname keep-date))))))
+
+          ;; One file is a Tramp file, the other one is local.
+          ((or t1 t2)
+           (cond
+            ;; Fast track on local machine.
+            ((tramp-local-host-p v)
+             (tramp-do-copy-or-rename-file-directly
+              op filename newname
+              ok-if-already-exists keep-date preserve-uid-gid))
+
+            ;; If the Tramp file has an out-of-band method, the
+            ;; corresponding copy-program can be invoked.
+            ((tramp-method-out-of-band-p v length)
+             (tramp-do-copy-or-rename-file-out-of-band
+              op filename newname keep-date))
+
+            ;; Use the inline method via a Tramp buffer.
+            (t (tramp-do-copy-or-rename-file-via-buffer
+                op filename newname keep-date))))
 
-       ;; Handle `preserve-extended-attributes'.  We ignore possible
-       ;; errors, because ACL strings could be incompatible.
-       (when attributes
-         (ignore-errors
-           (apply 'set-file-extended-attributes (list newname attributes))))
-
-       ;; In case of `rename', we must flush the cache of the source file.
-       (when (and t1 (eq op 'rename))
-         (with-parsed-tramp-file-name filename v1
-           (tramp-flush-file-property v1 (file-name-directory v1-localname))
-           (tramp-flush-file-property v1 v1-localname)))
-
-       ;; When newname did exist, we have wrong cached values.
-       (when t2
-         (with-parsed-tramp-file-name newname v2
-           (tramp-flush-file-property v2 (file-name-directory v2-localname))
-           (tramp-flush-file-property v2 v2-localname)))))))
+          (t
+           ;; One of them must be a Tramp file.
+           (error "Tramp implementation says this cannot happen")))
+
+         ;; Handle `preserve-extended-attributes'.  We ignore possible
+         ;; errors, because ACL strings could be incompatible.
+         (when attributes
+           (ignore-errors
+             (apply 'set-file-extended-attributes (list newname attributes))))
+
+         ;; In case of `rename', we must flush the cache of the source file.
+         (when (and t1 (eq op 'rename))
+           (with-parsed-tramp-file-name filename v1
+             (tramp-flush-file-property v1 (file-name-directory v1-localname))
+             (tramp-flush-file-property v1 v1-localname)))
+
+         ;; When newname did exist, we have wrong cached values.
+         (when t2
+           (with-parsed-tramp-file-name newname v2
+             (tramp-flush-file-property v2 (file-name-directory v2-localname))
+             (tramp-flush-file-property v2 v2-localname))))))))
 
 (defun tramp-do-copy-or-rename-file-via-buffer (op filename newname keep-date)
   "Use an Emacs buffer to copy or rename a file.
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index c88abbf..e92d1b6 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -1883,96 +1883,98 @@ This checks also `file-name-as-directory', 
`file-name-directory',
   (let (quoted)
     (let ((tmp-name1 (tramp--test-make-temp-name nil quoted))
          (tmp-name2 (tramp--test-make-temp-name nil quoted))
-         (tmp-name3 (tramp--test-make-temp-name nil quoted))
-         (tmp-name4 (tramp--test-make-temp-name 'local quoted))
-         (tmp-name5 (tramp--test-make-temp-name 'local quoted)))
-
-      ;; Copy on remote side.
-      (unwind-protect
-         (progn
-           (write-region "foo" nil tmp-name1)
-           (copy-file tmp-name1 tmp-name2)
-           (should (file-exists-p tmp-name2))
-           (with-temp-buffer
-             (insert-file-contents tmp-name2)
-             (should (string-equal (buffer-string) "foo")))
-           (should-error
-            (copy-file tmp-name1 tmp-name2)
-            :type 'file-already-exists)
-           (copy-file tmp-name1 tmp-name2 'ok)
-           (make-directory tmp-name3)
-           ;; This has been changed in Emacs 26.1.
-           (when (tramp--test-emacs26-p)
-             (should-error
-              (copy-file tmp-name1 tmp-name3)
-              :type 'file-already-exists))
-           (copy-file tmp-name1 (file-name-as-directory tmp-name3))
-           (should
-            (file-exists-p
-             (expand-file-name (file-name-nondirectory tmp-name1) tmp-name3))))
-
-       ;; Cleanup.
-       (ignore-errors (delete-file tmp-name1))
-       (ignore-errors (delete-file tmp-name2))
-       (ignore-errors (delete-directory tmp-name3 'recursive)))
-
-      ;; Copy from remote side to local side.
-      (unwind-protect
-         (progn
-           (write-region "foo" nil tmp-name1)
-           (copy-file tmp-name1 tmp-name4)
-           (should (file-exists-p tmp-name4))
-           (with-temp-buffer
-             (insert-file-contents tmp-name4)
-             (should (string-equal (buffer-string) "foo")))
-           (should-error
-            (copy-file tmp-name1 tmp-name4)
-            :type 'file-already-exists)
-           (copy-file tmp-name1 tmp-name4 'ok)
-           (make-directory tmp-name5)
-           ;; This has been changed in Emacs 26.1.
-           (when (tramp--test-emacs26-p)
-             (should-error
-              (copy-file tmp-name1 tmp-name5)
-              :type 'file-already-exists))
-           (copy-file tmp-name1 (file-name-as-directory tmp-name5))
-           (should
-            (file-exists-p
-             (expand-file-name (file-name-nondirectory tmp-name1) tmp-name5))))
-
-       ;; Cleanup.
-       (ignore-errors (delete-file tmp-name1))
-       (ignore-errors (delete-file tmp-name4))
-       (ignore-errors (delete-directory tmp-name5 'recursive)))
-
-      ;; Copy from local side to remote side.
-      (unwind-protect
-         (progn
-           (write-region "foo" nil tmp-name4 nil 'nomessage)
-           (copy-file tmp-name4 tmp-name1)
-           (should (file-exists-p tmp-name1))
-           (with-temp-buffer
-             (insert-file-contents tmp-name1)
-             (should (string-equal (buffer-string) "foo")))
-           (should-error
-            (copy-file tmp-name4 tmp-name1)
-            :type 'file-already-exists)
-           (copy-file tmp-name4 tmp-name1 'ok)
-           (make-directory tmp-name3)
-           ;; This has been changed in Emacs 26.1.
-           (when (tramp--test-emacs26-p)
-             (should-error
-              (copy-file tmp-name4 tmp-name3)
-              :type 'file-already-exists))
-           (copy-file tmp-name4 (file-name-as-directory tmp-name3))
-           (should
-            (file-exists-p
-             (expand-file-name (file-name-nondirectory tmp-name4) tmp-name3))))
+         (tmp-name3 (tramp--test-make-temp-name 'local quoted)))
+      (dolist (source-target
+              `(;; Copy on remote side.
+                (,tmp-name1 . ,tmp-name2)
+                ;; Copy from remote side to local side.
+                (,tmp-name1 . ,tmp-name3)
+                ;; Copy from local side to remote side.
+                (,tmp-name3 . ,tmp-name1)))
+       (let ((source (car source-target))
+             (target (cdr source-target)))
+
+         ;; Copy simple file.
+         (unwind-protect
+             (progn
+               (write-region "foo" nil source)
+               (should (file-exists-p source))
+               (copy-file source target)
+               (should (file-exists-p target))
+               (with-temp-buffer
+                 (insert-file-contents target)
+                 (should (string-equal (buffer-string) "foo")))
+               (should-error
+                (copy-file source target)
+                :type 'file-already-exists)
+               (copy-file source target 'ok))
+
+           ;; Cleanup.
+           (ignore-errors (delete-file source))
+           (ignore-errors (delete-file target)))
+
+         ;; Copy file to directory.
+         (unwind-protect
+             (progn
+               (write-region "foo" nil source)
+               (should (file-exists-p source))
+               (make-directory target)
+               (should (file-directory-p target))
+               ;; This has been changed in Emacs 26.1.
+               (when (tramp--test-emacs26-p)
+                 (should-error
+                  (copy-file source target)
+                  :type 'file-already-exists))
+               (copy-file source (file-name-as-directory target))
+               (should
+                (file-exists-p
+                 (expand-file-name (file-name-nondirectory source) target))))
+
+           ;; Cleanup.
+           (ignore-errors (delete-file source))
+           (ignore-errors (delete-directory target 'recursive)))
+
+         ;; Copy directory to existing directory.
+         (unwind-protect
+             (progn
+               (make-directory source)
+               (should (file-directory-p source))
+               (write-region "foo" nil (expand-file-name "foo" source))
+               (should (file-exists-p (expand-file-name "foo" source)))
+               (make-directory target)
+               (should (file-directory-p target))
+               ;; Directory `target' exists already, so we must use
+               ;; `file-name-as-directory'.
+               (copy-file source (file-name-as-directory target))
+               (should
+                (file-exists-p
+                 (expand-file-name
+                  (concat (file-name-nondirectory source) "/foo") target))))
+
+           ;; Cleanup.
+           (ignore-errors (delete-directory source 'recursive))
+           (ignore-errors (delete-directory target 'recursive)))
+
+         ;; Copy directory/file to non-existing directory.
+         (unwind-protect
+             (progn
+               (make-directory source)
+               (should (file-directory-p source))
+               (write-region "foo" nil (expand-file-name "foo" source))
+               (should (file-exists-p (expand-file-name "foo" source)))
+               (make-directory target)
+               (should (file-directory-p target))
+               (copy-file
+                source
+                (expand-file-name (file-name-nondirectory source) target))
+               (should
+                (file-exists-p
+                 (expand-file-name
+                  (concat (file-name-nondirectory source) "/foo") target))))
 
-       ;; Cleanup.
-       (ignore-errors (delete-file tmp-name1))
-       (ignore-errors (delete-file tmp-name4))
-       (ignore-errors (delete-directory tmp-name3 'recursive))))))
+           ;; Cleanup.
+           (ignore-errors (delete-directory source 'recursive))
+           (ignore-errors (delete-directory target 'recursive))))))))
 
 (ert-deftest tramp-test12-rename-file ()
   "Check `rename-file'."
@@ -1983,111 +1985,105 @@ This checks also `file-name-as-directory', 
`file-name-directory',
   (let (quoted)
     (let ((tmp-name1 (tramp--test-make-temp-name nil quoted))
          (tmp-name2 (tramp--test-make-temp-name nil quoted))
-         (tmp-name3 (tramp--test-make-temp-name nil quoted))
-         (tmp-name4 (tramp--test-make-temp-name 'local quoted))
-         (tmp-name5 (tramp--test-make-temp-name 'local quoted)))
-
-      ;; Rename on remote side.
-      (unwind-protect
-         (progn
-           (write-region "foo" nil tmp-name1)
-           (rename-file tmp-name1 tmp-name2)
-           (should-not (file-exists-p tmp-name1))
-           (should (file-exists-p tmp-name2))
-           (with-temp-buffer
-             (insert-file-contents tmp-name2)
-             (should (string-equal (buffer-string) "foo")))
-           (write-region "foo" nil tmp-name1)
-           (should-error
-            (rename-file tmp-name1 tmp-name2)
-            :type 'file-already-exists)
-           (rename-file tmp-name1 tmp-name2 'ok)
-           (should-not (file-exists-p tmp-name1))
-           (write-region "foo" nil tmp-name1)
-           (make-directory tmp-name3)
-           ;; This has been changed in Emacs 26.1.
-           (when (tramp--test-emacs26-p)
-             (should-error
-              (rename-file tmp-name1 tmp-name3)
-              :type 'file-already-exists))
-           (rename-file tmp-name1 (file-name-as-directory tmp-name3))
-           (should-not (file-exists-p tmp-name1))
-           (should
-            (file-exists-p
-             (expand-file-name (file-name-nondirectory tmp-name1) tmp-name3))))
-
-       ;; Cleanup.
-       (ignore-errors (delete-file tmp-name1))
-       (ignore-errors (delete-file tmp-name2))
-       (ignore-errors (delete-directory tmp-name3 'recursive)))
-
-      ;; Rename from remote side to local side.
-      (unwind-protect
-         (progn
-           (write-region "foo" nil tmp-name1)
-           (rename-file tmp-name1 tmp-name4)
-           (should-not (file-exists-p tmp-name1))
-           (should (file-exists-p tmp-name4))
-           (with-temp-buffer
-             (insert-file-contents tmp-name4)
-             (should (string-equal (buffer-string) "foo")))
-           (write-region "foo" nil tmp-name1)
-           (should-error
-            (rename-file tmp-name1 tmp-name4)
-            :type 'file-already-exists)
-           (rename-file tmp-name1 tmp-name4 'ok)
-           (should-not (file-exists-p tmp-name1))
-           (write-region "foo" nil tmp-name1)
-           (make-directory tmp-name5)
-           ;; This has been changed in Emacs 26.1.
-           (when (tramp--test-emacs26-p)
-             (should-error
-              (rename-file tmp-name1 tmp-name5)
-              :type 'file-already-exists))
-           (rename-file tmp-name1 (file-name-as-directory tmp-name5))
-           (should-not (file-exists-p tmp-name1))
-           (should
-            (file-exists-p
-             (expand-file-name (file-name-nondirectory tmp-name1) tmp-name5))))
-
-       ;; Cleanup.
-       (ignore-errors (delete-file tmp-name1))
-       (ignore-errors (delete-file tmp-name4))
-       (ignore-errors (delete-directory tmp-name5 'recursive)))
-
-      ;; Rename from local side to remote side.
-      (unwind-protect
-         (progn
-           (write-region "foo" nil tmp-name4 nil 'nomessage)
-           (rename-file tmp-name4 tmp-name1)
-           (should-not (file-exists-p tmp-name4))
-           (should (file-exists-p tmp-name1))
-           (with-temp-buffer
-             (insert-file-contents tmp-name1)
-             (should (string-equal (buffer-string) "foo")))
-           (write-region "foo" nil tmp-name4 nil 'nomessage)
-           (should-error
-            (rename-file tmp-name4 tmp-name1)
-            :type 'file-already-exists)
-           (rename-file tmp-name4 tmp-name1 'ok)
-           (should-not (file-exists-p tmp-name4))
-           (write-region "foo" nil tmp-name4 nil 'nomessage)
-           (make-directory tmp-name3)
-           ;; This has been changed in Emacs 26.1.
-           (when (tramp--test-emacs26-p)
-             (should-error
-              (rename-file tmp-name4 tmp-name3)
-              :type 'file-already-exists))
-           (rename-file tmp-name4 (file-name-as-directory tmp-name3))
-           (should-not (file-exists-p tmp-name4))
-           (should
-            (file-exists-p
-             (expand-file-name (file-name-nondirectory tmp-name4) tmp-name3))))
+         (tmp-name3 (tramp--test-make-temp-name 'local quoted)))
+      (dolist (source-target
+              `(;; Rename on remote side.
+                (,tmp-name1 . ,tmp-name2)
+                ;; Rename from remote side to local side.
+                (,tmp-name1 . ,tmp-name3)
+                ;; Rename from local side to remote side.
+                (,tmp-name3 . ,tmp-name1)))
+       (let ((source (car source-target))
+             (target (cdr source-target)))
+
+         ;; Rename simple file.
+         (unwind-protect
+             (progn
+               (write-region "foo" nil source)
+               (should (file-exists-p source))
+               (rename-file source target)
+               (should-not (file-exists-p source))
+               (should (file-exists-p target))
+               (with-temp-buffer
+                 (insert-file-contents target)
+                 (should (string-equal (buffer-string) "foo")))
+               (write-region "foo" nil source)
+               (should (file-exists-p source))
+               (should-error
+                (rename-file source target)
+                :type 'file-already-exists)
+               (rename-file source target 'ok)
+               (should-not (file-exists-p source)))
+
+           ;; Cleanup.
+           (ignore-errors (delete-file source))
+           (ignore-errors (delete-file target)))
+
+         ;; Rename file to directory.
+         (unwind-protect
+             (progn
+               (write-region "foo" nil source)
+               (should (file-exists-p source))
+               (make-directory target)
+               (should (file-directory-p target))
+               ;; This has been changed in Emacs 26.1.
+               (when (tramp--test-emacs26-p)
+                 (should-error
+                  (rename-file source target)
+                  :type 'file-already-exists))
+               (rename-file source (file-name-as-directory target))
+               (should-not (file-exists-p source))
+               (should
+                (file-exists-p
+                 (expand-file-name (file-name-nondirectory source) target))))
+
+           ;; Cleanup.
+           (ignore-errors (delete-file source))
+           (ignore-errors (delete-directory target 'recursive)))
+
+         ;; Rename directory to existing directory.
+         (unwind-protect
+             (progn
+               (make-directory source)
+               (should (file-directory-p source))
+               (write-region "foo" nil (expand-file-name "foo" source))
+               (should (file-exists-p (expand-file-name "foo" source)))
+               (make-directory target)
+               (should (file-directory-p target))
+               ;; Directory `target' exists already, so we must use
+               ;; `file-name-as-directory'.
+               (rename-file source (file-name-as-directory target))
+               (should-not (file-exists-p source))
+               (should
+                (file-exists-p
+                 (expand-file-name
+                  (concat (file-name-nondirectory source) "/foo") target))))
+
+           ;; Cleanup.
+           (ignore-errors (delete-directory source 'recursive))
+           (ignore-errors (delete-directory target 'recursive)))
+
+         ;; Rename directory/file to non-existing directory.
+         (unwind-protect
+             (progn
+               (make-directory source)
+               (should (file-directory-p source))
+               (write-region "foo" nil (expand-file-name "foo" source))
+               (should (file-exists-p (expand-file-name "foo" source)))
+               (make-directory target)
+               (should (file-directory-p target))
+               (rename-file
+                source
+                (expand-file-name (file-name-nondirectory source) target))
+               (should-not (file-exists-p source))
+               (should
+                (file-exists-p
+                 (expand-file-name
+                  (concat (file-name-nondirectory source) "/foo") target))))
 
-       ;; Cleanup.
-       (ignore-errors (delete-file tmp-name1))
-       (ignore-errors (delete-file tmp-name4))
-       (ignore-errors (delete-directory tmp-name3 'recursive))))))
+           ;; Cleanup.
+           (ignore-errors (delete-directory source 'recursive))
+           (ignore-errors (delete-directory target 'recursive))))))))
 
 (ert-deftest tramp-test13-make-directory ()
   "Check `make-directory'.