[rubygems/rubygems] Show better error when previous installation fails to be removed

Instead of guessing on the culprit.

We actually have a helper, `Bundler.rm_rf`, with exactly the behavior
that we want:

* Allow the passed folder to not exist.
* No exception swallowing other than that.

https://github.com/rubygems/rubygems/commit/5fa3e6f04a
This commit is contained in:
David Rodríguez 2022-05-25 11:03:33 +02:00 committed by git
parent 45177129a7
commit 6778d321a7
3 changed files with 17 additions and 18 deletions

View File

@ -79,10 +79,6 @@ module Bundler
case @permission_type
when :create
"executable permissions for all parent directories and write permissions for `#{parent_folder}`"
when :delete
permissions = "executable permissions for all parent directories and write permissions for `#{parent_folder}`"
permissions += ", and the same thing for all subdirectories inside #{@path}" if File.directory?(@path)
permissions
else
"#{@permission_type} permissions for that path"
end
@ -172,4 +168,16 @@ module Bundler
status_code(32)
end
class DirectoryRemovalError < BundlerError
def initialize(orig_exception, msg)
full_message = "#{msg}.\n" \
"The underlying error was #{orig_exception.class}: #{orig_exception.message}, with backtrace:\n" \
" #{orig_exception.backtrace.join("\n ")}\n\n" \
"Bundler Error Backtrace:"
super(full_message)
end
status_code(36)
end
end

View File

@ -93,14 +93,9 @@ module Bundler
private
def strict_rm_rf(dir)
# FileUtils.rm_rf should probably rise in case of permission issues like
# `rm -rf` does. However, it fails to delete the folder silently due to
# https://github.com/ruby/fileutils/issues/57. It should probably be fixed
# inside `fileutils` but for now I`m checking whether the folder was
# removed after it completes, and raising otherwise.
FileUtils.rm_rf dir
raise PermissionError.new(dir, :delete) if File.directory?(dir)
Bundler.rm_rf dir
rescue Errno::ENOTEMPTY => e
raise DirectoryRemovalError.new(e.cause, "Could not delete previous installation of `#{dir}`")
end
def validate_bundler_checksum(checksum)

View File

@ -755,12 +755,8 @@ RSpec.describe "bundle install with gem sources" do
end
expect(err).not_to include("ERROR REPORT TEMPLATE")
expect(err).to include(
"There was an error while trying to delete `#{foo_path}`. " \
"It is likely that you need to grant executable permissions for all parent directories " \
"and write permissions for `#{gems_path}`, and the same thing for all subdirectories inside #{foo_path}."
)
expect(err).to include("Could not delete previous installation of `#{foo_path}`.")
expect(err).to include("The underlying error was Errno::EACCES")
end
end