Merge pull request #6567 from mitchellh/b-prune-folders

core: remove saved synced folders not from Vagrantfile

lib/vagrant/action/builtin/mixin_synced_folders.rb

@@ -76,6 +76,16 @@ module Vagrant
           if opts[:merge]
             existing = cached_synced_folders(machine)
             if existing
+              if opts[:vagrantfile]
+                # Go through and find any cached that were from the
+                # Vagrantfile itself. We remove those if it was requested.
+                existing.each do |impl, fs|
+                  fs.each do |id, data|
+                    fs.delete(id) if data[:__vagrantfile]
+                  end
+                end
+              end
+
               folders.each do |impl, fs|
                 existing[impl] ||= {}
                 fs.each do |id, data|
@@ -101,7 +111,12 @@ module Vagrant
           return cached_synced_folders(machine) if opts[:cached]
 
           config = opts[:config]
-          config ||= machine.config.vm
+          root   = false
+          if !config
+            config = machine.config.vm
+            root   = true
+          end
+
           config_folders = config.synced_folders
           folders = {}
 
@@ -131,9 +146,17 @@ module Vagrant
               end
             end
 
+            # Get the data to store
+            data = data.dup
+            if root
+              # If these are the root synced folders (attached directly)
+              # to the Vagrantfile, then we mark it as such.
+              data[:__vagrantfile] = true
+            end
+
             # Keep track of this shared folder by the implementation.
             folders[impl] ||= {}
-            folders[impl][id] = data.dup
+            folders[impl][id] = data
           end
 
           # If we have folders with the "default" key, then determine the

lib/vagrant/action/builtin/synced_folders.rb

@@ -23,6 +23,7 @@ module Vagrant
             config: env[:synced_folders_config],
           }
 
+          @logger.info("SyncedFolders loading from cache: #{opts[:cached]}")
           folders = synced_folders(env[:machine], **opts)
           original_folders = folders
 
@@ -121,8 +122,11 @@ module Vagrant
 
             save_synced_folders(env[:machine], all)
           else
+            save_opts = { merge: true }
+            save_opts[:vagrantfile] = true if !opts[:config]
+
             # Save the synced folders
-            save_synced_folders(env[:machine], original_folders, merge: true)
+            save_synced_folders(env[:machine], original_folders, **save_opts)
           end
         end
       end

test/unit/vagrant/action/builtin/mixin_synced_folders_test.rb

@@ -114,11 +114,13 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do
       result = subject.synced_folders(machine)
       expect(result.length).to eq(2)
       expect(result[:default]).to eq({
-        "another" => folders["another"],
-        "foo" => folders["foo"],
-        "root" => folders["root"],
+        "another" => folders["another"].merge(__vagrantfile: true),
+        "foo" => folders["foo"].merge(__vagrantfile: true),
+        "root" => folders["root"].merge(__vagrantfile: true),
+      })
+      expect(result[:nfs]).to eq({
+        "nfs" => folders["nfs"].merge(__vagrantfile: true),
       })
-      expect(result[:nfs]).to eq({ "nfs" => folders["nfs"] })
     end
 
     it "should return the proper set of folders of a custom config" do
@@ -185,16 +187,20 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do
       result = subject.synced_folders(machine, cached: true)
       expect(result.length).to eq(2)
       expect(result[:default]).to eq({
-        "another" => old_folders["another"],
-        "foo" => old_folders["foo"],
-        "root" => old_folders["root"],
+        "another" => old_folders["another"].merge(__vagrantfile: true),
+        "foo" => old_folders["foo"].merge(__vagrantfile: true),
+        "root" => old_folders["root"].merge(__vagrantfile: true),
       })
-      expect(result[:nfs]).to eq({ "nfs" => old_folders["nfs"] })
+      expect(result[:nfs]).to eq({ "nfs" => old_folders["nfs"].merge(__vagrantfile: true) })
     end
 
     it "should be able to save and retrieve cached versions" do
-      folders["foo"] = { type: "default" }
-      result = subject.synced_folders(machine)
+      other_folders = {}
+      other = double("config")
+      other.stub(synced_folders: other_folders)
+
+      other_folders["foo"] = { type: "default" }
+      result = subject.synced_folders(machine, config: other)
       subject.save_synced_folders(machine, result)
 
       # Clear the folders and set some more
@@ -212,10 +218,36 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do
       expect(result.length).to eq(2)
       expect(result[:default]).to eq({
         "foo" => { type: "default" },
-        "bar" => { type: "default" },
+        "bar" => { type: "default", __vagrantfile: true},
       })
       expect(result[:nfs]).to eq({
-        "baz" => { type: "nfs" }
+        "baz" => { type: "nfs", __vagrantfile: true }
+      })
+    end
+
+    it "should remove items from the vagrantfile that were removed" do
+      folders["foo"] = { type: "default" }
+      result = subject.synced_folders(machine)
+      subject.save_synced_folders(machine, result)
+
+      # Clear the folders and set some more
+      folders.clear
+      folders["bar"] = { type: "default" }
+      folders["baz"] = { type: "nfs" }
+      result = subject.synced_folders(machine)
+      subject.save_synced_folders(machine, result, merge: true, vagrantfile: true)
+
+      # Clear one last time
+      folders.clear
+
+      # Read them all back
+      result = subject.synced_folders(machine, cached: true)
+      expect(result.length).to eq(2)
+      expect(result[:default]).to eq({
+        "bar" => { type: "default", __vagrantfile: true},
+      })
+      expect(result[:nfs]).to eq({
+        "baz" => { type: "nfs", __vagrantfile: true }
       })
     end
   end

test/unit/vagrant/action/builtin/synced_folders_test.rb

@@ -197,5 +197,32 @@ describe Vagrant::Action::Builtin::SyncedFolders do
       expect(ids.length).to eq(2)
       expect(ids[0]).to eq(ids[1])
     end
+
+    context "with folders from the machine" do
+      it "removes outdated folders not present in config" do
+        expect(subject).to receive(:save_synced_folders).with(
+          machine, anything, merge: true, vagrantfile: true)
+
+        subject.call(env)
+      end
+    end
+
+    context "with custom folders" do
+      before do
+        new_config = double("config")
+        env[:synced_folders_config] = new_config
+
+        allow(subject).to receive(:synced_folders).
+          with(machine, config: new_config, cached: false).
+          and_return({})
+      end
+
+      it "doesn't remove outdated folders not present in config" do
+        expect(subject).to receive(:save_synced_folders).with(
+          machine, anything, merge: true)
+
+        subject.call(env)
+      end
+    end
   end
 end