emacs-31 bcb83d7d2ad 2/3: Fix error handling in Tramp delete-{file, directory}

branch: emacs-31
commit bcb83d7d2ad1d34a6683fc466654437b1e25ad16
Author: Michael Albinus <[email protected]>
Commit: Michael Albinus <[email protected]>

    Fix error handling in Tramp delete-{file,directory}
    
    * lisp/net/tramp-smb.el (tramp-smb-handle-delete-directory):
    * lisp/net/tramp.el (tramp-skeleton-delete-directory): Fail if
    DIRECTORY is missing.
    (tramp-skeleton-delete-file): Don't fail if DIRECTORY is missing.
    
    * test/lisp/net/tramp-tests.el (tramp-test14-delete-directory):
    Adapt test.
---
 lisp/net/tramp-smb.el        | 57 ++++++++++++++++++++++----------------------
 lisp/net/tramp.el            | 24 ++++++++++---------
 test/lisp/net/tramp-tests.el | 10 ++++++++
 3 files changed, 51 insertions(+), 40 deletions(-)

diff --git a/lisp/net/tramp-smb.el b/lisp/net/tramp-smb.el
index ac54e47e376..6f88c7dedba 100644
--- a/lisp/net/tramp-smb.el
+++ b/lisp/net/tramp-smb.el
@@ -651,36 +651,35 @@ PRESERVE-UID-GID and PRESERVE-EXTENDED-ATTRIBUTES are completely ignored."
 (defun tramp-smb-handle-delete-directory (directory &optional recursive trash)
   "Like `delete-directory' for Tramp files."
   (tramp-skeleton-delete-directory directory recursive trash
-    (when (file-exists-p directory)
-      (when recursive
-	(mapc
-	 (lambda (file)
-	   (if (file-directory-p 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)))
+    (when recursive
+      (mapc
+       (lambda (file)
+	 (if (file-directory-p 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)))
+
+    ;; We must also flush the cache of the directory, because
+    ;; `file-attributes' reads the values from there.
+    (tramp-flush-directory-properties v localname)
+    (unless (tramp-smb-send-command
+	     v (format
+		"%s %s"
+		(if (tramp-smb-get-cifs-capabilities v)
+		    "posix_rmdir" "rmdir")
+		(tramp-smb-shell-quote-localname v)))
+      ;; Error.
+      (with-current-buffer (tramp-get-connection-buffer v)
+	(goto-char (point-min))
+	(search-forward-regexp tramp-smb-errors nil t)
+	(tramp-error v 'file-error "%s `%s'" (match-string 0) directory)))
 
-      ;; We must also flush the cache of the directory, because
-      ;; `file-attributes' reads the values from there.
-      (tramp-flush-directory-properties v localname)
-      (unless (tramp-smb-send-command
-	       v (format
-		  "%s %s"
-		  (if (tramp-smb-get-cifs-capabilities v)
-		      "posix_rmdir" "rmdir")
-		  (tramp-smb-shell-quote-localname v)))
-	;; Error.
-	(with-current-buffer (tramp-get-connection-buffer v)
-	  (goto-char (point-min))
-	  (search-forward-regexp tramp-smb-errors nil t)
-	  (tramp-error v 'file-error "%s `%s'" (match-string 0) directory)))
-
-      ;; "rmdir" does not report an error.  So we check ourselves.
-      ;; Deletion of a watched directory could be pending.
-      (when (and (not (tramp-directory-watched directory))
-		 (file-exists-p directory))
-        (tramp-error v 'file-error "`%s' not removed" directory)))))
+    ;; "rmdir" does not report an error.  So we check ourselves.
+    ;; Deletion of a watched directory could be pending.
+    (when (and (not (tramp-directory-watched directory))
+	       (file-exists-p directory))
+      (tramp-error v 'file-error "`%s' not removed" directory))))
 
 (defun tramp-smb-handle-delete-file (filename &optional trash)
   "Like `delete-file' for Tramp files."
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index c5ecbdd9675..78e1d34ebab 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -3734,14 +3734,15 @@ BODY is the backend specific code."
 		 ;; This variable exists since Emacs 30.1.
 		 (not (bound-and-true-p
 		       remote-file-name-inhibit-delete-by-moving-to-trash)))))
-       (if (and delete-by-moving-to-trash ,trash)
-	   ;; Move non-empty dir to trash only if recursive deletion was
-	   ;; requested.
-	   (if (not (or ,recursive (directory-empty-p ,directory)))
-	       (tramp-error
-		v 'file-error "Directory is not empty, not moving to trash")
-	     (move-file-to-trash ,directory))
-	 ,@body)
+       (tramp-barf-if-file-missing v ,directory
+	 (if (and delete-by-moving-to-trash ,trash)
+	     ;; Move non-empty dir to trash only if recursive deletion was
+	     ;; requested.
+	     (if (not (or ,recursive (directory-empty-p ,directory)))
+		 (tramp-error
+		  v 'file-error "Directory is not empty, not moving to trash")
+	       (move-file-to-trash ,directory))
+	   ,@body))
        (tramp-flush-directory-properties v localname))))
 
 (defmacro tramp-skeleton-delete-file (filename &optional trash &rest body)
@@ -3754,9 +3755,10 @@ BODY is the backend specific code."
 		 ;; This variable exists since Emacs 30.1.
 		 (not (bound-and-true-p
 		       remote-file-name-inhibit-delete-by-moving-to-trash)))))
-       (if (and delete-by-moving-to-trash ,trash)
-	   (move-file-to-trash ,filename)
-	 ,@body)
+       (ignore-errors
+	 (if (and delete-by-moving-to-trash ,trash)
+	     (move-file-to-trash ,filename)
+	   ,@body))
        (tramp-flush-file-properties v localname))))
 
 (defmacro tramp-skeleton-directory-files
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index b622a08dce3..6badf064efe 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -3322,6 +3322,16 @@ This tests also `file-directory-p' and `file-accessible-directory-p'."
   (dolist (quoted (if (tramp--test-expensive-test-p) '(nil t) '(nil)))
     (let* ((tmp-name1 (tramp--test-make-temp-name nil quoted))
 	   (tmp-name2 (expand-file-name "foo" tmp-name1)))
+      ;; Deleting a non-existing file should not fail.
+      (delete-file tmp-name1)
+      (delete-file tmp-name1 'trash)
+      ;; Deleting a non-existing directory should fail.
+      (should-error
+       (delete-directory tmp-name1)
+       :type 'file-missing)
+      (should-error
+       (delete-directory tmp-name1 nil 'trash)
+       :type 'file-missing)
       ;; Delete empty directory.
       (make-directory tmp-name1)
       (should (file-directory-p tmp-name1))