From 97715280c213e0490c3c0e25f5b91b6914804c3b Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 27 Jun 2017 17:16:50 -0700 Subject: [PATCH 1/2] Deep merge plugin list with system plugins. Discard specifications correctly. --- lib/vagrant/plugin/manager.rb | 15 +++++++++++++-- test/unit/vagrant/plugin/manager_test.rb | 4 ++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/vagrant/plugin/manager.rb b/lib/vagrant/plugin/manager.rb index 4f875fd6f..64891bd37 100644 --- a/lib/vagrant/plugin/manager.rb +++ b/lib/vagrant/plugin/manager.rb @@ -146,7 +146,7 @@ module Vagrant system[k] = v.merge("system" => true) end end - plugin_list = system.merge(@user_file.installed_plugins) + plugin_list = Util::DeepMerge.deep_merge(system, @user_file.installed_plugins) # Sort plugins by name Hash[ @@ -161,7 +161,15 @@ module Vagrant # # @return [Array] def installed_specs - installed = Set.new(installed_plugins.keys) + installed_plugin_info = installed_plugins + installed = Set.new(installed_plugin_info.keys) + installed_versions = Hash[ + installed_plugin_info.map{|plugin_name, plugin_info| + gem_version = plugin_info["gem_version"].to_s + gem_version = "> 0" if gem_version.empty? + [plugin_name, Gem::Requirement.new(gem_version)] + } + ] # Go through the plugins installed in this environment and # get the latest version of each. @@ -170,6 +178,9 @@ module Vagrant # Ignore specs that aren't in our installed list next if !installed.include?(spec.name) + next if installed_versions[spec.name] && + !installed_versions[spec.name].satisfied_by?(spec.version) + # If we already have a newer version in our list of installed, # then ignore it next if installed_map.key?(spec.name) && diff --git a/test/unit/vagrant/plugin/manager_test.rb b/test/unit/vagrant/plugin/manager_test.rb index da94c6ea6..8889c5ebd 100644 --- a/test/unit/vagrant/plugin/manager_test.rb +++ b/test/unit/vagrant/plugin/manager_test.rb @@ -4,7 +4,7 @@ require "pathname" require "vagrant/plugin" require "vagrant/plugin/manager" require "vagrant/plugin/state_file" - +require "vagrant/util/deep_merge" require File.expand_path("../../../base", __FILE__) describe Vagrant::Plugin::Manager do @@ -243,7 +243,7 @@ describe Vagrant::Plugin::Manager do expect(plugins.length).to eql(2) expect(plugins).to have_key("foo") expect(plugins["foo"]["gem_version"]).to eq("0.1.0") - expect(plugins["foo"]["system"]).to be_false + expect(plugins["foo"]["system"]).to be_true expect(plugins).to have_key("bar") expect(plugins["bar"]["system"]).to be_true end From 85d5f11f62a2d8316d7013376018ae6c2c54acfc Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 27 Jun 2017 17:17:13 -0700 Subject: [PATCH 2/2] Adjustments to handle plugin updates using proper constraints If a user provides the gem version using an explicit version or a constraint, the update action should honor that constraint and not simply replace it with an unbound constraint. This also removes system plugin specifications from being matched and preferred which prevents updates and can result in unexpected downgrades when running the update. --- lib/vagrant/bundler.rb | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/vagrant/bundler.rb b/lib/vagrant/bundler.rb index 275e970d2..4df5101f3 100644 --- a/lib/vagrant/bundler.rb +++ b/lib/vagrant/bundler.rb @@ -220,7 +220,7 @@ module Vagrant # Generate all required plugin deps plugin_deps = plugins.map do |name, info| if update[:gems] == true || (update[:gems].respond_to?(:include?) && update[:gems].include?(name)) - gem_version = '> 0' + gem_version = plugins[name]["gem_version"].to_s.empty? ? "> 0" : plugins[name]["gem_version"] skips << name else gem_version = info['gem_version'].to_s.empty? ? '> 0' : info['gem_version'] @@ -275,12 +275,16 @@ module Vagrant # Create the request set for the new plugins request_set = Gem::RequestSet.new(*plugin_deps) + system_plugins = plugins.map do |plugin_name, plugin_info| + plugin_name if plugin_info["system"] + end.compact + installer_set.system_plugins = system_plugins + installer_set = Gem::Resolver.compose_sets( installer_set, - generate_builtin_set, + generate_builtin_set(system_plugins), generate_plugin_set(skips) ) - @logger.debug("Generating solution set for installation.") # Generate the required solution set for new plugins @@ -349,11 +353,13 @@ module Vagrant end # Generate the builtin resolver set - def generate_builtin_set + def generate_builtin_set(system_plugins=[]) builtin_set = BuiltinSet.new @logger.debug("Generating new builtin set instance.") vagrant_internal_specs.each do |spec| - builtin_set.add_builtin_spec(spec) + if !system_plugins.include?(spec.name) + builtin_set.add_builtin_spec(spec) + end end builtin_set end @@ -428,9 +434,11 @@ module Vagrant # the entire set used for performing full resolutions on install. class VagrantSet < Gem::Resolver::InstallerSet attr_accessor :prefer_sources + attr_accessor :system_plugins def initialize(domain, defined_sources={}) @prefer_sources = defined_sources + @system_plugins = [] super(domain) end @@ -438,6 +446,11 @@ module Vagrant # for preferred sources def find_all(req) result = super + if system_plugins.include?(req.name) + result.delete_if do |spec| + spec.is_a?(Gem::Resolver::InstalledSpecification) + end + end subset = result.find_all do |idx_spec| preferred = false if prefer_sources[req.name]