diff --git a/lib/vagrant/action/builtin/handle_box.rb b/lib/vagrant/action/builtin/handle_box.rb index 932125798..bf13880e0 100644 --- a/lib/vagrant/action/builtin/handle_box.rb +++ b/lib/vagrant/action/builtin/handle_box.rb @@ -47,6 +47,38 @@ module Vagrant end def handle_metadata_box(env) + machine = env[:machine] + + if machine.box + @logger.info("Machine already has box. HandleBox will not run.") + return + end + + # Determine the set of formats that this box can be in + box_download_ca_cert = env[:machine].config.vm.box_download_ca_cert + box_download_client_cert = env[:machine].config.vm.box_download_client_cert + box_download_insecure = env[:machine].config.vm.box_download_insecure + box_formats = env[:machine].provider_options[:box_format] || + env[:machine].provider_name + + env[:ui].output(I18n.t( + "vagrant.box_auto_adding", name: machine.config.vm.box)) + env[:ui].detail("Box Provider: #{Array(box_formats).join(", ")}") + env[:ui].detail("Box Version: #{machine.config.vm.box_version}") + + begin + env[:action_runner].run(Vagrant::Action.action_box_add, { + box_url: machine.config.vm.box, + box_provider: box_formats, + box_version: machine.config.vm.box_version, + box_client_cert: box_download_client_cert, + box_download_ca_cert: box_download_ca_cert, + box_download_insecure: box_download_insecure, + }) + rescue Errors::BoxAlreadyExists + # Just ignore this, since it means the next part will succeed! + # This can happen in a multi-threaded environment. + end end =begin diff --git a/lib/vagrant/action/builtin/handle_box_url.rb b/lib/vagrant/action/builtin/handle_box_url.rb index 6a71be6a5..b77937883 100644 --- a/lib/vagrant/action/builtin/handle_box_url.rb +++ b/lib/vagrant/action/builtin/handle_box_url.rb @@ -1,96 +1,12 @@ -require "thread" - -require "log4r" - module Vagrant module Action module Builtin - # This built-in middleware handles the `box_url` setting, downloading - # the box if necessary. You should place this early in your middleware - # sequence for a provider after configuration validation but before - # you attempt to use any box. - class HandleBoxUrl - @@big_lock = Mutex.new - @@handle_box_url_locks = Hash.new { |h,k| h[k] = Mutex.new } - - def initialize(app, env) - @app = app - @logger = Log4r::Logger.new("vagrant::action::builtin::handle_box_url") - end - + class HandleBoxUrl < HandleBox def call(env) - if !env[:machine].config.vm.box || !env[:machine].config.vm.box_url - @logger.info("Skipping HandleBoxUrl because box or box_url not set.") - @app.call(env) - return - end - - if env[:machine].box - @logger.info("Skipping HandleBoxUrl because box is already available") - @app.call(env) - return - end - - # Get a "big lock" to make sure that our more fine grained - # lock access is thread safe. - lock = nil - @@big_lock.synchronize do - lock = @@handle_box_url_locks[env[:machine].config.vm.box] - end - - box_name = env[:machine].config.vm.box - box_url = env[:machine].config.vm.box_url - box_download_ca_cert = env[:machine].config.vm.box_download_ca_cert - box_download_checksum = env[:machine].config.vm.box_download_checksum - box_download_checksum_type = env[:machine].config.vm.box_download_checksum_type - box_download_client_cert = env[:machine].config.vm.box_download_client_cert - box_download_insecure = env[:machine].config.vm.box_download_insecure - - # Expand the CA cert file relative to the Vagrantfile path, if - # there is one. - if box_download_ca_cert - box_download_ca_cert = File.expand_path( - box_download_ca_cert, env[:machine].env.root_path) - end - - lock.synchronize do - # Check that we don't already have the box, which can happen - # if we're slow to acquire the lock because of another thread - box_formats = env[:machine].provider_options[:box_format] || - env[:machine].provider_name - if env[:box_collection].find(box_name, box_formats) - break - end - - # Add the box then reload the box collection so that it becomes - # aware of it. - env[:ui].info I18n.t( - "vagrant.actions.vm.check_box.not_found", - :name => box_name, - :provider => env[:machine].provider_name) - - begin - env[:action_runner].run(Vagrant::Action.action_box_add, { - :box_checksum => box_download_checksum, - :box_checksum_type => box_download_checksum_type, - :box_client_cert => box_download_client_cert, - :box_download_ca_cert => box_download_ca_cert, - :box_download_insecure => box_download_insecure, - :box_name => box_name, - :box_provider => box_formats, - :box_url => box_url, - }) - rescue Errors::BoxAlreadyExists - # Just ignore this, since it means the next part will succeed! - # This can happen in a multi-threaded environment. - end - end - - # Reload the environment and set the VM to be the new loaded VM. - env[:machine] = env[:machine].env.machine( - env[:machine].name, env[:machine].provider_name, true) - - @app.call(env) + env[:ui].warn("HandleBoxUrl middleware is deprecated. Use HandleBox instead.") + env[:ui].warn("This is a bug with the provider. Please contact the creator") + env[:ui].warn("of the provider you use to fix this.") + super end end end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 044c649ff..accb87fa0 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -4,6 +4,8 @@ en: Machine booted and ready! boot_waiting: |- Waiting for machine to boot. This may take a few minutes... + box_auto_adding: |- + Box '%{name}' could not be found. Attempting to find and install... box_add_choose_provider: |- This box can work with multiple providers! The providers that it can work with are listed below. Please review the list and choose diff --git a/test/unit/vagrant/action/builtin/handle_box_test.rb b/test/unit/vagrant/action/builtin/handle_box_test.rb index 510ab0368..b2a53800a 100644 --- a/test/unit/vagrant/action/builtin/handle_box_test.rb +++ b/test/unit/vagrant/action/builtin/handle_box_test.rb @@ -5,6 +5,7 @@ describe Vagrant::Action::Builtin::HandleBox do let(:app) { lambda { |env| } } let(:env) { { + action_runner: action_runner, machine: machine, ui: Vagrant::UI::Silent.new, } } @@ -13,12 +14,19 @@ describe Vagrant::Action::Builtin::HandleBox do let(:iso_env) do # We have to create a Vagrantfile so there is a root path - env = isolated_environment - env.vagrantfile("") - env.create_vagrant_env + isolated_environment.tap do |env| + env.vagrantfile("") + end end - let(:machine) { iso_env.machine(iso_env.machine_names[0], :dummy) } + let(:iso_vagrant_env) { iso_env.create_vagrant_env } + + let(:action_runner) { double("action_runner") } + let(:box) do + box_dir = iso_env.box3("foo", "1.0", :virtualbox) + Vagrant::Box.new("foo", :virtualbox, "1.0", box_dir) + end + let(:machine) { iso_vagrant_env.machine(iso_vagrant_env.machine_names[0], :dummy) } it "works if there is no box set" do machine.config.vm.box = nil @@ -28,4 +36,49 @@ describe Vagrant::Action::Builtin::HandleBox do subject.call(env) end + + context "without a box_url" do + before do + machine.stub(box: nil) + + machine.config.vm.box = "foo" + end + + it "doesn't do anything if a box exists" do + machine.stub(box: box) + + action_runner.should_receive(:run).never + app.should_receive(:call).with(env) + + subject.call(env) + end + + it "adds a box that doesn't exist" do + action_runner.should_receive(:run).with do |action, opts| + expect(opts[:box_url]).to eq(machine.config.vm.box) + expect(opts[:box_provider]).to eq(:dummy) + expect(opts[:box_version]).to eq(machine.config.vm.box_version) + true + end + + app.should_receive(:call).with(env) + + subject.call(env) + end + + it "adds a box using any format the provider allows" do + machine.provider_options[:box_format] = [:foo, :bar] + + action_runner.should_receive(:run).with do |action, opts| + expect(opts[:box_url]).to eq(machine.config.vm.box) + expect(opts[:box_provider]).to eq([:foo, :bar]) + expect(opts[:box_version]).to eq(machine.config.vm.box_version) + true + end + + app.should_receive(:call).with(env) + + subject.call(env) + end + end end