[rubygems/rubygems] Show proper error when previous installation of gem can't be deleted

Instead of showing the bug report template with an error at a random
place.

https://github.com/rubygems/rubygems/commit/882ad3ab57

3 files changed, 58 insertions, 2 deletions

diff --git a/lib/bundler/errors.rb b/lib/bundler/errors.rb
index 59898cd252..9ad7460e58 100644
--- a/lib/bundler/errors.rb
+++ b/lib/bundler/errors.rb
@@ -79,6 +79,10 @@ 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
diff --git a/lib/bundler/rubygems_gem_installer.rb b/lib/bundler/rubygems_gem_installer.rb
index 39c9031c4a..bb9f1cb3f5 100644
--- a/lib/bundler/rubygems_gem_installer.rb
+++ b/lib/bundler/rubygems_gem_installer.rb
@@ -16,8 +16,8 @@ module Bundler
       spec.loaded_from = spec_file
 
       # Completely remove any previous gem files
-      FileUtils.rm_rf gem_dir
-      FileUtils.rm_rf spec.extension_dir
+      strict_rm_rf gem_dir
+      strict_rm_rf spec.extension_dir
 
       SharedHelpers.filesystem_access(gem_dir, :create) do
         FileUtils.mkdir_p gem_dir, :mode => 0o755
@@ -92,6 +92,17 @@ 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)
+    end
+
     def validate_bundler_checksum(checksum)
       return true if Bundler.settings[:disable_checksum_validation]
       return true unless checksum
diff --git a/spec/bundler/commands/install_spec.rb b/spec/bundler/commands/install_spec.rb
index a477369e4c..2c7f0360e6 100644
--- a/spec/bundler/commands/install_spec.rb
+++ b/spec/bundler/commands/install_spec.rb
@@ -708,6 +708,47 @@ RSpec.describe "bundle install with gem sources" do
     end
   end
 
+  describe "when the path of a specific gem is not writable", :permissions do
+    let(:gems_path) { bundled_app("vendor/#{Bundler.ruby_scope}/gems") }
+    let(:foo_path) { gems_path.join("foo-1.0.0") }
+
+    before do
+      build_repo4 do
+        build_gem "foo", "1.0.0" do |s|
+          s.write "CHANGELOG.md", "foo"
+        end
+      end
+
+      gemfile <<-G
+        source "#{file_uri_for(gem_repo4)}"
+        gem 'foo'
+      G
+    end
+
+    it "should display a proper message to explain the problem" do
+      bundle "config set --local path vendor"
+      bundle :install
+      expect(out).to include("Bundle complete!")
+      expect(err).to be_empty
+
+      FileUtils.chmod("-x", foo_path)
+
+      begin
+        bundle "install --redownload", :raise_on_error => false
+      ensure
+        FileUtils.chmod("+x", foo_path)
+      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}."
+      )
+    end
+  end
+
   describe "when bundle cache path does not have write access", :permissions do
     let(:cache_path) { bundled_app("vendor/#{Bundler.ruby_scope}/cache") }