diff --git a/lib/vagrant/action/builtin/box_add.rb b/lib/vagrant/action/builtin/box_add.rb index 45a9f3129..d96b25d9a 100644 --- a/lib/vagrant/action/builtin/box_add.rb +++ b/lib/vagrant/action/builtin/box_add.rb @@ -114,8 +114,17 @@ module Vagrant version: metadata_version.version, provider: metadata_provider.name)) - # TODO(mitchellh): verify that the box we're adding - # doesn't already exist. + # Verify the box we're adding doesn't already exist + if !env[:box_force] + box = env[:box_collection].find( + metadata.name, metadata_provider.name, metadata_version.version) + if box + raise Errors::BoxAlreadyExists, + name: metadata.name, + provider: metadata_provider.name, + version: metadata_version.version + end + end # Now we have a URL, we have to download this URL. box = nil diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 36d278a69..7453d3c72 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -129,7 +129,7 @@ module Vagrant end class BoxAlreadyExists < VagrantError - error_key(:already_exists, "vagrant.actions.box.unpackage") + error_key(:box_add_exists) end class BoxChecksumInvalidType < VagrantError diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 51bd723f0..d8398954d 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -4,6 +4,13 @@ en: Machine booted and ready! boot_waiting: |- Waiting for machine to boot. This may take a few minutes... + box_add_exists: |- + The box you're attempting to add already exists. Remove it before + adding it again or add it with the `--force` flag. + + Name: %{name} + Provider: %{provider} + Version: %{version} box_add_with_version: |- Adding box '%{name}' (v%{version}) for '%{provider}' provider... box_added: |- @@ -1252,12 +1259,6 @@ en: output from attempting to unpackage (if any): %{output} - already_exists: |- - The box you're attempting to add already exists: - - Name: %{name} - Version: %{version} - Provider: %{providers} add: adding: |- Extracting box... diff --git a/test/unit/vagrant/action/builtin/box_add_test.rb b/test/unit/vagrant/action/builtin/box_add_test.rb index c9551be28..3439b159a 100644 --- a/test/unit/vagrant/action/builtin/box_add_test.rb +++ b/test/unit/vagrant/action/builtin/box_add_test.rb @@ -32,6 +32,10 @@ describe Vagrant::Action::Builtin::BoxAdd do FileChecksum.new(path, Digest::SHA1).checksum end + before do + box_collection.stub(find: nil) + end + context "with box file directly" do it "adds it" do box_path = iso_env.box2_file(:virtualbox) @@ -369,6 +373,41 @@ describe Vagrant::Action::Builtin::BoxAdd do to raise_error(Vagrant::Errors::BoxAddNoMatchingProvider) end + it "raises an error if a box already exists" 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] = tf.path + box_collection.should_receive(:find). + with("foo/bar", "virtualbox", "0.7").and_return(box) + box_collection.should_receive(:add).never + app.should_receive(:call).never + + expect { subject.call(env) }. + to raise_error(Vagrant::Errors::BoxAlreadyExists) + end + it "force adds a box if specified" do box_path = iso_env.box2_file(:virtualbox) tf = Tempfile.new("vagrant").tap do |f| @@ -396,6 +435,7 @@ describe Vagrant::Action::Builtin::BoxAdd do env[:box_force] = true env[:box_url] = tf.path + box_collection.stub(find: box) box_collection.should_receive(:add).with do |path, name, version, **opts| expect(checksum(path)).to eq(checksum(box_path)) expect(name).to eq("foo/bar")