Skip to content

Commit

Permalink
Revert "FileUtils.rm* methods swallows only Errno::ENOENT when force …
Browse files Browse the repository at this point in the history
…is true"

This reverts commit fa65d67.

This caused some incompatibility problems in real-world cases.
https://bugs.ruby-lang.org/issues/18784#change-98927
https://bugs.ruby-lang.org/issues/18784#change-98967
  • Loading branch information
mame committed Nov 7, 2022
1 parent edd8d3b commit 42983c2
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 31 deletions.
17 changes: 6 additions & 11 deletions lib/fileutils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,7 @@ def mv(src, dest, force: nil, noop: nil, verbose: nil, secure: nil)
#
# Keyword arguments:
#
# - <tt>force: true</tt> - ignores raised exceptions of Errno::ENOENT
# - <tt>force: true</tt> - ignores raised exceptions of StandardError
# and its descendants.
# - <tt>noop: true</tt> - does not remove files; returns +nil+.
# - <tt>verbose: true</tt> - prints an equivalent command:
Expand Down Expand Up @@ -1248,7 +1248,7 @@ def rm_f(list, noop: nil, verbose: nil)
#
# Keyword arguments:
#
# - <tt>force: true</tt> - ignores raised exceptions of Errno::ENOENT
# - <tt>force: true</tt> - ignores raised exceptions of StandardError
# and its descendants.
# - <tt>noop: true</tt> - does not remove entries; returns +nil+.
# - <tt>secure: true</tt> - removes +src+ securely;
Expand Down Expand Up @@ -1315,7 +1315,7 @@ def rm_rf(list, noop: nil, verbose: nil, secure: nil)
# see {Avoiding the TOCTTOU Vulnerability}[rdoc-ref:FileUtils@Avoiding+the+TOCTTOU+Vulnerability].
#
# Optional argument +force+ specifies whether to ignore
# raised exceptions of Errno::ENOENT and its descendants.
# raised exceptions of StandardError and its descendants.
#
# Related: {methods for deleting}[rdoc-ref:FileUtils@Deleting].
#
Expand Down Expand Up @@ -1384,12 +1384,10 @@ def remove_entry_secure(path, force = false)
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

Expand All @@ -1415,7 +1413,7 @@ def fu_stat_identical_entry?(a, b) #:nodoc:
# should be {interpretable as a path}[rdoc-ref:FileUtils@Path+Arguments].
#
# Optional argument +force+ specifies whether to ignore
# raised exceptions of Errno::ENOENT and its descendants.
# raised exceptions of StandardError and its descendants.
#
# Related: FileUtils.remove_entry_secure.
#
Expand All @@ -1425,12 +1423,10 @@ def remove_entry(path, force = false)
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

Expand All @@ -1441,15 +1437,14 @@ def remove_entry(path, force = false)
# should be {interpretable as a path}[rdoc-ref:FileUtils@Path+Arguments].
#
# Optional argument +force+ specifies whether to ignore
# raised exceptions of Errno::ENOENT and its descendants.
# raised exceptions of StandardError and its descendants.
#
# Related: {methods for deleting}[rdoc-ref:FileUtils@Deleting].
#
def remove_file(path, force = false)
Entry_.new(path).remove_file
rescue
raise unless force
raise unless Errno::ENOENT === $!
end
module_function :remove_file

Expand All @@ -1461,7 +1456,7 @@ def remove_file(path, force = false)
# should be {interpretable as a path}[rdoc-ref:FileUtils@Path+Arguments].
#
# Optional argument +force+ specifies whether to ignore
# raised exceptions of Errno::ENOENT and its descendants.
# raised exceptions of StandardError and its descendants.
#
# Related: {methods for deleting}[rdoc-ref:FileUtils@Deleting].
#
Expand Down
20 changes: 0 additions & 20 deletions test/fileutils/test_fileutils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1822,26 +1822,6 @@ def test_rm_rf
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

Expand Down

0 comments on commit 42983c2

Please sign in to comment.