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

ruby-changes:69431

From: David <ko1@a...>
Date: Mon, 25 Oct 2021 20:55:12 +0900 (JST)
Subject: [ruby-changes:69431] b4a43e4f57 (master): [rubygems/rubygems] Show proper error when previous installation of gem can't be deleted

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

From b4a43e4f577807303b0e465a27eefff2793fe3ea Mon Sep 17 00:00:00 2001
From: David Rodriguez <deivid.rodriguez@r...>
Date: Thu, 14 Oct 2021 12:03:57 +0200
Subject: [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
---
 lib/bundler/errors.rb                 |  4 ++++
 lib/bundler/rubygems_gem_installer.rb | 15 +++++++++++--
 spec/bundler/commands/install_spec.rb | 41 +++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/lib/bundler/errors.rb b/lib/bundler/errors.rb
index 59898cd2526..9ad7460e585 100644
--- a/lib/bundler/errors.rb
+++ b/lib/bundler/errors.rb
@@ -79,6 +79,10 @@ 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
diff --git a/lib/bundler/rubygems_gem_installer.rb b/lib/bundler/rubygems_gem_installer.rb
index 39c9031c4a8..bb9f1cb3f5c 100644
--- a/lib/bundler/rubygems_gem_installer.rb
+++ b/lib/bundler/rubygems_gem_installer.rb
@@ -16,8 +16,8 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/rubygems_gem_installer.rb#L16
       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 https://github.com/ruby/ruby/blob/trunk/lib/bundler/rubygems_gem_installer.rb#L92
 
     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 a477369e4cb..2c7f0360e6c 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 https://github.com/ruby/ruby/blob/trunk/spec/bundler/commands/install_spec.rb#L708
     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") }
 
-- 
cgit v1.2.1


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

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