From 5195bee9ea4c665dfb8d7fa9e41c26b6add3b434 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 6 May 2020 15:38:44 -0700 Subject: [PATCH] Check if plugin install provides specification During a plugin install, if the plugin is already installed and activated, no specification will be returned as there was nothing new installed. In this situation, look for the requested plugin within the activated specifications. If it is found, then proceed since the plugin is installed. If it is not found, return an error. --- lib/vagrant/bundler.rb | 10 ++++---- lib/vagrant/errors.rb | 4 +++ lib/vagrant/plugin/manager.rb | 31 ++++++++++++++++-------- templates/locales/en.yml | 4 +++ test/unit/vagrant/plugin/manager_test.rb | 19 +++++++++++++++ 5 files changed, 53 insertions(+), 15 deletions(-) diff --git a/lib/vagrant/bundler.rb b/lib/vagrant/bundler.rb index 92ee53a40..f4d2a0239 100644 --- a/lib/vagrant/bundler.rb +++ b/lib/vagrant/bundler.rb @@ -480,7 +480,7 @@ module Vagrant if update[:gems] == true || (update[:gems].respond_to?(:include?) && update[:gems].include?(name)) if Gem::Requirement.new(gem_version).exact? gem_version = "> 0" - @logger.debug("Detected exact version match for `#{name}` plugin update. Reset to loose constraint #{gem_version.inspect}.") + @logger.debug("Detected exact version match for `#{name}` plugin update. Reset to loosen constraint #{gem_version.inspect}.") end skips << name end @@ -565,10 +565,10 @@ module Vagrant activate_solution(solution) # Remove gems which are already installed - request_set.sorted_requests.delete_if do |activation_req| - rs_spec = activation_req.spec - if vagrant_internal_specs.detect{|ispec| ispec.name == rs_spec.name && ispec.version == rs_spec.version } - @logger.debug("Removing activation request from install. Already installed. (#{rs_spec.spec.full_name})") + request_set.sorted_requests.delete_if do |act_req| + rs = act_req.spec + if vagrant_internal_specs.detect{ |i| i.name == rs.name && i.version == rs.version } + @logger.debug("Removing activation request from install. Already installed. (#{rs.spec.full_name})") true end end diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 4271967ef..05e821bba 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -628,6 +628,10 @@ module Vagrant error_key(:plugin_install_license_not_found) end + class PluginInstallFailed < VagrantError + error_key(:plugin_install_failed) + end + class PluginInstallSpace < VagrantError error_key(:plugin_install_space) end diff --git a/lib/vagrant/plugin/manager.rb b/lib/vagrant/plugin/manager.rb index 3a0863252..9058e68b3 100644 --- a/lib/vagrant/plugin/manager.rb +++ b/lib/vagrant/plugin/manager.rb @@ -150,17 +150,28 @@ module Vagrant else result = local_spec end - # Add the plugin to the state file - plugin_file = opts[:env_local] ? @local_file : @user_file - plugin_file.add_plugin( - result.name, - version: opts[:version], - require: opts[:require], - sources: opts[:sources], - env_local: !!opts[:env_local], - installed_gem_version: result.version.to_s - ) + if result + # Add the plugin to the state file + plugin_file = opts[:env_local] ? @local_file : @user_file + plugin_file.add_plugin( + result.name, + version: opts[:version], + require: opts[:require], + sources: opts[:sources], + env_local: !!opts[:env_local], + installed_gem_version: result.version.to_s + ) + else + r = Gem::Dependency.new(name, opts[:version]) + result = Gem::Specification.find { |s| + s.satisfies_requirement?(r) && + s.activated? + } + raise Errors::PluginInstallFailed, + name: name if result.nil? + @logger.warn("Plugin install returned no result as no new plugins were installed.") + end # After install clean plugin gems to remove any cruft. This is useful # for removing outdated dependencies or other versions of an installed # plugin if the plugin is upgraded/downgraded diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 7ddadd519..abb0901d9 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1130,6 +1130,10 @@ en: The license file to install could not be found. Please verify the path you gave is correct. The path to the license file given was: '%{path}' + plugin_install_failed: |- + Vagrant was unable to install the requested plugin: '%{name}' + Please try to install this plugin again. If Vagrant is still unable + to install the requested plugin, please report this error. plugin_install_space: |- The directory where plugins are installed (the Vagrant home directory) has a space in it. On Windows, there is a bug in Ruby when compiling diff --git a/test/unit/vagrant/plugin/manager_test.rb b/test/unit/vagrant/plugin/manager_test.rb index 4df185f53..dcb3dbf5e 100644 --- a/test/unit/vagrant/plugin/manager_test.rb +++ b/test/unit/vagrant/plugin/manager_test.rb @@ -265,6 +265,25 @@ describe Vagrant::Plugin::Manager do expect(plugins["bar"]["gem_version"]).to eql("1.0") end + context "with existing activation" do + let(:value) { double("value") } + + before do + expect(bundler).to receive(:install).and_return([]) + allow(bundler).to receive(:clean) + end + + it "should locate existing activation if available" do + expect(Gem::Specification).to receive(:find).and_return(value) + expect(subject.install_plugin("foo")).to eq(value) + end + + it "should raise an error if no activation is located" do + expect(Gem::Specification).to receive(:find).and_return(nil) + expect { subject.install_plugin("foo") }.to raise_error(Vagrant::Errors::PluginInstallFailed) + end + end + describe "installation options" do let(:specs) do specs = Array.new(5) { Gem::Specification.new }