From 562ed26533a072b4a255ba7d9fe31b1260c367e6 Mon Sep 17 00:00:00 2001 From: Ievgen Prokhorenko Date: Sun, 19 Jul 2015 16:38:52 +0300 Subject: [PATCH] Fix #3570 'Box data left in ~/.vagrant.d/boxes after removal' --- lib/vagrant/action/builtin/box_remove.rb | 1 + lib/vagrant/box_collection.rb | 28 ++++++++++++++++--- .../vagrant/action/builtin/box_remove_test.rb | 26 +++++++++++++++++ 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/lib/vagrant/action/builtin/box_remove.rb b/lib/vagrant/action/builtin/box_remove.rb index 1d0cea7fd..4c242aa97 100644 --- a/lib/vagrant/action/builtin/box_remove.rb +++ b/lib/vagrant/action/builtin/box_remove.rb @@ -106,6 +106,7 @@ module Vagrant provider: box.provider, version: box.version)) box.destroy! + env[:box_collection].clean_up(box) # Passes on the removed box to the rest of the middleware chain env[:box_removed] = box diff --git a/lib/vagrant/box_collection.rb b/lib/vagrant/box_collection.rb index a7599f7c2..17c2f3927 100644 --- a/lib/vagrant/box_collection.rb +++ b/lib/vagrant/box_collection.rb @@ -13,6 +13,8 @@ module Vagrant # boxes. class BoxCollection TEMP_PREFIX = "vagrant-box-add-temp-" + VAGRANT_SLASH = "-VAGRANTSLASH-" + VAGRANT_COLON = "-VAGRANTCOLON-" # The directory where the boxes in this collection are stored. # @@ -346,6 +348,19 @@ module Vagrant end end + # Removes the whole directory of a given box if there are no + # other versions nor providers of the box exist. + def clean_up(box) + return false if exists?(box.name) + + box_directory = box.name + .gsub('/', VAGRANT_SLASH) + .gsub(':', VAGRANT_COLON) + + path = File.join(directory, box_directory) + FileUtils.rm_r(path) + end + protected # Returns the directory name for the box of the given name. @@ -354,16 +369,16 @@ module Vagrant # @return [String] def dir_name(name) name = name.dup - name.gsub!(":", "-VAGRANTCOLON-") if Util::Platform.windows? - name.gsub!("/", "-VAGRANTSLASH-") + name.gsub!(":", VAGRANT_COLON) if Util::Platform.windows? + name.gsub!("/", VAGRANT_SLASH) name end # Returns the directory name for the box cleaned up def undir_name(name) name = name.dup - name.gsub!("-VAGRANTCOLON-", ":") - name.gsub!("-VAGRANTSLASH-", "/") + name.gsub!(VAGRANT_COLON, ":") + name.gsub!(VAGRANT_SLASH, "/") name end @@ -440,5 +455,10 @@ module Vagrant ensure dir.rmtree if dir.exist? end + + # Checks if a box with a given name exists. + def exists?(box_name) + all.any? { |box| box.first.eql?(box_name) } + end end end diff --git a/test/unit/vagrant/action/builtin/box_remove_test.rb b/test/unit/vagrant/action/builtin/box_remove_test.rb index 9de3e467b..016fec243 100644 --- a/test/unit/vagrant/action/builtin/box_remove_test.rb +++ b/test/unit/vagrant/action/builtin/box_remove_test.rb @@ -30,6 +30,8 @@ describe Vagrant::Action::Builtin::BoxRemove do expect(box_collection).to receive(:find).with( "foo", :virtualbox, "1.0").and_return(box) + expect(box_collection).to receive(:clean_up).with(box) + .and_return(true) expect(box).to receive(:destroy!).once expect(app).to receive(:call).with(env).once @@ -50,6 +52,8 @@ describe Vagrant::Action::Builtin::BoxRemove do expect(box_collection).to receive(:find).with( "foo", :virtualbox, "1.0").and_return(box) + expect(box_collection).to receive(:clean_up).with(box) + .and_return(false) expect(box).to receive(:destroy!).once expect(app).to receive(:call).with(env).once @@ -70,6 +74,8 @@ describe Vagrant::Action::Builtin::BoxRemove do expect(box_collection).to receive(:find).with( "foo", :virtualbox, "1.0").and_return(box) + expect(box_collection).to receive(:clean_up).with(box) + .and_return(false) expect(box).to receive(:destroy!).once expect(app).to receive(:call).with(env).once @@ -78,6 +84,22 @@ describe Vagrant::Action::Builtin::BoxRemove do expect(env[:box_removed]).to equal(box) end + it "deletes the whole directory of the box if it's the last box on the system" do + box_collection.stub( + all: [ + ["foo", "1.0", :virtualbox], + ]) + + env[:box_name] = "foo" + + expect(box_collection).to receive(:find).with( + "foo", :virtualbox, "1.0").and_return(box) + expect(box_collection).to receive(:clean_up).with(box) + .and_return(true) + + subject.call(env) + end + context "checking if a box is in use" do def new_entry(name, provider, version, valid=true) Vagrant::MachineIndex::Entry.new.tap do |entry| @@ -110,6 +132,8 @@ describe Vagrant::Action::Builtin::BoxRemove do expect(box_collection).to receive(:find).with( "foo", :virtualbox, "1.0").and_return(box) expect(box).to receive(:destroy!).once + expect(box_collection).to receive(:clean_up).with(box) + .and_return(true) subject.call(env) end @@ -123,6 +147,8 @@ describe Vagrant::Action::Builtin::BoxRemove do expect(box_collection).to receive(:find).with( "foo", :virtualbox, "1.0").and_return(box) + expect(box_collection).to receive(:clean_up).with(box) + .and_return(true) expect(box).to receive(:destroy!).once subject.call(env)