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.
This commit is contained in:
Michael Albinus 2017-10-20 12:46:54 +02:00
parent d815de017b
commit b500e06f4d
4 changed files with 304 additions and 292 deletions

View file

@ -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 (file-directory-p filename)
(progn
(copy-directory filename newname t t)
(delete-directory filename 'recursive))
(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))
(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)
;; Rename by copy.
(copy-file
filename newname ok-if-already-exists 'keep-time 'preserve-uid-gid)
(delete-file filename))))))
(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)

View file

@ -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

View file

@ -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))
(if (file-directory-p filename)
(progn
(copy-directory filename newname keep-date t)
(when (eq op 'rename) (delete-directory filename 'recursive)))
(with-tramp-progress-reporter
v 0 (format "%s %s to %s"
(if (eq op 'copy) "Copying" "Renaming")
filename newname)
(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)))))
(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))
(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))
;; 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))
(with-tramp-progress-reporter
v 0 (format "%s %s to %s"
(if (eq op 'copy) "Copying" "Renaming")
filename newname)
;; 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))
;; 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))
;; 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))
;; 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))
;; Use the inline method via a Tramp buffer.
(t (tramp-do-copy-or-rename-file-via-buffer
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))))))
(t
;; One of them must be a Tramp file.
(error "Tramp implementation says this cannot happen")))
;; 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))
;; 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))))
;; 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))
;; 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)))
;; Use the inline method via a Tramp buffer.
(t (tramp-do-copy-or-rename-file-via-buffer
op filename newname keep-date))))
;; 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.

View file

@ -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)))
(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 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))))
;; 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 tmp-name1))
(ignore-errors (delete-file tmp-name2))
(ignore-errors (delete-directory tmp-name3 'recursive)))
;; Cleanup.
(ignore-errors (delete-file source))
(ignore-errors (delete-file target)))
;; 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))))
;; 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 tmp-name1))
(ignore-errors (delete-file tmp-name4))
(ignore-errors (delete-directory tmp-name5 'recursive)))
;; Cleanup.
(ignore-errors (delete-file source))
(ignore-errors (delete-directory target '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))))
;; 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-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)))
;; 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-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)))
(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 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))))
;; 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 tmp-name1))
(ignore-errors (delete-file tmp-name2))
(ignore-errors (delete-directory tmp-name3 'recursive)))
;; Cleanup.
(ignore-errors (delete-file source))
(ignore-errors (delete-file target)))
;; 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))))
;; 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 tmp-name1))
(ignore-errors (delete-file tmp-name4))
(ignore-errors (delete-directory tmp-name5 'recursive)))
;; Cleanup.
(ignore-errors (delete-file source))
(ignore-errors (delete-directory target '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))))
;; 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-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)))
;; 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-directory source 'recursive))
(ignore-errors (delete-directory target 'recursive))))))))
(ert-deftest tramp-test13-make-directory ()
"Check `make-directory'.