[前][次][番号順一覧][スレッド一覧]

ruby-changes:71979

From: David <ko1@a...>
Date: Fri, 27 May 2022 17:26:35 +0900 (JST)
Subject: [ruby-changes:71979] 6778d321a7 (master): [rubygems/rubygems] Show better error when previous installation fails to be removed

https://git.ruby-lang.org/ruby.git/commit/?id=6778d321a7

From 6778d321a7fa0ec56e3e02b6f3739a035c7ef41a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@r...>
Date: Wed, 25 May 2022 11:03:33 +0200
Subject: [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
---
 lib/bundler/errors.rb                 | 16 ++++++++++++----
 lib/bundler/rubygems_gem_installer.rb | 11 +++--------
 spec/bundler/commands/install_spec.rb |  8 ++------
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/lib/bundler/errors.rb b/lib/bundler/errors.rb
index 9ad7460e58..0bc1a860df 100644
--- a/lib/bundler/errors.rb
+++ b/lib/bundler/errors.rb
@@ -79,10 +79,6 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/errors.rb#L79
       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 https://github.com/ruby/ruby/blob/trunk/lib/bundler/errors.rb#L168
 
     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
diff --git a/lib/bundler/rubygems_gem_installer.rb b/lib/bundler/rubygems_gem_installer.rb
index 452583617b..87b9772c27 100644
--- a/lib/bundler/rubygems_gem_installer.rb
+++ b/lib/bundler/rubygems_gem_installer.rb
@@ -93,14 +93,9 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/rubygems_gem_installer.rb#L93
     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)
diff --git a/spec/bundler/commands/install_spec.rb b/spec/bundler/commands/install_spec.rb
index f189a70b10..07585237e3 100644
--- a/spec/bundler/commands/install_spec.rb
+++ b/spec/bundler/commands/install_spec.rb
@@ -755,12 +755,8 @@ RSpec.describe "bundle install with gem sources" do https://github.com/ruby/ruby/blob/trunk/spec/bundler/commands/install_spec.rb#L755
       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
 
-- 
cgit v1.2.1


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]