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

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/

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