From 21c1bb5e0502d0db4623b6414778894ae2b33154 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 31 Jan 2020 17:24:44 -0800 Subject: [PATCH] Add support for caching solutions. Remove GEMRC modifications. This pull request adds an enhancement to the internal Bundler class to cache solution sets. This prevents Vagrant from generating a solution for configured plugins on every run. Modifications to the configured plugin list (global or local) will result in the cached solution being invalidatd and resolved again. Also included is the removal of the GEMRC modifications required for Windows. --- lib/vagrant/bundler.rb | 278 +++++++++++++++++++---- lib/vagrant/plugin/manager.rb | 8 +- test/unit/vagrant/bundler_test.rb | 247 +++++++++++++++++++- test/unit/vagrant/plugin/manager_test.rb | 8 +- 4 files changed, 479 insertions(+), 62 deletions(-) diff --git a/lib/vagrant/bundler.rb b/lib/vagrant/bundler.rb index 7ba48435f..7ddf687cf 100644 --- a/lib/vagrant/bundler.rb +++ b/lib/vagrant/bundler.rb @@ -18,6 +18,149 @@ module Vagrant # Bundler as a way to properly resolve all dependencies of Vagrant and # all Vagrant-installed plugins. class Bundler + class SolutionFile + # @return [Pathname] path to plugin file + attr_reader :plugin_file + # @return [Pathname] path to solution file + attr_reader :solution_file + # @return [Array] list of required dependencies + attr_reader :dependency_list + + # @param [Pathname] plugin_file Path to plugin file + # @param [Pathname] solution_file Custom path to solution file + def initialize(plugin_file:, solution_file: nil) + @logger = Log4r::Logger.new("vagrant::bundler::signature_file") + @plugin_file = Pathname.new(plugin_file.to_s) + if solution_file + @solution_file = Pathname.new(solution_file.to_s) + else + @solution_file = Pathname.new(@plugin_file.to_s + ".sol") + end + @valid = false + @dependency_list = [].freeze + @logger.debug("new solution file instance plugin_file=#{plugin_file} " \ + "solution_file=#{solution_file}") + load + end + + # Set the list of dependencies for this solution + # + # @param [Array] dependency_list List of dependencies for the solution + def dependency_list=(dependency_list) + Array(dependency_list).each do |d| + if !d.is_a?(Gem::Dependency) + raise TypeError, "Expected `Gem::Dependency` but received `#{d.class}`" + end + end + @dependency_list = dependency_list.map(&:freeze).freeze + end + + # @return [Boolean] contained solution is valid + def valid? + @valid + end + + # @return [FalseClass] invalidate this solution file + def invalidate! + @logger.debug("manually invalidating solution file") + @valid = false + end + + # Delete the solution file + # + # @return [Boolean] true if file was deleted + def delete! + if !solution_file.exist? + @logger.debug("solution file does not exist. nothing to delete.") + return false + end + @logger.debug("deleting solution file - #{solution_file}") + solution_file.delete + true + end + + # Store the solution file + def store! + if !plugin_file.exist? + @logger.debug("plugin file does not exist, not storing solution") + return + end + if !solution_file.dirname.exist? + @logger.debug("creating directory for solution file: #{solution_file.dirname}") + solution_file.dirname.mkpath + end + @logger.debug("writing solution file contents to disk") + solution_file.write({ + dependencies: dependency_list.map { |d| + [d.name, d.requirements_list] + }, + checksum: plugin_file_checksum, + vagrant_version: Vagrant::VERSION + }.to_json) + @valid = true + end + + def to_s # :nodoc: + "" + end + + protected + + # Load the solution file for the plugin path provided + # if it exists. Validate solution is still applicable + # before injecting dependencies. + def load + if !plugin_file.exist? || !solution_file.exist? + @logger.debug("missing file so skipping loading") + return + end + solution = read_solution || return + return if !valid_solution?( + checksum: solution[:checksum], + version: solution[:vagrant_version] + ) + @logger.debug("loading solution dependency list") + @dependency_list = solution[:dependencies].map do |name, requirements| + Gem::Dependency.new(name, requirements) + end + @logger.debug("solution dependency list: #{dependency_list}") + @valid = true + end + + # Validate the given checksum matches the plugin file + # checksum + # + # @param [String] checksum Checksum value to validate + # @return [Boolean] + def valid_solution?(checksum:, version:) + file_checksum = plugin_file_checksum + @logger.debug("solution validation check CHECKSUM #{file_checksum} <-> #{checksum}" \ + " VERSION #{Vagrant::VERSION} <-> #{version}") + plugin_file_checksum == checksum && + Vagrant::VERSION == version + end + + # @return [String] checksum of plugin file + def plugin_file_checksum + digest = Digest::SHA256.new + digest.file(plugin_file.to_s) + digest.hexdigest + end + + # Read contents of solution file and parse + # + # @return [Hash] + def read_solution + @logger.debug("reading solution file - #{solution_file}") + begin + hash = JSON.load(solution_file.read) + Vagrant::Util::HashWithIndifferentAccess.new(hash) + rescue => err + @logger.warn("failed to load solution file, ignoring (error: #{err})") + end + end + end # Location of HashiCorp gem repository HASHICORP_GEMSTORE = "https://gems.hashicorp.com/".freeze @@ -34,26 +177,16 @@ module Vagrant # @return [Pathname] Global plugin path attr_reader :plugin_gem_path + # @return [Pathname] Global plugin solution set path + attr_reader :plugin_solution_path # @return [Pathname] Vagrant environment specific plugin path attr_reader :env_plugin_gem_path + # @return [Pathname] Vagrant environment data path + attr_reader :environment_data_path def initialize @plugin_gem_path = Vagrant.user_data_path.join("gems", RUBY_VERSION).freeze @logger = Log4r::Logger.new("vagrant::bundler") - - # TODO: Remove fix when https://github.com/rubygems/rubygems/pull/2735 - # gets merged and released - # - # Because of a rubygems bug, we need to set the gemrc file path - # through this method rather than relying on the environment varible - # GEMRC. On windows, that path gets split on `:`: and `;`, which means - # the drive letter gets treated as its own path. If that path exists locally, - # (like having a random folder called `c` where the library was invoked), - # it fails thinking the folder `c` is a gemrc file. - gemrc_val = ENV["GEMRC"] - ENV["GEMRC"] = "" - Gem.configuration = Gem::ConfigFile.new(["--config-file", gemrc_val]) - ENV["GEMRC"] = gemrc_val end # Enable Vagrant environment specific plugins at given data path @@ -62,11 +195,34 @@ module Vagrant # @return [Pathname] Path to environment specific gem directory def environment_path=(env_data_path) @env_plugin_gem_path = env_data_path.join("plugins", "gems", RUBY_VERSION).freeze + @environment_data_path = env_data_path + end + + # Use the given options to create a solution file instance + # for use during initialization. When a Vagrant environment + # is in use, solution files will be stored within the environment's + # data directory. This is because the solution for loading global + # plugins is dependent on any solution generated for local plugins. + # When no Vagrant environment is in use (running Vagrant without a + # Vagrantfile), the Vagrant user data path will be used for solution + # storage since only the global plugins will be used. + # + # @param [Hash] opts Options passed to #init! + # @return [SolutionFile] + def load_solution_file(opts) + return if !opts[:local] && !opts[:global] + return if opts[:local] && environment_data_path.nil? + solution_path = (environment_data_path || Vagrant.user_data_path) + "bundler" + solution_path += opts[:local] ? "local.sol" : "global.sol" + SolutionFile.new( + plugin_file: opts[:local] || opts[:global], + solution_file: solution_path + ) end # Initializes Bundler and the various gem paths so that we can begin # loading gems. - def init!(plugins, repair=false) + def init!(plugins, repair=false, **opts) if !@initial_specifications @initial_specifications = Gem::Specification.find_all{true} else @@ -74,45 +230,78 @@ module Vagrant Gem::Specification.reset end - # Add HashiCorp RubyGems source - if !Gem.sources.include?(HASHICORP_GEMSTORE) - sources = [HASHICORP_GEMSTORE] + Gem.sources.sources - Gem.sources.replace(sources) + solution_file = load_solution_file(opts) + @logger.debug("solution file in use for init: #{solution_file}") + + solution = nil + composed_set = generate_vagrant_set + + if solution_file&.valid? + @logger.debug("loading cached solution set") + solution = solution_file.dependency_list.map do |dep| + spec = composed_set.find_all(dep).first + if !spec + @logger.warn("failed to locate specification for dependency - #{dep}") + @logger.warn("invalidating solution file - #{solution_file}") + solution_file.invalidate! + break + end + dep_r = Gem::Resolver::DependencyRequest.new(dep, nil) + Gem::Resolver::ActivationRequest.new(spec, dep_r) + end end - # Generate dependencies for all registered plugins - plugin_deps = plugins.map do |name, info| - Gem::Dependency.new(name, info['installed_gem_version'].to_s.empty? ? '> 0' : info['installed_gem_version']) - end + if !solution_file&.valid? + @logger.debug("generating solution set for configured plugins") + # Add HashiCorp RubyGems source + if !Gem.sources.include?(HASHICORP_GEMSTORE) + sources = [HASHICORP_GEMSTORE] + Gem.sources.sources + Gem.sources.replace(sources) + end - @logger.debug("Current generated plugin dependency list: #{plugin_deps}") + # Generate dependencies for all registered plugins + plugin_deps = plugins.map do |name, info| + Gem::Dependency.new(name, info['installed_gem_version'].to_s.empty? ? '> 0' : info['installed_gem_version']) + end - # Load dependencies into a request set for resolution - request_set = Gem::RequestSet.new(*plugin_deps) - # Never allow dependencies to be remotely satisfied during init - request_set.remote = false + @logger.debug("Current generated plugin dependency list: #{plugin_deps}") - repair_result = nil - begin - # Compose set for resolution - composed_set = generate_vagrant_set - # Resolve the request set to ensure proper activation order - solution = request_set.resolve(composed_set) - rescue Gem::UnsatisfiableDependencyError => failure - if repair - raise failure if @init_retried - @logger.debug("Resolution failed but attempting to repair. Failure: #{failure}") - install(plugins) - @init_retried = true - retry - else - raise + # Load dependencies into a request set for resolution + request_set = Gem::RequestSet.new(*plugin_deps) + # Never allow dependencies to be remotely satisfied during init + request_set.remote = false + + repair_result = nil + begin + @logger.debug("resolving solution from available specification set") + # Resolve the request set to ensure proper activation order + solution = request_set.resolve(composed_set) + @logger.debug("solution set for configured plugins has been resolved") + rescue Gem::UnsatisfiableDependencyError => failure + if repair + raise failure if @init_retried + @logger.debug("Resolution failed but attempting to repair. Failure: #{failure}") + install(plugins) + @init_retried = true + retry + else + raise + end end end # Activate the gems + @logger.debug("activating solution set") activate_solution(solution) + if solution_file && !solution_file.valid? + solution_file.dependency_list = solution.map do |activation| + activation.request.dependency + end + solution_file.store! + @logger.debug("solution set stored to - #{solution_file}") + end + full_vagrant_spec_list = @initial_specifications + solution.map(&:full_spec) @@ -388,7 +577,8 @@ module Vagrant result = request_set.install_into(install_path.to_s, true, ignore_dependencies: true, prerelease: Vagrant.prerelease?, - wrappers: true + wrappers: true, + document: false ) result = result.map(&:full_spec) result.each do |spec| diff --git a/lib/vagrant/plugin/manager.rb b/lib/vagrant/plugin/manager.rb index 567347d33..3a0863252 100644 --- a/lib/vagrant/plugin/manager.rb +++ b/lib/vagrant/plugin/manager.rb @@ -51,7 +51,7 @@ module Vagrant @globalized = true @logger.debug("Enabling globalized plugins") plugins = installed_plugins - bundler_init(plugins) + bundler_init(plugins, global: user_file.path) plugins end @@ -66,7 +66,7 @@ module Vagrant @local_file = StateFile.new(env.local_data_path.join("plugins.json")) Vagrant::Bundler.instance.environment_path = env.local_data_path plugins = local_file.installed_plugins - bundler_init(plugins) + bundler_init(plugins, local: local_file.path) plugins end end @@ -80,7 +80,7 @@ module Vagrant # # @param [Hash] plugins List of plugins # @return [nil] - def bundler_init(plugins) + def bundler_init(plugins, **opts) if !Vagrant.plugins_init? @logger.warn("Plugin initialization is disabled") return nil @@ -99,7 +99,7 @@ module Vagrant ) end begin - Vagrant::Bundler.instance.init!(plugins) + Vagrant::Bundler.instance.init!(plugins, **opts) rescue StandardError, ScriptError => err @logger.error("Plugin initialization error - #{err.class}: #{err}") err.backtrace.each do |backtrace_line| diff --git a/test/unit/vagrant/bundler_test.rb b/test/unit/vagrant/bundler_test.rb index 99fe9d0cd..eb1554166 100644 --- a/test/unit/vagrant/bundler_test.rb +++ b/test/unit/vagrant/bundler_test.rb @@ -3,6 +3,243 @@ require_relative "../base" require "vagrant/bundler" +describe Vagrant::Bundler::SolutionFile do + let(:plugin_path) { Pathname.new(tmpdir) + "plugin_file" } + let(:solution_path) { Pathname.new(tmpdir) + "solution_file" } + let(:tmpdir) { @tmpdir ||= Dir.mktmpdir("vagrant-bundler-test") } + let(:subject) { + described_class.new( + plugin_file: plugin_path, + solution_file: solution_path + ) + } + + after do + if @tmpdir + FileUtils.rm_rf(@tmpdir) + @tmpdir = nil + end + end + + describe "#initialize" do + context "file paths" do + context "with solution_file not provided" do + let(:subject) { described_class.new(plugin_file: plugin_path) } + + it "should set the plugin_file" do + expect(subject.plugin_file.to_s).to eq(plugin_path.to_s) + end + + it "should set solution path to same directory" do + expect(subject.solution_file.to_s).to eq(plugin_path.to_s + ".sol") + end + end + + context "with custom solution_file provided" do + let(:subject) { described_class. + new(plugin_file: plugin_path, solution_file: solution_path) } + + it "should set the plugin file path" do + expect(subject.plugin_file.to_s).to eq(plugin_path.to_s) + end + + it "should set the solution file path to given value" do + expect(subject.solution_file.to_s).to eq(solution_path.to_s) + end + end + end + + context "initialization behavior" do + context "on creation" do + before { expect_any_instance_of(described_class).to receive(:load) } + + it "should load solution file during initialization" do + subject + end + end + + it "should be invalid by default" do + expect(subject.valid?).to be_falsey + end + end + end + + describe "#dependency_list=" do + it "should accept a list of Gem::Dependency instances" do + list = ["dep1", "dep2"].map{ |x| Gem::Dependency.new(x) } + expect(subject.dependency_list = list).to eq(list) + end + + it "should error if list includes instance not Gem::Dependency" do + list = ["dep1", "dep2"].map{ |x| Gem::Dependency.new(x) } << :invalid + expect{ subject.dependency_list = list }.to raise_error(TypeError) + end + end + + describe "#delete!" do + context "when file does not exist" do + before { subject.solution_file.delete if subject.solution_file.exist? } + + it "should return false" do + expect(subject.delete!).to be_falsey + end + + it "should not exist" do + subject.delete! + expect(subject.solution_file.exist?).to be_falsey + end + end + + context "when file does exist" do + before { subject.solution_file.write('x') } + + it "should return true" do + expect(subject.delete!).to be_truthy + end + + it "should not exist" do + expect(subject.solution_file.exist?).to be_truthy + subject.delete! + expect(subject.solution_file.exist?).to be_falsey + end + end + end + + describe "store!" do + context "when plugin file does not exist" do + before { subject.plugin_file.delete if subject.plugin_file.exist? } + + it "should return false" do + expect(subject.store!).to be_falsey + end + + it "should not create a solution file" do + subject.store! + expect(subject.solution_file.exist?).to be_falsey + end + end + + + context "when plugin file does exist" do + before { subject.plugin_file.write("x") } + + it "should return true" do + expect(subject.store!).to be_truthy + end + + it "should create a solution file" do + expect(subject.solution_file.exist?).to be_falsey + subject.store! + expect(subject.solution_file.exist?).to be_truthy + end + + context "stored file" do + let(:content) { + @content ||= JSON.load(subject.solution_file.read) + } + before { subject.store! } + after { @content = nil } + + it "should store JSON hash" do + expect(content).to be_a(Hash) + end + + it "should include dependencies key as array value" do + expect(content["dependencies"]).to be_a(Array) + end + + it "should include checksum key as string value" do + expect(content["checksum"]).to be_a(String) + end + + it "should include vagrant_version key as string value" do + expect(content["vagrant_version"]).to be_a(String) + end + + it "should include vagrant_version key that matches current version" do + expect(content["vagrant_version"]).to eq(Vagrant::VERSION) + end + end + end + end + + describe "behavior" do + context "when storing new solution set" do + let(:deps) { ["dep1", "dep2"].map{ |n| Gem::Dependency.new(n) } } + + context "when plugin file does not exist" do + before { subject.solution_file.delete if subject.solution_file.exist? } + + it "should not create a solution file" do + subject.dependency_list = deps + subject.store! + expect(subject.solution_file.exist?).to be_falsey + end + end + + context "when plugin file does exist" do + before { subject.plugin_file.write("x") } + + it "should create a solution file" do + subject.dependency_list = deps + subject.store! + expect(subject.solution_file.exist?).to be_truthy + end + + it "should update solution file instance to valid" do + expect(subject.valid?).to be_falsey + subject.dependency_list = deps + subject.store! + expect(subject.valid?).to be_truthy + end + + context "when solution file does exist" do + before do + subject.dependency_list = deps + subject.store! + end + + it "should be a valid solution" do + subject = described_class.new( + plugin_file: plugin_path, + solution_file: solution_path + ) + expect(subject.valid?).to be_truthy + end + + it "should have expected dependency list" do + subject = described_class.new( + plugin_file: plugin_path, + solution_file: solution_path + ) + expect(subject.dependency_list).to eq(deps) + end + + context "when plugin file has been changed" do + before { subject.plugin_file.write("xy") } + + it "should not be a valid solution" do + subject = described_class.new( + plugin_file: plugin_path, + solution_file: solution_path + ) + expect(subject.valid?).to be_falsey + end + + it "should have empty dependency list" do + subject = described_class.new( + plugin_file: plugin_path, + solution_file: solution_path + ) + expect(subject.dependency_list).to be_empty + end + end + end + end + end + end +end + describe Vagrant::Bundler do include_context "unit" @@ -29,16 +266,6 @@ describe Vagrant::Bundler do end describe "#initialize" do - let(:gemrc_location) { "C:\\My\\Config\\File" } - - it "should set up GEMRC through a flag instead of GEMRC" do - allow(ENV).to receive(:[]).with("VAGRANT_HOME") - allow(ENV).to receive(:[]).with("USERPROFILE") - - allow(ENV).to receive(:[]).with("GEMRC").and_return(gemrc_location) - expect(Gem::ConfigFile).to receive(:new).with(["--config-file", gemrc_location]) - init_subject = described_class.new - end end describe "#deinit" do diff --git a/test/unit/vagrant/plugin/manager_test.rb b/test/unit/vagrant/plugin/manager_test.rb index fea73f6e9..4df185f53 100644 --- a/test/unit/vagrant/plugin/manager_test.rb +++ b/test/unit/vagrant/plugin/manager_test.rb @@ -35,7 +35,7 @@ describe Vagrant::Plugin::Manager do end it "should init bundler with installed plugins" do - expect(subject).to receive(:bundler_init).with(plugins) + expect(subject).to receive(:bundler_init).with(plugins, anything) subject.globalize! end @@ -48,7 +48,7 @@ describe Vagrant::Plugin::Manager do let(:env) { double("env", local_data_path: local_data_path) } let(:local_data_path) { double("local_data_path") } let(:plugins) { double("plugins") } - let(:state_file) { double("state_file", installed_plugins: plugins) } + let(:state_file) { double("state_file", path: double("state_file_path"), installed_plugins: plugins) } before do allow(Vagrant::Plugin::StateFile).to receive(:new).and_return(state_file) @@ -71,7 +71,7 @@ describe Vagrant::Plugin::Manager do end it "should run bundler initialization" do - expect(subject).to receive(:bundler_init).with(plugins) + expect(subject).to receive(:bundler_init).with(plugins, anything) subject.localize!(env) end @@ -118,7 +118,7 @@ describe Vagrant::Plugin::Manager do end it "should init the bundler instance with plugins" do - expect(bundler).to receive(:init!).with(plugins) + expect(bundler).to receive(:init!).with(plugins, anything) subject.bundler_init(plugins) end