Fixing #594 -- Files on the local machine but not remote machine now purge. Note that this introduces a difference in behaviour between recursing locally and recursing remotely.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2436 980ebf18-57e1-0310-9a29-db15c13687c0

lib/puppet/type/pfile.rb

@@ -190,7 +190,10 @@ module Puppet
                 configured the purged files will be uploaded, but if you do not,
                 this will destroy data.  Only use this option for generated
                 files unless you really know what you are doing.  This option only
-                makes sense when recursively managing directories."
+                makes sense when recursively managing directories.
+                
+                Note that when using ``purge`` with ``source``, Puppet will purge any files
+                that are not on the remote system."
 
             defaultto :false
 
@@ -720,14 +723,22 @@ module Puppet
             if ret = self.localrecurse(recurse)
                 children += ret
             end
-            if @parameters.include?(:source) and ret = self.sourcerecurse(recurse)
-                children += ret
+
+            # These will be files pulled in by the file source
+            sourced = false
+            if @parameters.include?(:source)
+                ret, sourced = self.sourcerecurse(recurse)
+                if ret
+                    children += ret
+                end
             end
 
             # The purge check needs to happen after all of the other recursion.
             if self.purge?
                 children.each do |child|
-                    child[:ensure] = :absent unless child.managed?
+                    if (sourced and ! sourced.include?(child[:path])) or ! child.managed?
+                        child[:ensure] = :absent
+                    end
                 end
             end
             
@@ -852,6 +863,10 @@ module Puppet
 
             result = []
             found = []
+
+            # Keep track of all the files we found in the source, so we can purge
+            # appropriately.
+            sourced = []
             
             @parameters[:source].should.each do |source|
                 sourceobj, path = uri2obj(source)
@@ -892,15 +907,16 @@ module Puppet
                     end
 
                     found << name
+                    sourced << File.join(self[:path], name)
 
                     self.newchild(name, false, args)
                 }.reject {|c| c.nil? }
 
                 if self[:sourceselect] == :first
-                    return result
+                    return [result, sourced]
                 end
             end
-            return result
+            return [result, sourced]
         end
 
         # Set the checksum, from another property.  There are multiple

test/ral/types/file.rb

@@ -567,7 +567,7 @@ class TestFile < Test::Unit::TestCase
         return_nil = false
         
         # and monkey-patch it
-        [:localrecurse, :sourcerecurse, :linkrecurse].each do |m|
+        [:localrecurse, :linkrecurse].each do |m|
             dir.meta_def(m) do |recurse|
                 if return_nil # for testing nil return, of course
                     return nil
@@ -576,6 +576,16 @@ class TestFile < Test::Unit::TestCase
                 end
             end
         end
+
+        # We have to special-case this, because it returns a list of
+        # found files.
+        dir.meta_def(:sourcerecurse) do |recurse|
+            if return_nil # for testing nil return, of course
+                return nil
+            else
+                return [recurse], []
+            end
+        end
         
         # First try it with recurse set to false
         dir[:recurse] = false

test/ral/types/filesources.rb

@@ -273,9 +273,12 @@ class TestFileSources < Test::Unit::TestCase
         obj = Puppet::Type.newfile :source => source, :path => dest, :recurse => true
 
         result = nil
+        sourced = nil
         assert_nothing_raised do
-            result = obj.sourcerecurse(true)
+            result, sourced = obj.sourcerecurse(true)
         end
+
+        assert_equal([destfile], sourced, "Did not get correct list of sourced objects")
         dfileobj = @file[destfile]
         assert(dfileobj, "Did not create destfile object")
         assert_equal([dfileobj], result)
@@ -289,8 +292,9 @@ class TestFileSources < Test::Unit::TestCase
 
         result = nil
         assert_nothing_raised do
-            result = obj.sourcerecurse(true)
+            result, sourced = obj.sourcerecurse(true)
         end
+        assert_equal([destfile], sourced, "Did not get correct list of sourced objects")
         dfileobj = @file[destfile]
         assert(dfileobj, "Did not create destfile object with a missing source")
         assert_equal([dfileobj], result)
@@ -300,8 +304,9 @@ class TestFileSources < Test::Unit::TestCase
         obj[:source] = [nosource, tempfile()]
         
         assert_nothing_raised do
-            result = obj.sourcerecurse(true)
+            result, sourced = obj.sourcerecurse(true)
         end
+        assert_equal([], sourced, "Did not get correct list of sourced objects")
         assert_equal([], result, "Sourcerecurse failed when all sources are missing")
     end
 
@@ -1009,6 +1014,33 @@ class TestFileSources < Test::Unit::TestCase
             assert_equal("yay%s\n" % File.basename(file).sub("file", ''), File.read(path), "file was not copied correctly")
         end
     end
+
+    # #594
+    def test_purging_missing_remote_files
+        source = tempfile()
+        dest = tempfile()
+        s1 = File.join(source, "file1")
+        s2 = File.join(source, "file2")
+        d1 = File.join(dest, "file1")
+        d2 = File.join(dest, "file2")
+        Dir.mkdir(source)
+        [s1, s2].each { |name| File.open(name, "w") { |file| file.puts "something" } }
+
+        # We have to add a second parameter, because that's the only way to expose the "bug".
+        file = Puppet::Type.newfile(:path => dest, :source => source, :recurse => true, :purge => true, :mode => "755")
+
+        assert_apply(file)
+
+        assert(FileTest.exists?(d1), "File1 was not copied")
+        assert(FileTest.exists?(d2), "File2 was not copied")
+
+        File.unlink(s2)
+
+        assert_apply(file)
+
+        assert(FileTest.exists?(d1), "File1 was not kept")
+        assert(! FileTest.exists?(d2), "File2 was not purged")
+    end
 end
 
 # $Id$