bug#5436: 23.1.91; Deleting directories causes unusable file layout in freedesktop trashcan

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

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

#$%? missed a "not" in one of the comments. Only noticed just after 
send.  Further slight comments improvement attached. Sorry.





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

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: david@harpegolden.net-20100124202459-k128rxdh6lf7m2jh
# target_branch: http://bzr.savannah.gnu.org/r/emacs/trunk/
# testament_sha1: 148945194f5afb56b9c652a4a14e1a222aabe40b
# timestamp: 2010-01-24 20:25:06 +0000
# base_revision_id: rgm@gnu.org-20100123232525-hl30xzix3wevh4bn
# 
# Begin patch
=== modified file 'lisp/files.el'
--- lisp/files.el	2010-01-17 02:25:53 +0000
+++ lisp/files.el	2010-01-24 20:24:59 +0000
@@ -4667,19 +4667,37 @@
   (let ((handler (find-file-name-handler directory 'delete-directory)))
     (if handler
 	(funcall handler 'delete-directory directory recursive)
-      (if (and recursive (not (file-symlink-p directory)))
-	  (mapc
-	   (lambda (file)
-	     ;; This test is equivalent to
-	     ;; (and (file-directory-p fn) (not (file-symlink-p fn)))
-	     ;; but more efficient
-	     (if (eq t (car (file-attributes file)))
-		 (delete-directory file recursive)
-	       (delete-file file)))
-	   ;; We do not want to delete "." and "..".
-	   (directory-files
-	    directory 'full directory-files-no-dot-files-regexp)))
-      (delete-directory-internal directory))))
+      (if delete-by-moving-to-trash
+          ;; To mimic the non- delete-by-moving-to-trash case, which
+          ;; will (sensibly) fail (in delete-directory-internal) if
+          ;; the directory is not empty and recursion wasn't
+          ;; requested, only move non-empty directories to trash if
+          ;; recursive deletion was requested.  As move-file-to-trash
+          ;; moves directories, empty or otherwise, into the trash as
+          ;; a unit, we do not need to recurse ourselves here.
+          (if (and (not recursive)
+                   ;; Check if directory is not empty apart from "." and "..".
+                   (directory-files
+                    directory 'full directory-files-no-dot-files-regexp))
+              (error "Directory is not empty, not moving to trash.")
+            ;; Either recursive, or not recursive but directory is
+            ;; empty, so we can move directory to trash.
+            (move-file-to-trash directory))
+        ;; Recurse ourselves since we're not moving to trash.
+        (progn
+          (if (and recursive (not (file-symlink-p directory)))
+              (mapc
+               (lambda (file)
+                 ;; This test is equivalent to
+                 ;; (and (file-directory-p fn) (not (file-symlink-p fn)))
+                 ;; but more efficient
+                 (if (eq t (car (file-attributes file)))
+                     (delete-directory file recursive)
+                   (delete-file file)))
+               ;; We do not want to delete "." and "..".
+               (directory-files
+                directory 'full directory-files-no-dot-files-regexp)))
+          (delete-directory-internal directory))))))
 
 (defun copy-directory (directory newname &optional keep-time parents)
   "Copy DIRECTORY to NEWNAME.  Both args must be strings.
@@ -6170,7 +6188,8 @@
   "Move the file (or directory) named FILENAME to the trash.
 When `delete-by-moving-to-trash' is non-nil, this function is
 called by `delete-file' and `delete-directory' instead of
-deleting files outright.
+deleting files outright.  If FILENAME is a directory, it may
+be empty or non-empty.
 
 If the function `system-move-file-to-trash' is defined, call it
  with FILENAME as an argument.

=== modified file 'src/fileio.c'
--- src/fileio.c	2010-01-13 04:33:42 +0000
+++ src/fileio.c	2010-01-24 20:11:15 +0000
@@ -215,6 +215,12 @@
 /* Lisp function for moving files to trash.  */
 Lisp_Object Qmove_file_to_trash;
 
+/* Lisp function for recursively copying directories.  */
+Lisp_Object Qcopy_directory;
+
+/* Lisp function for recursively deleting directories.  */
+Lisp_Object Qdelete_directory;
+
 extern Lisp_Object Vuser_login_name;
 
 #ifdef WINDOWSNT
@@ -2241,7 +2247,11 @@
       && (NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname))))
 #endif
       )
-    newname = Fexpand_file_name (Ffile_name_nondirectory (file), newname);
+
+    if (!NILP (Ffile_directory_p (file)))
+      newname = Fexpand_file_name (Ffile_name_nondirectory (Fdirectory_file_name (file)), newname);
+    else
+      newname = Fexpand_file_name (Ffile_name_nondirectory (file), newname);
   else
     newname = Fexpand_file_name (newname, Qnil);
 
@@ -2279,15 +2289,23 @@
                                  NILP (ok_if_already_exists) ? Qnil : Qt);
           else
 #endif
-	    Fcopy_file (file, newname,
-			/* We have already prompted if it was an integer,
-			   so don't have copy-file prompt again.  */
-			NILP (ok_if_already_exists) ? Qnil : Qt,
-			Qt, Qt);
+            {
+              if ( Ffile_directory_p (file) )
+                call4 (Qcopy_directory, file, newname, Qt, Qnil);
+              else
+                Fcopy_file (file, newname,
+                            /* We have already prompted if it was an integer,
+                               so don't have copy-file prompt again.  */
+                            NILP (ok_if_already_exists) ? Qnil : Qt,
+                            Qt, Qt);
+            }
 
 	  count = SPECPDL_INDEX ();
 	  specbind (Qdelete_by_moving_to_trash, Qnil);
-	  Fdelete_file (file);
+          if ( Ffile_directory_p (file) )
+            call2 (Qdelete_directory, file, Qt);
+          else
+            Fdelete_file (file);
 	  unbind_to (count, Qnil);
 	}
       else
@@ -5727,6 +5745,10 @@
   Qdelete_by_moving_to_trash = intern_c_string ("delete-by-moving-to-trash");
   Qmove_file_to_trash = intern_c_string ("move-file-to-trash");
   staticpro (&Qmove_file_to_trash);
+  Qcopy_directory = intern_c_string ("copy-directory");
+  staticpro (&Qcopy_directory);
+  Qdelete_directory = intern_c_string ("delete-directory");
+  staticpro (&Qdelete_directory);
 
   defsubr (&Sfind_file_name_handler);
   defsubr (&Sfile_name_directory);

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWbRXlCwACo7/gFVf6Eh59///
/+f+gL////pgD+X193OoN8cK4oUAAAJlN3Kpu93vWKLb0DoGgAuyugJJIKeQYk2gyp6maZpqZJsk
ekyZABoA0BiaBoJJACYgQIhGoG1NAA9QDQAAAAD0hwNBpkNNGhhAyGhgjQ0yaNAMgxAAGgkKQmpG
00j0amaI0AZBiNqfqj0mahoAADQACSpqm1NND0gxNG9UNDIwEBhNGTJkGIGQaYIwiiIAiYCNTTCG
mUzQRp6gymgAyAA0NNNExgYBGfdarLlqqO4BLUdkYwwPrMiV5x9/gW7bMxn5xud5nTSkLjfczpJr
VW/1dWmyUfOcc92DrhLH0vXutnXOGnNNDEPTPI8yPbn+UPFbWhU183lhiZz6mrBw9jNV1F3LHGFP
FbyHVxuq5zhmfhki3wmUBZCBwL6F9ZB6mc16u8YtIg5aIfkrQlNoP00giiOlURBIIrgJJD4SQME4
H6Rfpb7IRvL7GkZ109GA8RCmMm/LujrURYGCSWIingxQSFrGbEBfC+VCgFiEwEEMQDBDEAdnnRN+
/n6adG3BNqVb+susrYwy2PXXrq0ikWwWxWBjElIbsbmjVytLrrrCkNmfNwpqTotF7rNZ64MI7B4O
h8XXseEN6T8IffKou6U58Kp+MJ8UR+Y7hvIM74KRceUmLjAk6ns2NPnJ3SVHuhTl5A/98HX8V9JE
xrkAfabmY09nl2GIGrsD6Wbo8Z0oeWJPkDksDzQyH+5osQCY5Zm1pnBWeLQgeZggfqGvRygcGO0t
GtONOONmmuwtHXAmR+EnUIEbxZxomZ84kh9hPIkzYtTST6Im0a1SGdbJ6D/kZAsXp8Ly5nvRstHU
YPBNcj9rydfa2ce5VmN3In0+qVeWbQmIESuhxVlyi1o/UclIy2hbiSq5HJv37NO8CbQoxa5QoYDG
EAXC7uDK6dtgFR4LqFle/RG+w7IbMid+MervYKjaMfQvEe20VykscZUx6kKMs2iUahSmeblwu4sA
rLbdqSNQbNJiEkGYSEnZM2Tp9X3voQyHY+z6PI4gDcD6oewHXoTl2X01Qk8hqClSBgMjBqkpQ9Qn
zgEvclhiVEPCyKEpgZQRDks+0SnjMS05Kc2Bmc3JQgsb/Uy+3PUa9mtTiZA6ER3tflHsDN4tgV7F
zJJJJISTYL+5r8/dpqYVXpZ6AbBWsxM6qyAdtZaXATcSiMAUtEPKe4rREe6AVB7JApvEW9+M0ken
UwWoKjFiYjSroJBvVwj6xpKgmhAsYgp1EYIFEgkMYigmyUUi5AhCeHVOTOJnCNEYzb1WBTWJanAk
ZBlSC2gShmWSZNqlR6UsRHtjYaBNmpPid4hIFu1xJY/W+iKo1ftQckCwjjr7BQc+Mzu3bLXmiRDD
CCg7tqaJDcgEAJswz/JG6N8aAot4HHiGdxic8T38KG3hPPlSQSOJHwPEDsnPQ73EUE0Dfd9JnA5p
eekxu/R58noqtuGy7DZyvNs+XnVAkHASBw5bFColMKlo0TkgCyCe7x616TeLfI5UctFQgm6lDprM
AUCkhcGt57vTy6ZHjDETzhdHAo+HyNyWgy5dFLmFW6FDykgIa9ZVhE3565+aKDy+X89WtDZTNRpR
JANp0w/WjjLLuouww4QMlL8zi7LvC+o0yVJzi49E96VDnL6nbrAgXvR4jS5NppIitBjZp8PEUNVE
xy/Xu62BH/bCPctc7bYWX5N9cB1HOonMjpQ7IuJLewjABHo4B7/Q+dCAgbGthA4c1N7vJR1TyNzS
VaswgynJO6jYkbpzJKI1ovuRtPPmjNaRx1LZ5JSEbJEVoOJ+tckJStIfzc1PfEbCNMRJDcuG12vD
Ek4uCoovdrDFXRESZxVTjRy6Wm5ohwFmBmBmBi9RUPPnwYMuXYR5Ts8112midzY12HiNizi5AzuN
TsKR4ngfJamtisivG5OIcDxw/ufcEE1oQ98QZ41IY4fNwsRmEjV0aBmuijalCtYRQ4fncgLh1Ch5
yK1ODRTKuuMpjGGEYko2grjdhEOqKVZIY0qbkNB7+ngXyAXURHjr0cUy5umRF4bh3sZKTjLqQl2P
Uz+MxeVuPTmjeD7JgmdqnVFADHr84kOGGwU7znOXMhC6PyViMOzMtkxrOI40JrfylrrbDryoVknY
QRJiGHDaF3wMxeZnCdMvKvkjujdDq8IdTB7hGMjzpyYZtKSkesFUFIXWnHDNh3ClAREWwg4JujjX
Igtzuble0asaTB9yGHMdzUqarx0oQnqbDloaRk/qIeaUHSNCu8jesCLwVhKCeo3EYVyvySUK7kkM
U2pyIlKpDekpFyToSgNUkAhLktZtUwQLr0hOwyAMokZMlxkyVXXOOqyMY2aSN1IYsiNUWkG5jbbY
24UUTTQdRFwrEdMtMm6ryELb4ysx0CQjEEQn1fAqeoYTT9pb9pV+v6+XLEsGExlR8y+ocaGgsPkY
D3Zs39NFW/DQISEQq7IBmN42DZxDYxtpjGPGEKIoLnCykSSqXFwVgvaU2RcH+0+l7T6hUAewqEj2
+0+1UaOo4b43xJ+115+8RxIksjxytFltkBKXCdGhAWt7mKSXyeq8iwbh9VwAeNKlmaS+QgjmD/X7
anJbOwS/mvUb06ERk3XGJEXYAk43QmqE0Ee66wP8p0IjJYfm83nMZDlfMj1iEx8l+gRERZGfILBQ
7HsDxtwWIIJrUj+HVEcBKSaLrwf9nm45uhMCnlEPE9P6DkASyXpYFiS/WXx6OvESlNFL5FdpYipt
xiMxM8WL3fn4uN/dYtYMfVj7438YeG++9IONhKbM+hPfJ9ye8ktIuMAhQCEokSwRzwebnI5fSlmr
Z07cL9nDKh0yX6UsqXmBkFfvfUAlYkiWB/cFcSAiF/ePHqr948DeSGQcQyDiGRMiGQc+yNDOwvDo
23tQNejGONQ00CIeDOYQlAkc4J566JspI0YAFPjaEoddTBDEjJsIKNDWQzuS2x6YanzCdSfs0iG0
K1qZtQsAaXBOjWh5cLKDhNbz/Zr7ZscjCfH4bcbnh5FQK/RsQmMkYGYZhhiY4HucZAZoQQ9KNhTG
aO7XRq01TmrDwFcDQCMIjTB2o7V10kjPyD5hdQ8ldOqp7ShbrNH0DzDXhC8UhlJTmYKyN6BAkhUz
8fiSmrZx0lp9hciP3wjoL8JPdfRE3ewu8DcW6a7RDqILE1zxt5rE2JboOqe7WZnQiPXZRQCuCJY5
JZJEbc+zQz6Lsc+xg6EQiNpJHT66ajKHO+mEiILhF5ws3c1FLlfISAeyDDlu8xj2daVsM6Bh1pU0
GegtiaE3shAfISJKd9icLpT1nlRG5U0el1bUSwHiew3HtDNAzT+BqA5hkDq2My28ZKC5fBwRb5ni
I6I9OjubyId5yILOlQjVkPEHvC2veRIwO5KHs+gUfuP2Oq5rskaCChYZm3Qpndzh6PpGDV6AggK0
koQQwEp3j0tQqxQMprJpqwrGFKJpqgdFiMKGfgiIFEe487KbDrBgU1hsLH7vRDVEc/DwO98cISSk
s+t7fKNlgpgiPo2JlKbf+FqZVtIPX+HF9Z6TpACEzS4d5lHr2/HTzeNLixM08iBKIymSb+F2C3ps
Pl1eHYFQNBgpeezryGNQgPLoE245Wm7oQOTMv3e+Yhlgh0mIbyDLuToTFOKmCaIgUx2qnvhV6oTI
Tn3FqJ+kNQQcQQhhPbOIwozypGsRyLEQiHxWoeARSIyHENBKCeZ0kj4A0TvPVzifT50Dp+bzzbqg
7ZJijSjIzOqgJU6WDaiQlATz6BFuDrgFDsxkQtVPnHp6D2s22+IlO1PsomnZYGTRIOznkI3a36B9
Ybu8HVyZ7QYFdpBuTapbVPIZrZtsTKF6nBLFLQ1o9gnJnRKzCWsHEg8xANSxEZU1kJB0D8QNdAxQ
IPkEuxTUDAiAYZwPEWjojfQhez3t1gXkgwox8RMEZFTB7hKnwvXUHMSBEbBDY64UxAhXGZEODKuk
Tf3cCOU/D5u0rFZWJuKLepdTYIbRKWxrPwTQiNDvPphJDEC4xSUggAgPam88patwNJgNgciBZ0Mr
RT6vZQFbsQczA5dEhkr6Od3vAPcQFidwkl1Q7UkYAOhtU0pMIjmnTr67K8DnxDFLqYoFoPVuEMhN
wmQnwBsbSkBEyG+WNMr8BIqgdqcvMCb0rZ+IiwLMAR2ix1cTaVNKjBvTvSVM2jlwy57FkIBqFTOs
9BAWwPi8WYi370CTeJRMUyk4DItkCpWYqwSEkkZkHtoElYmKkHCASZGU3uP/tBcXybtmqqunRNpt
sbOWKiNbTtM5CGkViwGNg2m02maevu+OpUR9dOOsgThiBhAaDh2T06wnQv5G5dusdhQt4YW2jiRQ
7xI5ABoVTwK3pYlR8oOC5CVshCqQuZYWm9IDPkR4pA5aDJGYujgRW4aDbg1BFVcZHCUkBIAghkWB
IWlYggIghwSt+guKMFZAL0yPDkQsgggUdHGClY+I3oyJaGC4TXpxhkKatPF4R8YO7IQi0iCLIiGE
7pDYrQ4msLVFoMSieFNKHgwcSwHSIX4ibxCGwYLFHS5VA4M3JNEZkLigUgGjzMjgwNCFolhBKYCS
aRPSJuMbF6HQngfufrJkiJP/i7kinChIWivKFgA=