From 2d019cb608563518819e8de8bc566f9b93a25622 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 8 Mar 2021 15:57:13 -0800 Subject: [PATCH 1/8] Provide specialized messages on plugin install failure if possible Inspect the error message received when failing to install a plugin. If it's something we can determine the cause, send back a cleaner error message to the user on how to resolve. --- lib/vagrant/errors.rb | 12 ++++++++++++ lib/vagrant/plugin/manager.rb | 22 ++++++++++++++++++++-- templates/locales/en.yml | 23 ++++++++++++++++++++--- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 782615bc4..5cb861c06 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -636,6 +636,18 @@ module Vagrant error_key(:provisioner_winrm_unsupported) end + class PluginNeedsDeveloperTools < VagrantError + error_key(:plugin_needs_developer_tools) + end + + class PluginMissingLibrary < VagrantError + error_key(:plugin_missing_library) + end + + class PluginMissingRubyDev < VagrantError + error_key(:plugin_missing_ruby_dev) + end + class PluginGemNotFound < VagrantError error_key(:plugin_gem_not_found) end diff --git a/lib/vagrant/plugin/manager.rb b/lib/vagrant/plugin/manager.rb index 9058e68b3..b73f07f9c 100644 --- a/lib/vagrant/plugin/manager.rb +++ b/lib/vagrant/plugin/manager.rb @@ -179,8 +179,26 @@ module Vagrant result rescue Gem::GemNotFoundException raise Errors::PluginGemNotFound, name: name - rescue Gem::Exception => e - raise Errors::BundlerError, message: e.to_s + rescue Gem::Exception => err + @logger.warn("Failed to install plugin: #{err}") + @logger.debug("#{err.class}: #{err}\n#{err.backtrace.join("\n")}") + # Try and determine a cause for the failure + case err.message + when /install development tools first/ + raise Errors::PluginNeedsDeveloperTools + when /library not found in default locations/ + lib = err.message.match(/(\w+) library not found in default locations/) + if lib.nil? + raise Errors::BundlerError, message: err.message + end + raise Errors::PluginMissingLibrary, + library: lib.captures.first, + name: name + when /find header files for ruby/ + raise Errors::PluginMissingRubyDev + else + raise Errors::BundlerError, message: err.message + end end # Uninstalls the plugin with the given name. diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 782904f49..edae9b477 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -794,9 +794,9 @@ en: matching this provider. For example, if you're using VirtualBox, the clone environment must also be using VirtualBox. cloud_init_not_found: |- - cloud-init is not found. Please ensure that cloud-init is installed and + cloud-init is not found. Please ensure that cloud-init is installed and available on path for guest '%{guest_name}'. - cloud_init_command_failed: |- + cloud_init_command_failed: |- cloud init command '%{cmd}' failed on guest '%{guest_name}'. command_deprecated: |- The command 'vagrant %{name}' has been deprecated and is no longer functional @@ -1238,6 +1238,23 @@ en: following command: vagrant plugin install --local + plugin_needs_developer_tools: |- + Vagrant failed to install the requested plugin because development tools + are required for installation but are not currently installed on this + machine. Please install development tools and then try this command + again. + plugin_missing_library: |- + Vagrant failed to install the requested plugin because it depends + on a library which is not currently installed on this system. The + following library is required by the '%{name}' plugin: + + %{library} + + Please install the library and then run the command again. + plugin_missing_ruby_dev: |- + Vagrant failed to install the requested plugin because the Ruby header + files could not be found. Install the ruby development package for your + system and then run this command again. powershell_not_found: |- Failed to locate the powershell executable on the available PATH. Please ensure powershell is installed and available on the local PATH, then @@ -2998,7 +3015,7 @@ en: pushes: file: no_destination: "File destination must be specified." - + autocomplete: installed: |- Autocomplete installed at paths: From 2d7694e3806b7618df212daf977df09b838adc34 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 8 Mar 2021 15:58:28 -0800 Subject: [PATCH 2/8] Activate all dependencies when starting up --- bin/vagrant | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/bin/vagrant b/bin/vagrant index ba7e40076..55c563c13 100755 --- a/bin/vagrant +++ b/bin/vagrant @@ -23,9 +23,9 @@ if idx = argv.index("--") argv = argv.slice(0, idx) end +require_relative "../lib/vagrant/version" # Fast path the version of Vagrant if argv.include?("-v") || argv.include?("--version") - require_relative "../lib/vagrant/version" puts "Vagrant #{Vagrant::VERSION}" exit 0 end @@ -82,6 +82,23 @@ end $stdout.sync = true $stderr.sync = true +# Before we start activate all our dependencies +# so we can provide correct resolutions later +vagrant_spec = Gem::Specification.find_all_by_name("vagrant").detect do |spec| + spec.version == Gem::Version.new(Vagrant::VERSION) +end + +dep_activator = proc do |spec| + spec.runtime_dependencies.each do |dep| + if gem(dep.name, *dep.requirement.as_list) || true + dep_spec = Gem::Specification.find_all_by_name(dep.name).detect(&:activated?) + dep_activator.call(dep_spec) if dep_spec + end + end +end + +dep_activator.call(vagrant_spec) if vagrant_spec + env = nil begin require 'log4r' From 282dadf784bc4bd2e5e610bd5eaac0699250804e Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 8 Mar 2021 15:58:47 -0800 Subject: [PATCH 3/8] When not running inside bundler, only use specifications that are activated --- lib/vagrant/bundler.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/vagrant/bundler.rb b/lib/vagrant/bundler.rb index 7abc01a97..85f9feb57 100644 --- a/lib/vagrant/bundler.rb +++ b/lib/vagrant/bundler.rb @@ -287,7 +287,6 @@ module Vagrant # 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 @@ -656,10 +655,16 @@ module Vagrant spec_dir = Gem::Specification.default_specifications_dir end directories = [spec_dir] - Gem::Specification.find_all{true}.each do |spec| - list[spec.full_name] = spec + if Vagrant.in_bundler? + Gem::Specification.find_all{true}.each do |spec| + list[spec.full_name] = spec + end + else + Gem::Specification.find_all(&:activated?).each do |spec| + list[spec.full_name] = spec + end end - if(!Object.const_defined?(:Bundler)) + if Vagrant.in_installer? directories += Gem::Specification.dirs.find_all do |path| !path.start_with?(Gem.user_dir) end From 32f495175954f095a50d82fa73d2ee41d51811c2 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 8 Mar 2021 16:25:13 -0800 Subject: [PATCH 4/8] Activate builtin specs during startup When starting up, and before any loading, find our current specification and activate all the internal dependencies while also collecting the activated specifications. Store these for later use when doing plugin resolutions. We bypass the builtin list when running in bundler since they will still show up as not activated, but we use the entire list regardless. --- bin/vagrant | 17 +++++++++++++---- lib/vagrant/bundler.rb | 5 +++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/bin/vagrant b/bin/vagrant index 55c563c13..c019f30ff 100755 --- a/bin/vagrant +++ b/bin/vagrant @@ -84,20 +84,26 @@ $stderr.sync = true # Before we start activate all our dependencies # so we can provide correct resolutions later +builtin_specs = [] + vagrant_spec = Gem::Specification.find_all_by_name("vagrant").detect do |spec| spec.version == Gem::Version.new(Vagrant::VERSION) end dep_activator = proc do |spec| spec.runtime_dependencies.each do |dep| - if gem(dep.name, *dep.requirement.as_list) || true - dep_spec = Gem::Specification.find_all_by_name(dep.name).detect(&:activated?) - dep_activator.call(dep_spec) if dep_spec + gem(dep.name, *dep.requirement.as_list) + dep_spec = Gem::Specification.find_all_by_name(dep.name).detect(&:activated?) + if dep_spec + builtin_specs << dep_spec + dep_activator.call(dep_spec) end end end -dep_activator.call(vagrant_spec) if vagrant_spec +if vagrant_spec + dep_activator.call(vagrant_spec) +end env = nil begin @@ -108,6 +114,9 @@ begin require 'vagrant/util/platform' require 'vagrant/util/experimental' + # Set our list of builtin specs + Vagrant::Bundler.instance.builtin_specs = builtin_specs + # Schedule the cleanup of things at_exit(&Vagrant::Bundler.instance.method(:deinit)) diff --git a/lib/vagrant/bundler.rb b/lib/vagrant/bundler.rb index 85f9feb57..f2010fed6 100644 --- a/lib/vagrant/bundler.rb +++ b/lib/vagrant/bundler.rb @@ -189,6 +189,8 @@ module Vagrant attr_reader :env_plugin_gem_path # @return [Pathname] Vagrant environment data path attr_reader :environment_data_path + # @return [Array, nil] List of builtin specs + attr_accessor :builtin_specs def initialize @plugin_gem_path = Vagrant.user_data_path.join("gems", RUBY_VERSION).freeze @@ -646,7 +648,6 @@ module Vagrant self_spec.activate @logger.info("Activated vagrant specification version - #{self_spec.version}") end - self_spec.runtime_dependencies.each { |d| gem d.name, *d.requirement.as_list } # discover all the gems we have available list = {} if Gem.respond_to?(:default_specifications_dir) @@ -660,7 +661,7 @@ module Vagrant list[spec.full_name] = spec end else - Gem::Specification.find_all(&:activated?).each do |spec| + builtin_specs.each do |spec| list[spec.full_name] = spec end end From 55dc18cb967e5107ebe302bb1aa148ff2b10957b Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 8 Mar 2021 17:14:14 -0800 Subject: [PATCH 5/8] Default the builtin specs when initializing --- lib/vagrant/bundler.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/vagrant/bundler.rb b/lib/vagrant/bundler.rb index f2010fed6..1165fdf2a 100644 --- a/lib/vagrant/bundler.rb +++ b/lib/vagrant/bundler.rb @@ -193,6 +193,7 @@ module Vagrant attr_accessor :builtin_specs def initialize + @builtin_specs = [] @plugin_gem_path = Vagrant.user_data_path.join("gems", RUBY_VERSION).freeze @logger = Log4r::Logger.new("vagrant::bundler") end From 320fb81824bb175a1063f234c7429f4c8cfd782e Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 8 Mar 2021 17:14:53 -0800 Subject: [PATCH 6/8] Clean up tests to handle modifications --- test/unit/bin/vagrant_test.rb | 1 + test/unit/vagrant/bundler_test.rb | 49 ++++++++++++++----------------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/test/unit/bin/vagrant_test.rb b/test/unit/bin/vagrant_test.rb index bc11309aa..dbbd52112 100644 --- a/test/unit/bin/vagrant_test.rb +++ b/test/unit/bin/vagrant_test.rb @@ -30,6 +30,7 @@ describe "vagrant bin" do allow(Kernel).to receive(:exit) allow(Vagrant::Environment).to receive(:new).and_return(env) allow(Vagrant).to receive(:in_installer?).and_return(true) + allow(self).to receive(:require_relative) end after { expect(run_vagrant).to eq(exit_code) } diff --git a/test/unit/vagrant/bundler_test.rb b/test/unit/vagrant/bundler_test.rb index 00cedc021..e81b314f9 100644 --- a/test/unit/vagrant/bundler_test.rb +++ b/test/unit/vagrant/bundler_test.rb @@ -778,42 +778,37 @@ describe Vagrant::Bundler do end end - context "when run time dependencies are defined" do - let(:vagrant_dep_specs) { [double("spec", name: "vagrant-dep", requirement: double("spec-req", as_list: []))] } - - it "should call #gem to activate the dependencies" do - expect(subject).to receive(:gem).with("vagrant-dep", any_args) - subject.send(:vagrant_internal_specs) - end - end - context "when bundler is not defined" do - before { expect(Object).to receive(:const_defined?).with(:Bundler).and_return(false) } + before { expect(Vagrant).to receive(:in_bundler?).and_return(false) } - it "should load gem specification directories" do - expect(Gem::Specification).to receive(:dirs).and_return(spec_dirs) - subject.send(:vagrant_internal_specs) - end + context "when running within the installer" do + before { expect(Vagrant).to receive(:in_installer?).and_return(true) } - context "when checking paths" do - let(:spec_dirs) { [double("spec-dir", start_with?: in_user_dir)] } - let(:in_user_dir) { true } - let(:user_dir) { double("user-dir") } - - before { allow(Gem).to receive(:user_dir).and_return(user_dir) } - - it "should check if path is within local user directory" do - expect(spec_dirs.first).to receive(:start_with?).with(user_dir).and_return(false) + it "should load gem specification directories" do + expect(Gem::Specification).to receive(:dirs).and_return(spec_dirs) subject.send(:vagrant_internal_specs) end - context "when path is not within user directory" do - let(:in_user_dir) { false } + context "when checking paths" do + let(:spec_dirs) { [double("spec-dir", start_with?: in_user_dir)] } + let(:in_user_dir) { true } + let(:user_dir) { double("user-dir") } - it "should use path when loading specs" do - expect(Gem::Specification).to receive(:each_spec) { |arg| expect(arg).to include(spec_dirs.first) } + before { allow(Gem).to receive(:user_dir).and_return(user_dir) } + + it "should check if path is within local user directory" do + expect(spec_dirs.first).to receive(:start_with?).with(user_dir).and_return(false) subject.send(:vagrant_internal_specs) end + + context "when path is not within user directory" do + let(:in_user_dir) { false } + + it "should use path when loading specs" do + expect(Gem::Specification).to receive(:each_spec) { |arg| expect(arg).to include(spec_dirs.first) } + subject.send(:vagrant_internal_specs) + end + end end end end From 80ec78929aa4c6c96073055838bf51f7ed5c3184 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 9 Mar 2021 08:05:55 -0800 Subject: [PATCH 7/8] Adjust syntax on method --- lib/vagrant.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vagrant.rb b/lib/vagrant.rb index d696bdff8..f3dcba0bc 100644 --- a/lib/vagrant.rb +++ b/lib/vagrant.rb @@ -81,7 +81,7 @@ if ENV["VAGRANT_LOG"] && ENV["VAGRANT_LOG"] != "" # See https://github.com/rest-client/rest-client/issues/34#issuecomment-290858 # for more information class VagrantLogger < Log4r::Logger - def << (msg) + def << msg debug(msg.strip) end end From ae48e960fc666c0782c40421d5aac0af0346243f Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 9 Mar 2021 08:06:10 -0800 Subject: [PATCH 8/8] Add test for behavior difference when running outside of installer --- test/unit/vagrant/bundler_test.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/unit/vagrant/bundler_test.rb b/test/unit/vagrant/bundler_test.rb index e81b314f9..69f425c66 100644 --- a/test/unit/vagrant/bundler_test.rb +++ b/test/unit/vagrant/bundler_test.rb @@ -781,7 +781,7 @@ describe Vagrant::Bundler do context "when bundler is not defined" do before { expect(Vagrant).to receive(:in_bundler?).and_return(false) } - context "when running within the installer" do + context "when running inside the installer" do before { expect(Vagrant).to receive(:in_installer?).and_return(true) } it "should load gem specification directories" do @@ -811,6 +811,15 @@ describe Vagrant::Bundler do end end end + + context "when running outside the installer" do + before { expect(Vagrant).to receive(:in_installer?).and_return(false) } + + it "should not load gem specification directories" do + expect(Gem::Specification).not_to receive(:dirs) + subject.send(:vagrant_internal_specs) + end + end end end