bug#27986: 26.0.50; 'rename-file' can rename files without confirmation

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]

Paul Eggert wrote:
> I reviewed all calls to the affected functions and have a few followup patches 
> that I plan to install separately soon, and mention here as I do.

I installed the patches (attached). Closing the bug report.

I'll CC: this to Michael, as the last patch is to the Tramp tests, and he may 
want Tramp to become consistent with the new behavior for rename-file etc.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-copy-directory-act-like-copy-file-etc.patch --]
[-- Type: text/x-patch; name="0001-Make-copy-directory-act-like-copy-file-etc.patch", Size: 5285 bytes --]

From e22794867d878d53675fcc91d2ef1ad2494a2ff2 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 10 Sep 2017 22:07:30 -0700
Subject: [PATCH 1/6] Make copy-directory act like copy-file etc.

Do the special dance with the destination only if it is a
directory name, for consistency with copy-file etc. (Bug#27986).
* doc/emacs/files.texi (Copying and Naming):
* doc/lispref/files.texi (Create/Delete Dirs):
* etc/NEWS: Document this.
* lisp/files.el (copy-directory): Treat NEWNAME as special
only if it is a directory name.
---
 doc/emacs/files.texi   |  8 ++++----
 doc/lispref/files.texi |  5 +++--
 etc/NEWS               |  4 ++--
 lisp/files.el          | 22 ++++++++++------------
 4 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index 0cf46b6..ca4f223 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -1572,10 +1572,10 @@ Copying and Naming
 
 @findex copy-directory
   @kbd{M-x copy-directory} copies directories, similar to the
-@command{cp -r} shell command.  If @var{new} is an existing directory,
-it creates a copy of the @var{old} directory and puts it in @var{new}.
-If @var{new} is not an existing directory, it copies all the contents
-of @var{old} into a new directory named @var{new}.
+@command{cp -r} shell command.  If @var{new} is a directory name, it
+creates a copy of the @var{old} directory and puts it in @var{new}.
+Otherwise it copies all the contents of @var{old} into a new directory
+named @var{new}.
 
 @cindex renaming files
 @findex rename-file
diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index eacaf04..901382f 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -2976,8 +2976,9 @@ Create/Delete Dirs
 
 @deffn Command copy-directory dirname newname &optional keep-time parents copy-contents
 This command copies the directory named @var{dirname} to
-@var{newname}.  If @var{newname} names an existing directory,
+@var{newname}.  If @var{newname} is a directory name,
 @var{dirname} will be copied to a subdirectory there.
+@xref{Directory Names}.
 
 It always sets the file modes of the copied files to match the
 corresponding original file.
@@ -2992,7 +2993,7 @@ Create/Delete Dirs
 
 The fifth argument @var{copy-contents}, if non-@code{nil}, means to
 copy the contents of @var{dirname} directly into @var{newname} if the
-latter is an existing directory, instead of copying @var{dirname} into
+latter is a directory name, instead of copying @var{dirname} into
 it as a subdirectory.
 @end deffn
 
diff --git a/etc/NEWS b/etc/NEWS
index 4187dd8..136d458 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1285,8 +1285,8 @@ documentation and had inherent races that led to security holes.  A
 call like (rename-file C D) that used the old, undocumented behavior
 can be written as (rename-file C (file-name-as-directory D)), a
 formulation portable to both older and newer versions of Emacs.
-Affected functions include add-name-to-file, copy-file,
-make-symbolic-link, and rename-file.
+Affected functions include add-name-to-file, copy-directory,
+copy-file, make-symbolic-link, and rename-file.
 
 \f
 * Lisp Changes in Emacs 26.1
diff --git a/lisp/files.el b/lisp/files.el
index 85e649f..7ab6f76 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5501,10 +5501,10 @@ copy-directory
 create parent directories if they don't exist.  Interactively,
 this happens by default.
 
-If NEWNAME names an existing directory, copy DIRECTORY as a
-subdirectory there.  However, if called from Lisp with a non-nil
-optional argument COPY-CONTENTS, copy the contents of DIRECTORY
-directly into NEWNAME instead."
+If NEWNAME is a directory name, copy DIRECTORY as a subdirectory
+there.  However, if called from Lisp with a non-nil optional
+argument COPY-CONTENTS, copy the contents of DIRECTORY directly
+into NEWNAME instead."
   (interactive
    (let ((dir (read-directory-name
 	       "Copy directory: " default-directory default-directory t nil)))
@@ -5526,19 +5526,17 @@ copy-directory
 
       ;; Compute target name.
       (setq directory (directory-file-name (expand-file-name directory))
-	    newname   (directory-file-name (expand-file-name newname)))
+	    newname (expand-file-name newname))
 
-      (cond ((not (file-directory-p newname))
-	     ;; If NEWNAME is not an existing directory, create it;
+      (cond ((not (directory-name-p newname))
+	     ;; If NEWNAME is not a directory name, create it;
 	     ;; that is where we will copy the files of DIRECTORY.
 	     (make-directory newname parents))
-	    ;; If NEWNAME is an existing directory and COPY-CONTENTS
+	    ;; If NEWNAME is a directory name and COPY-CONTENTS
 	    ;; is nil, copy into NEWNAME/[DIRECTORY-BASENAME].
 	    ((not copy-contents)
-	     (setq newname (concat
-			    (file-name-as-directory newname)
-			    (file-name-nondirectory
-			     (directory-file-name directory))))
+	     (setq newname (concat newname
+			    (file-name-nondirectory directory)))
 	     (and (file-exists-p newname)
 		  (not (file-directory-p newname))
 		  (error "Cannot overwrite non-directory %s with a directory"
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Make-write-file-act-like-copy-file-etc.patch --]
[-- Type: text/x-patch; name="0002-Make-write-file-act-like-copy-file-etc.patch", Size: 1741 bytes --]

From 61946d991b663c9d35a50b758d0108c3cbf8027b Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 10 Sep 2017 22:19:01 -0700
Subject: [PATCH 2/6] Make write-file act like copy-file etc.

Change write-file to be consistent with the new behavior
of copy-file, etc.
* etc/NEWS: Mention this.
* lisp/files.el (write-file): Treat the destination as special
only if it is a directory name.
---
 etc/NEWS      | 3 ++-
 lisp/files.el | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 136d458..4da4c37 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1286,7 +1286,8 @@ call like (rename-file C D) that used the old, undocumented behavior
 can be written as (rename-file C (file-name-as-directory D)), a
 formulation portable to both older and newer versions of Emacs.
 Affected functions include add-name-to-file, copy-directory,
-copy-file, make-symbolic-link, and rename-file.
+copy-file, format-write-file, make-symbolic-link, rename-file, and
+write-file.
 
 \f
 * Lisp Changes in Emacs 26.1
diff --git a/lisp/files.el b/lisp/files.el
index 7ab6f76..611a4c5 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4212,10 +4212,10 @@ write-file
 	 (not current-prefix-arg)))
   (or (null filename) (string-equal filename "")
       (progn
-	;; If arg is just a directory,
+	;; If arg is a directory name,
 	;; use the default file name, but in that directory.
-	(if (file-directory-p filename)
-	    (setq filename (concat (file-name-as-directory filename)
+	(if (directory-name-p filename)
+	    (setq filename (concat filename
 				   (file-name-nondirectory
 				    (or buffer-file-name (buffer-name))))))
 	(and confirm
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Make-gnus-copy-file-act-like-copy-file-etc.patch --]
[-- Type: text/x-patch; name="0003-Make-gnus-copy-file-act-like-copy-file-etc.patch", Size: 1626 bytes --]

From 739593d68742f45e4e35dfc99573c47a5031b646 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 10 Sep 2017 22:21:20 -0700
Subject: [PATCH 3/6] Make gnus-copy-file act like copy-file etc.

* etc/NEWS: Mention this.
* lisp/gnus/gnus-util.el (gnus-copy-file): Treat the destination
as special only if it is a directory name.
---
 etc/NEWS               | 4 ++--
 lisp/gnus/gnus-util.el | 3 ---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 4da4c37..fc40a3a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1286,8 +1286,8 @@ call like (rename-file C D) that used the old, undocumented behavior
 can be written as (rename-file C (file-name-as-directory D)), a
 formulation portable to both older and newer versions of Emacs.
 Affected functions include add-name-to-file, copy-directory,
-copy-file, format-write-file, make-symbolic-link, rename-file, and
-write-file.
+copy-file, format-write-file, gnus-copy-file, make-symbolic-link,
+rename-file, and write-file.
 
 \f
 * Lisp Changes in Emacs 26.1
diff --git a/lisp/gnus/gnus-util.el b/lisp/gnus/gnus-util.el
index b509d8a..93541f0 100644
--- a/lisp/gnus/gnus-util.el
+++ b/lisp/gnus/gnus-util.el
@@ -594,9 +594,6 @@ gnus-copy-file
 	 (read-file-name "Copy file to: " default-directory)))
   (unless to
     (setq to (read-file-name "Copy file to: " default-directory)))
-  (when (file-directory-p to)
-    (setq to (concat (file-name-as-directory to)
-		     (file-name-nondirectory file))))
   (copy-file file to))
 
 (defvar gnus-work-buffer " *gnus work*")
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-Adjust-ob-tangle-to-new-copy-file-behavior.patch --]
[-- Type: text/x-patch; name="0004-Adjust-ob-tangle-to-new-copy-file-behavior.patch", Size: 865 bytes --]

From 74b8615fcceba7b92c4938e1bcc92015f10ae899 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 10 Sep 2017 22:22:55 -0700
Subject: [PATCH 4/6] Adjust ob-tangle to new copy-file behavior

* lisp/org/ob-tangle.el (org-babel-tangle-publish):
Port to new copy-file behavior.
---
 lisp/org/ob-tangle.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/org/ob-tangle.el b/lisp/org/ob-tangle.el
index 3b05332..2dc55ca 100644
--- a/lisp/org/ob-tangle.el
+++ b/lisp/org/ob-tangle.el
@@ -197,6 +197,7 @@ org-babel-tangle-publish
   "Tangle FILENAME and place the results in PUB-DIR."
   (unless (file-exists-p pub-dir)
     (make-directory pub-dir t))
+  (setq pub-dir (file-name-as-directory pub-dir))
   (mapc (lambda (el) (copy-file el pub-dir t)) (org-babel-tangle-file filename)))
 
 ;;;###autoload
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0005-Adjust-thumbs-to-new-rename-file-behavior.patch --]
[-- Type: text/x-patch; name="0005-Adjust-thumbs-to-new-rename-file-behavior.patch", Size: 2260 bytes --]

From 2aa028825920207cca2bacb581111ab780e5d9ee Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 10 Sep 2017 22:28:08 -0700
Subject: [PATCH 5/6] Adjust thumbs to new rename-file behavior

* etc/NEWS: Mention this.
* lisp/thumbs.el (thumbs-rename-images): Treat the destination
as special only if it is a directory name.  When there is
a marked list, turn the destination into a directory name
if it is not already.
---
 etc/NEWS       |  2 +-
 lisp/thumbs.el | 15 ++++-----------
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index fc40a3a..3f1df23 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1287,7 +1287,7 @@ can be written as (rename-file C (file-name-as-directory D)), a
 formulation portable to both older and newer versions of Emacs.
 Affected functions include add-name-to-file, copy-directory,
 copy-file, format-write-file, gnus-copy-file, make-symbolic-link,
-rename-file, and write-file.
+rename-file, thumbs-rename-images, and write-file.
 
 \f
 * Lisp Changes in Emacs 26.1
diff --git a/lisp/thumbs.el b/lisp/thumbs.el
index 0665429..d0b5e22 100644
--- a/lisp/thumbs.el
+++ b/lisp/thumbs.el
@@ -523,23 +523,16 @@ thumbs-rename-images
   (interactive "FRename to file or directory: ")
   (let ((files (or thumbs-marked-list (list (thumbs-current-image))))
 	failures)
-    (if (and (not (file-directory-p newfile))
-	     thumbs-marked-list)
-	(if (file-exists-p newfile)
-	    (error "Renaming marked files to file name `%s'" newfile)
-	  (make-directory newfile t)))
+    (when thumbs-marked-list
+      (make-directory newfile t)
+      (setq newfile (file-name-as-directory newfile)))
     (if (yes-or-no-p (format "Really rename %d files? " (length files)))
 	(let ((thumbs-file-list (thumbs-file-alist))
 	      (inhibit-read-only t))
 	  (dolist (file files)
 	    (let (failure)
 	      (condition-case ()
-		  (if (file-directory-p newfile)
-		      (rename-file file
-				   (expand-file-name
-				    (file-name-nondirectory file)
-				    newfile))
-		    (rename-file file newfile))
+		  (rename-file file newfile)
 		(file-error (setq failure t)
 			    (push file failures)))
 	      (unless failure
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #7: 0006-Port-tramp-tests-to-new-copy-directory-behavior.patch --]
[-- Type: text/x-patch; name="0006-Port-tramp-tests-to-new-copy-directory-behavior.patch", Size: 1513 bytes --]

From 29963648dd11d53088f753e4f9b0491a7b981c0f Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 10 Sep 2017 23:04:10 -0700
Subject: [PATCH 6/6] Port tramp-tests to new copy-directory behavior

* test/lisp/net/tramp-tests.el (tramp-test15-copy-directory):
Use directory name as arg for copy-directory when we want
the special behavior.
---
 test/lisp/net/tramp-tests.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 735211c..4139d50 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -2117,7 +2117,7 @@ tramp--test-backtrace
 	    (should (file-directory-p tmp-name2))
 	    (should (file-exists-p tmp-name5))
 	    ;; Target directory does exist already.
-	    (copy-directory tmp-name1 tmp-name2)
+	    (copy-directory tmp-name1 (file-name-as-directory tmp-name2))
 	    (should (file-directory-p tmp-name3))
 	    (should (file-exists-p tmp-name6)))
 
@@ -2140,7 +2140,8 @@ tramp--test-backtrace
 	    ;; Target directory does exist already.
 	    (delete-file tmp-name5)
 	    (should-not (file-exists-p tmp-name5))
-	    (copy-directory tmp-name1 tmp-name2 nil 'parents 'contents)
+	    (copy-directory tmp-name1 (file-name-as-directory tmp-name2)
+			    nil 'parents 'contents)
 	    (should (file-directory-p tmp-name2))
 	    (should (file-exists-p tmp-name5))
 	    (should-not (file-directory-p tmp-name3))
-- 
2.7.4