sources now pass all tests

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

3 files changed, 248 insertions, 122 deletions

diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 4f937334d..212e47773 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -605,8 +605,12 @@ class Type < Puppet::Element
         # point to State objects
         # further, the lists of valid states and parameters are defined
         # at the class level
-        @states = Hash.new(false)
-        @parameters = Hash.new(false)
+        unless defined? @states
+            @states = Hash.new(false)
+        end
+        unless defined? @parameters
+            @parameters = Hash.new(false)
+        end
 
         @noop = false
 
@@ -820,7 +824,7 @@ class Type < Puppet::Element
         # now record how many changes we've resulted in
         Puppet::Metric.add(self.class,self,:changes,changes.length)
         if changes.length > 0
-            Puppet.info "%s resulted in %s changes" %
+            Puppet.info "%s: %s change(s)" %
                 [self.name, changes.length]
         end
         return changes.flatten
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 6ddcaa94a..bde232845 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -121,9 +121,19 @@ module Puppet
                         self.parent.delete(self.name)
                         return
                     else
-                        File.open(self.parent[:path]) { |file|
-                            sum = Digest::MD5.hexdigest(file.read)
-                        }
+                        begin
+                            File.open(self.parent[:path]) { |file|
+                                sum = Digest::MD5.hexdigest(file.read)
+                            }
+                        rescue Errno::EACCES => detail
+                            Puppet.notice "Cannot checksum %s: permission denied" %
+                                self.parent.name
+                            self.parent.delete(self.class.name)
+                        rescue => detail
+                            Puppet.notice "Cannot checksum %s: %s" %
+                                detail
+                            self.parent.delete(self.class.name)
+                        end
                     end
                 when "md5lite":
                     if FileTest.directory?(self.parent[:path])
@@ -537,13 +547,11 @@ module Puppet
 
             def retrieve
                 sum = nil
-                unless sum = self.parent.state(:checksum)
-                    raise Puppet::Error.new(
-                        "Cannot copy without knowing the sum state of %s" %
-                        self.parent.path
-                    )
+                if sum = self.parent.state(:checksum)
+                    @is = sum.is
+                else
+                    @is = -1
                 end
-                @is = sum.is
             end
 
             def should=(source)
@@ -575,17 +583,27 @@ module Puppet
             end
 
             def sync
+                if @is == -1
+                    self.retrieve # try again
+                    if @is == -1
+                        if @is == @should
+                            return nil
+                        end
+                    end
+                end
                 @backed = false
+                bak = self.parent[:backup] || ".puppet-bak"
+
                 # try backing ourself up before we overwrite
                 if FileTest.file?(self.parent.name)
                     if bucket = self.parent[:filebucket]
                         bucket.backup(self.parent.name)
                         @backed = true
-                    elsif str = self.parent[:backup]
+                    elsif self.parent[:backup]
                         # back the file up
                         begin
                             FileUtils.cp(self.parent.name,
-                                self.parent.name + self.parent[:backup])
+                                self.parent.name + bak)
                             @backed = true
                         rescue => detail
                             # since they said they want a backup, let's error out
@@ -597,6 +615,10 @@ module Puppet
                     end
                 end
 
+                unless self.parent[:backup]
+                    @backed = true
+                end
+
                 # okay, we've now got whatever backing up done we might need
                 # so just copy the files over
                 if @local
@@ -607,10 +629,10 @@ module Puppet
                             if FileTest.exists?(self.parent.name)
                                 # get the file here
                                 FileUtils.cp(@source, self.parent.name + ".tmp")
-                                if FileTest.exists?(self.parent.name + ".puppet-bak")
+                                if FileTest.exists?(self.parent.name + bak)
                                     Puppet.warning "Deleting backup of %s" %
                                         self.parent.name
-                                    File.unlink(self.parent.name + ".puppet-bak")
+                                    File.unlink(self.parent.name + bak)
                                 end
                                 # rename the existing one
                                 File.rename(
@@ -624,7 +646,10 @@ module Puppet
                                 )
                                 # if we've made a backup, then delete the old file
                                 if @backed
-                                    File.unlink(self.parent.name + ".puppet-bak")
+                                    #Puppet.err "Unlinking backup"
+                                    File.unlink(self.parent.name + bak)
+                                #else
+                                    #Puppet.err "Not unlinking backup"
                                 end
                             else
                                 # the easy case
@@ -674,10 +699,10 @@ module Puppet
 
             @parameters = [
                 :path,
+                :backup,
                 :source,
                 :recurse,
-                :filebucket,
-                :backup
+                :filebucket
             ]
 
             @name = :file
@@ -690,9 +715,23 @@ module Puppet
                 @arghash.delete(self.class.namevar)
 
                 @stat = nil
+                @parameters = Hash.new(false)
+
+                # default to a string, which is true
+                @parameters[:backup] = ".puppet-bak"
                 super
             end
 
+            def parambackup=(value)
+                if value == false or value == "false"
+                    @parameters[:backup] = false
+                elsif value == true or value == "true"
+                    @parameters[:backup] = ".puppet-bak"
+                else
+                    @parameters[:backup] = value
+                end
+            end
+
             def newchild(path, hash = {})
                 if path =~ %r{^#{File::SEPARATOR}}
                     raise Puppet::DevError.new(
@@ -756,7 +795,6 @@ module Puppet
                                 [path,detail.message]
                         )
                         Puppet.debug args.inspect
-                        puts detail.stack
                         child = nil
                     rescue => detail
                         Puppet.notice(
@@ -764,7 +802,6 @@ module Puppet
                                 [path,detail]
                         )
                         Puppet.debug args.inspect
-                        Puppet.err detail.class
                         child = nil
                     end
                 end
@@ -1015,6 +1052,11 @@ module Puppet
                         "Uh, somehow trying to manage non-dir %s" % self.name
                     )
                 end
+                unless FileTest.readable? self.name
+                    Puppet.notice "Cannot manage %s: permission denied" % self.name
+                    return
+                end
+
                 Dir.foreach(self.name) { |file|
                     next if file =~ /^\.\.?/ # skip . and ..
                     # XXX it's right here
diff --git a/test/types/tc_file.rb b/test/types/tc_file.rb
index 21ccc8a69..2808798f0 100644
--- a/test/types/tc_file.rb
+++ b/test/types/tc_file.rb
@@ -416,7 +416,7 @@ class TestFile < Test::Unit::TestCase
         return ret
     end
 
-    def mkranddirsandfiles(dirs = nil,files = nil,depth = 2)
+    def mkranddirsandfiles(dirs = nil,files = nil,depth = 3)
         if depth < 0
             return
         end
@@ -451,6 +451,14 @@ class TestFile < Test::Unit::TestCase
         }
     end
 
+    def file_list(dir)
+        list = nil
+        FileUtils.cd(dir) {
+            list = %x{find . 2>/dev/null}.chomp.split(/\n/)
+        }
+        return list
+    end
+
     def assert_trees_equal(fromdir,todir)
         assert(FileTest.directory?(fromdir))
         assert(FileTest.directory?(todir))
@@ -462,10 +470,8 @@ class TestFile < Test::Unit::TestCase
                 ! FileTest.readable?(file)
             }
         }
-        tolist = nil
-        FileUtils.cd(todir) {
-            tolist = %x{find . 2>/dev/null}.chomp.split(/\n/)
-        }
+        tolist = file_list(todir)
+
         assert_equal(fromlist,tolist)
 
         # and then do some verification that the files are actually set up
@@ -494,24 +500,14 @@ class TestFile < Test::Unit::TestCase
         }
     end
 
-    def delete_random_files(dir)
+    def random_files(dir)
         checked = 0
-        list = nil
-        FileUtils.cd(dir) {
-            list = %x{find . 2>/dev/null}.chomp.split(/\n/)
-        }
+        list = file_list(dir)
         list.reverse.each_with_index { |file,i|
             path = File.join(dir,file)
             stat = File.stat(dir)
-            if checked < 10 and i % 3 == 0
-                begin
-                    if stat.ftype == "directory"
-                    else
-                        File.unlink(path)
-                    end
-                rescue => detail
-                    # we probably won't be able to open our own secured files
-                    puts detail
+            if checked < 10 and (i % 3) == 2
+                unless yield path
                     next
                 end
                 checked += 1
@@ -519,103 +515,100 @@ class TestFile < Test::Unit::TestCase
         }
     end
 
-    def test_xcomplicatedlocalsource
-        path = "/tmp/Complsourcetest"
-        @@tmpfiles.push path
-        system("mkdir -p #{path}")
-
-        # okay, let's create a directory structure
-        fromdir = File.join(path,"fromdir")
-        Dir.mkdir(fromdir)
-        FileUtils.cd(fromdir) {
-            mkranddirsandfiles()
-        }
-
-        2.times { |index|
-            Puppet.err "Take %s" % index
-            initstorage
-            todir = File.join(path,"Todir")
-            tofile = nil
-            trans = nil
-
-            assert_nothing_raised {
-                tofile = Puppet::Type::PFile.new(
-                    :name => todir,
-                    "recurse" => true,
-                    "source" => fromdir
-                )
-            }
-            comp = Puppet::Component.new(
-                :name => "component"
-            )
-            comp.push tofile
-            assert_nothing_raised {
-                trans = comp.evaluate
-            }
-            assert_nothing_raised {
-                trans.evaluate
-            }
-
-            # until we have characterized how backups work, just get
-            # rid of them
-            FileUtils.cd(todir) {
-                %x{find . -name '*puppet-bak'}.chomp.split(/\n/).each { |file|
+    def delete_random_files(dir)
+        random_files(dir) { |file|
+            stat = File.stat(file)
+            begin
+                if stat.ftype == "directory"
+                    false
+                else
                     File.unlink(file)
-                }
-            }
-            assert_trees_equal(fromdir,todir)
-            clearstorage
-            Puppet::Type.allclear
-            delete_random_files(todir)
+                    true
+                end
+            rescue => detail
+                # we probably won't be able to open our own secured files
+                puts detail
+                false
+            end
         }
-
     end
 
-    def test_copywithfailures
-        path = "/tmp/Failuresourcetest"
-        @@tmpfiles.push path
-        system("mkdir -p #{path}")
-
-        # okay, let's create a directory structure
-        fromdir = File.join(path,"fromdir")
-        Dir.mkdir(fromdir)
-        FileUtils.cd(fromdir) {
-            mkranddirsandfiles()
+    def add_random_files(dir)
+        added = []
+        random_files(dir) { |file|
+            stat = File.stat(file)
+            begin
+                if stat.ftype == "directory"
+                    name = File.join(file,"file" + rand(100).to_s)
+                    File.open(name, "w") { |f|
+                        f.puts rand(10)
+                    }
+                    added << name
+                else
+                    false
+                end
+            rescue => detail
+                # we probably won't be able to open our own secured files
+                puts detail
+                false
+            end
         }
+        return added
+    end
 
-        todir = File.join(path,"Todir")
-        tofile = nil
-        trans = nil
-
-        fromlist = nil
-        FileUtils.cd(fromdir) {
-            fromlist = %x{find .}.chomp.split(/\n/)
+    def modify_random_files(dir)
+        modded = []
+        random_files(dir) { |file|
+            stat = File.stat(file)
+            begin
+                if stat.ftype == "directory"
+                    false
+                else
+                    File.open(file, "w") { |f|
+                        f.puts rand(10)
+                    }
+                    modded << name
+                    true
+                end
+            rescue => detail
+                # we probably won't be able to open our own secured files
+                puts detail
+                false
+            end
         }
+        return modded
+    end
 
-        # and then do some verification that the files are actually set up
-        # the same
-        checked = 0
-        fromlist.reverse.each_with_index { |file,i|
-            fromfile = File.join(fromdir,file)
-            fromstat = File.stat(fromdir)
-            if checked < 10 and i % 3 == 0
-                begin
-                    if fromstat.ftype == "directory"
-                        File.new(fromfile).chmod(0111)
-                    else
-                        File.new(fromfile).chmod(0000)
-                    end
-                rescue # we probably won't be able to open our own secured files
-                    next
+    def readonly_random_files(dir)
+        modded = []
+        random_files(dir) { |file|
+            stat = File.stat(file)
+            begin
+                if stat.ftype == "directory"
+                    File.new(file).chmod(0111)
+                else
+                    File.new(file).chmod(0000)
                 end
-                checked += 1
+                modded << file
+            rescue => detail
+                # we probably won't be able to open our own secured files
+                puts detail
+                false
             end
         }
+        return modded
+    end
+
+    def recursive_source_test(fromdir, todir)
+        initstorage
+        tofile = nil
+        trans = nil
 
         assert_nothing_raised {
             tofile = Puppet::Type::PFile.new(
                 :name => todir,
                 "recurse" => true,
+                "backup" => false,
                 "source" => fromdir
             )
         }
@@ -629,8 +622,95 @@ class TestFile < Test::Unit::TestCase
         assert_nothing_raised {
             trans.evaluate
         }
-        assert_trees_equal(fromdir,todir)
+
         clearstorage
         Puppet::Type.allclear
     end
+
+    def run_complex_sources
+        path = "/tmp/ComplexSourcesTest"
+        @@tmpfiles.push path
+
+        # first create the source directory
+        system("mkdir -p #{path}")
+
+
+        # okay, let's create a directory structure
+        fromdir = File.join(path,"fromdir")
+        Dir.mkdir(fromdir)
+        FileUtils.cd(fromdir) {
+            mkranddirsandfiles()
+        }
+
+        todir = File.join(path, "todir")
+        recursive_source_test(fromdir, todir)
+
+        return [fromdir,todir]
+    end
+
+    def test_complex_sources_twice
+        fromdir, todir = run_complex_sources
+        assert_trees_equal(fromdir,todir)
+        recursive_source_test(fromdir, todir)
+        assert_trees_equal(fromdir,todir)
+    end
+
+    def test_sources_with_deleted_destfiles
+        fromdir, todir = run_complex_sources
+        # then delete some files
+        delete_random_files(todir)
+
+        # and run
+        recursive_source_test(fromdir, todir)
+
+        # and make sure they're still equal
+        assert_trees_equal(fromdir,todir)
+    end
+
+    def test_sources_with_readonly_destfiles
+        fromdir, todir = run_complex_sources
+        readonly_random_files(todir)
+        recursive_source_test(fromdir, todir)
+
+        # and make sure they're still equal
+        assert_trees_equal(fromdir,todir)
+    end
+
+    def test_sources_with_modified_dest_files
+        fromdir, todir = run_complex_sources
+
+        # then modify some files
+        modify_random_files(todir)
+
+        recursive_source_test(fromdir, todir)
+
+        # and make sure they're still equal
+        assert_trees_equal(fromdir,todir)
+    end
+
+    def test_sources_with_added_destfiles
+        fromdir, todir = run_complex_sources
+        # and finally, add some new files
+        add_random_files(todir)
+
+        recursive_source_test(fromdir, todir)
+
+        fromtree = file_list(fromdir)
+        totree = file_list(todir)
+
+        assert(fromtree != totree)
+
+        # then remove our new files
+        FileUtils.cd(todir) {
+            %x{find . 2>/dev/null}.chomp.split(/\n/).each { |file|
+                if file =~ /file[0-9]+/
+                    File.unlink(file)
+                end
+            }
+        }
+
+        # and make sure they're still equal
+        assert_trees_equal(fromdir,todir)
+    end
+
 end