--- Begin Message ---
Subject: |
trash - runaway recursion for cross-device delete-file ops |
Date: |
Fri, 12 Sep 2008 03:07:31 +0100 |
User-agent: |
Mozilla-Thunderbird 2.0.0.16 (X11/20080724) |
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.
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)
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.
--- End Message ---
--- Begin Message ---
Subject: |
Re: bug#964: trash - runaway recursion for cross-device delete-file ops |
Date: |
Sat, 20 Sep 2008 17:40:39 -0400 |
User-agent: |
Gnus (www.gnus.org), GNU Emacs (www.gnu.org/software/emacs/) |
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.
--- End Message ---