recursive file copying now works even with some limited number of errors

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

6 files changed, 156 insertions, 69 deletions

diff --git a/lib/puppet/event.rb b/lib/puppet/event.rb
index 74804e6df..5bdd67c40 100644
--- a/lib/puppet/event.rb
+++ b/lib/puppet/event.rb
@@ -107,12 +107,13 @@ module Puppet
 				raise "Event.new called incorrectly"
 			end
 
+			@state = args[:state]
 			@event = args[:event]
 			@object = args[:object]
 			@transaction = args[:transaction]
 
-            info "%s: %s" %
-                [@object,@event]
+            Puppet.info "%s: %s(%s)" %
+                [@object,@state,@event]
 
             # initially, just stuff all instances into a central bucket
             # to be handled as a batch
diff --git a/lib/puppet/statechange.rb b/lib/puppet/statechange.rb
index f8533f40b..5219c6616 100644
--- a/lib/puppet/statechange.rb
+++ b/lib/puppet/statechange.rb
@@ -46,6 +46,7 @@ module Puppet
                 # should basically point to that, right?
                 return Puppet::Event.new(
                     :event => event,
+                    :state => @state.name,
                     :object => @state.parent,
                     :transaction => @transaction,
                     :message => self.to_s
@@ -61,6 +62,7 @@ module Puppet
                 #end
                 return Puppet::Event.new(
                     :event => pname + "_failed",
+                    :state => @state.name,
                     :object => @state.parent,
                     :transaction => @transaction,
                     :message => "Failed: " + self.to_s
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index 5cfe3d045..fe2e1b546 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -7,7 +7,7 @@ require 'puppet'
 module Puppet
     #------------------------------------------------------------
     class TransObject < Hash
-        attr_accessor :type
+        attr_accessor :type, :name
 
         @@ohash = {}
         @@oarray = []
@@ -33,6 +33,7 @@ module Puppet
         def initialize(name,type)
             self[:name] = name
             @type = type
+            @name = name
             self.class.add(self)
         end
 
@@ -40,9 +41,9 @@ module Puppet
             return [self.type,self[:name]].join('--')
         end
 
-        def name
-            return self[:name]
-        end
+        #def name
+        #    return self[:name]
+        #end
 
         def to_s
             return "%s(%s) => %s" % [@type,self[:name],super]
@@ -176,15 +177,25 @@ module Puppet
                         begin
                             object = child.to_type
                         rescue Puppet::Error => except
-                            Puppet.err "Failed to create %s: %s" %
-                                [child.type,except.message]
+                            Puppet.err "Failed to create %s %s: %s" %
+                                [child.type,child.name,except.message]
+                            if Puppet[:debug]
+                                puts except.stack
+                            end
+                            next
+                        rescue Puppet::Error => except
+                            Puppet.err "Failed to create %s %s: %s" %
+                                [child.type,child.name,except.message]
                             if Puppet[:debug]
                                 puts except.stack
                             end
                             next
                         rescue => except
                             Puppet.err "Failed to create %s %s: %s" %
-                                [child.type,child.inspect,except.message]
+                                [child.type,child.name,except.message]
+                            if Puppet[:debug]
+                                puts caller
+                            end
                             next
                         end
                         nametable[name] = object
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 166cfc0e5..ba64b184a 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -704,7 +704,6 @@ class Type < Puppet::Element
     # sync the changes to disk, and return the events generated by the changes
     def sync
         events = self.collect { |child|
-            Puppet.warning "Syncing %s" % child
             child.sync
         }.reject { |event|
             ! (event.is_a?(Symbol) or event.is_a?(String))
@@ -773,7 +772,6 @@ class Type < Puppet::Element
         #end
 
         # this only operates on states, not states + children
-        Puppet.warning "Calling retrieve on %s" % self.name
         self.retrieve
         unless self.insync?
             # add one to the number of out-of-sync instances
@@ -805,7 +803,7 @@ class Type < Puppet::Element
 
         # now record how many changes we've resulted in
         Puppet::Metric.add(self.class,self,:changes,changes.length)
-        Puppet.warning "%s resulted in %s changes" %
+        Puppet.info "%s resulted in %s changes" %
             [self.name, changes.length]
         return changes.flatten
     end
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 56ce2ee91..335058399 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -112,8 +112,8 @@ module Puppet
                 case @checktype
                 when "md5":
                     if FileTest.directory?(self.parent[:path])
-                        Puppet.info "Cannot MD5 sum directory %s" %
-                            self.parent[:path]
+                        #Puppet.info "Cannot MD5 sum directory %s" %
+                        #    self.parent[:path]
 
                         # because we cannot sum directories, just delete ourselves
                         # from the file
@@ -127,8 +127,8 @@ module Puppet
                     end
                 when "md5lite":
                     if FileTest.directory?(self.parent[:path])
-                        Puppet.info "Cannot MD5 sum directory %s" %
-                            self.parent[:path]
+                        #Puppet.info "Cannot MD5 sum directory %s" %
+                        #    self.parent[:path]
 
                         # because we cannot sum directories, just delete ourselves
                         # from the file
@@ -176,7 +176,8 @@ module Puppet
                     # if we still can't retrieve a checksum, it means that
                     # the file still doesn't exist
                     if @is == -1
-                        Puppet.warning "File %s still does not exist" % self.parent.name
+                        Puppet.warning "File %s does not exist -- cannot checksum" %
+                            self.parent.name
                         return nil
                     end
                 end
@@ -661,8 +662,8 @@ module Puppet
                 #Puppet::State::PFileSource,
             @states = [
                 Puppet::State::PFileCreate,
-                Puppet::State::PFileChecksum,
                 Puppet::State::PFileCopy,
+                Puppet::State::PFileChecksum,
                 Puppet::State::PFileUID,
                 Puppet::State::PFileGroup,
                 Puppet::State::PFileMode,
@@ -684,37 +685,25 @@ module Puppet
 
             @depthfirst = false
 
+            def self.newchild(hash)
+            end
+
             def initialize(hash)
                 if hash.include?("recurse")
                     hash[:recurse] = hash["recurse"]
                     hash.delete("recurse")
                 end
-                @arghash = hash.dup
-                super
-                #self.nameclean(@arghash)
-                #Puppet.err "arghash is %s" % @arghash.inspect
 
                 @stat = nil
 
-                # yay, super-hack!
-                #if @states.include?(:create)
-                #    if @states[:create].should == "directory"
-                #        if @states.include?(:source)
-                #            Puppet.warning "Deleting source for directory %s" %
-                #                self.name
-                #            @states.delete(:source)
-                #        end
-#
-#                        if @states.include?(:checksum)
-#                            Puppet.warning "Deleting checksum for directory %s" %
-#                                self.name
-#                            @states.delete(:checksum)
-#                        end
-#                    else
-#                        Puppet.info "Create is %s for %s" %
-#                            [@states[:create].should,self.name]
-#                    end
-#                end
+                @arghash = hash.dup
+                @arghash.each { |var,value|
+                    if var.is_a?(String)
+                        @arghash[var.intern] = value
+                        @arghash.delete(var)
+                    end
+                }
+                super
             end
 
             # pinning is like recursion, except that it's recursion across
@@ -730,12 +719,12 @@ module Puppet
                 if @source =~ /^file:\/\/(\/.+)/
                     @source = $1
                 elsif @source =~ /(\w+):\/\/(\/.+)/
-                    raise Puppet::Error("Protocol %s not supported" % $1)
+                    raise Puppet::Error.new("Protocol %s not supported" % $1)
                 end
 
                 # verify that the source exists
                 unless FileTest.exists?(@source)
-                    raise Puppet::Error(
+                    raise Puppet::Error.new(
                         "Files must exist to be sources; %s does not" % @source
                     )
                 end
@@ -745,9 +734,9 @@ module Puppet
                 # recursive creation of all of the objects, and then all the
                 # children of the tree will just pull existing objects
                 if obj = self.class[@source]
-                    Puppet.info "%s is already in memory" % @source
+                    #Puppet.info "%s is already in memory" % @source
                     if obj.managed?
-                        raise Puppet::Error(
+                        raise Puppet::Error.new(
                             "You cannot currently use managed files as sources;" +
                             "%s is managed" % @source
                         )
@@ -765,7 +754,7 @@ module Puppet
                         @sourceobj[:check] = check
                     end
                 else # the obj does not exist yet...
-                    Puppet.info "%s is not in memory" % @source
+                    #Puppet.info "%s is not in memory" % @source
                     args = {}
 
                     args[:check] = pinparams
@@ -785,7 +774,15 @@ module Puppet
                     # now create the tree of objects
                     # if recursion is turned on, this will create the whole tree
                     # and we'll just pick it up as our own recursive stuff
-                    @sourceobj = self.class.new(args)
+                    begin
+                        @sourceobj = self.class.new(args)
+                    rescue => detail
+                        # ok, so we somehow need to mark that we shouldn't
+                        # actually be copying the file...
+                        Puppet.notice "Cannot copy %s: %s" % [@source,detail]
+                        #self.delete(:copy)
+                        return
+                    end
                 end
 
                 # okay, now we've got the object; retrieve its values, so we
@@ -805,37 +802,37 @@ module Puppet
                     self[:create] = "directory"
                     # see amazingly crappy hack in initialize()
                     #self.delete(:source)
-                    Puppet.info "Not sourcing checksum of directory %s" % @source
+                    #Puppet.info "Not sourcing checksum of directory %s" % @source
 
                     # now, make sure that if they've got children we model those, too
                     curchildren = {}
                     if defined? @children
-                        Puppet.info "Collecting info about existing children"
+                        #Puppet.info "Collecting info about existing children"
                         @children.each { |child|
                             name = File.basename(child.name)
                             curchildren[name] = child
                         }
                     end
                     @sourceobj.each { |child|
-                        Puppet.info "Looking at %s => %s" %
-                            [@sourceobj.name, child.name]
+                        #Puppet.info "Looking at %s => %s" %
+                        #    [@sourceobj.name, child.name]
                         if child.is_a?(Puppet::Type::PFile)
                             name = File.basename(child.name)
 
                             if curchildren.include?(name) # the file's in both places
                                 # set the source accordingly
-                                Puppet.info "Adding %s as an existing child" % name
+                                #Puppet.info "Adding %s as an existing child" % name
                                 curchildren[name][:source] = child.name
                             else # they have it but we don't
                                 # XXX we have serious probability of repeating
                                 # bugs in paramrecurse
-                                Puppet.info "Adding %s as a new child" % child.name
+                                #Puppet.info "Adding %s as a new child" % child.name
                                 fullname = File.join(self.name, name)
                                 newchild = nil
 
                                 if newchild = self.class[fullname]
-                                    Puppet.info "%s is already being managed" %
-                                        fullname
+                                    #Puppet.info "%s is already being managed" %
+                                    #    fullname
                                     # first pin the file
                                     newchild[:source] = child.name
 
@@ -850,8 +847,6 @@ module Puppet
                                         end
                                     }
                                 else # create it anew
-                                    Puppet.info "Managing %s" % fullname
-
                                     # told you we'd repeat the name bugs from recurse
                                     args = {:name => fullname}
                                     @arghash.each { |var,value|
@@ -861,8 +856,17 @@ module Puppet
                                         args[var] = value
                                     }
                                     args[:source] = child.name
-                                    Puppet.info "Creating child with %s" % args.inspect
-                                    newchild = self.class.new(args)
+                                    #Puppet.info "Creating child with %s" % args.inspect
+                                    begin
+                                        newchild = self.class.new(args)
+                                    rescue => detail
+                                        Puppet.notice "Cannot copy %s: %s" % [@source,detail]
+                                        #self.delete(:copy)
+                                        next
+                                        #raise Puppet::Error.new(
+                                        #    "Cannot copy %s: %s" % [@source,detail]
+                                        #)
+                                    end
                                 end
                                 self.push newchild
                             end
@@ -912,12 +916,11 @@ module Puppet
             def paramrecurse=(value)
                 @parameters[:recurse] = value
                 unless FileTest.exist?(self.name) and self.stat.directory?
-                    Puppet.info "%s is not a directory; not recursing" %
-                        self.name
+                    #Puppet.info "%s is not a directory; not recursing" %
+                    #    self.name
                     return
                 end
 
-                Puppet.err "Recursing!"
                 recurse = value
                 # we might have a string, rather than a number
                 if recurse.is_a?(String)
@@ -957,7 +960,14 @@ module Puppet
                         else # create it anew
                             #notice "Creating new file with args %s" %
                             #    @arghash.inspect
-                            child = self.class.new(@arghash)
+                            begin
+                                child = self.class.new(@arghash)
+                            rescue => detail
+                                Puppet.notice(
+                                    "Cannot manage %s: %s" % [@arghash[:path],detail]
+                                )
+                                next
+                            end
                         end
                         self.push child
                     }
diff --git a/test/types/tc_file.rb b/test/types/tc_file.rb
index a7bedfdfb..0eeab9bb9 100644
--- a/test/types/tc_file.rb
+++ b/test/types/tc_file.rb
@@ -30,6 +30,7 @@ class TestFile < Test::Unit::TestCase
     def teardown
         Puppet::Type.allclear
         @@tmpfiles.each { |file|
+            system("chmod -R 755 %s" % file)
             system("rm -rf %s" % file)
         }
         system("rm -f %s" % Puppet[:statefile])
@@ -424,11 +425,13 @@ class TestFile < Test::Unit::TestCase
         # verify the file list is the same
         fromlist = nil
         FileUtils.cd(fromdir) {
-            fromlist = %x{find .}.chomp.split(/\n/)
+            fromlist = %x{find . 2>/dev/null}.chomp.split(/\n/).reject { |file|
+                ! FileTest.readable?(file)
+            }
         }
         tolist = nil
         FileUtils.cd(todir) {
-            tolist = %x{find .}.chomp.split(/\n/)
+            tolist = %x{find . 2>/dev/null}.chomp.split(/\n/)
         }
         assert_equal(fromlist,tolist)
 
@@ -458,7 +461,7 @@ class TestFile < Test::Unit::TestCase
         }
     end
 
-    def test_xcomplicatedlocalsource
+    def test_complicatedlocalsource
         path = "/tmp/complsourcetest"
         @@tmpfiles.push path
         system("mkdir -p #{path}")
@@ -471,15 +474,77 @@ class TestFile < Test::Unit::TestCase
         }
 
         todir = File.join(path,"todir")
-        fromfile = nil
         tofile = nil
         trans = nil
 
         assert_nothing_raised {
             tofile = Puppet::Type::PFile.new(
                 :name => todir,
-                :recurse => true,
-                :source => fromdir
+                "recurse" => true,
+                "source" => fromdir
+            )
+        }
+        comp = Puppet::Component.new(
+            :name => "component"
+        )
+        comp.push tofile
+        assert_nothing_raised {
+            trans = comp.evaluate
+        }
+        assert_nothing_raised {
+            trans.evaluate
+        }
+        assert_trees_equal(fromdir,todir)
+        clearstorage
+        Puppet::Type.allclear
+    end
+
+    def test_xcopywithfailures
+        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()
+        }
+
+        todir = File.join(path,"todir")
+        tofile = nil
+        trans = nil
+
+        fromlist = nil
+        FileUtils.cd(fromdir) {
+            fromlist = %x{find .}.chomp.split(/\n/)
+        }
+
+        # 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
+                end
+                checked += 1
+            end
+        }
+
+        assert_nothing_raised {
+            tofile = Puppet::Type::PFile.new(
+                :name => todir,
+                "recurse" => true,
+                "source" => fromdir
             )
         }
         comp = Puppet::Component.new(