From f0607c6df07040ea233ceea3251e1e6380afc7a0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 24 Jan 2014 12:50:07 -0800 Subject: [PATCH] core: BoxAdd supports multiple URLs properly --- lib/vagrant/action/builtin/box_add.rb | 88 ++++++++++++++----- lib/vagrant/errors.rb | 4 + templates/locales/en.yml | 11 +++ .../vagrant/action/builtin/box_add_test.rb | 57 ++++++++++++ 4 files changed, 137 insertions(+), 23 deletions(-) diff --git a/lib/vagrant/action/builtin/box_add.rb b/lib/vagrant/action/builtin/box_add.rb index f35b3f5ec..e185b82ea 100644 --- a/lib/vagrant/action/builtin/box_add.rb +++ b/lib/vagrant/action/builtin/box_add.rb @@ -22,30 +22,49 @@ module Vagrant def call(env) @download_interrupted = false - url = env[:box_url] + url = Array(env[:box_url]) # If we received a shorthand URL ("mitchellh/precise64"), # then expand it properly. expanded = false - uri = URI.parse(url) - if !uri.scheme && !File.file?(url) - expanded = true - url = "#{Vagrant.server_url}/#{url}" + url.each_index do |i| + uri = URI.parse(url[i]) + if !uri.scheme && !File.file?(url[i]) + expanded = true + url[i] = "#{Vagrant.server_url}/#{url[i]}" + end end - is_metadata = false - begin - is_metadata = metadata_url?(url, env) - rescue Errors::DownloaderError => e - raise if !expanded - raise Errors::BoxAddShortNotFound, - error: e.extra_data[:message], - name: env[:box_url], - url: url + # Test if any of our URLs point to metadata + is_metadata_results = url.map do |u| + begin + metadata_url?(u, env) + rescue Errors::DownloaderError => e + e + end + end + + if expanded && url.length == 1 + is_error = is_metadata_results.find do |b| + b.is_a?(Errors::DownloaderError) + end + + if is_error + raise Errors::BoxAddShortNotFound, + error: is_error.extra_data[:message], + name: env[:box_url], + url: url + end + end + + is_metadata = is_metadata_results.any? { |b| b === true } + if is_metadata && url.length > 1 + raise Errors::BoxAddMetadataMultiURL, + urls: url.join(", ") end if is_metadata - add_from_metadata(url, env) + add_from_metadata(url.first, env, expanded) else add_direct(url, env) end @@ -55,7 +74,10 @@ module Vagrant # Adds a box file directly (no metadata component, versioning, # etc.) - def add_direct(url, env) + # + # @param [Array] urls + # @param [Hash] env + def add_direct(urls, env) name = env[:box_name] if !name || name == "" raise Errors::BoxAddNameRequired @@ -65,7 +87,7 @@ module Vagrant provider = Array(provider) if provider box_add( - url, + urls, name, "0", provider, @@ -73,7 +95,7 @@ module Vagrant end # Adds a box given that the URL is a metadata document. - def add_from_metadata(url, env) + def add_from_metadata(url, env, expanded) original_url = env[:box_url] provider = env[:box_provider] provider = Array(provider) if provider @@ -93,6 +115,12 @@ module Vagrant File.open(metadata_path) do |f| metadata = BoxMetadata.new(f) end + rescue Errors::DownloaderError => e + raise if !expanded + raise Errors::BoxAddShortNotFound, + error: e.extra_data[:message], + name: original_url, + url: url ensure metadata_path.delete if metadata_path && metadata_path.file? end @@ -160,7 +188,7 @@ module Vagrant end box_add( - metadata_provider.url, + [metadata_provider.url], metadata.name, metadata_version.version, metadata_provider.name, env) @@ -232,13 +260,13 @@ module Vagrant # Shared helper to add a box once you know various details # about it. Shared between adding via metadata or by direct. # - # @param [String] url + # @param [Array] urls # @param [String] name # @param [String] version # @param [String] provider # @param [Hash] env # @return [Box] - def box_add(url, name, version, provider, env, **opts) + def box_add(urls, name, version, provider, env, **opts) env[:ui].output(I18n.t( "vagrant.box_add_with_version", name: name, @@ -260,7 +288,21 @@ module Vagrant # Now we have a URL, we have to download this URL. box = nil begin - box_url = download(url, env) + box_url = nil + + urls.each do |url| + env[:ui].detail(I18n.t( + "vagrant.box_downloading", url: url)) + + begin + box_url = download(url, env) + break + rescue Errors::DownloaderError => e + env[:ui].error(I18n.t( + "vagrant.box_download_error", message: e.message)) + box_url = nil + end + end # Add the box! box = env[:box_collection].add( @@ -273,7 +315,7 @@ module Vagrant # so we can resume the download later. if !@download_interrupted @logger.debug("Deleting temporary box: #{box_url}") - box_url.delete + box_url.delete if box_url end end diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 54a63ab26..596f073d9 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -120,6 +120,10 @@ module Vagrant error_key(:batch_multi_error) end + class BoxAddMetadataMultiURL < VagrantError + error_key(:box_add_metadata_multi_url) + end + class BoxAddNameMismatch < VagrantError error_key(:box_add_name_mismatch) end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 4caa5da4f..e0ce29031 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -20,6 +20,10 @@ en: Adding box '%{name}' (v%{version}) for '%{provider}' provider... box_added: |- Successfully added box '%{name}' for '%{provider}'! + box_downloading: |- + Downloading: %{url} + box_download_error: |- + Error downloading: %{message} box_expanding_url: |- URL: %{url} box_loading_metadata: |- @@ -289,6 +293,13 @@ en: Name: %{name} Provider: %{provider} Version: %{version} + box_add_metadata_multi_url: |- + Multiple URLs for a box can't be specified when adding + versioned boxes. Please specify a single URL to the box + metadata (JSON) information. The full list of URLs you + specified is shown below: + + %{urls} box_add_name_mismatch: |- The box you're adding has a name different from the name you requested. For boxes with metadata, you cannot override the name. diff --git a/test/unit/vagrant/action/builtin/box_add_test.rb b/test/unit/vagrant/action/builtin/box_add_test.rb index e821d25ff..152f60c84 100644 --- a/test/unit/vagrant/action/builtin/box_add_test.rb +++ b/test/unit/vagrant/action/builtin/box_add_test.rb @@ -77,6 +77,27 @@ describe Vagrant::Action::Builtin::BoxAdd do subject.call(env) end + it "adds from multiple URLs" do + box_path = iso_env.box2_file(:virtualbox) + + env[:box_name] = "foo" + env[:box_url] = [ + "/foo/bar/baz", + box_path.to_s, + ] + + box_collection.should_receive(:add).with do |path, name, version| + expect(checksum(path)).to eq(checksum(box_path)) + expect(name).to eq("foo") + expect(version).to eq("0") + true + end.and_return(box) + + app.should_receive(:call).with(env) + + subject.call(env) + end + it "adds from HTTP URL" do box_path = iso_env.box2_file(:virtualbox) with_web_server(box_path) do |port| @@ -252,6 +273,42 @@ describe Vagrant::Action::Builtin::BoxAdd do end end + it "raises an error if multiple metadata URLs are given" do + box_path = iso_env.box2_file(:virtualbox) + tf = Tempfile.new("vagrant").tap do |f| + f.write(<<-RAW) + { + "name": "foo/bar", + "versions": [ + { + "version": "0.5" + }, + { + "version": "0.7", + "providers": [ + { + "name": "virtualbox", + "url": "#{box_path}" + } + ] + } + ] + } + RAW + f.close + end + + env[:box_url] = [ + "/foo/bar/baz", + tf.path, + ] + box_collection.should_receive(:add).never + app.should_receive(:call).never + + expect { subject.call(env) }. + to raise_error(Vagrant::Errors::BoxAddMetadataMultiURL) + end + it "adds the latest version of a box with only one provider" do box_path = iso_env.box2_file(:virtualbox) tf = Tempfile.new("vagrant").tap do |f|