From cf0ba53fbb7eaeb5734d044312fb2d52555b9c1b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 19 Jul 2013 00:48:46 -0400 Subject: [PATCH] box_url works with all box formats a provider supports [GH-1752] --- CHANGELOG.md | 2 + lib/vagrant/action/builtin/handle_box_url.rb | 6 +-- lib/vagrant/box_collection.rb | 49 +++++++++++++------- templates/locales/en.yml | 2 +- 4 files changed, 37 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddd9926d9..fcce264f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ IMPROVEMENTS: BUG FIXES: + - `box_url` now handles the case where the provider doesn't perfectly + match the provider in use, but the provider supports it. [GH-1752] - Fix uninitialized constant error when configuring Arch Linux network. [GH-1734] - Debian/Ubuntu change hostname works properly if eth0 is configured with hot-plugging. [GH-1929] diff --git a/lib/vagrant/action/builtin/handle_box_url.rb b/lib/vagrant/action/builtin/handle_box_url.rb index 6bf552104..1f2b61589 100644 --- a/lib/vagrant/action/builtin/handle_box_url.rb +++ b/lib/vagrant/action/builtin/handle_box_url.rb @@ -43,9 +43,9 @@ module Vagrant # First see if we actually have the box now. has_box = false - formats = env[:machine].provider_options[:box_format] || + box_formats = env[:machine].provider_options[:box_format] || env[:machine].provider_name - if env[:box_collection].find(box_name, formats) + if env[:box_collection].find(box_name, box_formats) has_box = true break end @@ -61,7 +61,7 @@ module Vagrant begin env[:action_runner].run(Vagrant::Action.action_box_add, { :box_name => box_name, - :box_provider => env[:machine].provider_name, + :box_provider => box_formats, :box_url => box_url }) rescue Errors::BoxAlreadyExists diff --git a/lib/vagrant/box_collection.rb b/lib/vagrant/box_collection.rb index 3d29a67f8..f980a309c 100644 --- a/lib/vagrant/box_collection.rb +++ b/lib/vagrant/box_collection.rb @@ -70,31 +70,35 @@ module Vagrant # the box represents will be added. # @param [Boolean] force If true, any existing box with the same name # and provider will be replaced. - def add(path, name, provider=nil, force=false) + def add(path, name, formats=nil, force=false) + formats = [formats] if formats && !formats.is_a?(Array) + provider = nil + with_collection_lock do # A helper to check if a box exists. We store this in a variable # since we call it multiple times. - check_box_exists = lambda do |box_provider| - box = find(name, box_provider) + check_box_exists = lambda do |box_formats| + box = find(name, box_formats) next if !box if !force - @logger.error("Box already exists, can't add: #{name} #{box_provider}") - raise Errors::BoxAlreadyExists, :name => name, :provider => box_provider + @logger.error("Box already exists, can't add: #{name} #{box_formats.join(", ")}") + raise Errors::BoxAlreadyExists, :name => name, :formats => box_formats.join(", ") end # We're forcing, so just delete the old box - @logger.info("Box already exists, but forcing so removing: #{name} #{box_provider}") + @logger.info( + "Box already exists, but forcing so removing: #{name} #{box_formats.join(", ")}") box.destroy! end - log_provider = provider ? provider : "any provider" + log_provider = formats ? formats.join(", ") : "any provider" @logger.debug("Adding box: #{name} (#{log_provider}) from #{path}") # Verify the box doesn't exist early if we're given a provider. This # can potentially speed things up considerably since we don't need # to unpack any files. - check_box_exists.call(provider) if provider + check_box_exists.call(formats) if formats # Verify that a V1 box doesn't exist. If it does, then we signal # to the user that we need an upgrade. @@ -121,26 +125,35 @@ module Vagrant with_temp_dir(temp_dir) do |final_temp_dir| # Get an instance of the box we just added before it is finalized # in the system so we can inspect and use its metadata. - box = Box.new(name, provider, final_temp_dir) + box = Box.new(name, nil, final_temp_dir) # Get the provider, since we'll need that to at the least add it # to the system or check that it matches what is given to us. box_provider = box.metadata["provider"] - if provider - # Verify that the given provider matches what the box has. - if box_provider.to_sym != provider - @logger.error("Added box provider doesnt match expected: #{box_provider}") - raise Errors::BoxProviderDoesntMatch, :expected => provider, :actual => box_provider + if formats + found = false + formats.each do |format| + # Verify that the given provider matches what the box has. + if box_provider.to_sym == format + found = true + break + end + end + + if !found + @logger.error("Added box provider doesnt match expected: #{log_provider}") + raise Errors::BoxProviderDoesntMatch, + :expected => log_provider, :actual => box_provider end else - # We weren't given a provider, so store this one. - provider = box_provider.to_sym - # Verify the box doesn't already exist - check_box_exists.call(provider) + check_box_exists.call([box_provider]) end + # We weren't given a provider, so store this one. + provider = box_provider.to_sym + # Create the directory for this box, not including the provider box_dir = @directory.join(name) box_dir.mkpath diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 55b69a696..df79cf17c 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1006,7 +1006,7 @@ en: The box you're attempting to add already exists: Name: %{name} - Provider: %{provider} + Provider: %{formats} add: adding: |- Extracting box...