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

[David De La Harpe Golden <david@harpegolden.net>]

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

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.







[-- Attachment #2: trash_xdevice_fix_r1.diff --]
[-- Type: text/x-patch, Size: 1141 bytes --]

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))))))
 
 \f
 (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)

[-- Attachment #3: ChangeLog.trash_xdevice_fix_r1 --]
[-- Type: text/plain, Size: 219 bytes --]

2008-09-11  David De La Harpe Golden <david@harpegolden.net>

	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.