From: Paul Eggert <eggert@cs.ucla.edu>
To: Michael Albinus <michael.albinus@gmx.de>, Eli Zaretskii <eliz@gnu.org>
Cc: thievol@posteo.net, 58919-done@debbugs.gnu.org
Subject: bug#58919: 28.2; dired-copy-file-recursive fails to overwrite directory
Date: Sat, 17 Dec 2022 14:40:09 -0800 [thread overview]
Message-ID: <7c209ab7-056b-8300-09b9-87549f6084be@cs.ucla.edu> (raw)
In-Reply-To: <87y1r69x2e.fsf@gmx.de>
[-- 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
next prev parent reply other threads:[~2022-12-17 22:40 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-31 8:54 bug#58919: 28.2; dired-copy-file-recursive fails to overwrite directory Thierry Volpiatto
2022-10-31 13:01 ` Eli Zaretskii
2022-11-01 17:34 ` Thierry Volpiatto
2022-11-01 18:04 ` Paul Eggert
2022-11-01 18:09 ` Eli Zaretskii
2022-11-01 19:21 ` Michael Albinus
2022-12-11 10:46 ` Eli Zaretskii
2022-12-16 23:22 ` Paul Eggert
2022-12-17 8:04 ` Eli Zaretskii
2022-12-17 9:52 ` Michael Albinus
2022-12-17 10:40 ` Eli Zaretskii
2022-12-17 22:40 ` Paul Eggert [this message]
2022-12-18 19:35 ` Michael Albinus
2022-12-18 20:54 ` Paul Eggert
2022-12-23 10:26 ` Michael Albinus
2022-12-24 9:11 ` Paul Eggert
2022-12-24 10:13 ` Michael Albinus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7c209ab7-056b-8300-09b9-87549f6084be@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=58919-done@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=michael.albinus@gmx.de \
--cc=thievol@posteo.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).