bug#58919: 28.2; dired-copy-file-recursive fails to overwrite directory

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

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

On 12/17/22 01:52, Michael Albinus wrote:
> Since file name handlers still raise an error in case DIR exists and
> PARENTS is nil, we might see surprises in code assuming the new
> behavior. I guess I'll add a change in tramp-*-handle-make-directory
> like
>
> --8<---------------cut here---------------start------------->8---
>      (if (and (null parents) (file-exists-p dir))
> 	(if (>= emacs-major-version 29)
>              t
> 	  (tramp-error v 'file-already-exists dir)))
> --8<---------------cut here---------------end--------------->8---

That doesn't look right, as there is no change as to whether 
make-directory signals an error. Nor is there any change in behavior 
when PARENTS is nil. The only change in behavior is when PARENTS is 
non-nil and DIRECTORY already exists as a directory: in this case, Emacs 
29 returns non-nil whereas earlier Emacs returns (undocumented) nil.

So I think all that it would be nice to do is make sure the handlers 
ordinarily return nil, except that they return non-nil in the 
abovementioned special case. If a handler currently signals an error, it 
should continue to do so in the same way as it did before. Strictly 
speaking, modifying the handlers in this way won't affect whether they 
are compatible with Emacs 28- (since their return value is undocumented 
there) nor will it affect whether they work in Emacs 29 (since Emacs 29 
ignores their return value). But it might affect whether the handlers 
will work with Emacs 30, which might start assuming the Emacs 29 API for 
make-directory handlers.

Come to think of it, if an existing make-directory handler returns 
non-nil now (which it is allowed to in Emacs 28 as the return value is 
undocumented), then the proposed changes would sometimes have caused 
make-directory to return that non-nil value to its caller, even when the 
Emacs 29 doc says make-directory should return nil. As far as I know no 
such make-directory handler does so now, but to be safe I installed the 
attached additional patch to make sure Emacss 29 make-directory returns 
nil in this situation. As the combined set of patches should fix the 
original bug report I'm marking it as done.

In Emacs 30 we could remove this additional patch once we've fixed all 
the handlers, along with omitting some of the other code that supports 
calling Emacs 28-style handlers in Emacs 30 environments. Or we could 
leave it in; there's no rush, I imagine.

[-- Attachment #2: 0001-Don-t-assume-make-directory-handler-returns-nil.patch --]
[-- Type: text/x-patch, Size: 1381 bytes --]

From 4a8ff671b0e93e96f7fca4204cdbc83f99a3387c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 17 Dec 2022 14:09:03 -0800
Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20assume=20make-directory=20handl?=
 =?UTF-8?q?er=20returns=20nil?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/files.el (make-directory): Ignore what the make-directory
handler returns, as its return value was not documented in Emacs 28.
---
 lisp/files.el | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lisp/files.el b/lisp/files.el
index 3cf7833ae02..cc7d7e2af94 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -6224,7 +6224,12 @@ make-directory
   ;; make sure we find its make-directory handler.
   (setq dir (expand-file-name dir))
   (let ((mkdir (if-let ((handler (find-file-name-handler dir 'make-directory)))
-                   #'(lambda (dir) (funcall handler 'make-directory dir))
+		   #'(lambda (dir)
+		       ;; Use 'ignore' since the handler might be designed for
+		       ;; Emacs 28-, so it might return an (undocumented)
+		       ;; non-nil value, whereas the Emacs 29+ convention is
+		       ;; to return nil here.
+		       (ignore (funcall handler 'make-directory dir)))
                  #'make-directory-internal)))
     (if (not parents)
         (funcall mkdir dir)
-- 
2.38.1