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.

3 files changed, 133 insertions, 117 deletions

diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index e75efcf483f..bf21db2e8d8 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 2b29a1b55f5..51d24cbc1b0 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 bdb7a132408..52a6b8fac0b 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.