From 983115cf3c8f75b1afbe3274f02c1529e1ce3a81 Mon Sep 17 00:00:00 2001
From: Yusuke Endoh <mame@ruby-lang.org>
Date: Tue, 26 Jul 2022 21:31:27 +0900
Subject: [PATCH] [ruby/fileutils] FileUtils.rm* methods swallows only
 Errno::ENOENT when force is true
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

... instead of any StandardError.

To behave like the standard `rm` command, it should only ignore
exceptions about not existing files, not every exception. This should
make debugging some errors easier, because the expectation is that `rm
-rf` will succeed if and only if, all given files (previously existent
or not) are removed. However, due to this exception swallowing, this is
not always the case.

From the `rm` man page

> COMPATIBILITY
>
> The rm utility differs from historical implementations in that the -f
> option only masks attempts to remove non-existent files instead of
> masking a large variety of errors.

https://github.com/ruby/fileutils/commit/fa65d676ec

Co-Authored-By: David Rodríguez <deivid.rodriguez@riseup.net>
---
 lib/fileutils.rb                 | 17 +++++++++++------
 test/fileutils/test_fileutils.rb | 20 ++++++++++++++++++++
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/lib/fileutils.rb b/lib/fileutils.rb
index 4ba7d18a5d..74bb904e28 100644
--- a/lib/fileutils.rb
+++ b/lib/fileutils.rb
@@ -1165,7 +1165,7 @@ module FileUtils
   #
   # Keyword arguments:
   #
-  # - <tt>force: true</tt> - ignores raised exceptions of StandardError
+  # - <tt>force: true</tt> - ignores raised exceptions of Errno::ENOENT
   #   and its descendants.
   # - <tt>noop: true</tt> - does not remove files; returns +nil+.
   # - <tt>verbose: true</tt> - prints an equivalent command:
@@ -1248,7 +1248,7 @@ module FileUtils
   #
   # Keyword arguments:
   #
-  # - <tt>force: true</tt> - ignores raised exceptions of StandardError
+  # - <tt>force: true</tt> - ignores raised exceptions of Errno::ENOENT
   #   and its descendants.
   # - <tt>noop: true</tt> - does not remove entries; returns +nil+.
   # - <tt>secure: true</tt> - removes +src+ securely;
@@ -1315,7 +1315,7 @@ module FileUtils
   # see {Avoiding the TOCTTOU Vulnerability}[rdoc-ref:FileUtils@Avoiding+the+TOCTTOU+Vulnerability].
   #
   # Optional argument +force+ specifies whether to ignore
-  # raised exceptions of StandardError and its descendants.
+  # raised exceptions of Errno::ENOENT and its descendants.
   #
   # Related: {methods for deleting}[rdoc-ref:FileUtils@Deleting].
   #
@@ -1384,10 +1384,12 @@ module FileUtils
         ent.remove
       rescue
         raise unless force
+        raise unless Errno::ENOENT === $!
       end
     end
   rescue
     raise unless force
+    raise unless Errno::ENOENT === $!
   end
   module_function :remove_entry_secure
 
@@ -1413,7 +1415,7 @@ module FileUtils
   # should be {interpretable as a path}[rdoc-ref:FileUtils@Path+Arguments].
   #
   # Optional argument +force+ specifies whether to ignore
-  # raised exceptions of StandardError and its descendants.
+  # raised exceptions of Errno::ENOENT and its descendants.
   #
   # Related: FileUtils.remove_entry_secure.
   #
@@ -1423,10 +1425,12 @@ module FileUtils
         ent.remove
       rescue
         raise unless force
+        raise unless Errno::ENOENT === $!
       end
     end
   rescue
     raise unless force
+    raise unless Errno::ENOENT === $!
   end
   module_function :remove_entry
 
@@ -1437,7 +1441,7 @@ module FileUtils
   # should be {interpretable as a path}[rdoc-ref:FileUtils@Path+Arguments].
   #
   # Optional argument +force+ specifies whether to ignore
-  # raised exceptions of StandardError and its descendants.
+  # raised exceptions of Errno::ENOENT and its descendants.
   #
   # Related: {methods for deleting}[rdoc-ref:FileUtils@Deleting].
   #
@@ -1445,6 +1449,7 @@ module FileUtils
     Entry_.new(path).remove_file
   rescue
     raise unless force
+    raise unless Errno::ENOENT === $!
   end
   module_function :remove_file
 
@@ -1456,7 +1461,7 @@ module FileUtils
   # should be {interpretable as a path}[rdoc-ref:FileUtils@Path+Arguments].
   #
   # Optional argument +force+ specifies whether to ignore
-  # raised exceptions of StandardError and its descendants.
+  # raised exceptions of Errno::ENOENT and its descendants.
   #
   # Related: {methods for deleting}[rdoc-ref:FileUtils@Deleting].
   #
diff --git a/test/fileutils/test_fileutils.rb b/test/fileutils/test_fileutils.rb
index 05ba8d184a..bce7271a3b 100644
--- a/test/fileutils/test_fileutils.rb
+++ b/test/fileutils/test_fileutils.rb
@@ -1822,6 +1822,26 @@ cd -
     assert_file_not_exist 'tmpdatadir'
   end
 
+  def test_rm_rf_no_permissions
+    check_singleton :rm_rf
+
+    return if /mswin|mingw/ =~ RUBY_PLATFORM
+
+    mkdir 'tmpdatadir'
+    touch 'tmpdatadir/tmpdata'
+    chmod "-x", 'tmpdatadir'
+
+    begin
+      assert_raise Errno::EACCES do
+        rm_rf 'tmpdatadir'
+      end
+
+      assert_file_exist 'tmpdatadir'
+    ensure
+      chmod "+x", 'tmpdatadir'
+    end
+  end
+
   def test_rmdir
     check_singleton :rmdir