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

22 files changed, 395 insertions, 569 deletions

diff --git a/Rakefile b/Rakefile
index 0e2e62d3a..25f5a23fa 100644
--- a/Rakefile
+++ b/Rakefile
@@ -28,7 +28,7 @@ project = Rake::RedLabProject.new("puppet") do |p|
 
     p.add_dependency('facter', '1.1.0')
 
-    p.epmhosts = %w{culain}
+    #p.epmhosts = %w{culain}
     p.sunpkghost = "sol10b"
     p.rpmhost = "fedora1"
 end
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
diff --git a/test/other/relationships.rb b/test/other/relationships.rb
index e9c1d1c9c..164d52d2a 100755
--- a/test/other/relationships.rb
+++ b/test/other/relationships.rb
@@ -237,6 +237,30 @@ class TestRelationships < Test::Unit::TestCase
         graph = trans.relgraph
         assert(graph.edge?(file, exec), "autorequire edge was not created")
     end
+    
+    def test_requires?
+        # Test the first direction
+        file1 = Puppet::Type.newfile(:title => "one", :path => tempfile,
+            :ensure => :directory)
+        file2 = Puppet::Type.newfile(:title => "two", :path => tempfile,
+            :ensure => :directory)
+        
+        file1[:require] = file2
+        assert(file1.requires?(file2), "requires? failed to catch :require relationship")
+        file1.delete(:require)
+        assert(! file1.requires?(file2), "did not delete relationship")
+        file1[:subscribe] = file2
+        assert(file1.requires?(file2), "requires? failed to catch :subscribe relationship")
+        file1.delete(:subscribe)
+        assert(! file1.requires?(file2), "did not delete relationship")
+        file2[:before] = file1
+        assert(file1.requires?(file2), "requires? failed to catch :before relationship")
+        file2.delete(:before)
+        assert(! file1.requires?(file2), "did not delete relationship")
+        file2[:notify] = file1
+        assert(file1.requires?(file2), "requires? failed to catch :notify relationship")
+    end
+        
 end
 
 # $Id$
diff --git a/test/other/transactions.rb b/test/other/transactions.rb
index 7342b57ec..9fc58526a 100755
--- a/test/other/transactions.rb
+++ b/test/other/transactions.rb
@@ -599,7 +599,7 @@ class TestTransactions < Test::Unit::TestCase
     
     # Test mid-evaluation generation.
     def test_eval_generate
-        $evaluated = {}
+        $evaluated = []
         type = mkgenerator() do
             def eval_generate
                 ret = []
@@ -612,7 +612,7 @@ class TestTransactions < Test::Unit::TestCase
             end
             
             def evaluate
-                $evaluated[self.title] = true
+                $evaluated << self.title
                 return []
             end
         end
@@ -627,7 +627,7 @@ class TestTransactions < Test::Unit::TestCase
         # Now apply the resources, and make sure they appropriately generate
         # things.
         assert_nothing_raised("failed to apply yay") do
-            trans.apply(yay)
+            trans.eval_resource(yay)
         end
         ya = type["ya"]
         assert(ya, "Did not generate ya")
@@ -648,7 +648,7 @@ class TestTransactions < Test::Unit::TestCase
         
         # Now make sure it in turn eval_generates appropriately
         assert_nothing_raised("failed to apply yay") do
-            trans.apply(type["ya"])
+            trans.eval_resource(type["ya"])
         end
 
         %w{y}.each do |name|
@@ -659,14 +659,12 @@ class TestTransactions < Test::Unit::TestCase
         end
         
         assert_nothing_raised("failed to eval_generate with nil response") do
-            trans.apply(type["y"])
+            trans.eval_resource(type["y"])
         end
+        assert(trans.relgraph.edge?(yay, ya), "no edge was created for ya => yay")
         
-        assert_equal(%w{yay ya y rah}, trans.sorted_resources.collect { |r| r.title },
-            "Did not eval_generate correctly")
-            
         assert_nothing_raised("failed to apply rah") do
-            trans.apply(rah)
+            trans.eval_resource(rah)
         end
 
         ra = type["ra"]
@@ -701,12 +699,13 @@ class TestTransactions < Test::Unit::TestCase
         
         # Now, start over and make sure that everything gets evaluated.
         trans = comp.evaluate
+        $evaluated.clear
         assert_nothing_raised do
             trans.evaluate
         end
         
-        assert_equal(%w{yay ya y rah ra r}.sort, $evaluated.keys.sort,
-            "Not all resources were evaluated")
+        assert_equal(%w{yay ya y rah ra r}, $evaluated,
+            "Not all resources were evaluated or not in the right order")
     end
     
     def test_tags
diff --git a/test/types/component.rb b/test/types/component.rb
index 3fdc7bfe2..3b55bab0f 100755
--- a/test/types/component.rb
+++ b/test/types/component.rb
@@ -10,6 +10,7 @@ require 'puppettest/support/resources'
 
 class TestComponent < Test::Unit::TestCase
 	include PuppetTest
+	include PuppetTest::Support::Resources
     def setup
         super
         @@used = {}
@@ -92,20 +93,6 @@ class TestComponent < Test::Unit::TestCase
         comp.finalize
         comp
     end
-
-    def test_ordering
-        list = nil
-        comp = mkrandcomp(30,5)
-        assert_nothing_raised {
-            list = comp.flatten
-        }
-
-        list.each_with_index { |obj, index|
-            obj.eachdependency { |dep|
-                assert(list.index(dep) < index)
-            }
-        }
-    end
         
     def test_to_graph
         one, two, middle, top = mktree
@@ -124,193 +111,4 @@ class TestComponent < Test::Unit::TestCase
             end
         end
     end
-
-    def test_correctsorting
-        tmpfile = tempfile()
-        @@tmpfiles.push tmpfile
-        trans = nil
-        cmd = nil
-        File.open(tmpfile, File::WRONLY|File::CREAT|File::TRUNC) { |of|
-            of.puts rand(100)
-        }
-        file = Puppet.type(:file).create(
-            :path => tmpfile,
-            :checksum => "md5"
-        )
-        assert_nothing_raised {
-            cmd = Puppet.type(:exec).create(
-                :command => "pwd",
-                :path => "/usr/bin:/bin:/usr/sbin:/sbin",
-                :subscribe => [[file.class.name,file.name]],
-                :refreshonly => true
-            )
-        }
-
-        order = nil
-        assert_nothing_raised {
-            order = Puppet.type(:component).sort([file, cmd])
-        }
-
-        [cmd, file].each { |obj|
-            assert_equal(1, order.find_all { |o| o.name == obj.name }.length)
-        }
-    end
-
-    def test_correctflattening
-        tmpfile = tempfile()
-        @@tmpfiles.push tmpfile
-        trans = nil
-        cmd = nil
-        File.open(tmpfile, File::WRONLY|File::CREAT|File::TRUNC) { |of|
-            of.puts rand(100)
-        }
-        file = Puppet.type(:file).create(
-            :path => tmpfile,
-            :checksum => "md5"
-        )
-        assert_nothing_raised {
-            cmd = Puppet.type(:exec).create(
-                :command => "pwd",
-                :path => "/usr/bin:/bin:/usr/sbin:/sbin",
-                :subscribe => [[file.class.name,file.name]],
-                :refreshonly => true
-            )
-        }
-
-        comp = newcomp(cmd, file)
-        comp.finalize
-        objects = nil
-        assert_nothing_raised {
-            objects = comp.flatten
-        }
-
-        [cmd, file].each { |obj|
-            assert_equal(1, objects.find_all { |o| o.name == obj.name }.length)
-        }
-
-        assert(objects[0] == file, "File was not first object")
-        assert(objects[1] == cmd, "Exec was not second object")
-    end
-
-    def test_deepflatten
-        tmpfile = tempfile()
-        @@tmpfiles.push tmpfile
-        trans = nil
-        cmd = nil
-        File.open(tmpfile, File::WRONLY|File::CREAT|File::TRUNC) { |of|
-            of.puts rand(100)
-        }
-        file = Puppet.type(:file).create(
-            :path => tmpfile,
-            :checksum => "md5"
-        )
-        assert_nothing_raised {
-            cmd = Puppet.type(:exec).create(
-                :command => "pwd",
-                :path => "/usr/bin:/bin:/usr/sbin:/sbin",
-                :refreshonly => true
-            )
-        }
-
-        fcomp = newcomp("fflatten", file)
-        ecomp = newcomp("eflatten", cmd)
-
-        # this subscription can screw up the sorting
-        ecomp[:subscribe] = [[fcomp.class.name,fcomp.name]]
-
-        comp = newcomp("bflatten", ecomp, fcomp)
-        comp.finalize
-        objects = nil
-        assert_nothing_raised {
-            objects = comp.flatten
-        }
-
-        assert_equal(objects.length, 2, "Did not get two sorted objects")
-        objects.each { |o|
-            assert(o.is_a?(Puppet::Type), "Object %s is not a Type" % o.class)
-        }
-
-        assert(objects[0] == file, "File was not first object")
-        assert(objects[1] == cmd, "Exec was not second object")
-    end
-
-    def test_deepflatten2
-        tmpfile = tempfile()
-        @@tmpfiles.push tmpfile
-        trans = nil
-        cmd = nil
-        File.open(tmpfile, File::WRONLY|File::CREAT|File::TRUNC) { |of|
-            of.puts rand(100)
-        }
-        file = Puppet.type(:file).create(
-            :path => tmpfile,
-            :checksum => "md5"
-        )
-        assert_nothing_raised {
-            cmd = Puppet.type(:exec).create(
-                :command => "pwd",
-                :path => "/usr/bin:/bin:/usr/sbin:/sbin",
-                :refreshonly => true
-            )
-        }
-
-        ocmd = nil
-        assert_nothing_raised {
-            ocmd = Puppet.type(:exec).create(
-                :command => "echo true",
-                :path => "/usr/bin:/bin:/usr/sbin:/sbin",
-                :refreshonly => true
-            )
-        }
-
-        fcomp = newcomp("fflatten", file)
-        ecomp = newcomp("eflatten", cmd)
-        ocomp = newcomp("oflatten", ocmd)
-
-        # this subscription can screw up the sorting
-        cmd[:subscribe] = [[fcomp.class.name,fcomp.name]]
-        ocmd[:subscribe] = [[cmd.class.name,cmd.name]]
-
-        comp = newcomp("bflatten", ocomp, ecomp, fcomp)
-        comp.finalize
-        objects = nil
-        assert_nothing_raised {
-            objects = comp.flatten
-        }
-
-        assert_equal(objects.length, 3, "Did not get three sorted objects")
-
-        objects.each { |o|
-            assert(o.is_a?(Puppet::Type), "Object %s is not a Type" % o.class)
-        }
-
-        assert(objects[0] == file, "File was not first object")
-        assert(objects[1] == cmd, "Exec was not second object")
-        assert(objects[2] == ocmd, "Other exec was not second object")
-    end
-
-    def test_moreordering
-        dir = tempfile()
-
-        comp = Puppet.type(:component).create(
-            :name => "ordertesting"
-        )
-
-        10.times { |i|
-            fileobj = Puppet.type(:file).create(
-                :path => File.join(dir, "file%s" % i),
-                :ensure => "file"
-            )
-            comp.push(fileobj)
-        }
-
-        dirobj = Puppet.type(:file).create(
-            :path => dir,
-            :ensure => "directory"
-        )
-
-        comp.push(dirobj)
-
-        assert_apply(comp)
-    end
 end
diff --git a/test/types/exec.rb b/test/types/exec.rb
index 305f27d56..fd304e7d1 100755
--- a/test/types/exec.rb
+++ b/test/types/exec.rb
@@ -263,23 +263,28 @@ class TestExec < Test::Unit::TestCase
             :command => "cat %s %s" % [exe, oexe],
             :path => ENV["PATH"]
         )
-
-        comp = newcomp(ofile, exec, cat, file, baseobj)
-        comp.finalize
+        
+        rels = nil
+        assert_nothing_raised do
+            rels = exec.autorequire
+        end
 
         # Verify we get the script itself
-        assert(exec.requires?(file), "Exec did not autorequire %s" % file)
+        assert(rels.detect { |r| r.source == file }, "Exec did not autorequire its command")
 
         # Verify we catch the cwd
-        assert(exec.requires?(baseobj), "Exec did not autorequire cwd")
+        assert(rels.detect { |r| r.source == baseobj }, "Exec did not autorequire its cwd")
 
         # Verify we don't require ourselves
+        assert(! rels.detect { |r| r.source == ofile }, "Exec incorrectly required mentioned file")
         assert(!exec.requires?(ofile), "Exec incorrectly required file")
 
-        # Verify that we catch inline files
         # We not longer autorequire inline files
-        assert(! cat.requires?(ofile), "Exec required second inline file")
-        assert(! cat.requires?(file), "Exec required inline file")
+        assert_nothing_raised do
+            rels = cat.autorequire
+        end
+        assert(! rels.detect { |r| r.source == ofile }, "Exec required second inline file")
+        assert(! rels.detect { |r| r.source == file }, "Exec required inline file")
     end
 
     def test_ifonly
diff --git a/test/types/file.rb b/test/types/file.rb
index d5b493788..c57129cd5 100755
--- a/test/types/file.rb
+++ b/test/types/file.rb
@@ -792,7 +792,7 @@ class TestFile < Test::Unit::TestCase
         }
 
         assert_nothing_raised {
-            dirobj.generate
+            dirobj.eval_generate
         }
 
         assert_nothing_raised {
@@ -817,12 +817,12 @@ class TestFile < Test::Unit::TestCase
             :name => subfile,
             :ensure => "file"
         )
-        comp = newcomp(baseobj, subobj)
-        comp.finalize
-
-        assert(subobj.requires?(baseobj), "File did not require basedir")
-        assert(!subobj.requires?(subobj), "File required itself")
-        assert_events([:directory_created, :file_created], comp)
+        edge = nil
+        assert_nothing_raised do
+            edge = subobj.autorequire.shift
+        end
+        assert_equal(baseobj, edge.source, "file did not require its parent dir")
+        assert_equal(subobj, edge.target, "file did not require its parent dir")
     end
 
     def test_content
@@ -1623,11 +1623,12 @@ class TestFile < Test::Unit::TestCase
             )
             comp = newcomp(user, group, home)
         }
-        comp.finalize
-        comp.retrieve
-
-        assert(home.requires?(user), "File did not require owner")
-        assert(home.requires?(group), "File did not require group")
+        
+        # Now make sure we get a relationship for each of these
+        rels = nil
+        assert_nothing_raised { rels = home.autorequire }
+        assert(rels.detect { |e| e.source == user }, "owner was not autorequired")
+        assert(rels.detect { |e| e.source == group }, "group was not autorequired")
     end
 
     # Testing #309 -- //my/file => /my/file
@@ -1760,36 +1761,6 @@ class TestFile < Test::Unit::TestCase
             end
         end
     end
-
-    def test_check_checksums
-        dir = tempfile()
-        Dir.mkdir(dir)
-        subdir = File.join(dir, "sub")
-        Dir.mkdir(subdir)
-        file = File.join(dir, "file")
-        File.open(file, "w") { |f| f.puts "yay" }
-
-        obj = Puppet::Type.type(:file).create(
-            :path => dir, :check => :checksum, :recurse => true
-        )
-
-        assert_apply(obj)
-        File.open(file, "w") { |f| f.puts "rah" }
-        sleep 1
-        system("touch %s" % subdir)
-        Puppet::Storage.store
-        Puppet::Storage.load
-        assert_apply(obj)
-        [file, subdir].each do |path|
-            sub = Puppet::Type.type(:file)[path]
-            assert(sub, "did not find obj for %s" % path)
-            sub.retrieve
-
-            assert_nothing_raised do
-                sub.state(:checksum).sync
-            end
-        end
-    end
 end
 
 # $Id$
diff --git a/test/types/filesources.rb b/test/types/filesources.rb
index bae4c7d5f..c1c601b59 100755
--- a/test/types/filesources.rb
+++ b/test/types/filesources.rb
@@ -141,7 +141,7 @@ class TestFileSources < Test::Unit::TestCase
         assert(state, "did not get source state")
         
         # Make sure the munge didn't actually change the source
-        assert_equal(source, state.should, "munging changed the source")
+        assert_equal([source], state.should, "munging changed the source")
         
         # First try it with a missing source
         assert_nothing_raised do
@@ -411,7 +411,6 @@ class TestFileSources < Test::Unit::TestCase
         # then delete a file
         File.unlink(two)
 
-        puts "yay"
         # and run
         recursive_source_test(fromdir, todir)
 
@@ -422,9 +421,16 @@ class TestFileSources < Test::Unit::TestCase
     end
 
     def test_sources_with_readonly_destfiles
-        fromdir, todir = run_complex_sources
+        fromdir, todir, one, two = run_complex_sources
         assert(FileTest.exists?(todir))
-        readonly_random_files(todir)
+        File.chmod(0600, one)
+        recursive_source_test(fromdir, todir)
+
+        # and make sure they're still equal
+        assert_trees_equal(fromdir,todir)
+        
+        # Now try it with the directory being read-only
+        File.chmod(0111, todir)
         recursive_source_test(fromdir, todir)
 
         # and make sure they're still equal
@@ -432,11 +438,12 @@ class TestFileSources < Test::Unit::TestCase
     end
 
     def test_sources_with_modified_dest_files
-        fromdir, todir = run_complex_sources
+        fromdir, todir, one, two = run_complex_sources
 
         assert(FileTest.exists?(todir))
-        # then modify some files
-        modify_random_files(todir)
+        
+        # Modify a dest file
+        File.open(two, "w") { |f| f.puts "something else" }
 
         recursive_source_test(fromdir, todir)
 
@@ -470,6 +477,7 @@ class TestFileSources < Test::Unit::TestCase
         assert_trees_equal(fromdir,todir)
     end
 
+    # Make sure added files get correctly caught during recursion
     def test_RecursionWithAddedFiles
         basedir = tempfile()
         Dir.mkdir(basedir)
@@ -478,37 +486,28 @@ class TestFileSources < Test::Unit::TestCase
         file2 = File.join(basedir, "file2")
         subdir1 = File.join(basedir, "subdir1")
         file3 = File.join(subdir1, "file")
-        File.open(file1, "w") { |f| 3.times { f.print rand(100) } }
+        File.open(file1, "w") { |f| f.puts "yay" }
         rootobj = nil
         assert_nothing_raised {
             rootobj = Puppet.type(:file).create(
                 :name => basedir,
                 :recurse => true,
-                :check => %w{type owner}
+                :check => %w{type owner},
+                :mode => 0755
             )
-
-            rootobj.evaluate
         }
+        
+        assert_apply(rootobj)
+        assert_equal(0755, filemode(file1))
 
-        klass = Puppet.type(:file)
-        assert(klass[basedir])
-        assert(klass[file1])
-        assert_nil(klass[file2])
-
-        File.open(file2, "w") { |f| 3.times { f.print rand(100) } }
-
-        assert_nothing_raised {
-            rootobj.evaluate
-        }
-        assert(klass[file2])
+        File.open(file2, "w") { |f| f.puts "rah" }
+        assert_apply(rootobj)
+        assert_equal(0755, filemode(file2))
 
         Dir.mkdir(subdir1)
-        File.open(file3, "w") { |f| 3.times { f.print rand(100) } }
-
-        assert_nothing_raised {
-            rootobj.evaluate
-        }
-        assert(klass[file3])
+        File.open(file3, "w") { |f| f.puts "foo" }
+        assert_apply(rootobj)
+        assert_equal(0755, filemode(file3))
     end
 
     def mkfileserverconf(mounts)
@@ -890,7 +889,8 @@ class TestFileSources < Test::Unit::TestCase
         assert_apply(file)
 
         # Make sure it doesn't change.
-        assert_equal("yayness\n", File.read(dest))
+        assert_equal("yayness\n", File.read(dest),
+            "File got replaced when :replace was false")
 
         # Now set it to true and make sure it does change.
         assert_nothing_raised {
@@ -899,7 +899,8 @@ class TestFileSources < Test::Unit::TestCase
         assert_apply(file)
 
         # Make sure it doesn't change.
-        assert_equal("funtest\n", File.read(dest))
+        assert_equal("funtest\n", File.read(dest),
+            "File was not replaced when :replace was true")
     end
 
     # Testing #285.  This just makes sure that URI parsing works correctly.
diff --git a/test/types/tidy.rb b/test/types/tidy.rb
index 20694762f..513b05319 100755
--- a/test/types/tidy.rb
+++ b/test/types/tidy.rb
@@ -27,9 +27,9 @@ class TestTidy < Test::Unit::TestCase
 
     def test_tidydirs
         dir = mktmpdir
-        file = File.join(dir, "tidytesting")
+        file = File.join(dir, "file")
         File.open(file, "w") { |f|
-            f.puts rand(100)
+            f.puts "some stuff"
         }
 
         tidy = Puppet.type(:tidy).create(
diff --git a/test/types/user.rb b/test/types/user.rb
index a3a2e14f0..958434fa5 100755
--- a/test/types/user.rb
+++ b/test/types/user.rb
@@ -390,12 +390,14 @@ class TestUser < Test::Unit::TestCase
             )
             comp = newcomp(user, group, home, ogroup)
         }
-        comp.finalize
-        comp.retrieve
-
-        assert(user.requires?(group), "User did not require group")
-        assert(home.requires?(user), "Homedir did not require user")
-        assert(user.requires?(ogroup), "User did not require other groups")
+        
+        rels = nil
+        assert_nothing_raised() { rels = user.autorequire }
+
+        assert(rels.detect { |r| r.source == group }, "User did not require group")
+        assert(rels.detect { |r| r.source == ogroup }, "User did not require other groups")
+        assert_nothing_raised() { rels = home.autorequire }
+        assert(rels.detect { |r| r.source == user }, "Homedir did not require user")
     end
 
     def test_simpleuser