Subject: bug#964 closed by Glenn Morris <[email-masked]> (Re: bug#964: trash - runaway recursion for cross-device delete-file ops)

From: [email-masked] (Emacs bug Tracking System)

This is an automatic notification regarding your bug report
which was filed against the emacs package:

#964: trash - runaway recursion for cross-device delete-file ops

It has been closed by Glenn Morris <[email-masked]>.

Their explanation is attached below along with your original report.
If this explanation is unsatisfactory and you have not received a
better one in a separate message then please contact Glenn Morris <[email-masked]> by
replying to this email.

-- 
964: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=964
Emacs Bug Tracking System
Contact [email-masked] with problems
>From [email-masked] Sat Sep 20 14:42:44 2008
From: Glenn Morris <[email-masked]>
To: David De La Harpe Golden <[email-masked]>
Cc: [email-masked]
Subject: Re: bug#964: trash - runaway recursion for cross-device delete-file ops
Date: Sat, 20 Sep 2008 17:40:39 -0400

David De La Harpe Golden wrote:

> (i) Well, binding delete-by-moving-to-trash to nil around the
> rename-file in move-file-to-trash might be adequate, fixes immediate
> issue?  trivial patch attached.

Thanks; applied.

> (ii) As it stands, won't an actual rename-file will sometimes move a
> copy of "renamed" files to trash (i.e. when they're on a different
> device) if delete-by-moving-to-trash is on?  Not sure that's very nice -
> maybe should bind delete-by-moving-to-trash to nil around its call to
> delete-file, or maybe just use C unlink()?

I installed a fix using the former approach.

This feature doesn't seem very well thought-out on non-Windows platforms.

>From [email-masked] Thu Sep 11 19:07:49 2008
Date: Fri, 12 Sep 2008 03:07:31 +0100
From: David De La Harpe Golden <[email-masked]>
To: [email-masked]
Subject: trash - runaway recursion for cross-device delete-file ops

This is a multi-part message in MIME format.
--------------040105050802060801010406
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Package: emacs
Version: 23.0.60
Severity: normal
Tags: patch

Steps to reproduce:

Turn on delete-by-moving-to-trash on gnu+linux.
Make a file on a different filesystem to your home dir.
Try to delete-file the file in emacs once.
Look inside ~/.Trash, full of hundreds of redundant
copies of the file.

Reason:

When delete-by-moving-to-trash is on, delete-file calls move-file-to-trash.

move-file-to-trash, when using its fallback emacs trashcan
implementation* (i.e. when there's no system-move-file-to-trash) picks a
name for the trashed file, and calls rename-file.

rename-file tries a C rename(), but that doesn't work cross-device on
gnu+linux (and several other OSes), so rename-file falls back to copying
the file to the new name, and then calls delete-file to get rid of the
old one (~line 2247 of fileio.c)

move-file-to-trash decides the existing filename under .Trash is taken,
seeing as a file exists with that name, picks a new name, and calls
rename-file again...

... Lather, rinse, repeat, until emacs gets bored with a
"Variable binding depth exceeds max-specpdl-size"

Fix?:

(i) Well, binding delete-by-moving-to-trash to nil around the
rename-file in move-file-to-trash might be adequate, fixes immediate
issue?  trivial patch attached.  Though another strikes me:

(ii) As it stands, won't an actual rename-file will sometimes move a
copy of "renamed" files to trash (i.e. when they're on a different
device) if delete-by-moving-to-trash is on?  Not sure that's very nice -
maybe should bind delete-by-moving-to-trash to nil around its call to
delete-file, or maybe just use C unlink()?

* which really isn't suitable for gnu+linux/freedesktop.org desktops,
it seems to be something very like (if not identical to) the macosx
convention, but that's a matter for a different bug.

--------------040105050802060801010406
Content-Type: text/x-patch;
 name="trash_xdevice_fix_r1.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="trash_xdevice_fix_r1.diff"

Index: lisp/files.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/files.el,v
retrieving revision 1.995
diff -U 8 -r1.995 files.el
--- lisp/files.el	2 Sep 2008 16:10:44 -0000	1.995
+++ lisp/files.el	12 Sep 2008 01:43:51 -0000
@@ -5820,17 +5820,18 @@
            ;; make new-fn unique.
            ;; example: "~/.Trash/abc.txt" -> "~/.Trash/abc.txt.~1~"
            (let ((version-control t))
              (setq new-fn (car (find-backup-file-name new-fn)))))
       ;; stop processing if fn is same or parent directory of trash-dir.
       (and (string-match fn trash-dir)
            (error "Filename `%s' is same or parent directory of trash-directory"
                   filename))
-      (rename-file fn new-fn)))))
+      (let ((delete-by-moving-to-trash nil))
+	(rename-file fn new-fn))))))
 
 
 (define-key ctl-x-map "\C-f" 'find-file)
 (define-key ctl-x-map "\C-r" 'find-file-read-only)
 (define-key ctl-x-map "\C-v" 'find-alternate-file)
 (define-key ctl-x-map "\C-s" 'save-buffer)
 (define-key ctl-x-map "s" 'save-some-buffers)
 (define-key ctl-x-map "\C-w" 'write-file)

--------------040105050802060801010406
Content-Type: text/plain;
 name="ChangeLog.trash_xdevice_fix_r1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="ChangeLog.trash_xdevice_fix_r1"

2008-09-11  David De La Harpe Golden <[email-masked]>

	files.el: in move-file-to-trash, bind delete-by-moving-to-trash to
	nil around rename-file to avoid recursive trashing if rename-file
	calls delete-file.

--------------040105050802060801010406--