Auto merge of #5520 - kerrizor:kerrizor/stop-bundler-from-uninstalling-itself, r=segiddins

Only uninstall plugins that we attempted to install

Previously, when encountering any kind of exception when installing or
registering a plugin, we would uninstall _everything_ in `specs`, which included
not just every installed plugin but Bundler itself 😱

This patch prevents this from happening, by only attempting to delete code
that is a) part of the installation attempt and b) NOT a currently registered
command. This second restriction is to avoid a broken state,  in the case
where we attempt to install the same plugin twice. In that case, if we uninstall
the code for the plugin, but leave the command registered, this causes
attempts to reinstall the plugin to fail, as the command is still in the
registry, and attempts to USE the command will fail because the code behind
it has been destroyed.

Paired with @roseaboveit

3 files changed, 19 insertions, 1 deletions

diff --git a/lib/bundler/plugin.rb b/lib/bundler/plugin.rb
index e700ae71f1..66f485ef8e 100644
--- a/lib/bundler/plugin.rb
+++ b/lib/bundler/plugin.rb
@@ -37,7 +37,11 @@ module Bundler
 
       save_plugins names, specs
     rescue PluginError => e
-      specs.values.map {|spec| Bundler.rm_rf(spec.full_gem_path) } if specs
+      if specs
+        specs_to_delete = Hash[specs.select {|k, _v| names.include?(k) && !index.commands.values.include?(k) }]
+        specs_to_delete.values.each {|spec| Bundler.rm_rf(spec.full_gem_path) }
+      end
+
       Bundler.ui.error "Failed to install plugin #{name}: #{e.message}\n  #{e.backtrace.join("\n ")}"
     end
 
diff --git a/lib/bundler/plugin/index.rb b/lib/bundler/plugin/index.rb
index 7f89d26178..8dde072f16 100644
--- a/lib/bundler/plugin/index.rb
+++ b/lib/bundler/plugin/index.rb
@@ -20,6 +20,8 @@ module Bundler
         end
       end
 
+      attr_reader :commands
+
       def initialize
         @plugin_paths = {}
         @commands = {}
diff --git a/spec/plugins/install_spec.rb b/spec/plugins/install_spec.rb
index 4ce9e46bff..e2d351181c 100644
--- a/spec/plugins/install_spec.rb
+++ b/spec/plugins/install_spec.rb
@@ -70,6 +70,17 @@ RSpec.describe "bundler plugin install" do
 
   context "malformatted plugin" do
     it "fails when plugins.rb is missing" do
+      update_repo2 do
+        build_plugin "foo", "1.1"
+        build_plugin "kung-foo", "1.1"
+      end
+
+      bundle "plugin install foo kung-foo --version '1.0' --source file://#{gem_repo2}"
+
+      expect(out).to include("Installing foo 1.0")
+      expect(out).to include("Installing kung-foo 1.0")
+      plugin_should_be_installed("foo", "kung-foo")
+
       build_repo2 do
         build_gem "charlie"
       end
@@ -80,6 +91,7 @@ RSpec.describe "bundler plugin install" do
 
       expect(global_plugin_gem("charlie-1.0")).not_to be_directory
 
+      plugin_should_be_installed("foo", "kung-foo")
       plugin_should_not_be_installed("charlie")
     end