okay, sources seem to be much more understandable here -- only the recursion function is recursive

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

6 files changed, 496 insertions, 246 deletions

diff --git a/lib/puppet/statechange.rb b/lib/puppet/statechange.rb
index 5219c6616..71995b864 100644
--- a/lib/puppet/statechange.rb
+++ b/lib/puppet/statechange.rb
@@ -15,6 +15,12 @@ module Puppet
             @state = state
             @path = [state.path,"change"].flatten
             @is = state.is
+
+            if state.is == state.should
+                raise Puppet::Error(
+                    "Tried to create a change for in-sync state %s" % state.name
+                )
+            end
             @should = state.should
 
             @run = false
@@ -28,13 +34,21 @@ module Puppet
                 return nil
             end
 
+            if @state.is == @state.should
+                raise Puppet::Error.new(
+                    "Tried to change in-sync state %s" % state.name
+                )
+            end
+
             begin
                 event = @state.sync
                 @run = true
                 
                 # default to a simple event type
                 if event.nil?
-                    event = @state.parent.class.name.id2name + "_changed"
+                    #event = @state.parent.class.name.id2name + "_changed"
+                    # they didn't actually change anything
+                    return
                 elsif ! event.is_a?(Symbol)
                     Puppet.warning "State '%s' returned invalid event '%s'; resetting to default" %
                         [@state.class,event]
@@ -52,7 +66,7 @@ module Puppet
                     :message => self.to_s
                 )
             rescue => detail
-                Puppet.err "%s failed: %s" % [self.to_s,detail]
+                #Puppet.err "%s failed: %s" % [self.to_s,detail]
                 raise
                 # there should be a way to ask the state what type of event
                 # it would have generated, but...
@@ -89,8 +103,10 @@ module Puppet
             @state.should = @is
             @state.retrieve
 
-            Puppet.notice "Rolling %s backward" % self
-            return self.go
+            unless @state.insync?
+                Puppet.notice "Rolling %s backward" % self
+                return self.go
+            end
 
             #raise "Moving statechanges backward is currently unsupported"
             #@type.change(@path,@should,@is)
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index fe2e1b546..ab1f0f611 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -63,41 +63,6 @@ module Puppet
     #------------------------------------------------------------
 
     #------------------------------------------------------------
-    class TransSetting
-        attr_accessor :type, :name, :args, :evalcount
-
-        def initialize
-            @evalcount = 0
-        end
-
-        def evaluate
-            @evalcount += 0
-            if type = Puppet::Type.type(self.type)
-                # call the settings
-                name = self.name
-                unless name.is_a?(Symbol)
-                    name = name.intern
-                end
-                if type.allowedmethod(name)
-                    Puppet.debug "Sending %s->%s(%s)" %
-                        [type.name,self.name,self.args]
-                    begin
-                        type.send(self.name,self.args)
-                    rescue => detail
-                        raise "Failed to execute '%s.%s(%s)'" %
-                            [type,self.name,self.args]
-                    end
-                else
-                    err("%s does not respond to %s" % [self.type,self.name])
-                end
-            else
-                raise "Could not find object type %s" % setting.type
-            end
-        end
-    end
-    #------------------------------------------------------------
-
-    #------------------------------------------------------------
     # just a linear container for objects
     class TransBucket < Array
         attr_accessor :name, :type
@@ -148,9 +113,6 @@ module Puppet
                 if child.is_a?(Puppet::TransBucket)
                     # just perform the same operation on any children
                     container.push(child.to_type)
-                elsif child.is_a?(Puppet::TransSetting)
-                    # XXX this is wrong, but for now just evaluate the settings
-                    child.evaluate
                 elsif child.is_a?(Puppet::TransObject)
                     # do a simple little naming hack to see if the object already
                     # exists in our scope
@@ -172,6 +134,7 @@ module Puppet
                             # override any existing values
                             object[var] = value
                         }
+                        object.parent = self
                     else # the object does not exist yet in our scope
                         # now we have the object instantiated, in our scope
                         begin
@@ -183,13 +146,6 @@ module Puppet
                                 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.name,except.message]
@@ -201,6 +157,7 @@ module Puppet
                         nametable[name] = object
 
                         # this sets the order of the object
+                        Puppet.notice "Pushing %s" % object.name
                         container.push object
                     end
                 else
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 212e47773..2e0fd3a63 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -37,6 +37,8 @@ class Type < Puppet::Element
     attr_accessor :children, :parameters, :parent
     include Enumerable
 
+    @@retrieved = Hash.new(0)
+
     # an array to contain all instances of Type
     @@allobjects = Array.new
 
@@ -441,12 +443,18 @@ class Type < Puppet::Element
                 else
                     #Puppet.warning "Creating state %s for %s" %
                     #    [stateklass.name,self.name]
-                    @states[name] = stateklass.new(
-                        :parent => self,
-                        :should => value
-                    )
-                    #debug "Adding parent to %s" % name
-                    #@states[name].parent = self
+                    begin
+                        # make sure the state doesn't have any errors
+                        newstate = stateklass.new(
+                            :parent => self,
+                            :should => value
+                        )
+                        @states[name] = newstate
+                    rescue => detail
+                        # the state failed, so just ignore it
+                        Puppet.debug "State %s failed: %s" %
+                            [name, detail]
+                    end
                 end
             end
         elsif self.class.validparameter?(name)
@@ -471,7 +479,7 @@ class Type < Puppet::Element
         if @states.has_key?(attr)
             @states.delete(attr)
         else
-            raise "Undefined state '#{attr}' in #{self}"
+            raise Puppet::DevError.new("Undefined state '#{attr}' in #{self}")
         end
     end
     #---------------------------------------------------------------
@@ -549,6 +557,7 @@ class Type < Puppet::Element
         end
         childs.each { |child|
             @children.push(child)
+            child.parent = self
         }
     end
     #---------------------------------------------------------------
@@ -702,7 +711,7 @@ class Type < Puppet::Element
     # some classes (e.g., FileTypeRecords) will have to override this
     def path
         if defined? @parent
-            return [@parent.name, self.name].flatten
+            return [@parent.path, self.name].flatten
         else
             return [self.name]
         end
@@ -768,11 +777,17 @@ class Type < Puppet::Element
     # this returns any changes resulting from testing, thus 'collect'
     # rather than 'each'
     def evaluate
+        #Puppet.err "Evaluating %s" % self.path.join(":")
         unless defined? @evalcount
             Puppet.err "No evalcount defined on '%s' of type '%s'" %
                 [self.name,self.class]
             @evalcount = 0
         end
+        @@retrieved[self] += 1
+        if self.name =~ /e\/dav_fs.load/ and @@retrieved[self] > 1
+            Puppet.notice "%s(%s) %s" %
+                [@@retrieved[self], @evalcount, self.path.join(":")]
+        end
         # if we're a metaclass and we've already evaluated once...
         #if self.metaclass and @evalcount > 0
         #    return
@@ -792,18 +807,29 @@ class Type < Puppet::Element
         #end
 
         # this only operates on states, not states + children
-        self.retrieve
-        unless self.insync?
+        #self.retrieve
+        #unless self.insync?
+
+        # states() is a private method, returning an ordered list
+        changes << states().each { |state|
+            @@retrieved[state] += 1
+            #if self.name =~ /e\/dav_fs.load/
+            #    Puppet.notice "%s %s" % [@@retrieved[state], state.path]
+            #end
+            #unless @@retrieved[state] > 0
+                state.retrieve
+            #end
+        }.find_all { |state|
+            ! state.insync?
+        }.collect { |state|
+            Puppet::StateChange.new(state)
+        }
+
+        if changes.length > 0
             # add one to the number of out-of-sync instances
             Puppet::Metric.add(self.class,self,:outofsync,1)
-
-            # states() is a private method, returning an ordered list
-            changes << states().find_all { |state|
-                ! state.insync?
-            }.collect { |state|
-                Puppet::StateChange.new(state)
-            }
         end
+        #end
 
         changes << @children.collect { |child|
             child.evaluate
diff --git a/lib/puppet/type/component.rb b/lib/puppet/type/component.rb
index 0e9ae5c20..901d89326 100644
--- a/lib/puppet/type/component.rb
+++ b/lib/puppet/type/component.rb
@@ -62,9 +62,11 @@ Component
         def push(*ary)
             ary.each { |child|
                 unless child.is_a?(Puppet::Element)
-                    debug "Got object of type %s" % child.class
-                    raise "Containers can only contain Puppet::Elements, not %s" %
+                    Puppet.debug "Got object of type %s" % child.class
+                    raise Puppet::DevError.new(
+                        "Containers can only contain Puppet::Elements, not %s" %
                         child.class
+                    )
                 end
                 @children.push child
                 child.parent = self
@@ -79,11 +81,11 @@ Component
             }
         end
 
-        def retrieve
-            self.collect { |child|
-                child.retrieve
-            }
-        end
+        #def retrieve
+        #    self.collect { |child|
+        #        child.retrieve
+        #    }
+        #end
 
         def to_s
             return "component(%s)" % self.name
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index bde232845..30e46a6d0 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -34,7 +34,7 @@ module Puppet
             end
 
             def retrieve
-                if stat = self.parent.stat(true)
+                if stat = @parent.stat(true)
                     @is = stat.ftype
                 else
                     @is = -1
@@ -52,11 +52,11 @@ module Puppet
                 begin
                     case @should
                     when "file":
-                        File.open(self.parent[:path],"w") { # just create an empty file
+                        File.open(@parent[:path],"w") { # just create an empty file
                         }
                         event = :file_created
                     when "directory":
-                        Dir.mkdir(self.parent.name)
+                        Dir.mkdir(@parent.name)
                         event = :directory_created
                     else
                         error = Puppet::Error.new(
@@ -82,18 +82,18 @@ module Puppet
             def should=(value)
                 @checktype = value
                 state = Puppet::Storage.state(self)
-                if hash = state[self.parent[:path]]
+                if hash = state[@parent[:path]]
                     if hash.include?(@checktype)
                         @should = hash[@checktype]
                         #Puppet.debug "Found checksum %s for %s" %
-                        #    [@should,self.parent[:path]]
+                        #    [@should,@parent[:path]]
                     else
                         #Puppet.debug "Found checksum for %s but not of type %s" %
-                        #    [self.parent[:path],@checktype]
+                        #    [@parent[:path],@checktype]
                         @should = nil
                     end
                 #else
-                    #Puppet.debug "No checksum for %s" % self.parent[:path]
+                    #Puppet.debug "No checksum for %s" % @parent[:path]
                 end
             end
 
@@ -102,8 +102,8 @@ module Puppet
                     @checktype = "md5"
                 end
 
-                unless FileTest.exists?(self.parent.name)
-                    Puppet.info "File %s does not exist" % self.parent.name
+                unless FileTest.exists?(@parent.name)
+                    Puppet.info "File %s does not exist" % @parent.name
                     self.is = -1
                     return
                 end
@@ -111,45 +111,45 @@ module Puppet
                 sum = ""
                 case @checktype
                 when "md5":
-                    if FileTest.directory?(self.parent[:path])
+                    if FileTest.directory?(@parent[:path])
                         #Puppet.info "Cannot MD5 sum directory %s" %
-                        #    self.parent[:path]
+                        #    @parent[:path]
 
                         # because we cannot sum directories, just delete ourselves
                         # from the file
                         # is/should so we won't sync
-                        self.parent.delete(self.name)
+                        @parent.delete(self.name)
                         return
                     else
                         begin
-                            File.open(self.parent[:path]) { |file|
+                            File.open(@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)
+                                @parent.name
+                            @parent.delete(self.class.name)
                         rescue => detail
                             Puppet.notice "Cannot checksum %s: %s" %
                                 detail
-                            self.parent.delete(self.class.name)
+                            @parent.delete(self.class.name)
                         end
                     end
                 when "md5lite":
-                    if FileTest.directory?(self.parent[:path])
+                    if FileTest.directory?(@parent[:path])
                         #Puppet.info "Cannot MD5 sum directory %s" %
-                        #    self.parent[:path]
+                        #    @parent[:path]
 
                         # because we cannot sum directories, just delete ourselves
                         # from the file
                         # is/should so we won't sync
                         return
                     else
-                        File.open(self.parent[:path]) { |file|
+                        File.open(@parent[:path]) { |file|
                             text = file.read(512)
                             if text.nil?
                                 Puppet.info "Not checksumming empty file %s" %
-                                    self.parent.name
+                                    @parent.name
                                 sum = 0
                             else
                                 sum = Digest::MD5.hexdigest(text)
@@ -157,9 +157,9 @@ module Puppet
                         }
                     end
                 when "timestamp","mtime":
-                    sum = File.stat(self.parent[:path]).mtime.to_s
+                    sum = File.stat(@parent[:path]).mtime.to_s
                 when "time":
-                    sum = File.stat(self.parent[:path]).ctime.to_s
+                    sum = File.stat(@parent[:path]).ctime.to_s
                 end
 
                 self.is = sum
@@ -174,20 +174,20 @@ module Puppet
             def sync
                 if @is.nil?
                     error = Puppet::Error.new "Checksum state for %s is somehow nil" %
-                        self.parent.name
+                        @parent.name
                     raise error
                 end
 
                 if @is == -1
                     self.retrieve
                     Puppet.debug "%s(%s): after refresh, is '%s'" %
-                        [self.class.name,self.parent.name,@is]
+                        [self.class.name,@parent.name,@is]
 
                     # if we still can't retrieve a checksum, it means that
                     # the file still doesn't exist
                     if @is == -1
                         Puppet.warning "File %s does not exist -- cannot checksum" %
-                            self.parent.name
+                            @parent.name
                         return nil
                     end
                 end
@@ -207,41 +207,41 @@ module Puppet
             def updatesum
                 result = false
                 state = Puppet::Storage.state(self)
-                unless state.include?(self.parent.name)
+                unless state.include?(@parent.name)
                     Puppet.debug "Initializing state hash for %s" %
-                        self.parent.name
+                        @parent.name
 
-                    state[self.parent.name] = Hash.new
+                    state[@parent.name] = Hash.new
                 end
 
                 if @is == -1
                     error = Puppet::Error.new("%s has invalid checksum" %
-                        self.parent.name)
+                        @parent.name)
                     raise error
                 #elsif @should == -1
                 #    error = Puppet::Error.new("%s has invalid 'should' checksum" %
-                #        self.parent.name)
+                #        @parent.name)
                 #    raise error
                 end
 
                 # if we're replacing, vs. updating
-                if state[self.parent.name].include?(@checktype)
+                if state[@parent.name].include?(@checktype)
                     unless defined? @should
                         raise Puppet::Error.new(
                             ("@should is not initialized for %s, even though we " +
-                            "found a checksum") % self.parent[:path]
+                            "found a checksum") % @parent[:path]
                         )
                     end
                     Puppet.debug "Replacing checksum %s with %s" %
-                        [state[self.parent.name][@checktype],@is]
+                        [state[@parent.name][@checktype],@is]
                     Puppet.debug "@is: %s; @should: %s" % [@is,@should]
                     result = true
                 else
                     Puppet.debug "Creating checksum %s for %s of type %s" %
-                        [self.is,self.parent.name,@checktype]
+                        [self.is,@parent.name,@checktype]
                     result = false
                 end
-                state[self.parent.name][@checktype] = @is
+                state[@parent.name][@checktype] = @is
                 return result
             end
         end
@@ -292,6 +292,10 @@ module Puppet
                 if FileTest.symlink?(@link)
                     self.is = File.readlink(@link)
                     return
+                elsif FileTest.exists?(@link)
+                    Puppet.err "Cannot replace %s with a link" % @link
+                    @should = nil
+                    @is = nil
                 else
                     self.is = nil
                     return
@@ -302,12 +306,12 @@ module Puppet
             # to us
             def should=(link)
                 @link = link
-                @should = self.parent[:path]
+                @should = @parent[:path]
 
                 # unless we're fully qualified or we've specifically allowed
                 # relative links.  Relative links are currently disabled, until
                 # someone actually asks for them
-                #unless @should =~ /^\// or self.parent[:relativelinks]
+                #unless @should =~ /^\// or @parent[:relativelinks]
                 unless @should =~ /^\//
                     @should = File.expand_path @should
                 end
@@ -317,7 +321,7 @@ module Puppet
             # a file
             def sync
                 if @is
-                    self.class.remove(@is)
+                    self.class.remove(@link)
                 end
                 self.class.create(@should,@link)
 
@@ -331,53 +335,106 @@ module Puppet
             @event = :inode_changed
 
             def retrieve
-                unless stat = self.parent.stat(true)
+                # if we're not root, then we can't chown anyway
+                unless Process.uid == 0
+                    @parent.delete(self.name)
+                    @should = nil
+                    @is = nil
+                    unless defined? @@notified
+                        Puppet.notice "Cannot manage ownership unless running as root"
+                        @@notified = true
+                        return
+                    end
+                end
+
+                unless stat = @parent.stat(true)
                     @is = -1
                     return
                 end
 
                 self.is = stat.uid
-                if defined? @should
-                    unless @should.is_a?(Integer)
-                        begin
-                            user = Etc.getpwnam(@should)
-                            if user.gid == ""
-                                error = Puppet::Error.new(
-                                    "Could not retrieve uid for '%s'" %
-                                        self.parent.name)
-                                raise error
-                            end
-                            #Puppet.debug "converting %s to integer '%d'" %
-                            #    [@should,user.uid]
-                            @should = user.uid
-                        rescue => detail
+            end
+
+            def should=(value)
+                unless Process.uid == 0
+                    @should = nil
+                    @is = nil
+                    unless defined? @@notified
+                        Puppet.notice "Cannot manage ownership unless running as root"
+                        #@parent.delete(self.name)
+                        @@notified = true
+                        return
+                    end
+                    raise Puppet::Error.new(
+                        "Cannot manage ownership unless running as root"
+                    )
+                end
+                if value.is_a?(Integer)
+                    # verify the user is a valid user
+                    begin
+                        user = Etc.getpwuid(value)
+                        if user.uid == ""
                             error = Puppet::Error.new(
-                                "Could not get any info on user '%s'" % @should)
+                                "Could not retrieve uid for '%s'" %
+                                    @parent.name)
                             raise error
                         end
+                    rescue ArgumentError => detail
+                        raise Puppet::Error.new("User ID %s does not exist" %
+                            value
+                        )
+                    rescue => detail
+                        raise Puppet::Error.new(
+                            "Could not find user '%s': %s" % [value, detail])
+                        raise error
+                    end
+                else
+                    begin
+                        user = Etc.getpwnam(value)
+                        if user.uid == ""
+                            error = Puppet::Error.new(
+                                "Could not retrieve uid for '%s'" %
+                                    @parent.name)
+                            raise error
+                        end
+                        value = user.uid
+                    rescue ArgumentError => detail
+                        raise Puppet::Error.new("User %s does not exist" %
+                            value
+                        )
+                    rescue => detail
+                        error = Puppet::Error.new(
+                            "Could not find user '%s': %s" % [value, detail])
+                        raise error
                     end
                 end
 
-                #Puppet.debug "chown state is %d" % self.is
+                @should = value
             end
 
             def sync
+                unless Process.uid == 0
+                    raise Puppet::Error.new(
+                        "Got told to sync owner as non-root user"
+                    )
+                end
+
                 if @is == -1
-                    self.parent.stat(true)
+                    @parent.stat(true)
                     self.retrieve
                     Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
                 end
 
-                unless self.parent.stat
+                unless @parent.stat
                     Puppet.err "PFile '%s' does not exist; cannot chown" %
-                        self.parent[:path]
+                        @parent[:path]
                 end
 
                 begin
-                    File.chown(self.should,-1,self.parent[:path])
+                    File.chown(self.should,-1,@parent[:path])
                 rescue => detail
                     error = Puppet::Error.new("failed to chown '%s' to '%s': %s" %
-                        [self.parent[:path],self.should,detail])
+                        [@parent[:path],self.should,detail])
                     raise error
                 end
 
@@ -404,33 +461,61 @@ module Puppet
                     should = Integer(should)
                 end
                 @should = should
+                if FileTest.exists?(@parent.name)
+                    self.dirfix
+                end
+            end
+
+            def dirfix
+                # if we're a directory, we need to be executable for all cases
+                # that are readable
+                if FileTest.directory?(@parent.name)
+                    if @should & 0400
+                        @should |= 0100
+                    end
+                    if @should & 040
+                        @should |= 010
+                    end
+                    if @should & 04
+                        @should |= 01
+                    end
+                end
+
+                @fixed = true
             end
 
             def retrieve
-                stat = self.parent.stat(true)
-                self.is = stat.mode & 007777
+                if stat = @parent.stat(true)
+                    self.is = stat.mode & 007777
+                else
+                    self.is = -1
+                end
 
                 #Puppet.debug "chmod state is %o" % self.is
             end
 
             def sync
                 if @is == -1
-                    self.parent.stat(true)
+                    @parent.stat(true)
                     self.retrieve
                     Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
                 end
 
-                unless self.parent.stat
+                unless @parent.stat
                     Puppet.err "PFile '%s' does not exist; cannot chmod" %
-                        self.parent[:path]
+                        @parent[:path]
                     return
                 end
 
+                unless defined? @fixed
+                    self.dirfix
+                end
+
                 begin
-                    File.chmod(self.should,self.parent[:path])
+                    File.chmod(self.should,@parent[:path])
                 rescue => detail
                     error = Puppet::Error.new("failed to chmod %s: %s" %
-                        [self.parent.name, detail.message])
+                        [@parent.name, detail.message])
                     raise error
                 end
                 return :inode_changed
@@ -454,7 +539,7 @@ module Puppet
             # this just doesn't seem right...
             def sync
                 unless defined? @is or @is == -1
-                    self.parent.stat(true)
+                    @parent.stat(true)
                     self.retrieve
                     Puppet.debug "%s: should is '%s'" % [self.class.name,self.should]
                 end
@@ -474,7 +559,7 @@ module Puppet
             @event = :inode_changed
 
             def retrieve
-                stat = self.parent.stat(true)
+                stat = @parent.stat(true)
 
                 self.is = stat.gid
 
@@ -482,58 +567,86 @@ module Puppet
                 # but i don't see a good way around it right now
                 # mmmm, should
                 if defined? @should
-                    unless self.should.is_a?(Integer)
-                        begin
-                            require 'puppet/fact'
-                            group = Etc.getgrnam(self.should)
-                            # apparently os x is six shades of weird
-                            os = Puppet::Fact["Operatingsystem"]
-
-                            gid = ""
-                            case os
-                            when "Darwin":
-                                gid = group.passwd
-                            else
-                                gid = group.gid
-                            end
-                            if gid == ""
-                                error = Puppet::Error.new(
-                                    "Could not retrieve gid for %s" % self.parent.name)
-                                raise error
-                            end
-                            #Puppet.debug "converting %s to integer %d" %
-                            #    [self.should,gid]
-                            self.should = gid
-                        rescue => detail
-                            error = Puppet::Error.new(
-                                "Could not get any info on group %s: %s" % self.should)
-                            raise error
-                        end
+                else
+                    @parent.delete(self.name)
+                end
+            end
+
+            def should=(value)
+                require 'puppet/fact'
+                method = nil
+                gid = nil
+                gname = nil
+
+                if value.is_a?(Integer)
+                    method = :getgrgid
+                else
+                    method = :getgrnam
+                end
+
+                begin
+                    group = Etc.send(method,value)
+                    # apparently os x is six shades of weird
+                    os = Puppet::Fact["Operatingsystem"]
+
+                    case os
+                    when "Darwin":
+                        gid = group.passwd
+                    else
+                        gid = group.gid
                     end
+                    gname = group.name
+
+                    if gid.nil?
+                        raise Puppet::Error.new(
+                            "Could not retrieve gid for %s" % @parent.name)
+                    end
+                rescue ArgumentError => detail
+                    raise Puppet::Error.new(
+                        "Could not find group %s" % value)
+                rescue => detail
+                    raise Puppet::Error.new(
+                        "Could not find group %s: %s" % [self.should,detail])
+                end
+
+                # now make sure the user is allowed to change to that group
+                unless Process.uid == 0
+                    groups = %x{groups}.chomp.split(/\s/)
+                    unless groups.include?(gname)
+                        Puppet.notice "Cannot chgrp: not in group %s" % gname
+                        raise Puppet::Error.new(
+                            "Cannot chgrp: not in group %s" % gname)
+                    end
+                end
+
+                if gid.nil?
+                    raise Puppet::Error.new(
+                        "Nil gid for %s" % @parent.name)
+                else
+                    @should = gid
                 end
-                #Puppet.debug "chgrp state is %d" % self.is
             end
 
             def sync
                 Puppet.debug "setting chgrp state to %s" % self.should
                 if @is == -1
-                    self.parent.stat(true)
+                    @parent.stat(true)
                     self.retrieve
                     Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
                 end
 
-                unless self.parent.stat
+                unless @parent.stat
                     Puppet.err "PFile '%s' does not exist; cannot chgrp" %
-                        self.parent[:path]
+                        @parent[:path]
                     return
                 end
 
                 begin
                     # set owner to nil so it's ignored
-                    File.chown(nil,self.should,self.parent[:path])
+                    File.chown(nil,self.should,@parent[:path])
                 rescue => detail
                     error = Puppet::Error.new( "failed to chgrp %s to %s: %s" %
-                        [self.parent[:path], self.should, detail.message])
+                        [@parent[:path], self.should, detail.message])
                     raise error
                 end
                 return :inode_changed
@@ -547,8 +660,19 @@ module Puppet
 
             def retrieve
                 sum = nil
-                if sum = self.parent.state(:checksum)
-                    @is = sum.is
+                if sum = @parent.state(:checksum)
+                    if @parent.name =~ /e\/dav_fs.load/
+                        puts caller
+                        Puppet.notice "Ah: %s" % @parent.name
+                    end
+                    if sum.is
+                        if sum.is == -1
+                            sum.retrieve
+                        end
+                        @is = sum.is
+                    else
+                        @is = -1
+                    end
                 else
                     @is = -1
                 end
@@ -570,52 +694,54 @@ module Puppet
                         )
                     end
                 when "directory":
-                    error = Puppet::Error.new(
-                        "Somehow got told to copy dir %s" % self.parent.name)
-                    raise error
+                    raise Puppet::Error.new(
+                        "Somehow got told to copy dir %s" % @parent.name)
                 else
-                    error = Puppet::Error.new(
+                    raise Puppet::Error.new(
                         "Cannot use files of type %s as source" % stat.ftype)
-                    raise error
                 end
 
                 @should = sourcesum
             end
 
             def sync
+                if @is.nil?
+                    Puppet.err "@is is nil"
+                end
                 if @is == -1
                     self.retrieve # try again
-                    if @is == -1
-                        if @is == @should
-                            return nil
-                        end
+                    if @is == @should
+                        return nil
+                    else
+                        Puppet.err "@is: %s; @should: %s" % [@is, @should]
                     end
                 end
                 @backed = false
-                bak = self.parent[:backup] || ".puppet-bak"
+                bak = @parent[:backup] || ".puppet-bak"
 
+                Puppet.notice "@is: %s; @should: %s" % [@is, @should]
                 # try backing ourself up before we overwrite
-                if FileTest.file?(self.parent.name)
-                    if bucket = self.parent[:filebucket]
-                        bucket.backup(self.parent.name)
+                if FileTest.file?(@parent.name)
+                    if bucket = @parent[:filebucket]
+                        bucket.backup(@parent.name)
                         @backed = true
-                    elsif self.parent[:backup]
+                    elsif @parent[:backup]
                         # back the file up
                         begin
-                            FileUtils.cp(self.parent.name,
-                                self.parent.name + bak)
+                            FileUtils.cp(@parent.name,
+                                @parent.name + bak)
                             @backed = true
                         rescue => detail
                             # since they said they want a backup, let's error out
                             # if we couldn't make one
                             error = Puppet::Error.new("Could not back %s up: %s" %
-                                [self.parent.name, detail.message])
+                                [@parent.name, detail.message])
                             raise error
                         end
                     end
                 end
 
-                unless self.parent[:backup]
+                unless @parent[:backup]
                     @backed = true
                 end
 
@@ -626,49 +752,49 @@ module Puppet
                     case stat.ftype
                     when "file":
                         begin
-                            if FileTest.exists?(self.parent.name)
+                            if FileTest.exists?(@parent.name)
                                 # get the file here
-                                FileUtils.cp(@source, self.parent.name + ".tmp")
-                                if FileTest.exists?(self.parent.name + bak)
+                                FileUtils.cp(@source, @parent.name + ".tmp")
+                                if FileTest.exists?(@parent.name + bak)
                                     Puppet.warning "Deleting backup of %s" %
-                                        self.parent.name
-                                    File.unlink(self.parent.name + bak)
+                                        @parent.name
+                                    File.unlink(@parent.name + bak)
                                 end
                                 # rename the existing one
                                 File.rename(
-                                    self.parent.name,
-                                    self.parent.name + ".puppet-bak"
+                                    @parent.name,
+                                    @parent.name + ".puppet-bak"
                                 )
                                 # move the new file into place
                                 File.rename(
-                                    self.parent.name + ".tmp",
-                                    self.parent.name
+                                    @parent.name + ".tmp",
+                                    @parent.name
                                 )
                                 # if we've made a backup, then delete the old file
                                 if @backed
                                     #Puppet.err "Unlinking backup"
-                                    File.unlink(self.parent.name + bak)
+                                    File.unlink(@parent.name + bak)
                                 #else
                                     #Puppet.err "Not unlinking backup"
                                 end
                             else
                                 # the easy case
-                                FileUtils.cp(@source, self.parent.name)
+                                FileUtils.cp(@source, @parent.name)
                             end
                         rescue => detail
                             # since they said they want a backup, let's error out
                             # if we couldn't make one
                             error = Puppet::Error.new("Could not copy %s to %s: %s" %
-                                [@source, self.parent.name, detail.message])
+                                [@source, @parent.name, detail.message])
                             raise error
                         end
                     when "directory":
                         raise Puppet::Error.new(
                             "Somehow got told to copy directory %s" %
-                                self.parent.name)
+                                @parent.name)
                     when "link":
                         dest = File.readlink(@source)
-                        Puppet::State::PFileLink.create(@dest,self.parent.path)
+                        Puppet::State::PFileLink.create(@dest,@parent.path)
                     else
                         raise Puppet::Error.new(
                             "Cannot use files of type %s as source" % stat.ftype)
@@ -682,7 +808,7 @@ module Puppet
     end
     class Type
         class PFile < Type
-            attr_reader :params, :source
+            attr_reader :params, :source, :srcbase
 
             # class instance variable
                 #Puppet::State::PFileSource,
@@ -700,8 +826,8 @@ module Puppet
             @parameters = [
                 :path,
                 :backup,
-                :source,
                 :recurse,
+                :source,
                 :filebucket
             ]
 
@@ -710,18 +836,70 @@ module Puppet
 
             @depthfirst = false
 
+            if Process.uid == 0
+                @@pinparams = [:mode, :owner, :group, :checksum]
+            else
+                @@pinparams = [:mode, :group, :checksum]
+            end
+
+
+            def self.recursecompare(source,dest)
+                if FileTest.directory?(source.name)
+                    # find all of the children of our source
+                    mkchilds = source.reject { |schild|
+                        schild.is_a?(Puppet::State)
+                    }.collect { |schild|
+                        File.basename(schild.name)
+                    }
+
+                    # add them to our repository
+                    dest.reject { |child|
+                        child.is_a?(Puppet::State)
+                    }.collect { |schild|
+                        name = File.basename(child.name)
+                        if mkchilds.include?(name)
+                            mkchilds.delete(name)
+                        end
+                    }
+
+                    # okay, now we know which ones we still need to make
+                    mkchilds.each { |child|
+                        Puppet.notice "Making non-existent file %s" % child
+                        child = self.newchild(child)
+                        child.parent = self
+                    }
+                end
+            end
+
             def initialize(hash)
                 @arghash = self.argclean(hash)
                 @arghash.delete(self.class.namevar)
 
+                if @arghash.include?(:source)
+                    @arghash.delete(:source)
+                end
+
                 @stat = nil
                 @parameters = Hash.new(false)
 
                 # default to a string, which is true
                 @parameters[:backup] = ".puppet-bak"
+                @srcbase = nil
                 super
             end
 
+            def path
+                if defined? @parent
+                    if @parent.is_a?(self.class)
+                        return [@parent.path, File.basename(self.name)].flatten
+                    else
+                        return [@parent.path, self.name].flatten
+                    end
+                else
+                    return [self.name]
+                end
+            end
+
             def parambackup=(value)
                 if value == false or value == "false"
                     @parameters[:backup] = false
@@ -744,7 +922,9 @@ module Puppet
                 args = @arghash.dup
 
                 args[:path] = path
+
                 unless hash.include?(:source) # it's being manually overridden
+
                     if args.include?(:source)
                         #Puppet.notice "Rewriting source for %s" % path
                         name = File.basename(path)
@@ -780,6 +960,7 @@ module Puppet
 
                 child = nil
                 if child = self.class[path]
+                    #raise "Ruh-roh"
                     args.each { |var,value|
                         next if var == :path
                         next if var == :name
@@ -805,6 +986,9 @@ module Puppet
                         child = nil
                     end
                 end
+                if child
+                    child.parent = self
+                end
                 return child
             end
 
@@ -816,8 +1000,6 @@ module Puppet
                     return nil
                 end
 
-                pinparams = [:mode, :owner, :group, :checksum]
-
                 obj = nil
                 # XXX i'm pretty sure this breaks the closure rules, doesn't it?
                 # shouldn't i be looking it up through other mechanisms?
@@ -831,7 +1013,7 @@ module Puppet
                     else
                         # verify they're looking up the correct info
                         check = []
-                        pinparams.each { |param|
+                        @@pinparams.each { |param|
                             unless obj.state(param)
                                 check.push param
                             end
@@ -843,7 +1025,7 @@ module Puppet
                     #Puppet.info "%s is not in memory" % @source
                     args = {}
 
-                    args[:check] = pinparams
+                    args[:check] = @@pinparams
                     args[:name] = @source
 
                     if @arghash.include?(:recurse)
@@ -902,24 +1084,65 @@ module Puppet
                     return
                 end
 
-                # Check whether we'll be creating the file or whether it already
-                # exists.  The root of the destination tree will cause the
-                # recursive creation of all of the objects, and then all the
-                # children of the tree will just pull existing objects
                 unless @sourceobj = self.newsource(@source)
                     return
                 end
 
+#                    # okay, we now have the whole source tree in memory, being modelled
+#                    # now we just need to compare it with what we have, to see
+#                    # if we're missing any files
+#
+#                    # we're assuming that 'paramrecurse=' has already been called
+#
+#                    if FileTest.directory?(@source)
+#                        mkchilds = @sourceobj.reject { |schild|
+#                            schild.is_a?(Puppet::State)
+#                        }.collect { |schild|
+#                            File.basename(schild.name)
+#                        }
+#
+#                        @children.each { |child|
+#                            name = File.basename(child.name)
+#                            if mkchilds.include?(name)
+#                                mkchilds.delete(name)
+#                            end
+#                        }
+#
+#                        # okay, now we know which ones we still need to make
+#                        mkchilds.each { |child|
+#                            Puppet.notice "Making non-existent file %s" % child
+#                            child = self.newchild(child)
+#                            self.push child
+#                        }
+#                    end
+#
+#                    @srcbase = @source
+#
+#                    # now, the sourceobj models the entire tree at once
+#                    # and we've already recursed through what exists locally
+#                end
+#
+#                # Check whether we'll be creating the file or whether it already
+#                # exists.  The root of the destination tree will cause the
+#                # recursive creation of all of the objects, and then all the
+#                # children of the tree will just pull existing objects
+#                unless @sourceobj = self.newsource(@source)
+#                    return
+#                end
+
                 # okay, now we've got the object; retrieve its values, so we
                 # can make them our 'should' values
                 @sourceobj.retrieve
 
-                # if the pin states, these can be done easily
-                [:owner, :group, :mode].each { |state|
+                @@pinparams.each { |state|
+                    next if state == :checksum
                     unless @states.include?(state)
                         # this copies the source's 'is' value to our 'should'
                         # but doesn't override existing settings
-                        self[state] = @sourceobj[state]
+                        # it might be nil -- e.g., we might not be root
+                        if value = @sourceobj[state]
+                            self[state] = @sourceobj[state]
+                        end
                     end
                 }
 
@@ -995,24 +1218,18 @@ module Puppet
                         if child.is_a?(Puppet::Type::PFile)
                             name = File.basename(child.name)
 
-                            if curchildren.include?(name) # the file's in both places
+                            if curchildren.include?(name)
+                                # the file's in both places
                                 # set the source accordingly
-                                #Puppet.info "Adding %s as an existing child" % name
                                 curchildren[name][:source] = child.name
                             else # they have it but we don't
-                                #Puppet.info "Adding %s as a new child" % child.name
                                 fullname = File.join(self.name, name)
-
-                                if FileTest.exists?(self.name) and ! FileTest.directory?(self.name)
-                                    Puppet.err "Source: %s" % @source
-                                    Puppet.err "Dest: %s" % self.name
-                                    Puppet.err "Child: %s" % name
-                                    Puppet.err "Child: %s" % child.name
-                                    caller
-                                    exit
-                                end
                                 if kid = self.newchild(name,:source => child.name)
-                                    self.push kid
+                                    if @children.include?(kid)
+                                        Puppet.notice "Child already included"
+                                    else
+                                        self.push kid
+                                    end
                                 end
                             end
                         end
@@ -1057,13 +1274,45 @@ module Puppet
                     return
                 end
 
+                added = []
                 Dir.foreach(self.name) { |file|
                     next if file =~ /^\.\.?/ # skip . and ..
                     # XXX it's right here
                     if child = self.newchild(file, :recurse => recurse)
-                        self.push child
+                        if @children.include?(child)
+                            Puppet.notice "Child already included"
+                        else
+                            self.push child
+                            added.push file
+                        end
                     end
                 }
+
+                # here's where we handle sources; it's special, because it can
+                # require us to build a structure of files that don't yet exist
+                if @parameters.include?(:source)
+                    unless FileTest.directory?(@parameters[:source])
+                        raise Puppet::Error("Cannot use file %s as a source for a dir" %
+                            @parameters[:source])
+                    end
+                    Dir.foreach(@parameters[:source]) { |file|
+                        next if file =~ /^\.\.?/ # skip . and ..
+                        unless added.include?(file)
+                            Puppet.notice "Adding absent source-file %s" % file
+                            if child = self.newchild(file,
+                                :recurse => recurse,
+                                :source => File.join(self.name, file)
+                            )
+                                if @children.include?(child)
+                                    Puppet.notice "Child already included"
+                                else
+                                    self.push child
+                                    added.push file
+                                end
+                            end
+                        end
+                    }
+                end
             end
 
             # a wrapper method to make sure the file exists before doing anything
diff --git a/lib/puppet/type/state.rb b/lib/puppet/type/state.rb
index 00a1f0815..20e3a234d 100644
--- a/lib/puppet/type/state.rb
+++ b/lib/puppet/type/state.rb
@@ -41,16 +41,16 @@ class State < Puppet::Element
     #---------------------------------------------------------------
     # if we're not in sync, return a statechange capable of putting us
     # in sync
-    def evaluate
-        #debug "evaluating %s" % self
-        self.retrieve
-        if self.insync?
-            #debug "%s is in sync" % self
-            return nil
-        else
-            return Puppet::StateChange.new(self)
-        end
-    end
+    #def evaluate
+    #    #debug "evaluating %s" % self
+    #    self.retrieve
+    #    if self.insync?
+    #        #debug "%s is in sync" % self
+    #        return nil
+    #    else
+    #        return Puppet::StateChange.new(self)
+    #    end
+    #end
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------