Most of the graph handling is now done, and all of the recursive types (basically just file, tidy, and component) now correctly use the generation mechanisms in the transactions, instead of sticking them in their @children array. Now I just need to go through the rest of the tests and make sure everything passes.

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

13 files changed, 292 insertions, 266 deletions

diff --git a/lib/puppet/metatype/attributes.rb b/lib/puppet/metatype/attributes.rb
index 764aba8ee..28cefec6a 100644
--- a/lib/puppet/metatype/attributes.rb
+++ b/lib/puppet/metatype/attributes.rb
@@ -131,6 +131,21 @@ class Puppet::Type
 
         return ens
     end
+    
+    # Deal with any options passed into parameters.
+    def self.handle_param_options(name, options)
+        # If it's a boolean parameter, create a method to test the value easily
+        if options[:boolean]
+            define_method(name.to_s + "?") do
+                val = self[name]
+                if val == :true or val == true
+                    return true
+                end
+            end
+        end
+        
+        # If this param handles relationships, store that information
+    end
 
     # Is the parameter in question a meta-parameter?
     def self.metaparam?(param)
@@ -153,18 +168,21 @@ class Puppet::Type
 
     # Create a new metaparam.  Requires a block and a name, stores it in the
     # @parameters array, and does some basic checking on it.
-    def self.newmetaparam(name, &block)
+    def self.newmetaparam(name, options = {}, &block)
         @@metaparams ||= []
         @@metaparamhash ||= {}
         name = symbolize(name)
 
         param = genclass(name,
-            :parent => Puppet::Parameter,
+            :parent => options[:parent] || Puppet::Parameter,
             :prefix => "MetaParam",
             :hash => @@metaparamhash,
             :array => @@metaparams,
+            :attributes => options[:attributes],
             &block
         )
+        
+        handle_param_options(name, options)
 
         param.ismetaparameter
 
@@ -192,14 +210,18 @@ class Puppet::Type
     # Create a new parameter.  Requires a block and a name, stores it in the
     # @parameters array, and does some basic checking on it.
     def self.newparam(name, options = {}, &block)
+        options[:attributes] ||= {}
+        options[:attributes][:element] = self
         param = genclass(name,
             :parent => options[:parent] || Puppet::Parameter,
-            :attributes => { :element => self },
+            :attributes => options[:attributes],
             :block => block,
             :prefix => "Parameter",
             :array => @parameters,
             :hash => @paramhash
         )
+        
+        handle_param_options(name, options)
 
         # These might be enabled later.
 #        define_method(name) do
diff --git a/lib/puppet/metatype/container.rb b/lib/puppet/metatype/container.rb
index d7c509699..364639fd5 100644
--- a/lib/puppet/metatype/container.rb
+++ b/lib/puppet/metatype/container.rb
@@ -10,6 +10,10 @@ class Puppet::Type
             return false
         end
     end
+    
+    def depthfirst?
+        self.class.depthfirst?
+    end
 
     def parent=(parent)
         if self.parentof?(parent)
diff --git a/lib/puppet/metatype/manager.rb b/lib/puppet/metatype/manager.rb
index d2749b87d..bad41570a 100644
--- a/lib/puppet/metatype/manager.rb
+++ b/lib/puppet/metatype/manager.rb
@@ -113,6 +113,10 @@ module Manager
         klass = rmclass(name,
             :hash => @types
         )
+        
+        if respond_to?("new" + name.to_s)
+            metaclass.send(:remove_method, "new" + name.to_s)
+        end
     end
 
     # Return a Type instance by name.
diff --git a/lib/puppet/metatype/metaparams.rb b/lib/puppet/metatype/metaparams.rb
index f28103a87..df226b146 100644
--- a/lib/puppet/metatype/metaparams.rb
+++ b/lib/puppet/metatype/metaparams.rb
@@ -87,7 +87,6 @@ class Puppet::Type
                         [state, self.class.name]
                 end
                 next unless stateklass.checkable?
-
                 @parent.newstate(state)
             }
         end
@@ -134,74 +133,6 @@ class Puppet::Type
         result
     end
 
-    # For each object we require, subscribe to all events that it generates. We
-    # might reduce the level of subscription eventually, but for now...
-    newmetaparam(:require) do
-        desc "One or more objects that this object depends on.
-            This is used purely for guaranteeing that changes to required objects
-            happen before the dependent object.  For instance:
-            
-                # Create the destination directory before you copy things down
-                file { \"/usr/local/scripts\":
-                    ensure => directory
-                }
-
-                file { \"/usr/local/scripts/myscript\":
-                    source => \"puppet://server/module/myscript\",
-                    mode => 755,
-                    require => file[\"/usr/local/scripts\"]
-                }
-
-            Note that Puppet will autorequire everything that it can, and
-            there are hooks in place so that it's easy for elements to add new
-            ways to autorequire objects, so if you think Puppet could be
-            smarter here, let us know.
-
-            In fact, the above code was redundant -- Puppet will autorequire
-            any parent directories that are being managed; it will
-            automatically realize that the parent directory should be created
-            before the script is pulled down.
-            
-            Currently, exec elements will autorequire their CWD (if it is
-            specified) plus any fully qualified paths that appear in the
-            command.   For instance, if you had an ``exec`` command that ran
-            the ``myscript`` mentioned above, the above code that pulls the
-            file down would be automatically listed as a requirement to the
-            ``exec`` code, so that you would always be running againts the
-            most recent version.
-            "
-
-        # Take whatever dependencies currently exist and add these.
-        # Note that this probably doesn't behave correctly with unsubscribe.
-        munge do |requires|
-            @parent.store_relationship(:require, requires)
-        end
-    end
-
-    # For each object we require, subscribe to all events that it generates.
-    # We might reduce the level of subscription eventually, but for now...
-    newmetaparam(:subscribe) do
-        desc "One or more objects that this object depends on.  Changes in the
-            subscribed to objects result in the dependent objects being
-            refreshed (e.g., a service will get restarted).  For instance:
-            
-                class nagios {
-                    file { \"/etc/nagios/nagios.conf\":
-                        source => \"puppet://server/module/nagios.conf\",
-                        alias => nagconf # just to make things easier for me
-                    }
-                    service { nagios:
-                        running => true,
-                        subscribe => file[nagconf]
-                    }
-                }
-            "
-
-        munge do |requires|
-            @parent.store_relationship(:subscribe, requires)
-        end
-    end
-
     newmetaparam(:loglevel) do
         desc "Sets the level that information will be logged.
              The log levels have the biggest impact when logs are sent to
@@ -305,8 +236,133 @@ class Puppet::Type
             end
         end
     end
+    
+    class RelationshipMetaparam < Puppet::Parameter
+        class << self
+            attr_accessor :direction, :events, :callback, :subclasses
+        end
+        
+        @subclasses = []
+        
+        def self.inherited(sub)
+            @subclasses << sub
+        end
+        
+        def munge(rels)
+            @parent.store_relationship(self.class.name, rels)
+        end
+        
+        # Create edges from each of our relationships.    :in
+        # relationships are specified by the event-receivers, and :out
+        # relationships are specified by the event generator.  This
+        # way 'source' and 'target' are consistent terms in both edges
+        # and events -- that is, an event targets edges whose source matches
+        # the event's source.  The direction of the relationship determines
+        # which resource is applied first and which resource is considered
+        # to be the event generator.
+        def to_edges
+            @value.collect do |value|
+                # we just have a name and a type, and we need to convert it
+                # to an object...
+                tname, name = value
+                object = nil
+                unless type = Puppet::Type.type(tname)
+                    self.fail "Could not find type %s" % tname.inspect
+                end
+                unless object = type[name]
+                    self.fail "Could not retrieve object '%s' of type '%s'" %
+                        [name,type]
+                end
+                self.debug("subscribes to %s" % [object])
+
+                # Are we requiring them, or vice versa?  See the builddepends
+                # method for further docs on this.
+                if self.class.direction == :in
+                    source = object
+                    target = @parent
+                else
+                    source = @parent
+                    target = object
+                end
+
+                # ok, both sides of the connection store some information
+                # we store the method to call when a given subscription is 
+                # triggered, but the source object decides whether 
+                subargs = {
+                    :event => self.class.events
+                }
+
+                if method = self.class.callback
+                    subargs[:callback] = method
+                end
+                rel = Puppet::Relationship.new(source, target, subargs)
+            end
+        end
+    end
+    
+    def self.relationship_params
+        RelationshipMetaparam.subclasses
+    end
+
+    # For each object we require, subscribe to all events that it generates. We
+    # might reduce the level of subscription eventually, but for now...
+    newmetaparam(:require, :parent => RelationshipMetaparam, :attributes => {:direction => :in, :events => :NONE}) do
+        desc "One or more objects that this object depends on.
+            This is used purely for guaranteeing that changes to required objects
+            happen before the dependent object.  For instance:
+            
+                # Create the destination directory before you copy things down
+                file { \"/usr/local/scripts\":
+                    ensure => directory
+                }
+
+                file { \"/usr/local/scripts/myscript\":
+                    source => \"puppet://server/module/myscript\",
+                    mode => 755,
+                    require => file[\"/usr/local/scripts\"]
+                }
 
-    newmetaparam(:notify) do
+            Note that Puppet will autorequire everything that it can, and
+            there are hooks in place so that it's easy for elements to add new
+            ways to autorequire objects, so if you think Puppet could be
+            smarter here, let us know.
+
+            In fact, the above code was redundant -- Puppet will autorequire
+            any parent directories that are being managed; it will
+            automatically realize that the parent directory should be created
+            before the script is pulled down.
+            
+            Currently, exec elements will autorequire their CWD (if it is
+            specified) plus any fully qualified paths that appear in the
+            command.   For instance, if you had an ``exec`` command that ran
+            the ``myscript`` mentioned above, the above code that pulls the
+            file down would be automatically listed as a requirement to the
+            ``exec`` code, so that you would always be running againts the
+            most recent version.
+            "
+    end
+
+    # For each object we require, subscribe to all events that it generates.
+    # We might reduce the level of subscription eventually, but for now...
+    newmetaparam(:subscribe, :parent => RelationshipMetaparam, :attributes => {:direction => :in, :events => :ALL_EVENTS, :callback => :refresh}) do
+        desc "One or more objects that this object depends on.  Changes in the
+            subscribed to objects result in the dependent objects being
+            refreshed (e.g., a service will get restarted).  For instance:
+            
+                class nagios {
+                    file { \"/etc/nagios/nagios.conf\":
+                        source => \"puppet://server/module/nagios.conf\",
+                        alias => nagconf # just to make things easier for me
+                    }
+                    service { nagios:
+                        running => true,
+                        subscribe => file[nagconf]
+                    }
+                }
+            "
+    end
+    
+    newmetaparam(:notify, :parent => RelationshipMetaparam, :attributes => {:direction => :out, :events => :ALL_EVENTS, :callback => :refresh}) do
         desc %{This parameter is the opposite of **subscribe** -- it sends events
             to the specified object:
 
@@ -320,14 +376,9 @@ class Puppet::Type
                 }
             
             This will restart the sshd service if the sshd config file changes.}
-
-
-        munge do |notifies|
-            @parent.store_relationship(:notify, notifies)
-        end
     end
 
-    newmetaparam(:before) do
+    newmetaparam(:before, :parent => RelationshipMetaparam, :attributes => {:direction => :out, :events => :NONE}) do
         desc %{This parameter is the opposite of **require** -- it guarantees
             that the specified object is applied later than the specifying
             object:
@@ -345,10 +396,6 @@ class Puppet::Type
             
             This will make sure all of the files are up to date before the
             make command is run.}
-
-        munge do |notifies|
-            @parent.store_relationship(:before, notifies)
-        end
     end
 end # Puppet::Type
 
diff --git a/lib/puppet/metatype/relationships.rb b/lib/puppet/metatype/relationships.rb
index 5f2471460..467b6187b 100644
--- a/lib/puppet/metatype/relationships.rb
+++ b/lib/puppet/metatype/relationships.rb
@@ -48,86 +48,28 @@ class Puppet::Type
         return reqs
     end
 
-    # Build the dependencies associated with an individual object.  :in
-    # relationships are specified by the event-receivers, and :out
-    # relationships are specified by the event generator.  This
-    # way 'source' and 'target' are consistent terms in both edges
-    # and events -- that is, an event targets edges whose source matches
-    # the event's source.  Note that the direction of the relationship
-    # doesn't actually mean anything until you start using events --
-    # the same information is present regardless.
+    # Build the dependencies associated with an individual object.
     def builddepends
         # Handle the requires
-        {:require => [:NONE, nil, :in],
-            :subscribe => [:ALL_EVENTS, :refresh, :in],
-            :notify => [:ALL_EVENTS, :refresh, :out],
-            :before => [:NONE, nil, :out]}.collect do |type, args|
-                if self[type]
-                    handledepends(self[type], *args)
-                end
-            end.flatten.reject { |r| r.nil? }
-    end
-
-    def handledepends(requires, event, method, direction)
-        # Requires are specified in the form of [type, name], so they're always
-        # an array.  But we want them to be an array of arrays.
-        unless requires[0].is_a?(Array)
-            requires = [requires]
-        end
-        requires.collect { |rname|
-            # we just have a name and a type, and we need to convert it
-            # to an object...
-            type = nil
-            object = nil
-            tname = rname[0]
-            unless type = Puppet::Type.type(tname)
-                self.fail "Could not find type %s" % tname.inspect
-            end
-            name = rname[1]
-            unless object = type[name]
-                self.fail "Could not retrieve object '%s' of type '%s'" %
-                    [name,type]
-            end
-            self.debug("subscribes to %s" % [object])
-
-            # Are we requiring them, or vice versa?  See the builddepends
-            # method for further docs on this.
-            if direction == :in
-                source = object
-                target = self
-            else
-                source = self
-                target = object
-            end
-
-            # ok, both sides of the connection store some information
-            # we store the method to call when a given subscription is 
-            # triggered, but the source object decides whether 
-            subargs = {
-                :event => event
-            }
-
-            if method
-                subargs[:callback] = method
+        self.class.relationship_params.collect do |klass|
+            if param = @metaparams[klass.name]
+                param.to_edges
             end
-            rel = Puppet::Relationship.new(source, target, subargs)
-        }
+        end.flatten.reject { |r| r.nil? }
     end
-
-    # Unsubscribe from a given object, possibly with a specific event.
-    def unsubscribe(object, event = nil)
-        # First look through our own relationship params
-        [:require, :subscribe].each do |param|
-            if values = self[param]
-                newvals = values.reject { |d|
-                    d == [object.class.name, object.title]
-                }
-                if newvals.length != values.length
-                    self.delete(param)
-                    self[param] = newvals
-                end
+    
+    # Does this resource have a relationship with the other?  We have to
+    # check each object for both directions of relationship.
+    def requires?(other)
+        them = [other.class.name, other.title]
+        me = [self.class.name, self.title]
+        self.class.relationship_params.each do |param|
+            case param.direction
+            when :in: return true if v = self[param.name] and v.include?(them)
+            when :out: return true if v = other[param.name] and v.include?(me)
             end
         end
+        return false
     end
 
     # we've received an event
@@ -154,6 +96,22 @@ class Puppet::Type
             }
         end
     end
+    
+    # Unsubscribe from a given object, possibly with a specific event.
+    def unsubscribe(object, event = nil)
+        # First look through our own relationship params
+        [:require, :subscribe].each do |param|
+            if values = self[param]
+                newvals = values.reject { |d|
+                    d == [object.class.name, object.title]
+                }
+                if newvals.length != values.length
+                    self.delete(param)
+                    self[param] = newvals
+                end
+            end
+        end
+    end
 end
 
 # $Id$
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 5bd5afd2f..adb5eb134 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -29,28 +29,6 @@ class Transaction
     # Apply all changes for a resource, returning a list of the events
     # generated.
     def apply(resource)
-        # First make sure there are no failed dependencies.  To do this,
-        # we check for failures in any of the vertexes above us.  It's not
-        # enough to check the immediate dependencies, which is why we use
-        # a tree from the reversed graph.
-        @relgraph.reversal.tree_from_vertex(resource, :dfs).keys.each do |dep|
-            skip = false
-            if fails = failed?(dep)
-                resource.notice "Dependency %s[%s] has %s failures" %
-                    [dep.class.name, dep.name, @failures[dep]]
-                skip = true
-            end
-
-            if skip
-                resource.warning "Skipping because of failed dependencies"
-                @resourcemetrics[:skipped] += 1
-                return []
-            end
-        end
-        
-        # If the resource needs to generate new objects at eval time, do it now.
-        eval_generate(resource)
-
         begin
             changes = resource.evaluate
         rescue => detail
@@ -139,20 +117,24 @@ class Transaction
     def eval_generate(resource)
         if resource.respond_to?(:eval_generate)
             if children = resource.eval_generate
+                depthfirst = resource.depthfirst?
                 dependents = @relgraph.adjacent(resource, :direction => :out, :type => :edges)
                 targets = @relgraph.adjacent(resource, :direction => :in, :type => :edges)
                 children.each do |gen_child|
-                    gen_child.info "generated"
-                    @relgraph.add_edge!(resource, gen_child)
+                    if depthfirst
+                        @relgraph.add_edge!(gen_child, resource)
+                    else
+                        @relgraph.add_edge!(resource, gen_child)
+                    end
                     dependents.each do |edge|
                         @relgraph.add_edge!(gen_child, edge.target, edge.label)
                     end
                     targets.each do |edge|
                         @relgraph.add_edge!(edge.source, gen_child, edge.label)
                     end
-                    @sorted_resources.insert(@sorted_resources.index(resource) + 1, gen_child)
                     @generated << gen_child
                 end
+                return children
             end
         end
     end
@@ -161,25 +143,35 @@ class Transaction
     def eval_resource(resource)
         events = []
         
-        unless tagged?(resource)
-            resource.debug "Not tagged with %s" % tags.join(", ")
-            return events
-        end
-        
-        unless scheduled?(resource)
-            resource.debug "Not scheduled"
-            return events
-        end
-        
-        @resourcemetrics[:scheduled] += 1
+        if skip?(resource)
+            @resourcemetrics[:skipped] += 1
+        else
+            @resourcemetrics[:scheduled] += 1
+            
+            # We need to generate first regardless, because the recursive
+            # actions sometimes change how the top resource is applied.
+            children = eval_generate(resource)
+            
+            if resource.depthfirst? and children
+                children.each do |child|
+                    events += eval_resource(child)
+                end
+            end
 
-        # Perform the actual changes
-        seconds = thinmark do
-            events = apply(resource)
-        end
+            # Perform the actual changes
+            seconds = thinmark do
+                events += apply(resource)
+            end
+            
+            if ! resource.depthfirst? and children
+                children.each do |child|
+                    events += eval_resource(child)
+                end
+            end
 
-        # Keep track of how long we spend in each type of resource
-        @timemetrics[resource.class.name] += seconds
+            # Keep track of how long we spend in each type of resource
+            @timemetrics[resource.class.name] += seconds
+        end
 
         # Check to see if there are any events for this resource
         if triggedevents = trigger(resource)
@@ -231,6 +223,24 @@ class Transaction
             return false
         end
     end
+
+    # Does this resource have any failed dependencies?
+    def failed_dependencies?(resource)
+        # First make sure there are no failed dependencies.  To do this,
+        # we check for failures in any of the vertexes above us.  It's not
+        # enough to check the immediate dependencies, which is why we use
+        # a tree from the reversed graph.
+        skip = false
+        @relgraph.reversal.tree_from_vertex(resource, :dfs).keys.each do |dep|
+            if fails = failed?(dep)
+                resource.notice "Dependency %s[%s] has %s failures" %
+                    [dep.class.name, dep.name, @failures[dep]]
+                skip = true
+            end
+        end
+        
+        return skip
+    end
     
     # Collect any dynamically generated resources.
     def generate
@@ -443,6 +453,21 @@ class Transaction
         self.ignoreschedules or resource.scheduled?
     end
     
+    # Should this resource be skipped?
+    def skip?(resource)
+        skip = false
+        if ! tagged?(resource)
+            resource.debug "Not tagged with %s" % tags.join(", ")
+        elsif ! scheduled?(resource)
+            resource.debug "Not scheduled"
+        elsif failed_dependencies?(resource)
+            resource.warning "Skipping because of failed dependencies"
+        else
+            return false
+        end
+        return true
+    end
+    
     # The tags we should be checking.
     def tags
         # Allow the tags to be overridden
diff --git a/lib/puppet/type/component.rb b/lib/puppet/type/component.rb
index cadd586c8..f203179a8 100644
--- a/lib/puppet/type/component.rb
+++ b/lib/puppet/type/component.rb
@@ -23,42 +23,6 @@ module Puppet
             defaultto "component"
         end
 
-        # topo sort functions
-        def self.sort(objects)
-            list = []
-            tmplist = {}
-
-            objects.each { |obj|
-                self.recurse(obj, tmplist, list)
-            }
-
-            return list.flatten
-        end
-
-        # FIXME this method assumes that dependencies themselves
-        # are never components
-        def self.recurse(obj, inlist, list)
-            if inlist.include?(obj.object_id)
-                return
-            end
-            inlist[obj.object_id] = true
-            begin
-                obj.eachdependency { |req|
-                    self.recurse(req, inlist, list)
-                }
-            rescue Puppet::Error => detail
-                raise Puppet::Error, "%s: %s" % [obj.path, detail]
-            end
-
-            if obj.is_a? self
-                obj.each { |child|
-                    self.recurse(child, inlist, list)
-                }
-            else
-                list << obj
-            end
-        end
-
         # Remove a child from the component.
         def delete(child)
             if @children.include?(child)
@@ -116,12 +80,6 @@ module Puppet
                 return false
             end
         end
-        
-        # Return a flattened array containing all of the children
-        # and all child components' children, sorted in order of dependencies.
-        def flatten
-            self.class.sort(@children).flatten
-        end
 
         # Initialize a new component
         def initialize(args)
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index b3bbba3b9..24f961a62 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -113,7 +113,7 @@ module Puppet
             end
         end
 
-        newparam(:replace) do
+        newparam(:replace, :boolean => true) do
             desc "Whether or not to replace a file that is
                 sourced but exists.  This is useful for using file sources
                 purely for initialization."
@@ -121,7 +121,7 @@ module Puppet
             defaultto :true
         end
 
-        newparam(:force) do
+        newparam(:force, :boolean => true) do
             desc "Force the file operation.  Currently only used when replacing
                 directories with links."
             newvalues(:true, :false)
@@ -160,7 +160,7 @@ module Puppet
             defaultto :ignore
         end
 
-        newparam(:purge) do
+        newparam(:purge, :boolean => true) do
             desc "Whether unmanaged files should be purged.  If you have a filebucket
                 configured the purged files will be uploaded, but if you do not,
                 this will destroy data.  Only use this option for generated
@@ -413,10 +413,6 @@ module Puppet
                 end
             end
 
-            if @arghash[:target]
-                warning "%s vs %s" % [@arghash[:ensure], @arghash[:target]]
-            end
-
             @stat = nil
         end
 
@@ -441,6 +437,7 @@ module Puppet
 
             # Now that we know our corresponding target is a directory,
             # change our type
+            info "setting ensure to target"
             self[:ensure] = :directory
 
             unless FileTest.readable? target
@@ -778,10 +775,6 @@ module Puppet
             unless stat = self.stat(true)
                 self.debug "File does not exist"
                 @states.each { |name,state|
-                    # We've already retrieved the source, and we don't
-                    # want to overwrite whatever it did.  This is a bit
-                    # of a hack, but oh well, source is definitely special.
-                    # next if name == :source
                     state.is = :absent
                 }
                 
diff --git a/lib/puppet/type/pfile/checksum.rb b/lib/puppet/type/pfile/checksum.rb
index a91f7e017..c4ae6e8c3 100755
--- a/lib/puppet/type/pfile/checksum.rb
+++ b/lib/puppet/type/pfile/checksum.rb
@@ -306,12 +306,12 @@ module Puppet
 
             # if we're replacing, vs. updating
             if sum = cache(checktype())
-                unless defined? @should
-                    raise Puppet::Error.new(
-                        ("@should is not initialized for %s, even though we " +
-                        "found a checksum") % @parent[:path]
-                    )
-                end
+                # unless defined? @should
+                #     raise Puppet::Error.new(
+                #         ("@should is not initialized for %s, even though we " +
+                #         "found a checksum") % @parent[:path]
+                #     )
+                # end
                 
                 if @is == sum
                     info "Sums are already equal"
diff --git a/lib/puppet/type/pfile/ensure.rb b/lib/puppet/type/pfile/ensure.rb
index c998e0f7f..6f7b15d49 100755
--- a/lib/puppet/type/pfile/ensure.rb
+++ b/lib/puppet/type/pfile/ensure.rb
@@ -60,7 +60,6 @@ module Puppet
         end
 
         newvalue(:directory) do
-            p @is
             mode = @parent.should(:mode)
             parent = File.dirname(@parent[:path])
             unless FileTest.exists? parent
diff --git a/lib/puppet/type/pfile/source.rb b/lib/puppet/type/pfile/source.rb
index 8ac60422c..77f90bc9f 100755
--- a/lib/puppet/type/pfile/source.rb
+++ b/lib/puppet/type/pfile/source.rb
@@ -67,6 +67,10 @@ module Puppet
             source.sub(/\/$/, '')
         end
         
+        def change_to_s
+            "replacing from source %s" % @source
+        end
+        
         def checksum
             if defined?(@stats)
                 @stats[:checksum]
@@ -133,6 +137,10 @@ module Puppet
             unless @stats[:type] == "file" 
                 return true
             end
+            
+            if @parent.is(:ensure) != :absent and ! @parent.replace?
+                return true
+            end
             # Now, we just check to see if the checksums are the same
             return @parent.is(:checksum) == @stats[:checksum]
         end
@@ -184,6 +192,7 @@ module Puppet
                 # be inherited from the source?
                 unless @parent.argument?(stat)
                     @parent[stat] = value
+                    @parent.state(stat).retrieve
                 end
             }
             
@@ -197,15 +206,14 @@ module Puppet
         # Make sure we're also checking the checksum
         def should=(value)
             super
+
+            checks = (PINPARAMS + [:ensure])
+            checks.delete(:checksum)
             
-            # @parent[:check] = [:checksum, :ensure]
+            @parent[:check] = checks
             unless @parent.state(:checksum)
                 @parent[:checksum] = :md5
             end
-            
-            unless @parent.state(:ensure)
-                @parent[:check] = :ensure
-            end
         end
 
         def sync
diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb
index 7e0a04353..cebbde74a 100755
--- a/lib/puppet/type/tidy.rb
+++ b/lib/puppet/type/tidy.rb
@@ -168,7 +168,15 @@ module Puppet
             end
 
             def insync?
-                insync_age? and insync_size?
+                if @is.is_a?(Symbol)
+                    if @is == :absent
+                        return true
+                    else
+                        return false
+                    end
+                else
+                    insync_age? and insync_size?
+                end
             end
 
             def retrieve
diff --git a/lib/puppet/util/posix.rb b/lib/puppet/util/posix.rb
index 75726b3da..01c3e25aa 100755
--- a/lib/puppet/util/posix.rb
+++ b/lib/puppet/util/posix.rb
@@ -12,7 +12,7 @@ module Puppet::Util::POSIX
         else
             method = (prefix + "nam").intern
         end
-        
+
         begin
             return Etc.send(method, id).send(field)
         rescue ArgumentError => detail