From b4a43e4f577807303b0e465a27eefff2793fe3ea Mon Sep 17 00:00:00 2001 From: David Rodriguez Date: Thu, 14 Oct 2021 12:03:57 +0200 Subject: [PATCH] [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 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") }