Fix #3570 'Box data left in ~/.vagrant.d/boxes after removal'

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

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!(":", VAGRANT_COLON) if Util::Platform.windows?
-      name.gsub!("/", "-VAGRANTSLASH-")
+      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!(VAGRANT_COLON, ":")
-      name.gsub!("-VAGRANTSLASH-", "/")
+      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

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)