diff --git a/lib/vagrant/action/builtin/box_remove.rb b/lib/vagrant/action/builtin/box_remove.rb index 93a1dd355..faa1d685c 100644 --- a/lib/vagrant/action/builtin/box_remove.rb +++ b/lib/vagrant/action/builtin/box_remove.rb @@ -12,24 +12,67 @@ module Vagrant def call(env) box_name = env[:box_name] - box_provider = env[:box_provider].to_sym + box_provider = env[:box_provider] + box_provider = box_provider.to_sym if box_provider + box_version = env[:box_version] - box = nil - begin - box = env[:box_collection].find(box_name, box_provider) - rescue Vagrant::Errors::BoxUpgradeRequired - env[:box_collection].upgrade(box_name) - retry + boxes = {} + env[:box_collection].all.each do |n, v, p| + boxes[n] ||= {} + boxes[n][p] ||= [] + boxes[n][p] << v end - raise Vagrant::Errors::BoxNotFound, :name => box_name, :provider => box_provider if !box + all_box = boxes[box_name] + if !all_box + raise Errors::BoxRemoveNotFound, name: box_name + end + + all_versions = nil + if !box_provider + if all_box.length == 1 + # There is only one provider, just use that. + all_versions = all_box.values.first + box_provider = all_box.keys.first + else + raise Errors::BoxRemoveMultiProvider, + name: box_name, + providers: all_box.keys.map(&:to_s).sort.join(", ") + end + else + all_versions = all_box[box_provider] + if !all_versions + raise Errors::BoxRemoveProviderNotFound, + name: box_name, + provider: box_provider.to_s, + providers: all_box.keys.map(&:to_s).sort.join(", ") + end + end + + if !box_version + if all_versions.length == 1 + # There is only one version, just use that. + box_version = all_versions.first + else + # There are multiple versions, we can't choose. + raise Errors::BoxRemoveMultiVersion, + name: box_name, + provider: box_provider.to_s, + versions: all_versions.join(", ") + end + end + + box = env[:box_collection].find( + box_name, box_provider, box_version) + env[:ui].info(I18n.t("vagrant.commands.box.removing", - :name => box_name, - :provider => box_provider)) + :name => box.name, + :provider => box.provider)) box.destroy! # Passes on the removed box to the rest of the middleware chain env[:box_removed] = box + @app.call(env) end end diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 61c2c4ec2..1984c5efd 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -160,14 +160,26 @@ module Vagrant error_key(:box_metadata_malformed) end - class BoxNotFound < VagrantError - error_key(:box_not_found) - end - class BoxProviderDoesntMatch < VagrantError error_key(:box_provider_doesnt_match) end + class BoxRemoveNotFound < VagrantError + error_key(:box_remove_not_found) + end + + class BoxRemoveProviderNotFound < VagrantError + error_key(:box_remove_provider_not_found) + end + + class BoxRemoveMultiProvider < VagrantError + error_key(:box_remove_multi_provider) + end + + class BoxRemoveMultiVersion < VagrantError + error_key(:box_remove_multi_version) + end + class BoxUnpackageFailure < VagrantError error_key(:untar_failure, "vagrant.actions.box.unpackage") end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 47d80ead7..611367c29 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -315,12 +315,33 @@ en: that this issue can be fixed. %{error} - box_not_found: Box '%{name}' with '%{provider}' provider could not be found. box_provider_doesnt_match: |- The box you attempted to add doesn't match the provider you specified. Provider expected: %{expected} Provider of box: %{actual} + box_remove_multi_provider: |- + You requested to remove the box '%{name}'. This box has + multiple providers. You must explicitly select a single + provider to remove with `--provider`. + + Available providers: %{providers} + box_remove_multi_version: |- + You requested to remove the box '%{name}' with provider + '%{provider}'. This box has multiple versions. You must + explicitly specify which version you want to remove with + the `--box-version` flag. + + Versions: %{versions} + box_remove_not_found: |- + The box you requested to be removed could not be found. No + boxes named '%{name}' could be found. + box_remove_provider_not_found: |- + You requested to remove the box '%{name}' with provider + '%{provider}'. The box '%{name}' exists but not with + the provider specified. Please double-check and try again. + + The providers for this are: %{providers} box_upgrade_required: |- The box '%{name}' is still stored on disk in the Vagrant 1.0.x format. This box must be upgraded in order to work properly with diff --git a/test/unit/vagrant/action/builtin/box_remove_test.rb b/test/unit/vagrant/action/builtin/box_remove_test.rb new file mode 100644 index 000000000..19a15b3f3 --- /dev/null +++ b/test/unit/vagrant/action/builtin/box_remove_test.rb @@ -0,0 +1,108 @@ +require File.expand_path("../../../../base", __FILE__) + +describe Vagrant::Action::Builtin::BoxRemove do + include_context "unit" + + let(:app) { lambda { |env| } } + let(:env) { { + box_collection: box_collection, + ui: Vagrant::UI::Silent.new, + } } + + subject { described_class.new(app, env) } + + let(:box_collection) { double("box_collection") } + let(:iso_env) { isolated_environment } + + let(:box) do + box_dir = iso_env.box3("foo", "1.0", :virtualbox) + Vagrant::Box.new("foo", :virtualbox, "1.0", box_dir) + end + + it "deletes the box if it is the only option" do + box_collection.stub(all: [["foo", "1.0", :virtualbox]]) + + env[:box_name] = "foo" + + box_collection.should_receive(:find).with( + "foo", :virtualbox, "1.0").and_return(box) + box.should_receive(:destroy!).once + app.should_receive(:call).with(env).once + + subject.call(env) + + expect(env[:box_removed]).to equal(box) + end + + it "deletes the box with the specified provider if given" do + box_collection.stub( + all: [ + ["foo", "1.0", :virtualbox], + ["foo", "1.0", :vmware], + ]) + + env[:box_name] = "foo" + env[:box_provider] = "virtualbox" + + box_collection.should_receive(:find).with( + "foo", :virtualbox, "1.0").and_return(box) + box.should_receive(:destroy!).once + app.should_receive(:call).with(env).once + + subject.call(env) + + expect(env[:box_removed]).to equal(box) + end + + it "errors if the box doesn't exist" do + box_collection.stub(all: []) + + app.should_receive(:call).never + + expect { subject.call(env) }. + to raise_error(Vagrant::Errors::BoxRemoveNotFound) + end + + it "errors if the specified provider doesn't exist" do + env[:box_name] = "foo" + env[:box_provider] = "bar" + + box_collection.stub(all: [["foo", "1.0", :virtualbox]]) + + app.should_receive(:call).never + + expect { subject.call(env) }. + to raise_error(Vagrant::Errors::BoxRemoveProviderNotFound) + end + + it "errors if there are multiple providers" do + env[:box_name] = "foo" + + box_collection.stub( + all: [ + ["foo", "1.0", :virtualbox], + ["foo", "1.0", :vmware], + ]) + + app.should_receive(:call).never + + expect { subject.call(env) }. + to raise_error(Vagrant::Errors::BoxRemoveMultiProvider) + end + + it "errors if the specified provider has multiple versions" do + env[:box_name] = "foo" + env[:box_provider] = "virtualbox" + + box_collection.stub( + all: [ + ["foo", "1.0", :virtualbox], + ["foo", "1.1", :virtualbox], + ]) + + app.should_receive(:call).never + + expect { subject.call(env) }. + to raise_error(Vagrant::Errors::BoxRemoveMultiVersion) + end +end