switching all relationships to be centrally maintained and to use symbolic references, rather than literal ones; also going through and making all tests pass again after mucking with services

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

15 files changed, 667 insertions, 444 deletions

diff --git a/lib/puppet/event.rb b/lib/puppet/event.rb
index d01863de2..e2c364194 100644
--- a/lib/puppet/event.rb
+++ b/lib/puppet/event.rb
@@ -1,8 +1,3 @@
-#!/usr/local/bin/ruby -w
-
-# $Id$
-
-# included so we can test object types
 require 'puppet'
 require 'puppet/type'
 
@@ -12,33 +7,183 @@ module Puppet
     # eventually, these will be passed on to some central event system
 	class Event
         include Puppet
+
         # subscriptions are permanent associations determining how different
         # objects react to an event
         class Subscription
             include Puppet
-            attr_accessor :source, :event, :target
+            attr_accessor :event, :callback
 
-            def initialize(hash)
-                @triggered = false
+            # Remove the existing subscriptions and such
+            def self.clear
+                self.init
+            end
+
+            # Remove a subscription
+            def self.delete(sub)
+                type, name = sub.targetarray
+                if @dependencies[type][name].include?(sub)
+                    @dependencies[type][name].delete(sub)
+                end
+
+                type, name = sub.sourcearray
+                if @subscriptions[type][name].include?(sub)
+                    @subscriptions[type][name].delete(sub)
+                end
+            end
+
+            # Initialize our class variables.  This is in a method so it can
+            # be called to clear the variables, too.
+            def self.init
+                # A hash of subscriptions and another of dependencies, organized by
+                # type, then by name.  I'm storing them all here, so that I don't
+                # have to store the subscriptions with the individual objects,
+                # which makes creating and destroying objects as necessary much
+                # easier.
+                @subscriptions = Hash.new { |hash, key|
+                    hash[key] = Hash.new { |shash, skey|
+                        # Each object has an array of subscriptions
+                        shash[skey] = []
+                    }
+                }
+
+                @dependencies = Hash.new { |hash, key|
+                    hash[key] = Hash.new { |shash, skey|
+                        # Each object has an array of subscriptions
+                        shash[skey] = []
+                    }
+                }
+            end
+
+            self.init
 
+            # Store the new subscription in a central hash.
+            def self.newsub(sub)
+                # The dependencies map allows me to look up a subscription by
+                # target -- find out which objects a given object is subscribed
+                # to, and thus find out which objects that given object depends
+                # upon.
+                # DEPENDENCIES == TARGET
+                ttype, tname = sub.targetarray
+                @dependencies[ttype][tname] << sub
+
+                # Subscriptions are the list of subscriptions for a given object,
+                # i.e., the list of all objects that care about a given object's
+                # events.
+                # SUBSCRIPTION == SOURCE
+                stype, sname = sub.sourcearray
+                @subscriptions[stype][sname] << sub
+            end
+
+            # Trigger the subscriptions related to an event, and then pass it up
+            # as appropriate
+            def self.trigger(source, event, transaction)
+                type, name = self.split(source)
+
+                @subscriptions[type][name].each { |sub|
+                    if sub.match?(event)
+                        sub.trigger(transaction)
+                    end
+                }
+            end
+
+            # Look up an object by type and name.  This is used because we
+            # store symbolic links in our subscription hash rather than storing
+            # actual object references.
+            def self.retrieve(ary)
+                type, name = ary
+                typeobj = Puppet::Type.type(type)
+
+                unless typeobj
+                    return nil
+                end
+
+                obj = typeobj[name]
+                return obj
+            end
+
+            # Split an object into its type and name
+            def self.split(object)
+                return [object.class.name, object.name]
+            end
+
+            # Retrieve all of the subscriptions that result in a dependency.
+            # We return the whole dependency here, because it is being returned
+            # to the object that made the subscription.
+            def self.dependencies(target)
+                type, name = self.split(target)
+                return @dependencies[type][name]
+            end
+
+            # Return all objects that are subscribed to us.  We are only willing
+            # to return the object, not the subscription object, because the
+            # source shouldn't need to know things like the event or method that
+            # we're subscribed to.
+            def self.subscribers(source)
+                type, name = self.split(source)
+                return @subscriptions[type][name].collect { |sub|
+                    sub.target
+                }
+            end
+
+            # The hash here must include the target and source objects, the event,
+            # and the callback to call.
+            def initialize(hash)
                 hash.each { |param,value|
                     # assign each value appropriately
                     # this is probably wicked-slow
                     self.send(param.to_s + "=",value)
                 }
+
+                self.class.newsub(self)
                 #Puppet.debug "New Subscription: '%s' => '%s'" %
                 #    [@source,@event]
             end
 
-            # the transaction is passed in so that we can notify it if
-            # something fails
+            # Determine whether the passed event matches our event
+            def match?(event)
+                if event == :NONE or @event == :NONE
+                    return false
+                elsif @event == :ALL_EVENTS or event == :ALL_EVENTS or event == @event
+                    return true
+                else
+                    return false
+                end
+            end
+
+            # The source is the event source.
+            def source=(object)
+                type, name = self.class.split(object)
+                @source = [type, name]
+            end
+
+            def source
+                self.class.retrieve(@source)
+            end
+
+            def sourcearray
+                @source
+            end
+
+            # The target is the object who will receive the callbacks, i.e.,
+            # a source generates an event, which results in a callback on the
+            # target.
+            def target=(object)
+                type, name = self.class.split(object)
+                @target = [type, name]
+            end
+
+            def target
+                self.class.retrieve(@target)
+            end
+
+            def targetarray
+                @target
+            end
+
+            # Trigger a subscription, which basically calls the associated method
+            # on the target object.
             def trigger(transaction)
-                # this is potentially incomplete, because refreshing an object
-                # could theoretically kick off an event, which would not get run
-                # or, because we're executing the first subscription rather than
-                # the last, a later-refreshed object could somehow be connected
-                # to the "old" object rather than "new"
-                # but we're pretty far from that being a problem
                 event = nil
 
                 if @event == :NONE
@@ -46,30 +191,35 @@ module Puppet
                     return
                 end
 
-                if transaction.triggercount(self) > 0
+                if transaction.triggered?(self.target, @callback) > 0
                     Puppet.debug "%s has already run" % self
                 else
-                    Puppet.debug "'%s' matched '%s'; triggering '%s' on '%s'" %
-                        [@source,@event,@method,@target]
+                    # We need to call the method, so that it gets retrieved
+                    # as a real object.
+                    target = self.target
+                    #Puppet.debug "'%s' matched '%s'; triggering '%s' on '%s'" %
+                    #    [@source,@event,@method,target]
                     begin
-                        if @target.respond_to?(@method)
-                            event = @target.send(@method)
+                        if target.respond_to?(@callback)
+                            event = target.send(@callback)
                         else
-                            Puppet.debug "'%s' of type '%s' does not respond to '%s'" %
-                                [@target,@target.class,@method.inspect]
+                            Puppet.debug(   
+                                "'%s' of type '%s' does not respond to '%s'" %
+                                [target,target.class,@callback.inspect]
+                            )
                         end
                     rescue => detail
-                        # um, what the heck do i do when an object fails to refresh?
-                        # shouldn't that result in the transaction rolling back?
-                        # the 'onerror' metaparam will be used to determine
-                        # behaviour in that case
+                        # um, what the heck do i do when an object fails to
+                        # refresh?  shouldn't that result in the transaction
+                        # rolling back?  the 'onerror' metaparam will be used
+                        # to determine behaviour in that case
                         Puppet.err "'%s' failed to %s: '%s'" %
-                            [@target,@method,detail]
+                            [target,@callback,detail]
                         raise
                         #raise "We need to roll '%s' transaction back" %
                             #transaction
                     end
-                    transaction.triggered(self)
+                    transaction.triggered(target, @callback)
                 end
                 return event
             end
@@ -81,36 +231,6 @@ module Puppet
 
         @@subscriptions = []
 
-        # I think this method is obsolete
-        def self.process
-            Puppet.debug "Processing events"
-            @@events.each { |event|
-                @@subscriptions.find_all { |sub|
-                    #debug "Sub source: '%s'; event object: '%s'" %
-                    #    [sub.source.inspect,event.object.inspect]
-                    sub.source == event.object and
-                        (sub.event == event.event or
-                         sub.event == :ALL_EVENTS)
-                }.each { |sub|
-                    Puppet.debug "Found subscription to %s" % event
-                    sub.trigger(event.transaction)
-                }
-            }
-
-            @@events.clear
-        end
-
-        # I think this method is obsolete
-        def self.subscribe(hash)
-            if hash[:event] == '*'
-                hash[:event] = :ALL_EVENTS
-            end
-            sub = Subscription.new(hash)
-
-            # add to the correct area
-            @@subscriptions.push sub
-        end
-
 		def initialize(args)
             unless args.include?(:event) and args.include?(:source)
 				raise Puppet::DevError, "Event.new called incorrectly"
@@ -136,4 +256,4 @@ module Puppet
 	end
 end
 
-
+# $Id$
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 59e106902..a39ec55d1 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -85,7 +85,8 @@ class Transaction
         #@triggerevents = []
         events.each { |event|
             object = event.source
-            object.propagate(event)
+            #Puppet::Event::Subscriptions.propagate(object, event, self)
+            object.propagate(event, self)
         }
 
         #events += @triggerevents
@@ -157,7 +158,7 @@ class Transaction
         #@triggerevents = []
         events.each { |event|
             object = event.source
-            object.propagate(event)
+            object.propagate(event, self)
         }
 
         #events += @triggerevents
@@ -166,6 +167,7 @@ class Transaction
 
     #---------------------------------------------------------------
     def triggered(object, method)
+        Puppet.notice "Triggered %s" % method
         @triggered[object][method] += 1
         #@triggerevents << ("%s_%sed" % [object.class.name.to_s, method.to_s]).intern
     end
@@ -173,6 +175,7 @@ class Transaction
 
     #---------------------------------------------------------------
     def triggered?(object, method)
+        Puppet.notice "Looking for triggered %s" % method
         @triggered[object][method]
     end
     #---------------------------------------------------------------
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 79a8fb572..421cdf6ca 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -1,41 +1,19 @@
-#!/usr/local/bin/ruby -w
-
-# $Id$
+# This class is the abstract base class for the mechanism for organizing
+# work.  No work is actually done by this class or its subclasses; rather,
+# the subclasses include states which do the actual work.
+#   See state.rb for how work is actually done.
 
-# included so we can test object types
 require 'puppet'
 require 'puppet/log'
 require 'puppet/element'
 require 'puppet/event'
 require 'puppet/metric'
 require 'puppet/type/state'
-
-
 # see the bottom of the file for the rest of the inclusions
 
-#---------------------------------------------------------------
-# This class is the abstract base class for the mechanism for organizing
-# work.  No work is actually done by this class or its subclasses; rather,
-# the subclasses include states which do the actual work.
-#   See state.rb for how work is actually done.
-
-# our duck type interface -- if your object doesn't match this interface,
-# it won't work
-
-# all of our first-class objects (objects, states, and components) will
-# respond to these methods
-# although states don't inherit from Puppet::Type
-#   although maybe Puppet::State should...
-
-# the default behaviour that this class provides is to just call a given
-# method on each contained object, e.g., in calling 'sync', we just run:
-# object.each { |subobj| subobj.sync() }
-
-# to use this interface, just define an 'each' method and 'include Puppet::Type'
-
 module Puppet
 class Type < Puppet::Element
-    attr_accessor :children, :parameters, :parent
+    attr_accessor :children, :parameters, :parent, :implicit
     attr_accessor :file, :line
     include Enumerable
 
@@ -102,15 +80,10 @@ class Type < Puppet::Element
     @@metaparamdoc[:loglevel] = "Sets the level that information will be logged:
          debug, info, verbose, notice, warning, err, alert, emerg or crit"
 
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
     # class methods dealing with Type management
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
 
     public
 
-    #---------------------------------------------------------------
     # these objects are used for mapping type names (e.g., 'file')
     # to actual object classes; because Type.inherited is
     # called before the <subclass>.name method is defined, we need
@@ -133,7 +106,6 @@ class Type < Puppet::Element
         attr_reader :name, :namevar, :states, :validstates, :parameters
     end
 
-    #---------------------------------------------------------------
     # Create @@typehash from @@typeary.  This is meant to be run
     # multiple times -- whenever it is discovered that the two
     # objects have differents lengths.
@@ -149,16 +121,12 @@ class Type < Puppet::Element
             end
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # iterate across all of the subclasses of Type
     def self.eachtype
         @@typeary.each { |type| yield type }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # The work that gets done for every subclass of Type
     # this is an implicit method called by Ruby for us
     def self.inherited(sub)
@@ -171,9 +139,7 @@ class Type < Puppet::Element
         # get executed, which, um, sucks
         @@typeary.push(sub)
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # all of the variables that must be initialized for each subclass
     def self.initvars
         # all of the instances of this class
@@ -201,9 +167,7 @@ class Type < Puppet::Element
         end
 
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # return a Type instance by name
     def self.type(type)
         unless @@typeary.length == @@typehash.length
@@ -213,18 +177,11 @@ class Type < Puppet::Element
         end
         @@typehash[type]
     end
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
     # class methods dealing with type instance management
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
 
     public
 
-    #---------------------------------------------------------------
     # retrieve a named instance of the current type
     def self.[](name)
         if @objects.has_key?(name)
@@ -233,9 +190,7 @@ class Type < Puppet::Element
             return nil
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # add an instance by name to the class list of instances
     def self.[]=(name,object)
         newobj = nil
@@ -260,28 +215,23 @@ class Type < Puppet::Element
         # and then add it to the master list
         Puppet::Type.push(object)
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # remove all type instances; this is mostly only useful for testing
     def self.allclear
         @@allobjects.clear
+        Puppet::Event::Subscription.clear
         @@typeary.each { |subtype|
             subtype.clear
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # remove all of the instances of a single type
     def self.clear
         if defined? @objects
             @objects.clear
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # remove a specified object
     def self.delete(object)
         if @@allobjects.include?(object)
@@ -292,9 +242,7 @@ class Type < Puppet::Element
             @objects.delete(object.name)
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # iterate across each of the type's instances
     def self.each
         return unless defined? @objects
@@ -302,14 +250,11 @@ class Type < Puppet::Element
             yield instance
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # does the type have an object with the given name?
     def self.has_key?(name)
         return @objects.has_key?(name)
     end
-    #---------------------------------------------------------------
 
     # Allow an outside party to specify the 'is' value for a state.  The
     # arguments are an array because you can't use parens with 'is=' calls.
@@ -329,7 +274,6 @@ class Type < Puppet::Element
         end
     end
 
-    #---------------------------------------------------------------
     # add an object to the master list of Type instances
     # I'm pretty sure this is currently basically unused
     def self.push(object)
@@ -337,18 +281,11 @@ class Type < Puppet::Element
         #debug("adding %s of type %s to master list" %
         #    [object.name,object.class])
     end
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
     # class and instance methods dealing with parameters and states
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
 
     public
 
-    #---------------------------------------------------------------
     # build a per-Type hash, mapping the states to their names
     def self.buildstatehash
         unless defined? @validstates
@@ -368,9 +305,7 @@ class Type < Puppet::Element
             end
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # set the parameters for a type; probably only used by FileRecord
     # objects
     def self.parameters=(params)
@@ -383,9 +318,7 @@ class Type < Puppet::Element
             end
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # does the name reflect a valid state?
     def self.validstate?(name)
         unless @validstates.length == @states.length
@@ -397,9 +330,7 @@ class Type < Puppet::Element
             return false
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # does the name reflect a valid parameter?
     def self.validparameter?(name)
         unless defined? @parameters
@@ -411,9 +342,7 @@ class Type < Puppet::Element
             return false
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     def self.validarg?(name)
         if name.is_a?(String)
             name = name.intern
@@ -424,9 +353,7 @@ class Type < Puppet::Element
             return false
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # abstract accessing parameters and states, and normalize
     # access to always be symbols, not strings
     # XXX this returns a _value_, not an object
@@ -455,9 +382,7 @@ class Type < Puppet::Element
             raise TypeError.new("Invalid parameter %s" % [name])
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # abstract setting parameters and states, and normalize
     # access to always be symbols, not strings
     def []=(name,value)
@@ -499,9 +424,7 @@ class Type < Puppet::Element
             raise Puppet::Error, "Invalid parameter %s" % [name]
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # remove a state from the object; useful in testing or in cleanup
     # when an error has been encountered
     def delete(attr)
@@ -511,25 +434,21 @@ class Type < Puppet::Element
             raise Puppet::DevError.new("Undefined state '#{attr}' in #{self}")
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # remove a state from the object; useful in testing or in cleanup
     # when an error has been encountered
     def destroy
         self.class.delete(self)
 
-        @dependencies.each { |dep|
-            dep.unsubscribe(self)
+        Puppet::Event::Subscription.dependencies(self).each { |dep|
+            self.unsubscribe(dep)
         }
 
         if defined? @parent and @parent
             @parent.delete(self)
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # iterate across all children, and then iterate across states
     # we do children first so we're sure that all dependent objects
     # are checked first
@@ -553,9 +472,7 @@ class Type < Puppet::Element
             }
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # iterate across the existing states
     def eachstate
         # states() is a private method
@@ -563,9 +480,7 @@ class Type < Puppet::Element
             yield state
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # retrieve the 'is' value for a specified state
     def is(state)
         if @states.include?(state)
@@ -574,9 +489,7 @@ class Type < Puppet::Element
             return nil
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # retrieve the 'should' value for a specified state
     def should(state)
         if @states.include?(state)
@@ -585,16 +498,12 @@ class Type < Puppet::Element
             return nil
         end
     end
-    #---------------------------------------------------------------
     
-    #---------------------------------------------------------------
     # create a log at specified level
     def log(msg)
         Puppet::Log.create(@metaparams[:loglevel],msg)
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # is the instance a managed instance?  A 'yes' here means that
     # the instance was created from the language, vs. being created
     # in order resolve other questions, such as finding a package
@@ -612,9 +521,7 @@ class Type < Puppet::Element
             return @managed
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # create a new state
     def newstate(name, hash = {})
         if stateklass = self.class.validstate?(name) 
@@ -651,9 +558,7 @@ class Type < Puppet::Element
             raise Puppet::Error, "Invalid parameter %s" % name
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # return the value of a parameter
     def parameter(name)
         unless name.is_a? Symbol
@@ -661,9 +566,7 @@ class Type < Puppet::Element
         end
         return @parameters[name]
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     def push(*childs)
         unless defined? @children
             @children = []
@@ -673,9 +576,7 @@ class Type < Puppet::Element
             child.parent = self
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # return an actual type by name; to return the value, use 'inst[name]'
     # FIXME this method should go away
     def state(name)
@@ -684,11 +585,9 @@ class Type < Puppet::Element
         end
         return @states[name]
     end
-    #---------------------------------------------------------------
 
     private
 
-    #---------------------------------------------------------------
     def states
         #debug "%s has %s states" % [self,@states.length]
         tmpstates = []
@@ -702,27 +601,29 @@ class Type < Puppet::Element
         end
         return tmpstates
     end
-    #---------------------------------------------------------------
 
 
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
     # instance methods related to instance intrinsics
     # e.g., initialize() and name()
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
 
     public
 
-    #---------------------------------------------------------------
     # Force users to call this, so that we can merge objects if
     # necessary.  FIXME This method should be responsible for most of the
     # error handling.
     def self.create(hash)
+        # Handle this new object being implicit
+        implicit = hash[:implicit] || false
+        if hash.include?(:implicit)
+            hash.delete(:implicit)
+        end
+
         if name =   hash["name"] || hash[:name] ||
                     hash[self.namevar] || hash[self.namevar.to_s]
             # if the object already exists
             if retobj = self[name]
+                # if only one of our objects is implicit, then it's easy to see
+                # who wins -- the non-implicit one.
                 # merge the new data
                 retobj.merge(hash)
 
@@ -753,22 +654,27 @@ class Type < Puppet::Element
                 self.to_s
         end
     end
-    #---------------------------------------------------------------
+
+    def self.implicitcreate(hash)
+        unless hash.include?(:implicit)
+            hash[:implicit] = true
+        end
+        obj = self.create(hash)
+        obj.implicit = true
+
+        return obj
+    end
 
     # and then make 'new' private
     class << self
         private :new
     end
 
-    #---------------------------------------------------------------
     # initialize the type instance
     def initialize(hash)
         @children = []
         @evalcount = 0
 
-        @subscriptions = []
-        @dependencies = []
-
         # callbacks are per object and event
         @callbacks = Hash.new { |chash, key|
             chash[key] = {}
@@ -855,9 +761,7 @@ class Type < Puppet::Element
         #puts caller
         self.class[self.name] = self
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # merge new information with an existing object, checking for conflicts
     # and such
     def merge(hash)
@@ -877,9 +781,7 @@ class Type < Puppet::Element
             end
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # derive the instance name based on class.namevar
     def name
         unless defined? @name and @name
@@ -901,9 +803,7 @@ class Type < Puppet::Element
 
         return @name
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # fix any namevar => param translations
     def argclean(hash)
         # we have to set the name of our object before anything else,
@@ -935,9 +835,7 @@ class Type < Puppet::Element
 
         return hash
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # return the full path to us, for logging and rollback
     # some classes (e.g., FileTypeRecords) will have to override this
     def path
@@ -947,9 +845,7 @@ class Type < Puppet::Element
             return [self.name]
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # retrieve the current value of all contained states
     def retrieve
         # it's important to use the method here, as it follows the order
@@ -958,9 +854,7 @@ class Type < Puppet::Element
             state.retrieve
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # sync the changes to disk, and return the events generated by the changes
     # FIXME this method is essentially obviated, but it's still used by tests
     # and i don't feel like fixing it yet
@@ -977,24 +871,16 @@ class Type < Puppet::Element
         Puppet::Metric.addevents(self.class,self,events)
         return events
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # convert to a string
     def to_s
         self.name
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
     # instance methods dealing with actually doing work
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
 
     public
 
-    #---------------------------------------------------------------
     # this is a retarded hack method to get around the difference between
     # component children and file children
     def self.depthfirst?
@@ -1004,9 +890,7 @@ class Type < Puppet::Element
             return false
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # this method is responsible for collecting state changes
     # we always descend into the children before we evaluate our current
     # states
@@ -1094,9 +978,7 @@ class Type < Puppet::Element
         end
         return changes.flatten
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # if all contained objects are in sync, then we're in sync
     # FIXME I don't think this is used on the type instances any more
     def insync?
@@ -1112,24 +994,16 @@ class Type < Puppet::Element
         #Puppet.debug("%s sync status is %s" % [self,insync])
         return insync
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
     # Meta-parameter methods:  These methods deal with the results
     # of specifying metaparameters
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     def self.eachmetaparam
         @@metaparams.each { |param|
             yield param
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # This just marks states that we definitely want to retrieve values
     # on.  There is currently no way to uncheck a parameter.
     def metacheck=(args)
@@ -1149,34 +1023,26 @@ class Type < Puppet::Element
             self.newstate(state)
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # Is the parameter in question a meta-parameter?
     def self.metaparam?(param)
         @@metaparams.include?(param)
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # for each object we require, subscribe to all events that it
     # generates
     # we might reduce the level of subscription eventually, but for now...
     def metarequire=(requires)
         self.handledepends(requires, :NONE, nil)
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # for each object we require, subscribe to all events that it
     # generates
     # we might reduce the level of subscription eventually, but for now...
     def metasubscribe=(requires)
         self.handledepends(requires, :ALL_EVENTS, :refresh)
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     def metanoop=(noop)
         if noop == "true" or noop == true
             @noop = true
@@ -1186,20 +1052,15 @@ class Type < Puppet::Element
             raise Puppet::Error.new("Invalid noop value '%s'" % noop)
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     def metaonerror=(response)
         Puppet.debug("Would have called metaonerror")
         @onerror = response
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     def metaschedule=(schedule)
         @schedule = schedule
     end
-    #---------------------------------------------------------------
     def metaloglevel=(loglevel)
         if loglevel.is_a?(String)
             loglevel.intern
@@ -1214,42 +1075,31 @@ class Type < Puppet::Element
             raise Puppet::Error.new("Invalid loglevel '%s%'" % loglevel)       
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
     # Subscription and relationship methods
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
-    def addcallback(object, event, method)
-        @callbacks[object][event] = method
-    end
-    #---------------------------------------------------------------
+    #def addcallback(object, event, method)
+    #    @callbacks[object][event] = method
+    #end
 
-    #---------------------------------------------------------------
     # return all objects subscribed to the current object
     def eachdependency
-        @dependencies.each { |dep|
-            yield dep
+        Puppet::Event::Subscription.dependencies(self).each { |dep|
+            yield dep.source
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # return all objects subscribed to the current object
-    def eachsubscriber
-        @subscriptions.each { |sub|
-            yield sub.target
-        }
-    end
-    #---------------------------------------------------------------
+    #def eachsubscriber
+    #    Puppet::Event::Subscriptions.subscribers?(self).each { |sub|
+    #        yield sub.targetobject
+    #    }
+    #end
 
-    #---------------------------------------------------------------
     def handledepends(requires, event, method)
-        # FIXME this should probably test whether requires[0] is an array
-        unless requires.is_a?(Array)
+        # 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.each { |rname|
@@ -1268,77 +1118,72 @@ class Type < Puppet::Element
             end
             Puppet.debug("%s subscribes to %s" % [self.name,object])
 
-            unless @dependencies.include?(object)
-                @dependencies << object
-            end
+            #unless @dependencies.include?(object)
+            #    @dependencies << object
+            #end
 
             # pure requires don't call methods
-            next if method.nil?
+            #next if method.nil?
 
             # 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 
-            sub = object.subscribe(
+            subargs = {
                 :event => event,
+                :source => object,
                 :target => self
-            )
-            if self.respond_to?(method)
-                self.addcallback(object, event, method)
+            }
+            if method and self.respond_to?(method)
+                subargs[:callback] = method
             end
+            Puppet::Event::Subscription.new(subargs)
+            #if self.respond_to?(method)
+            #    self.addcallback(object, event, method)
+            #end
             #object.addnotify(self)
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
-    def propagate(event)
-        self.subscribers?(event).each { |object|
-            object.trigger(event, self)
-        }
+    # Trigger any associated subscriptions, and then pass the event up to our
+    # parent.
+    def propagate(event, transaction)
+        Puppet::Event::Subscription.trigger(self, event, transaction)
 
         if defined? @parent
-            @parent.propagate(event)
+            @parent.propagate(event, transaction)
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     def subscribe(hash)
         hash[:source] = self
-        sub = Puppet::Event::Subscription.new(hash)
+        Puppet::Event::Subscription.new(hash)
 
         # add to the correct area
-        @subscriptions.push sub
+        #@subscriptions.push sub
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
+    # Unsubscribe from a given object, possibly with a specific event.
     def unsubscribe(object, event = nil)
-        @subscriptions.find_all { |sub|
+        Puppet::Event::Subscription.dependencies(self).find_all { |sub|
             if event
-                sub.target == object and sub.event = event
+                sub.match?(event)
             else
-                sub.target == object
+                sub.source == object
             end
         }.each { |sub|
-            @subscriptions.delete(sub)
+            Puppet::Event::Subscription.delete(sub)
         }
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
     # return all of the subscriptions to a given event
-    def subscribers?(event)
-        @subscriptions.find_all { |sub|
-            sub.event == event.event or
-                sub.event == :ALL_EVENTS
-        }.collect { |sub|
-            sub.target
-        }
-    end
-    #---------------------------------------------------------------
+    #def subscribers?(event)
+    #    Puppet::Event::Subscription.subscriptions(self).find_all { |sub|
+    #        sub.match?(event)
+    #    }.collect { |sub|
+    #        sub.target
+    #    }
+    #end
 
-    #---------------------------------------------------------------
     # we've received an event
     # we only support local events right now, so we can pass actual
     # objects around, including the transaction object
@@ -1363,23 +1208,14 @@ class Type < Puppet::Element
             }
         end
     end
-    #---------------------------------------------------------------
 
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
     # Documentation methods
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
     def self.paramdoc(param)
         @paramdoc[param]
     end
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
     def self.metaparamdoc(metaparam)
         @@metaparamdoc[metaparam]
     end
-    #---------------------------------------------------------------
-    #---------------------------------------------------------------
 end # Puppet::Type
 end
 
@@ -1398,3 +1234,5 @@ require 'puppet/type/tidy'
 #require 'puppet/type/typegen'
 #require 'puppet/type/typegen/filetype'
 #require 'puppet/type/typegen/filerecord'
+
+# $Id$
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 980c83fdd..82581998a 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -345,22 +345,22 @@ module Puppet
 
             # If we're not root, we can check the values but we cannot change them
             def should=(value)
-                unless Process.uid == 0
-                    @should = nil
-                    @is = nil
-                    unless defined? @@notifieduid
-                        Puppet.notice "Cannot manage ownership unless running as root"
-                        #@parent.delete(self.name)
-                        @@notifieduid = true
-                    end
-                    return
+                #unless Process.uid == 0
+                #    @should = nil
+                #    @is = nil
+                #    unless defined? @@notifieduid
+                #        Puppet.notice "Cannot manage ownership unless running as root"
+                #        #@parent.delete(self.name)
+                #        @@notifieduid = true
+                #    end
+                #    return
                     #if @parent.state(:owner)
                     #    @parent.delete(:owner)
                     #end
                     #raise Puppet::Error.new(
                     #    "Cannot manage ownership unless running as root"
                     #)
-                end
+                #end
                 if value.is_a?(Integer)
                     # verify the user is a valid user
                     begin
@@ -561,16 +561,16 @@ module Puppet
                 gid = nil
                 gname = nil
 
-                unless Process.uid == 0
-                    unless defined? @@notifiedgroup
-                        Puppet.notice(  
-                            "Cannot manage group unless running as root"
-                        )
-                        @@notifiedgroup = true
-                    end
-                    return
-                end
-
+                #unless Process.uid == 0
+                #    unless defined? @@notifiedgroup
+                #        Puppet.notice(  
+                #            "Cannot manage group unless running as root"
+                #        )
+                #        @@notifiedgroup = true
+                #    end
+                #    return
+                #end
+#
                 if value.is_a?(Integer)
                     method = :getgrgid
                 else
@@ -624,16 +624,19 @@ module Puppet
                 end
             end
 
+            # Normal users will only be able to manage certain groups.  Right now,
+            # we'll just let it fail, but we should probably set things up so
+            # that users get warned if they try to change to an unacceptable group.
             def sync
-                unless Process.uid == 0
-                    unless defined? @@notifiedgroup
-                        Puppet.notice(  
-                            "Cannot manage group ownership unless running as root"
-                        )
-                        @@notifiedgroup = true
-                    end
-                    return nil
-                end
+                #unless Process.uid == 0
+                #    unless defined? @@notifiedgroup
+                #        Puppet.notice(  
+                #            "Cannot manage group ownership unless running as root"
+                #        )
+                #        @@notifiedgroup = true
+                #    end
+                #    return nil
+                #end
 
                 Puppet.debug "setting chgrp state to %s" % self.should
                 if @is == :notfound
@@ -660,6 +663,7 @@ module Puppet
             end
         end
 
+        # Copy files from a local or remote source.
         class PFileSource < Puppet::State
             attr_accessor :source, :local
             @doc = "Copy a file over the current file.  Uses `checksum` to
@@ -668,6 +672,7 @@ module Puppet
                 types are *puppet* and *file*."
             @name = :source
 
+            # Ask the file server to describe our file.
             def describe
                 source = @source
 
@@ -700,15 +705,22 @@ module Puppet
                 return args
             end
 
+            # This basically calls describe() on our file, and then sets all
+            # of the local states appropriately.  If the remote file is a normal
+            # file then we set it to copy; if it's a directory, then we just mark
+            # that the local directory should be created.
             def retrieve
                 sum = nil
                 
+                # Describe the remote file.
                 @stats = self.describe
                 if @stats.nil? or @stats[:type].nil?
                     @is = :notdescribed
                     return nil
                 end
 
+                # Take each of the stats and set them as states on the local file
+                # if a value has not already been provided.
                 @stats.each { |stat, value|
                     next if stat == :checksum
                     next if stat == :type
@@ -723,6 +735,8 @@ module Puppet
                         end
                     end
                 }
+
+                # If we're a normal file, then set things up to copy the file down.
                 case @stats[:type]
                 when "file":
                     if sum = @parent.state(:checksum)
@@ -749,6 +763,8 @@ module Puppet
                             @parent.delete(:create)
                         end
                     end
+                # If we're a directory, then do not copy anything, and instead just
+                # create the directory using the 'create' state.
                 when "directory":
                     if state = @parent.state(:create)
                         unless state.should == "directory"
@@ -761,6 +777,7 @@ module Puppet
                     # we'll let the :create state do our work
                     @should = true
                     @is = true
+                # FIXME We should at least support symlinks, I would think...
                 else
                     Puppet.err "Cannot use files of type %s as sources" %
                         @stats[:type]
@@ -769,15 +786,26 @@ module Puppet
                 end
             end
 
+            # The special thing here is that we need to make sure that 'should'
+            # is only set for files, not directories.
             def should=(source)
-                @source = source
+                if ! defined? @stats or @stats.nil?
+                    @source = source
 
-                # stupid hack for now; it'll get overriden
-                @should = source
+                    # stupid hack for now; it'll get overriden
+                    @should = source
+                else
+                    if @stats[:type] == "directory"
+                        @should = true
+                        @is = true
+                    else
+                        @source = source
+                        @should = source
+                    end
+                end
             end
 
             def sync
-                Puppet.notice "syncing %s" % @parent.name
                 if @is == :notdescribed
                     self.retrieve # try again
                     if @is == :notdescribed
@@ -790,6 +818,9 @@ module Puppet
                 end
 
                 unless @stats[:type] == "file"
+                    if @stats[:type] == "directory"
+                            [@parent.name, @is.inspect, @should.inspect]
+                    end
                     raise Puppet::DevError, "Got told to copy non-file %s" %
                         @parent.name
                 end
@@ -804,8 +835,6 @@ module Puppet
                     return nil
                 end
 
-                Puppet.notice "retrieved %s" % path
-
                 unless sourceobj.server.local
                     contents = CGI.unescape(contents)
                 end
@@ -1147,14 +1176,13 @@ module Puppet
                         next if var == :name
                         # behave idempotently
                         unless child.should(var) == value
-                            #Puppet.warning "%s is %s, not %s" % [var, child[var], value]
                             child[var] = value
                         end
                     }
                 else # create it anew
                     #notice "Creating new file with args %s" % args.inspect
                     begin
-                        child = klass.create(args)
+                        child = klass.implicitcreate(args)
                         child.parent = self
                         @children << child
                     rescue Puppet::Error => detail
@@ -1301,6 +1329,10 @@ module Puppet
                 unless stat = self.stat(true)
                     Puppet.debug "File %s does not exist" % self.name
                     @states.each { |name,state|
+                        # We've already retreived 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 = :notfound
                     }
                     return
diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb
index 3e87a136a..4308291fa 100644
--- a/lib/puppet/type/service.rb
+++ b/lib/puppet/type/service.rb
@@ -4,10 +4,45 @@
 # can only be managed through the interface of an init script
 # which is why they have a search path for initscripts and such
 
-require 'puppet/type/service/init'
-
 module Puppet
     class State
+        class ServiceEnabled < State
+            @doc = "Whether a service should be enabled to start at boot.
+                **true**/*false*/*runlevel*"
+            @name = :enabled
+
+            def retrieve
+                @is = @parent.enabled?
+            end
+
+            def should=(should)
+                case should
+                when true: @should = :enabled
+                when false: @should = :disabled
+                else
+                    raise Puppet::Error, "Invalid 'enabled' value %s" % should
+                end
+            end
+
+            def sync
+                case @should
+                when :enabled
+                    unless @parent.respond_to?(:enable)
+                        raise Puppet::Error, "Service %s does not support enabling"
+                    end
+                    @parent.enable
+                    return :service_enabled
+                when :disabled
+                    unless @parent.respond_to?(:disable)
+                        raise Puppet::Error,
+                            "Service %s does not support disabling"
+                    end
+                    @parent.disable
+                    return :service_disabled
+                end
+            end
+        end
+
         class ServiceRunning < State
             @doc = "Whether a service should be running.  **true**/*false*"
             @name = :running
@@ -16,9 +51,9 @@ module Puppet
             # i should probably just be using booleans, but for now, i'm not...
             def should=(should)
                 case should
-                when false,0,"0":
+                when false,0,"0", "stopped", :stopped:
                     should = :stopped
-                when true,1,"1":
+                when true,1,"1", :running, "running":
                     should = :running
                 else
                     Puppet.warning "%s: interpreting '%s' as false" %
@@ -106,12 +141,8 @@ module Puppet
 
             # Return the service type we're using.  Default to the Service
             # class itself, but could be set to a module.
-            def self.svctype
-                if defined? @svctype
-                    return @svctype
-                else
-                    return self
-                end
+            class << self
+                attr_accessor :svctype
             end
 
             # Execute a command.  Basically just makes sure it exits with a 0
@@ -176,7 +207,9 @@ module Puppet
             # happen if, for instance, it has an init script (and thus responds to
             # 'statuscmd') but does not have 'hasstatus' enabled.
             def status
-                if self[:status] or (self.respond_to?(:statuscmd) and self.statuscmd)
+                if self[:status] or (
+                    self.respond_to?(:statuscmd) and self.statuscmd
+                )
                     cmd = self[:status] || self.statuscmd
                     output = %x(#{cmd} 2>&1)
                     Puppet.debug "%s status returned %s" %
@@ -226,15 +259,20 @@ module Puppet
             end
 
             # Now load any overlay modules to provide additional functionality
-            case Facter["operatingsystem"].value
+            os = Facter["operatingsystem"].value
+            case os
             when "Linux":
                 case Facter["distro"].value
                 when "Debian":
                     require 'puppet/type/service/init'
                     @svctype = Puppet::ServiceTypes::InitSvc
+
+                    # and then require stupid debian-specific stuff
+                    require 'puppet/type/service/debian'
+                    include Puppet::ServiceTypes::DebianSvc
                 end
             when "SunOS":
-                release = Integer(Facter["operatingsystemrelease"].value)
+                release = Float(Facter["operatingsystemrelease"].value)
                 if release < 5.10
                     require 'puppet/type/service/init'
                     @svctype = Puppet::ServiceTypes::InitSvc
diff --git a/lib/puppet/type/service/smf.rb b/lib/puppet/type/service/smf.rb
new file mode 100755
index 000000000..4202708c8
--- /dev/null
+++ b/lib/puppet/type/service/smf.rb
@@ -0,0 +1,105 @@
+module Puppet
+    module ServiceTypes
+        module SMFSvc
+            # Mark that our init script supports 'status' commands.
+            def hasstatus=(value)
+                case value
+                when true, "true": @parameters[:hasstatus] = true
+                when false, "false": @parameters[:hasstatus] = false
+                else
+                    raise Puppet::Error, "Invalid 'hasstatus' value %s" %
+                        value.inspect
+                end
+            end
+
+            # it'd be nice if i didn't throw the output away...
+            # this command returns true if the exit code is 0, and returns
+            # false otherwise
+            def initcmd(cmd)
+                script = self.initscript
+
+                Puppet.debug "Executing '%s %s' as initcmd for '%s'" %
+                    [script,cmd,self]
+
+                rvalue = Kernel.system("%s %s" %
+                        [script,cmd])
+
+                Puppet.debug "'%s' ran with exit status '%s'" %
+                    [cmd,rvalue]
+
+
+                rvalue
+            end
+
+            # Where is our init script?
+            def initscript
+                if defined? @initscript
+                    return @initscript
+                else
+                    @initscript = self.search(self.name)
+                end
+            end
+
+            # Store the search path for init scripts.  This will generally not
+            # be called.
+            def parampath=(ary)
+                unless ary.is_a?(Array)
+                    ary = [ary]
+                end
+                @parameters[:path] = ary
+                @searchpaths = ary.find_all { |dir|
+                    File.directory?(dir)
+                }
+            end
+
+            # Enable a service, to it's started at boot time.  This basically
+            # just creates links in the RC directories, which means that, well,
+            # we need to know where the rc directories are.
+            # FIXME This should probably be a state or something, and
+            # it should actually create use Symlink objects...
+            #def enable
+            #end
+
+            #def disable
+            #end
+
+            def search(name)
+                @searchpaths.each { |path|
+                    fqname = File.join(path,name)
+                    begin
+                        stat = File.stat(fqname)
+                    rescue
+                        # should probably rescue specific errors...
+                        Puppet.debug("Could not find %s in %s" % [name,path])
+                        next
+                    end
+
+                    # if we've gotten this far, we found a valid script
+                    return fqname
+                }
+                raise Puppet::Error, "Could not find init script for '%s'" % name
+            end
+
+            # The start command is just the init scriptwith 'start'.
+            def startcmd
+                self.initscript + " start"
+            end
+
+            # If it was specified that the init script has a 'status' command, then
+            # we just return that; otherwise, we return false, which causes it to
+            # fallback to other mechanisms.
+            def statuscmd
+                if self[:hasstatus]
+                    return self.initscript + " status"
+                else
+                    return false
+                end
+            end
+
+            # The stop command is just the init script with 'stop'.
+            def stopcmd
+                self.initscript + " stop"
+            end
+        end
+    end
+end
diff --git a/lib/puppet/type/symlink.rb b/lib/puppet/type/symlink.rb
index 050904297..79780e869 100755
--- a/lib/puppet/type/symlink.rb
+++ b/lib/puppet/type/symlink.rb
@@ -167,7 +167,7 @@ module Puppet
                     :source => @target
                 }
 
-                dir = Puppet::Type::PFile.create(args)
+                dir = Puppet::Type::PFile.implicitcreate(args)
                 dir.parent = self
                 Puppet.debug "Got dir %s" % dir.name
                 self.push dir
diff --git a/test/other/tc_events.rb b/test/other/tc_events.rb
index 5f3ddcd67..c555e17f0 100755
--- a/test/other/tc_events.rb
+++ b/test/other/tc_events.rb
@@ -11,9 +11,9 @@ require 'test/unit'
 # $Id$
 
 class TestEvents < TestPuppet
-    def setup
-        Puppet[:loglevel] = :debug if __FILE__ == $0
+    def teardown
         super
+        Puppet::Event::Subscription.clear
     end
 
     def test_simplesubscribe
@@ -30,17 +30,9 @@ class TestEvents < TestPuppet
 
         @@tmpfiles << "/tmp/eventtestingA"
 
-        comp = Puppet::Type::Component.create(
-            :name => "eventtesting"
-        )
-        comp.push exec
-        trans = comp.evaluate
-        events = nil
-        assert_nothing_raised {
-            events = trans.evaluate
-        }
+        comp = newcomp("eventtesting", file, exec)
 
-        assert_equal(1, events.length)
+        trans = assert_events(comp, [:file_created], "events")
 
         assert_equal(1, trans.triggered?(exec, :refresh))
     end
@@ -74,7 +66,7 @@ class TestEvents < TestPuppet
         assert_equal(0, trans.triggered?(exec, :refresh))
     end
 
-    def test_ladderrequire
+    def test_zladderrequire
         comps = {}
         objects = {}
         fname = "/tmp/eventtestfuntest"
diff --git a/test/other/tc_relationships.rb b/test/other/tc_relationships.rb
index c8927972b..d2544ffa4 100755
--- a/test/other/tc_relationships.rb
+++ b/test/other/tc_relationships.rb
@@ -9,57 +9,103 @@ require 'puppettest'
 require 'test/unit'
 
 class TestRelationships < TestPuppet
-    def setup
-        super
-        @groups = %x{groups}.chomp.split(/ /)
-        unless @groups.length > 1
-            p @groups
-            raise "You must be a member of more than one group to test this"
-        end
-    end
-
     def newfile
         assert_nothing_raised() {
-            cfile = File.join($puppetbase,"examples/root/etc/configfile")
-            unless Puppet::Type::PFile.has_key?(cfile)
-                Puppet::Type::PFile.create(
-                    :path => cfile,
-                    :check => [:mode, :owner, :group]
-                )
-            end
-            return Puppet::Type::PFile[cfile]
+            return Puppet::Type::PFile.create(
+                :path => tempfile,
+                :check => [:mode, :owner, :group]
+            )
         }
     end
 
-    def newservice
-        assert_nothing_raised() {
-            unless Puppet::Type::Service.has_key?("sleeper")
-                Puppet::Type::Service.create(
-                    :name => "sleeper",
-                    :path => File.join($puppetbase,"examples/root/etc/init.d"),
-                    :check => [:running]
-                )
-            end
-            return Puppet::Type::Service["sleeper"]
+    def test_simplerel
+        file1 = newfile()
+        file2 = newfile()
+        assert_nothing_raised {
+            file1[:require] = [file2.class.name, file2.name]
         }
-    end
 
-    def newcomp(name,*args)
-        comp = nil
-        assert_nothing_raised() {
-            comp = Puppet::Component.new(:name => name)
+        deps = []
+        assert_nothing_raised {
+            file1.eachdependency { |obj|
+                deps << obj
+            }
         }
 
-        args.each { |arg|
-            assert_nothing_raised() {
-                comp.push arg
+        assert_equal(1, deps.length, "Did not get dependency")
+
+        assert_nothing_raised {
+            file1.unsubscribe(file2)
+        }
+
+        deps = []
+        assert_nothing_raised {
+            file1.eachdependency { |obj|
+                deps << obj
             }
         }
 
-        return comp
+        assert_equal(0, deps.length, "Still have dependency")
     end
 
-    def test_simplerel
+    def test_newsub
+        file1 = newfile()
+        file2 = newfile()
+
+        sub = nil
+        assert_nothing_raised("Could not create subscription") {
+            sub = Puppet::Event::Subscription.new(
+                :source => file1,
+                :target => file2,
+                :event => :ALL_EVENTS,
+                :callback => :refresh
+            )
+        }
+
+        subs = nil
+
+        assert_nothing_raised {
+            subs = Puppet::Event::Subscription.subscribers(file1)
+        }
+        assert_equal(1, subs.length, "Got incorrect number of subs")
+        assert_equal(sub.target, subs[0], "Got incorrect sub")
+
+        deps = nil
+        assert_nothing_raised {
+            deps = Puppet::Event::Subscription.dependencies(file2)
+        }
+        assert_equal(1, deps.length, "Got incorrect number of deps")
+        assert_equal(sub, deps[0], "Got incorrect dep")
+    end
+
+    def test_eventmatch
+        file1 = newfile()
+        file2 = newfile()
+
+        sub = nil
+        assert_nothing_raised("Could not create subscription") {
+            sub = Puppet::Event::Subscription.new(
+                :source => file1,
+                :target => file2,
+                :event => :ALL_EVENTS,
+                :callback => :refresh
+            )
+        }
+
+        assert(sub.match?(:anything), "ALL_EVENTS did not match")
+        assert(! sub.match?(:NONE), "ALL_EVENTS matched :NONE")
+
+        sub.event = :file_created
+
+        assert(sub.match?(:file_created), "event did not match")
+        assert(sub.match?(:ALL_EVENTS), "ALL_EVENTS did not match")
+        assert(! sub.match?(:NONE), "ALL_EVENTS matched :NONE")
+
+        sub.event = :NONE
+
+        assert(! sub.match?(:file_created), "Invalid match")
+        assert(! sub.match?(:ALL_EVENTS), "ALL_EVENTS matched")
+        assert(! sub.match?(:NONE), "matched :NONE")
     end
 end
 
diff --git a/test/other/tc_transactions.rb b/test/other/tc_transactions.rb
index 946d53279..6b6213b81 100644
--- a/test/other/tc_transactions.rb
+++ b/test/other/tc_transactions.rb
@@ -74,6 +74,7 @@ class TestTransactions < FileTesting
             return Puppet::Type::Service.create(
                 :name => "sleeper",
                 :path => File.join($puppetbase,"examples/root/etc/init.d"),
+                :hasstatus => true,
                 :check => [:running]
             )
         }
diff --git a/test/server/tc_bucket.rb b/test/server/tc_bucket.rb
index 264a8da13..8a7e71511 100644
--- a/test/server/tc_bucket.rb
+++ b/test/server/tc_bucket.rb
@@ -22,6 +22,7 @@ class TestBucket < ServerTest
             spin
             name = File.basename(file)
             tmppath = File.join(tmpdir,name)
+            @@tmpfiles << tmppath
 
             # copy the files to our tmp directory so we can modify them...
             File.open(tmppath,File::WRONLY|File::TRUNC|File::CREAT) { |wf|
@@ -167,6 +168,7 @@ class TestBucket < ServerTest
         files = filelist()
 
         tmpdir = File.join(tmpdir(),"tmpfiledir")
+        @@tmpfiles << tmpdir
         FileUtils.mkdir_p(tmpdir)
 
         bucket = nil
diff --git a/test/types/tc_basic.rb b/test/types/tc_basic.rb
index 585caaa95..f0257fef0 100644
--- a/test/types/tc_basic.rb
+++ b/test/types/tc_basic.rb
@@ -1,7 +1,7 @@
 if __FILE__ == $0
     $:.unshift '..'
     $:.unshift '../../lib'
-    $puppetbase = "../../../../language/trunk"
+    $puppetbase = "../.."
 end
 
 require 'puppet'
@@ -40,6 +40,7 @@ class TestBasic < Test::Unit::TestCase
             @sleeper = Puppet::Type::Service.create(
                 :name => "sleeper",
                 :path => File.join($puppetbase,"examples/root/etc/init.d"),
+                :hasstatus => true,
                 :running => 1
             )
         }
diff --git a/test/types/tc_file.rb b/test/types/tc_file.rb
index 169b32995..8b19dc670 100644
--- a/test/types/tc_file.rb
+++ b/test/types/tc_file.rb
@@ -86,8 +86,66 @@ class TestFile < FileTesting
             end
         end
 
-        # we can really only test changing ownership if we're root
-        if Process.uid == 0
+        uid, name = users.shift
+        us = {}
+        us[uid] = name
+        users.each { |uid, name|
+            # just make sure we don't try to manage users
+            assert_nothing_raised() {
+                file.sync
+            }
+            assert_nothing_raised() {
+                file[:owner] = name
+            }
+            assert_nothing_raised() {
+                file.retrieve
+            }
+            assert_nothing_raised() {
+                file.sync
+            }
+        }
+    end
+
+    def test_zgroup
+        file = mktestfile()
+        [%x{groups}.chomp.split(/ /), Process.groups].flatten.each { |group|
+            assert_nothing_raised() {
+                file[:group] = group
+            }
+            assert(file.state(:group))
+            assert(file.state(:group).should)
+        }
+    end
+
+    if Process.uid == 0
+        def test_ownerasroot
+            file = mktestfile()
+
+            users = {}
+            count = 0
+
+            # collect five users
+            Etc.passwd { |passwd|
+                if count > 5
+                    break
+                else
+                    count += 1
+                end
+                users[passwd.uid] = passwd.name
+            }
+
+            fake = {}
+            # find a fake user
+            while true
+                a = rand(1000)
+                begin
+                    Etc.getpwuid(a)
+                rescue
+                    fake[a] = "fakeuser"
+                    break
+                end
+            end
+
             users.each { |uid, name|
                 assert_nothing_raised() {
                     file[:owner] = name
@@ -122,51 +180,33 @@ class TestFile < FileTesting
                     file[:owner] = uid
                 }
             }
-        else
-            uid, name = users.shift
-            us = {}
-            us[uid] = name
-            users.each { |uid, name|
-                # just make sure we don't try to manage users
+        end
+
+        def test_groupasroot
+            file = mktestfile()
+            [%x{groups}.chomp.split(/ /), Process.groups].flatten.each { |group|
                 assert_nothing_raised() {
-                    file.sync
+                    file[:group] = group
                 }
+                assert(file.state(:group))
+                assert(file.state(:group).should)
                 assert_nothing_raised() {
-                    file[:owner] = name
+                    file.evaluate
                 }
                 assert_nothing_raised() {
-                    file.retrieve
+                    file.sync
+                }
+                assert_nothing_raised() {
+                    file.evaluate
                 }
                 assert(file.insync?())
                 assert_nothing_raised() {
-                    file.sync
+                    file.delete(:group)
                 }
             }
         end
-    end
-
-    def test_group
-        file = mktestfile()
-        [%x{groups}.chomp.split(/ /), Process.groups].flatten.each { |group|
-            assert_nothing_raised() {
-                file[:group] = group
-            }
-            assert(file.state(:group))
-            assert(file.state(:group).should)
-            assert_nothing_raised() {
-                file.evaluate
-            }
-            assert_nothing_raised() {
-                file.sync
-            }
-            assert_nothing_raised() {
-                file.evaluate
-            }
-            assert(file.insync?())
-            assert_nothing_raised() {
-                file.delete(:group)
-            }
-        }
+    else
+        $stderr.puts "Run as root for complete owner and group testing"
     end
 
     def test_create
@@ -371,6 +411,7 @@ class TestFile < FileTesting
 
     def test_recursion
         path = "/tmp/filerecursetest"
+        @@tmpfiles.push path
         tmpfile = File.join(path,"testing")
         system("mkdir -p #{path}")
         cyclefile(path)
@@ -382,7 +423,6 @@ class TestFile < FileTesting
             of.puts "goodness"
         }
         cyclefile(path)
-        @@tmpfiles.push path
     end
 
 =begin
diff --git a/test/types/tc_query.rb b/test/types/tc_query.rb
index 5ba0e0c64..31e0d23a9 100644
--- a/test/types/tc_query.rb
+++ b/test/types/tc_query.rb
@@ -1,7 +1,7 @@
 if __FILE__ == $0
     $:.unshift '..'
     $:.unshift '../../lib'
-    $puppetbase = "../../../../language/trunk"
+    $puppetbase = "../.."
 end
 
 require 'puppet'
@@ -43,6 +43,7 @@ class TestQuery < Test::Unit::TestCase
                 Puppet::Type::Service.create(
                     :name => "sleeper",
                     :path => File.join($puppetbase,"examples/root/etc/init.d"),
+                    :hasstatus => true,
                     :check => [:running]
                 )
             end
diff --git a/test/types/tc_service.rb b/test/types/tc_service.rb
index c0df3419f..82827c734 100644
--- a/test/types/tc_service.rb
+++ b/test/types/tc_service.rb
@@ -74,14 +74,18 @@ class TestService < TestPuppet
         assert(sleeper.insync?)
     end
 
-    def test_processStartWithPattern
-        sleeper = mksleeper(:pattern => "bin/sleeper")
+    case Puppet::Type::Service.svctype
+    when Puppet::ServiceTypes::InitSvc
+        def test_processStartWithPattern
+            sleeper = mksleeper(:pattern => "bin/sleeper")
 
-        cyclesleeper(sleeper)
-    end
+            cyclesleeper(sleeper)
+        end
 
-    def test_processStartWithStatus
-        sleeper = mksleeper(:hasstatus => true)
-        cyclesleeper(sleeper)
+        def test_processStartWithStatus
+            sleeper = mksleeper(:hasstatus => true)
+            cyclesleeper(sleeper)
+        end
+    #when Puppet::ServiceTypes::SMFSvc
     end
 end