ruby-changes:67471
From: David <ko1@a...>
Date: Tue, 31 Aug 2021 19:07:46 +0900 (JST)
Subject: [ruby-changes:67471] f934096638 (master): [rubygems/rubygems] Make plugin installation idempotent
https://git.ruby-lang.org/ruby.git/commit/?id=f934096638 From f934096638ec5850b65e45dc7230900107441288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@r...> Date: Thu, 19 Aug 2021 11:57:16 +0200 Subject: [rubygems/rubygems] Make plugin installation idempotent The error had not be caught be specs because `bundle install` was returning a zero exit code when plugin installation errors happened. So I fixed that issue too. https://github.com/rubygems/rubygems/commit/90cde87856 --- lib/bundler/plugin.rb | 12 ++++++++---- spec/bundler/bundler/plugin_spec.rb | 7 +++++++ spec/bundler/plugins/command_spec.rb | 6 ++---- spec/bundler/plugins/install_spec.rb | 6 +++--- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/lib/bundler/plugin.rb b/lib/bundler/plugin.rb index ab4b209..158c69e 100644 --- a/lib/bundler/plugin.rb +++ b/lib/bundler/plugin.rb @@ -13,6 +13,7 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/plugin.rb#L13 class MalformattedPlugin < PluginError; end class UndefinedCommandError < PluginError; end class UnknownSourceError < PluginError; end + class PluginInstallError < PluginError; end PLUGIN_FILE_NAME = "plugins.rb".freeze @@ -38,12 +39,11 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/plugin.rb#L39 specs = Installer.new.install(names, options) save_plugins names, specs - rescue PluginError => e + rescue PluginError specs_to_delete = specs.select {|k, _v| names.include?(k) && !index.commands.values.include?(k) } specs_to_delete.each_value {|spec| Bundler.rm_rf(spec.full_gem_path) } - names_list = names.map {|name| "`#{name}`" }.join(", ") - Bundler.ui.error "Failed to install the following plugins: #{names_list}. The underlying error was: #{e.message}.\n #{e.backtrace.join("\n ")}" + raise end # Uninstalls plugins by the given names @@ -245,6 +245,8 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/plugin.rb#L245 # @param [Array<String>] names of inferred source plugins that can be ignored def save_plugins(plugins, specs, optional_plugins = []) plugins.each do |name| + next if index.installed?(name) + spec = specs[name] save_plugin(name, spec, optional_plugins.include?(name)) @@ -269,11 +271,13 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/plugin.rb#L271 # @param [Boolean] optional_plugin, removed if there is conflict with any # other plugin (used for default source plugins) # - # @raise [MalformattedPlugin] if validation or registration raises any error + # @raise [PluginInstallError] if validation or registration raises any error def save_plugin(name, spec, optional_plugin = false) validate_plugin! Pathname.new(spec.full_gem_path) installed = register_plugin(name, spec, optional_plugin) Bundler.ui.info "Installed plugin #{name}" if installed + rescue PluginError => e + raise PluginInstallError, "Failed to install plugin `#{spec.name}`, due to #{e.class} (#{e.message})" end # Runs the plugins.rb file in an isolated namespace, records the plugin diff --git a/spec/bundler/bundler/plugin_spec.rb b/spec/bundler/bundler/plugin_spec.rb index d1a05bb..8a1a6cd 100644 --- a/spec/bundler/bundler/plugin_spec.rb +++ b/spec/bundler/bundler/plugin_spec.rb @@ -65,6 +65,8 @@ RSpec.describe Bundler::Plugin do https://github.com/ruby/ruby/blob/trunk/spec/bundler/bundler/plugin_spec.rb#L65 end it "passes the name and options to installer" do + allow(index).to receive(:installed?). + with("new-plugin") allow(installer).to receive(:install).with(["new-plugin"], opts) do { "new-plugin" => spec } end.once @@ -73,6 +75,8 @@ RSpec.describe Bundler::Plugin do https://github.com/ruby/ruby/blob/trunk/spec/bundler/bundler/plugin_spec.rb#L75 end it "validates the installed plugin" do + allow(index).to receive(:installed?). + with("new-plugin") allow(subject). to receive(:validate_plugin!).with(lib_path("new-plugin")).once @@ -80,6 +84,8 @@ RSpec.describe Bundler::Plugin do https://github.com/ruby/ruby/blob/trunk/spec/bundler/bundler/plugin_spec.rb#L84 end it "registers the plugin with index" do + allow(index).to receive(:installed?). + with("new-plugin") allow(index).to receive(:register_plugin). with("new-plugin", lib_path("new-plugin").to_s, [lib_path("new-plugin").join("lib").to_s], []).once subject.install ["new-plugin"], opts @@ -96,6 +102,7 @@ RSpec.describe Bundler::Plugin do https://github.com/ruby/ruby/blob/trunk/spec/bundler/bundler/plugin_spec.rb#L102 end.once allow(subject).to receive(:validate_plugin!).twice + allow(index).to receive(:installed?).twice allow(index).to receive(:register_plugin).twice subject.install ["new-plugin", "another-plugin"], opts end diff --git a/spec/bundler/plugins/command_spec.rb b/spec/bundler/plugins/command_spec.rb index 4567a39..3a7adf4 100644 --- a/spec/bundler/plugins/command_spec.rb +++ b/spec/bundler/plugins/command_spec.rb @@ -69,12 +69,10 @@ RSpec.describe "command plugins" do https://github.com/ruby/ruby/blob/trunk/spec/bundler/plugins/command_spec.rb#L69 end end - bundle "plugin install copycat --source #{file_uri_for(gem_repo2)}" + bundle "plugin install copycat --source #{file_uri_for(gem_repo2)}", :raise_on_error => false expect(out).not_to include("Installed plugin copycat") - expect(err).to include("Failed to install the following plugins: `copycat`") - - expect(err).to include("Command(s) `mahcommand` declared by copycat are already registered.") + expect(err).to include("Failed to install plugin `copycat`, due to Bundler::Plugin::Index::CommandConflict (Command(s) `mahcommand` declared by copycat are already registered.)") end end diff --git a/spec/bundler/plugins/install_spec.rb b/spec/bundler/plugins/install_spec.rb index c719e85..b382d3c 100644 --- a/spec/bundler/plugins/install_spec.rb +++ b/spec/bundler/plugins/install_spec.rb @@ -109,9 +109,9 @@ RSpec.describe "bundler plugin install" do https://github.com/ruby/ruby/blob/trunk/spec/bundler/plugins/install_spec.rb#L109 build_gem "charlie" end - bundle "plugin install charlie --source #{file_uri_for(gem_repo2)}" + bundle "plugin install charlie --source #{file_uri_for(gem_repo2)}", :raise_on_error => false - expect(err).to include("Failed to install the following plugins: `charlie`. The underlying error was: plugins.rb was not found") + expect(err).to include("Failed to install plugin `charlie`, due to Bundler::Plugin::MalformattedPlugin (plugins.rb was not found in the plugin.)") expect(global_plugin_gem("charlie-1.0")).not_to be_directory @@ -128,7 +128,7 @@ RSpec.describe "bundler plugin install" do https://github.com/ruby/ruby/blob/trunk/spec/bundler/plugins/install_spec.rb#L128 end end - bundle "plugin install chaplin --source #{file_uri_for(gem_repo2)}" + bundle "plugin install chaplin --source #{file_uri_for(gem_repo2)}", :raise_on_error => false expect(global_plugin_gem("chaplin-1.0")).not_to be_directory -- cgit v1.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/