From 3ace82cc5b97cd669206fe3fedddd3d6be652739 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 31 Aug 2018 13:38:36 -0700 Subject: [PATCH] Update Vagrant.has_plugin? helper to function prior to plugin loading Due to the Vagrantfile being loaded prior to plugin loading to determine project local plugin information the Vagrant.has_plugin? helper will always return false when the Vagrantfile is first loaded. To prevent this behavior we can check for plugins in the plugin data files prior to the plugins being loaded, and after they have been loaded we can fallback to the original specification based check. --- lib/vagrant.rb | 9 +- lib/vagrant/plugin/manager.rb | 36 +++++++- test/unit/vagrant/plugin/manager_test.rb | 100 +++++++++++++++++++++++ test/unit/vagrant_test.rb | 2 + 4 files changed, 138 insertions(+), 9 deletions(-) diff --git a/lib/vagrant.rb b/lib/vagrant.rb index 64183c092..ff5d42ad1 100644 --- a/lib/vagrant.rb +++ b/lib/vagrant.rb @@ -151,16 +151,9 @@ module Vagrant return true if plugin("2").manager.registered.any? { |p| p.name == name } end - # Make the requirement object - version = Gem::Requirement.new([version]) if version - # Now check the plugin gem names require "vagrant/plugin/manager" - Plugin::Manager.instance.installed_specs.any? do |s| - match = s.name == name - next match if !version - next match && version.satisfied_by?(s.version) - end + Plugin::Manager.instance.plugin_installed?(name, version) end # Returns a superclass to use when creating a plugin for Vagrant. diff --git a/lib/vagrant/plugin/manager.rb b/lib/vagrant/plugin/manager.rb index d409ba788..567347d33 100644 --- a/lib/vagrant/plugin/manager.rb +++ b/lib/vagrant/plugin/manager.rb @@ -41,12 +41,14 @@ module Vagrant @system_file = StateFile.new(system_path) if system_path && system_path.file? @local_file = nil + @globalized = @localized = false end # Enable global plugins # # @return [Hash] list of plugins def globalize! + @globalized = true @logger.debug("Enabling globalized plugins") plugins = installed_plugins bundler_init(plugins) @@ -56,8 +58,9 @@ module Vagrant # Enable environment local plugins # # @param [Environment] env Vagrant environment - # @return [Hash] list of plugins + # @return [Hash, nil] list of plugins def localize!(env) + @localized = true if env.local_data_path @logger.debug("Enabling localized plugins") @local_file = StateFile.new(env.local_data_path.join("plugins.json")) @@ -68,6 +71,11 @@ module Vagrant end end + # @return [Boolean] local and global plugins are loaded + def ready? + @globalized && @localized + end + # Initialize bundler with given plugins # # @param [Hash] plugins List of plugins @@ -335,6 +343,32 @@ module Vagrant end nil end + + # Check if the requested plugin is installed + # + # @param [String] name Name of plugin + # @param [String] version Specific version of the plugin + # @return [Boolean] + def plugin_installed?(name, version=nil) + # Make the requirement object + version = Gem::Requirement.new([version.to_s]) if version + + # If plugins are loaded, check for match in loaded specs + if ready? + return installed_specs.any? do |s| + match = s.name == name + next match if !version + next match && version.satisfied_by?(s.version) + end + end + + # Plugins are not loaded yet so check installed plugin data + plugin_info = installed_plugins[name] + return false if !plugin_info + return !!plugin_info if version.nil? || plugin_info["installed_gem_version"].nil? + installed_version = Gem::Version.new(plugin_info["installed_gem_version"]) + version.satisfied_by?(installed_version) + end end end end diff --git a/test/unit/vagrant/plugin/manager_test.rb b/test/unit/vagrant/plugin/manager_test.rb index aa6b11b7c..fea73f6e9 100644 --- a/test/unit/vagrant/plugin/manager_test.rb +++ b/test/unit/vagrant/plugin/manager_test.rb @@ -80,6 +80,35 @@ describe Vagrant::Plugin::Manager do end end + describe "#ready?" do + let(:plugins) { double("plugins") } + let(:env) { double("env", local_data_path: nil) } + + before do + allow(subject).to receive(:bundler_init) + end + + it "should be false by default" do + expect(subject.ready?).to be_falsey + end + + it "should be false when only globalize! has been called" do + subject.globalize! + expect(subject.ready?).to be_falsey + end + + it "should be false when only localize! has been called" do + subject.localize!(env) + expect(subject.ready?).to be_falsey + end + + it "should be true when both localize! and globalize! have been called" do + subject.globalize! + subject.localize!(env) + expect(subject.ready?).to be_truthy + end + end + describe "#bundler_init" do let(:plugins) { {"plugin_name" => {}} } @@ -111,6 +140,77 @@ describe Vagrant::Plugin::Manager do end end + describe "#plugin_installed?" do + let(:ready) { true } + let(:specs) { [] } + + before do + allow(subject).to receive(:ready?).and_return(ready) + allow(subject).to receive(:installed_specs).and_return(specs) + end + + context "when manager is ready" do + it "should return false when plugin is not found" do + expect(subject.plugin_installed?("vagrant-plugin")).to be_falsey + end + + context "when plugin is installed" do + let(:specs) { [Gem::Specification.new("vagrant-plugin", "1.2.3")] } + + it "should return true" do + expect(subject.plugin_installed?("vagrant-plugin")).to be_truthy + end + + it "should return true when version matches installed version" do + expect(subject.plugin_installed?("vagrant-plugin", "1.2.3")).to be_truthy + end + + it "should return true when version requirement is satisified by version" do + expect(subject.plugin_installed?("vagrant-plugin", "> 1.0")).to be_truthy + end + + it "should return false when version requirement is not satisified by version" do + expect(subject.plugin_installed?("vagrant-plugin", "2.0")).to be_falsey + end + end + end + + context "when manager is not ready" do + let(:ready) { false } + let(:plugins) { {} } + before { allow(subject).to receive(:installed_plugins).and_return(plugins) } + + it "should check installed plugin data" do + expect(subject).to receive(:installed_plugins).and_return(plugins) + subject.plugin_installed?("vagrant-plugin") + end + + it "should return false when plugin is not found" do + expect(subject.plugin_installed?("vagrant-plugin")).to be_falsey + end + + context "when plugin is installed" do + let(:plugins) { {"vagrant-plugin" => {"installed_gem_version" => "1.2.3"}} } + + it "should return true" do + expect(subject.plugin_installed?("vagrant-plugin")).to be_truthy + end + + it "should return true when version matches installed version" do + expect(subject.plugin_installed?("vagrant-plugin", "1.2.3")).to be_truthy + end + + it "should return true when version requirement is satisified by version" do + expect(subject.plugin_installed?("vagrant-plugin", "> 1.0")).to be_truthy + end + + it "should return false when version requirement is not satisified by version" do + expect(subject.plugin_installed?("vagrant-plugin", "2.0")).to be_falsey + end + end + end + end + describe "#install_plugin" do it "installs the plugin and adds it to the state file" do specs = Array.new(5) { Gem::Specification.new } diff --git a/test/unit/vagrant_test.rb b/test/unit/vagrant_test.rb index c90091e80..ae6e78a51 100644 --- a/test/unit/vagrant_test.rb +++ b/test/unit/vagrant_test.rb @@ -70,6 +70,7 @@ describe Vagrant do specs = [Gem::Specification.new] specs[0].name = "foo" allow(Vagrant::Plugin::Manager.instance).to receive(:installed_specs).and_return(specs) + allow(Vagrant::Plugin::Manager.instance).to receive(:ready?).and_return(true) expect(described_class.has_plugin?("foo")).to be(true) expect(described_class.has_plugin?("bar")).to be(false) @@ -79,6 +80,7 @@ describe Vagrant do specs = [Gem::Specification.new] specs[0].name = "foo" specs[0].version = "1.2.3" + allow(Vagrant::Plugin::Manager.instance).to receive(:ready?).and_return(true) allow(Vagrant::Plugin::Manager.instance).to receive(:installed_specs).and_return(specs) expect(described_class.has_plugin?("foo", "~> 1.2.0")).to be(true)