From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Mike Kupfer Newsgroups: gmane.emacs.bugs Subject: bug#58721: 28.2; dired with delete-by-moving-to-trash can't trash directory twice Date: Sun, 20 Nov 2022 17:08:18 -0800 Message-ID: <66176.1668992898@alto> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="14978"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 58721@debbugs.gnu.org To: Gustavo Barros , Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Nov 21 02:09:25 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1owvJJ-0003h0-QP for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 21 Nov 2022 02:09:21 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1owvJ2-0001DY-3k; Sun, 20 Nov 2022 20:09:04 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1owvJ0-0001DQ-Rj for bug-gnu-emacs@gnu.org; Sun, 20 Nov 2022 20:09:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1owvJ0-0004vA-CV for bug-gnu-emacs@gnu.org; Sun, 20 Nov 2022 20:09:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1owvJ0-0001q3-3n for bug-gnu-emacs@gnu.org; Sun, 20 Nov 2022 20:09:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Mike Kupfer Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 21 Nov 2022 01:09:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 58721 X-GNU-PR-Package: emacs Original-Received: via spool by 58721-submit@debbugs.gnu.org id=B58721.16689929117027 (code B ref 58721); Mon, 21 Nov 2022 01:09:02 +0000 Original-Received: (at 58721) by debbugs.gnu.org; 21 Nov 2022 01:08:31 +0000 Original-Received: from localhost ([127.0.0.1]:44940 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1owvIV-0001pG-6P for submit@debbugs.gnu.org; Sun, 20 Nov 2022 20:08:31 -0500 Original-Received: from shell1.rawbw.com ([198.144.192.42]:64815 ident=root) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1owvIP-0001p3-OE for 58721@debbugs.gnu.org; Sun, 20 Nov 2022 20:08:29 -0500 Original-Received: from alto (135-180-174-133.dsl.dynamic.sonic.net [135.180.174.133] (may be forged)) (authenticated bits=0) by shell1.rawbw.com (8.15.1/8.15.1) with ESMTPSA id 2AL18IvA090311 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sun, 20 Nov 2022 17:08:23 -0800 (PST) (envelope-from mkupfer@alum.berkeley.edu) X-Authentication-Warning: shell1.rawbw.com: Host 135-180-174-133.dsl.dynamic.sonic.net [135.180.174.133] (may be forged) claimed to be alto In-Reply-To: Your message of "Mon, 31 Oct 2022 10:16:47 -0300." X-Mailer: MH-E 8.6+git; nmh 1.7.1; Emacs 29.0.50 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:248493 Archived-At: --=-=-= Content-Type: text/plain Gustavo Barros wrote: > On Mon, 31 Oct 2022 at 09:49, Eli Zaretskii wrote: > > > What is the expected semantics of moving a symlink to trashcan? Is it > > supposed to move the symlink or its target? (I'd think it's the > > former, but maybe my instincts are wrong.) If the expectations are > > that the symlink is moved, then all we need to do is to treat symlinks > > as regular files, by augmenting file-directory-p not to dupe us. > > I'm not sure either, but my instincts are the same as yours. If that's > any reference, I just tested here, and that's what "gio trash" does > (moves the symlink, not the target). Yes, I tried a few graphical file browsers (Thunar, Caja, and Dolphin), and they all move the symlink, not the target, to Trash. After getting my test setup straightened out, I think I have a fix for the symlink issue and for the issue that Gustavo originally reported (cross-filesystem trashing fails when there's already a directory with the same name in Trash). I've committed these fixes separately; see attached. Gustavo, can you try these out and make sure they handle your use case(s)? > > I'm okay with filing another bug report about rename-file, and > > discussing this there. But that's a separate issue, and fix of this > > bug should not depend on that. > > Understood. Okay. I'm not planning to follow up on this, Gustavo, so if you'd like to lobby for a change to rename-file, you'll need to open a bug for it (if you haven't already). cheers, mike --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Fix-cross-filesystem-directory-trashing-Bug-58721.patch Content-Description: fix duplicate trashing >From f749a1dd7875a83e6415d8c512c5659f0f23c834 Mon Sep 17 00:00:00 2001 From: Mike Kupfer Date: Sun, 30 Oct 2022 10:31:11 -0700 Subject: [PATCH 1/2] Fix cross-filesystem directory trashing (Bug#58721) * lisp/files.el (move-file-to-trash): When trashing a directory with the same name as something that's already in the trash, copy it into the trash folder and then delete it, rather than using rename-file. --- lisp/files.el | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/lisp/files.el b/lisp/files.el index a2825322580..de4f68ebd51 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -8596,10 +8596,27 @@ move-file-to-trash (setq files-base (substring (file-name-nondirectory info-fn) 0 (- (length ".trashinfo")))) (write-region nil nil info-fn nil 'quiet info-fn))) - ;; Finally, try to move the file to the trashcan. + ;; Finally, try to move the item to the trashcan. If + ;; it's a file, just move it. Things are more + ;; complicated for directories. If the target + ;; directory already exists (due to uniquification) + ;; and the trash directory is in a different + ;; filesystem, rename-file will error out, even when + ;; 'overwrite' is non-nil. Rather than worry about + ;; whether we're crossing filesystems, just check if + ;; we've moving a directory and the target directory + ;; already exists. That handles both the + ;; same-filesystem and cross-filesystem cases. (let ((delete-by-moving-to-trash nil) (new-fn (file-name-concat trash-files-dir files-base))) - (rename-file fn new-fn overwrite))))))))) + (if (or (not is-directory) + (not (file-exists-p new-fn))) + (rename-file fn new-fn overwrite) + (copy-directory fn + (file-name-as-directory new-fn) + t nil t) + (delete-directory fn t)))))))))) + (defsubst file-attribute-type (attributes) -- 2.30.2 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0002-Fix-trashing-of-symlink-that-points-at-a-directory.patch Content-Description: fix trashing of symlink >From 4aefb5b5964e1aaad0c74238e14ad0a1e3dc4e7a Mon Sep 17 00:00:00 2001 From: Mike Kupfer Date: Sun, 20 Nov 2022 16:44:20 -0800 Subject: [PATCH 2/2] Fix trashing of symlink that points at a directory * lisp/files.el (move-file-to-trash): Redefine is-directory so that it is false for symlinks. --- lisp/files.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lisp/files.el b/lisp/files.el index de4f68ebd51..a78d1627689 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -8566,7 +8566,8 @@ move-file-to-trash ;; Make a .trashinfo file. Use O_EXCL, as per trash-spec 1.0. (let* ((files-base (file-name-nondirectory fn)) - (is-directory (file-directory-p fn)) + (is-directory (and (file-directory-p fn) + (not (file-symlink-p fn)))) (overwrite nil) info-fn) ;; We're checking further down whether the info file -- 2.30.2 --=-=-=--