diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
index 84e1a0360..12f496a6e 100644
--- a/lib/puppet/property.rb
+++ b/lib/puppet/property.rb
@@ -1,324 +1,339 @@
 # The virtual base class for properties, which are the self-contained building
 # blocks for actually doing work on the system.
 
 require 'puppet'
 require 'puppet/parameter'
 
 class Puppet::Property < Puppet::Parameter
   require 'puppet/property/ensure'
 
   # Because 'should' uses an array, we have a special method for handling
   # it.  We also want to keep copies of the original values, so that
   # they can be retrieved and compared later when merging.
   attr_reader :shouldorig
 
   attr_writer :noop
 
   class << self
     attr_accessor :unmanaged
     attr_reader :name
 
     # Return array matching info, defaulting to just matching
     # the first value.
     def array_matching
       @array_matching ||= :first
     end
 
     # Set whether properties should match all values or just the first one.
     def array_matching=(value)
       value = value.intern if value.is_a?(String)
       raise ArgumentError, "Supported values for Property#array_matching are 'first' and 'all'" unless [:first, :all].include?(value)
       @array_matching = value
     end
   end
 
   # Look up a value's name, so we can find options and such.
   def self.value_name(name)
     if value = value_collection.match?(name)
       value.name
     end
   end
 
   # Retrieve an option set when a value was defined.
   def self.value_option(name, option)
     if value = value_collection.value(name)
       value.send(option)
     end
   end
 
   # Define a new valid value for a property.  You must provide the value itself,
   # usually as a symbol, or a regex to match the value.
   #
   # The first argument to the method is either the value itself or a regex.
   # The second argument is an option hash; valid options are:
   # * <tt>:method</tt>: The name of the method to define.  Defaults to 'set_<value>'.
   # * <tt>:required_features</tt>: A list of features this value requires.
   # * <tt>:event</tt>: The event that should be returned when this value is set.
   # * <tt>:call</tt>: When to call any associated block.  The default value
   #   is `instead`, which means to call the value instead of calling the
   #   provider.  You can also specify `before` or `after`, which will
   #   call both the block and the provider, according to the order you specify
   #   (the `first` refers to when the block is called, not the provider).
   def self.newvalue(name, options = {}, &block)
     value = value_collection.newvalue(name, options, &block)
 
     define_method(value.method, &value.block) if value.method and value.block
     value
   end
 
   # Call the provider method.
   def call_provider(value)
       provider.send(self.class.name.to_s + "=", value)
   rescue NoMethodError
       self.fail "The #{provider.class.name} provider can not handle attribute #{self.class.name}"
   end
 
   # Call the dynamically-created method associated with our value, if
   # there is one.
   def call_valuemethod(name, value)
     if method = self.class.value_option(name, :method) and self.respond_to?(method)
       begin
         event = self.send(method)
       rescue Puppet::Error
         raise
       rescue => detail
         puts detail.backtrace if Puppet[:trace]
         error = Puppet::Error.new("Could not set '#{value} on #{self.class.name}: #{detail}", @resource.line, @resource.file)
         error.set_backtrace detail.backtrace
         raise error
       end
     elsif block = self.class.value_option(name, :block)
       # FIXME It'd be better here to define a method, so that
       # the blocks could return values.
       self.instance_eval(&block)
     else
       devfail "Could not find method for value '#{name}'"
     end
   end
 
   # How should a property change be printed as a string?
   def change_to_s(current_value, newvalue)
     begin
       if current_value == :absent
         return "defined '#{name}' as '#{should_to_s(newvalue)}'"
       elsif newvalue == :absent or newvalue == [:absent]
         return "undefined '#{name}' from '#{is_to_s(current_value)}'"
       else
         return "#{name} changed '#{is_to_s(current_value)}' to '#{should_to_s(newvalue)}'"
       end
     rescue Puppet::Error, Puppet::DevError
       raise
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       raise Puppet::DevError, "Could not convert change '#{name}' to string: #{detail}"
     end
   end
 
   # Figure out which event to return.
   def event_name
     value = self.should
 
     event_name = self.class.value_option(value, :event) and return event_name
 
     name == :ensure or return (name.to_s + "_changed").to_sym
 
     return (resource.type.to_s + case value
     when :present; "_created"
     when :absent; "_removed"
     else
       "_changed"
     end).to_sym
   end
 
   # Return a modified form of the resource event.
   def event
     resource.event :name => event_name, :desired_value => should, :property => self, :source_description => path
   end
 
   attr_reader :shadow
 
   # initialize our property
   def initialize(hash = {})
     super
 
     if ! self.metaparam? and klass = Puppet::Type.metaparamclass(self.class.name)
       setup_shadow(klass)
     end
   end
 
   # Determine whether the property is in-sync or not.  If @should is
   # not defined or is set to a non-true value, then we do not have
   # a valid value for it and thus consider the property to be in-sync
   # since we cannot fix it.  Otherwise, we expect our should value
   # to be an array, and if @is matches any of those values, then
   # we consider it to be in-sync.
-  def insync?(is)
+  #
+  # Don't override this method.
+  def safe_insync?(is)
+    # If there is no @should value, consider the property to be in sync.
     return true unless @should
 
+    # Otherwise delegate to the (possibly derived) insync? method.
+    insync?(is)
+  end
+
+  def self.method_added(sym)
+    raise "Puppet::Property#safe_insync? shouldn't be overridden; please override insync? instead" if sym == :safe_insync?
+  end
+
+  # This method should be overridden by derived classes if necessary
+  # to provide extra logic to determine whether the property is in
+  # sync.
+  def insync?(is)
     self.devfail "#{self.class.name}'s should is not array" unless @should.is_a?(Array)
 
     # an empty array is analogous to no should values
     return true if @should.empty?
 
     # Look for a matching value
     return (is == @should or is == @should.collect { |v| v.to_s }) if match_all?
 
     @should.each { |val| return true if is == val or is == val.to_s }
 
     # otherwise, return false
     false
   end
 
   # because the @should and @is vars might be in weird formats,
   # we need to set up a mechanism for pretty printing of the values
   # default to just the values, but this way individual properties can
   # override these methods
   def is_to_s(currentvalue)
     currentvalue
   end
 
   # Send a log message.
   def log(msg)
 
           Puppet::Util::Log.create(
 
       :level => resource[:loglevel],
       :message => msg,
 
       :source => self
     )
   end
 
   # Should we match all values, or just the first?
   def match_all?
     self.class.array_matching == :all
   end
 
   # Execute our shadow's munge code, too, if we have one.
   def munge(value)
     self.shadow.munge(value) if self.shadow
 
     super
   end
 
   # each property class must define the name method, and property instances
   # do not change that name
   # this implicitly means that a given object can only have one property
   # instance of a given property class
   def name
     self.class.name
   end
 
   # for testing whether we should actually do anything
   def noop
     # This is only here to make testing easier.
     if @resource.respond_to?(:noop?)
       @resource.noop?
     else
       if defined?(@noop)
         @noop
       else
         Puppet[:noop]
       end
     end
   end
 
   # By default, call the method associated with the property name on our
   # provider.  In other words, if the property name is 'gid', we'll call
   # 'provider.gid' to retrieve the current value.
   def retrieve
     provider.send(self.class.name)
   end
 
   # Set our value, using the provider, an associated block, or both.
   def set(value)
     # Set a name for looking up associated options like the event.
     name = self.class.value_name(value)
 
     call = self.class.value_option(name, :call) || :none
 
     if call == :instead
       call_valuemethod(name, value)
     elsif call == :none
       # They haven't provided a block, and our parent does not have
       # a provider, so we have no idea how to handle this.
       self.fail "#{self.class.name} cannot handle values of type #{value.inspect}" unless @resource.provider
       call_provider(value)
     else
       # LAK:NOTE 20081031 This is a change in behaviour -- you could
       # previously specify :call => [;before|:after], which would call
       # the setter *in addition to* the block.  I'm convinced this
       # was never used, and it makes things unecessarily complicated.
       # If you want to specify a block and still call the setter, then
       # do so in the block.
       devfail "Cannot use obsolete :call value '#{call}' for property '#{self.class.name}'"
     end
   end
 
   # If there's a shadowing metaparam, instantiate it now.
   # This allows us to create a property or parameter with the
   # same name as a metaparameter, and the metaparam will only be
   # stored as a shadow.
   def setup_shadow(klass)
     @shadow = klass.new(:resource => self.resource)
   end
 
   # Only return the first value
   def should
     return nil unless defined?(@should)
 
     self.devfail "should for #{self.class.name} on #{resource.name} is not an array" unless @should.is_a?(Array)
 
     if match_all?
       return @should.collect { |val| self.unmunge(val) }
     else
       return self.unmunge(@should[0])
     end
   end
 
   # Set the should value.
   def should=(values)
     values = [values] unless values.is_a?(Array)
 
     @shouldorig = values
 
     values.each { |val| validate(val) }
     @should = values.collect { |val| self.munge(val) }
   end
 
   def should_to_s(newvalue)
     [newvalue].flatten.join(" ")
   end
 
   def sync
     devfail "Got a nil value for should" unless should
     set(should)
   end
 
   # Verify that the passed value is valid.
   # If the developer uses a 'validate' hook, this method will get overridden.
   def unsafe_validate(value)
     super
     validate_features_per_value(value)
   end
 
   # Make sure that we've got all of the required features for a given value.
   def validate_features_per_value(value)
     if features = self.class.value_option(self.class.value_name(value), :required_features)
       features = Array(features)
       needed_features = features.collect { |f| f.to_s }.join(", ")
       raise ArgumentError, "Provider must have features '#{needed_features}' to set '#{self.class.name}' to '#{value}'" unless provider.satisfies?(features)
     end
   end
 
   # Just return any should value we might have.
   def value
     self.should
   end
 
   # Match the Parameter interface, but we really just use 'should' internally.
   # Note that the should= method does all of the validation and such.
   def value=(value)
     self.should = value
   end
 end
diff --git a/lib/puppet/property/keyvalue.rb b/lib/puppet/property/keyvalue.rb
index 0181708f9..57d0ea2d9 100644
--- a/lib/puppet/property/keyvalue.rb
+++ b/lib/puppet/property/keyvalue.rb
@@ -1,88 +1,86 @@
 #This subclass of property manages string key value pairs.
 
 #In order to use this property:
 # - the @should value must be an array of keyvalue pairs separated by the 'separator'
 # - the retrieve method should return a hash with the keys as symbols
 # IMPORTANT NOTE: In order for this property to work there must also be a 'membership' parameter
 # The class that inherits from property should override that method with the symbol for the membership
 
 require 'puppet/property'
 
 module Puppet
   class Property
     class KeyValue < Property
 
       def hash_to_key_value_s(hash)
         hash.select { |k,v| true }.map { |pair| pair.join(separator) }.join(delimiter)
       end
 
       def should_to_s(should_value)
         hash_to_key_value_s(should_value)
       end
 
       def is_to_s(current_value)
         hash_to_key_value_s(current_value)
       end
 
       def membership
         :key_value_membership
       end
 
       def inclusive?
         @resource[membership] == :inclusive
       end
 
       def hashify(key_value_array)
         #turns string array into a hash
         key_value_array.inject({}) do |hash, key_value|
           tmp = key_value.split(separator)
           hash[tmp[0].intern] = tmp[1]
           hash
         end
       end
 
       def process_current_hash(current)
         return {} if current == :absent
 
         #inclusive means we are managing everything so if it isn't in should, its gone
         current.each_key { |key| current[key] = nil } if inclusive?
         current
       end
 
       def should
         return nil unless @should
 
         members = hashify(@should)
         current = process_current_hash(retrieve)
 
         #shared keys will get overwritten by members
         current.merge(members)
       end
 
       def separator
         "="
       end
 
       def delimiter
         ";"
       end
 
       def retrieve
         #ok, some 'convention' if the keyvalue property is named properties, provider should implement a properties method
         if key_hash = provider.send(name) and key_hash != :absent
           return key_hash
         else
           return :absent
         end
       end
 
       def insync?(is)
-        return true unless @should
-
         return true unless is
 
         (is == self.should)
       end
     end
   end
 end
diff --git a/lib/puppet/property/list.rb b/lib/puppet/property/list.rb
index dcee85db7..b86dc87f2 100644
--- a/lib/puppet/property/list.rb
+++ b/lib/puppet/property/list.rb
@@ -1,77 +1,75 @@
 require 'puppet/property'
 
 module Puppet
   class Property
     class List < Property
 
       def should_to_s(should_value)
         #just return the should value
         should_value
       end
 
       def is_to_s(currentvalue)
         if currentvalue == :absent
           return "absent"
         else
           return currentvalue.join(delimiter)
         end
       end
 
       def membership
         :membership
       end
 
       def add_should_with_current(should, current)
         should += current if current.is_a?(Array)
         should.uniq
       end
 
       def inclusive?
         @resource[membership] == :inclusive
       end
 
       #dearrayify was motivated because to simplify the implementation of the OrderedList property
       def dearrayify(array)
         array.sort.join(delimiter)
       end
 
       def should
         return nil unless @should
 
         members = @should
         #inclusive means we are managing everything so if it isn't in should, its gone
         members = add_should_with_current(members, retrieve) if ! inclusive?
 
         dearrayify(members)
       end
 
       def delimiter
         ","
       end
 
       def retrieve
         #ok, some 'convention' if the list property is named groups, provider should implement a groups method
         if tmp = provider.send(name) and tmp != :absent
           return tmp.split(delimiter)
         else
           return :absent
         end
       end
 
       def prepare_is_for_comparison(is)
         if is == :absent
           is = []
         end
         dearrayify(is)
       end
 
       def insync?(is)
-        return true unless @should
-
         return true unless is
 
         (prepare_is_for_comparison(is) == self.should)
       end
     end
   end
 end
diff --git a/lib/puppet/provider/file/posix.rb b/lib/puppet/provider/file/posix.rb
index 415a5af40..f7b8c9797 100644
--- a/lib/puppet/provider/file/posix.rb
+++ b/lib/puppet/provider/file/posix.rb
@@ -1,99 +1,97 @@
 Puppet::Type.type(:file).provide :posix do
   desc "Uses POSIX functionality to manage file's users and rights."
 
   confine :feature => :posix
 
   include Puppet::Util::POSIX
   include Puppet::Util::Warnings
 
   require 'etc'
 
   def id2name(id)
     return id.to_s if id.is_a?(Symbol)
     return nil if id > Puppet[:maximum_uid].to_i
 
     begin
       user = Etc.getpwuid(id)
     rescue TypeError
       return nil
     rescue ArgumentError
       return nil
     end
 
     if user.uid == ""
       return nil
     else
       return user.name
     end
   end
 
   def is_owner_insync?(current, should)
-    return true unless should
-
     should.each do |value|
       if value =~ /^\d+$/
         uid = Integer(value)
       elsif value.is_a?(String)
         fail "Could not find user #{value}" unless uid = uid(value)
       else
         uid = value
       end
 
       return true if uid == current
     end
 
     unless Puppet.features.root?
       warnonce "Cannot manage ownership unless running as root"
       return true
     end
 
     false
   end
 
   # Determine if the user is valid, and if so, return the UID
   def validuser?(value)
     Integer(value) rescue uid(value) || false
   end
 
   def retrieve(resource)
     unless stat = resource.stat(false)
       return :absent
     end
 
     currentvalue = stat.uid
 
     # On OS X, files that are owned by -2 get returned as really
     # large UIDs instead of negative ones.  This isn't a Ruby bug,
     # it's an OS X bug, since it shows up in perl, too.
     if currentvalue > Puppet[:maximum_uid].to_i
       self.warning "Apparently using negative UID (#{currentvalue}) on a platform that does not consistently handle them"
       currentvalue = :silly
     end
 
     currentvalue
   end
 
   def sync(path, links, should)
     # Set our method appropriately, depending on links.
     if links == :manage
       method = :lchown
     else
       method = :chown
     end
 
     uid = nil
     should.each do |user|
       break if uid = validuser?(user)
     end
 
     raise Puppet::Error, "Could not find user(s) #{should.join(",")}" unless uid
 
     begin
       File.send(method, uid, nil, path)
     rescue => detail
       raise Puppet::Error, "Failed to set owner to '#{uid}': #{detail}"
     end
 
     :file_changed
   end
 end
diff --git a/lib/puppet/provider/file/win32.rb b/lib/puppet/provider/file/win32.rb
index 23aa491dc..21e7ca974 100644
--- a/lib/puppet/provider/file/win32.rb
+++ b/lib/puppet/provider/file/win32.rb
@@ -1,74 +1,72 @@
 Puppet::Type.type(:file).provide :microsoft_windows do
   desc "Uses Microsoft Windows functionality to manage file's users and rights."
 
   confine :feature => :microsoft_windows
 
   include Puppet::Util::Warnings
 
   require 'sys/admin' if Puppet.features.microsoft_windows?
 
   def id2name(id)
     return id.to_s if id.is_a?(Symbol)
     return nil if id > Puppet[:maximum_uid].to_i
     # should translate ID numbers to usernames
     id
   end
 
   def is_owner_insync?(current, should)
-    return true unless should
-
     should.each do |value|
       if value =~ /^\d+$/
         uid = Integer(value)
       elsif value.is_a?(String)
         fail "Could not find user #{value}" unless uid = uid(value)
       else
         uid = value
       end
 
       return true if uid == current
     end
 
     unless Puppet.features.root?
       warnonce "Cannot manage ownership unless running as root"
       return true
     end
 
     false
   end
 
   # Determine if the user is valid, and if so, return the UID
   def validuser?(value)
     info "Is '#{value}' a valid user?"
     return 0
     begin
       number = Integer(value)
       return number
     rescue ArgumentError
       number = nil
     end
     (number = uid(value)) && number
   end
 
   def retrieve(resource)
     unless stat = resource.stat(false)
       return :absent
     end
 
     currentvalue = stat.uid
 
     # On OS X, files that are owned by -2 get returned as really
     # large UIDs instead of negative ones.  This isn't a Ruby bug,
     # it's an OS X bug, since it shows up in perl, too.
     if currentvalue > Puppet[:maximum_uid].to_i
       self.warning "Apparently using negative UID (#{currentvalue}) on a platform that does not consistently handle them"
       currentvalue = :silly
     end
 
     currentvalue
   end
 
   def sync(path, links, should)
     info("should set '%s'%%owner to '%s'" % [path, should])
   end
 end
diff --git a/lib/puppet/provider/zone/solaris.rb b/lib/puppet/provider/zone/solaris.rb
index c11444993..f46337b14 100644
--- a/lib/puppet/provider/zone/solaris.rb
+++ b/lib/puppet/provider/zone/solaris.rb
@@ -1,257 +1,257 @@
 Puppet::Type.type(:zone).provide(:solaris) do
   desc "Provider for Solaris Zones."
 
   commands :adm => "/usr/sbin/zoneadm", :cfg => "/usr/sbin/zonecfg"
   defaultfor :operatingsystem => :solaris
 
   mk_resource_methods
 
   # Convert the output of a list into a hash
   def self.line2hash(line)
     fields = [:id, :name, :ensure, :path]
 
     properties = {}
     line.split(":").each_with_index { |value, index|
       next unless fields[index]
       properties[fields[index]] = value
     }
 
     # Configured but not installed zones do not have IDs
     properties.delete(:id) if properties[:id] == "-"
 
     properties[:ensure] = symbolize(properties[:ensure])
 
     properties
   end
 
   def self.instances
     # LAK:NOTE See http://snurl.com/21zf8  [groups_google_com]
     x = adm(:list, "-cp").split("\n").collect do |line|
       new(line2hash(line))
     end
   end
 
   # Perform all of our configuration steps.
   def configure
     # If the thing is entirely absent, then we need to create the config.
     # Is there someway to get this on one line?
     str = "create -b #{@resource[:create_args]}\nset zonepath=#{@resource[:path]}\n"
 
     # Then perform all of our configuration steps.  It's annoying
     # that we need this much internal info on the resource.
     @resource.send(:properties).each do |property|
-      str += property.configtext + "\n" if property.is_a? ZoneConfigProperty and ! property.insync?(properties[property.name])
+      str += property.configtext + "\n" if property.is_a? ZoneConfigProperty and ! property.safe_insync?(properties[property.name])
     end
 
     str += "commit\n"
     setconfig(str)
   end
 
   def destroy
     zonecfg :delete, "-F"
   end
 
   def exists?
     properties[:ensure] != :absent
   end
 
   # Clear out the cached values.
   def flush
     @property_hash.clear
   end
 
   def install(dummy_argument=:work_arround_for_ruby_GC_bug)
     if @resource[:clone] # TODO: add support for "-s snapshot"
       zoneadm :clone, @resource[:clone]
     elsif @resource[:install_args]
       zoneadm :install, @resource[:install_args].split(" ")
     else
       zoneadm :install
     end
   end
 
   # Look up the current status.
   def properties
     if @property_hash.empty?
       @property_hash = status || {}
       if @property_hash.empty?
         @property_hash[:ensure] = :absent
       else
         @resource.class.validproperties.each do |name|
           @property_hash[name] ||= :absent
         end
       end
 
     end
     @property_hash.dup
   end
 
   # We need a way to test whether a zone is in process.  Our 'ensure'
   # property models the static states, but we need to handle the temporary ones.
   def processing?
     if hash = status
       case hash[:ensure]
       when "incomplete", "ready", "shutting_down"
         true
       else
         false
       end
     else
       false
     end
   end
 
   # Collect the configuration of the zone.
   def getconfig
     output = zonecfg :info
 
     name = nil
     current = nil
     hash = {}
     output.split("\n").each do |line|
       case line
       when /^(\S+):\s*$/
         name = $1
         current = nil # reset it
       when /^(\S+):\s*(.+)$/
         hash[$1.intern] = $2
       when /^\s+(\S+):\s*(.+)$/
         if name
           hash[name] = [] unless hash.include? name
 
           unless current
             current = {}
             hash[name] << current
           end
           current[$1.intern] = $2
         else
           err "Ignoring '#{line}'"
         end
       else
         debug "Ignoring zone output '#{line}'"
       end
     end
 
     hash
   end
 
   # Execute a configuration string.  Can't be private because it's called
   # by the properties.
   def setconfig(str)
     command = "#{command(:cfg)} -z #{@resource[:name]} -f -"
     debug "Executing '#{command}' in zone #{@resource[:name]} with '#{str}'"
     IO.popen(command, "w") do |pipe|
       pipe.puts str
     end
 
     unless $CHILD_STATUS == 0
       raise ArgumentError, "Failed to apply configuration"
     end
   end
 
   def start
     # Check the sysidcfg stuff
     if cfg = @resource[:sysidcfg]
       zoneetc = File.join(@resource[:path], "root", "etc")
       sysidcfg = File.join(zoneetc, "sysidcfg")
 
       # if the zone root isn't present "ready" the zone
       # which makes zoneadmd mount the zone root
       zoneadm :ready unless File.directory?(zoneetc)
 
       unless File.exists?(sysidcfg)
         begin
           File.open(sysidcfg, "w", 0600) do |f|
             f.puts cfg
           end
         rescue => detail
           puts detail.stacktrace if Puppet[:debug]
           raise Puppet::Error, "Could not create sysidcfg: #{detail}"
         end
       end
     end
 
     zoneadm :boot
   end
 
   # Return a hash of the current status of this zone.
   def status
     begin
       output = adm "-z", @resource[:name], :list, "-p"
     rescue Puppet::ExecutionFailure
       return nil
     end
 
     main = self.class.line2hash(output.chomp)
 
     # Now add in the configuration information
     config_status.each do |name, value|
       main[name] = value
     end
 
     main
   end
 
   def ready
     zoneadm :ready
   end
 
   def stop
     zoneadm :halt
   end
 
   def unconfigure
     zonecfg :delete, "-F"
   end
 
   def uninstall
     zoneadm :uninstall, "-F"
   end
 
   private
 
   # Turn the results of getconfig into status information.
   def config_status
     config = getconfig
     result = {}
 
     result[:autoboot] = config[:autoboot] ? config[:autoboot].intern : :absent
     result[:pool] = config[:pool]
     result[:shares] = config[:shares]
     if dir = config["inherit-pkg-dir"]
       result[:inherit] = dir.collect { |dirs| dirs[:dir] }
     end
     result[:iptype] = config[:"ip-type"]
     if net = config["net"]
       result[:ip] = net.collect do |params|
         if params[:defrouter]
           "#{params[:physical]}:#{params[:address]}:#{params[:defrouter]}"
         elsif params[:address]
           "#{params[:physical]}:#{params[:address]}"
         else
           params[:physical]
         end
       end
     end
 
     result
   end
 
   def zoneadm(*cmd)
       adm("-z", @resource[:name], *cmd)
   rescue Puppet::ExecutionFailure => detail
       self.fail "Could not #{cmd[0]} zone: #{detail}"
   end
 
   def zonecfg(*cmd)
     # You apparently can't get the configuration of the global zone
     return "" if self.name == "global"
 
     begin
       cfg("-z", self.name, *cmd)
     rescue Puppet::ExecutionFailure => detail
       self.fail "Could not #{cmd[0]} zone: #{detail}"
     end
   end
 end
 
diff --git a/lib/puppet/transaction/resource_harness.rb b/lib/puppet/transaction/resource_harness.rb
index 0abab10a5..4a3d35e0d 100644
--- a/lib/puppet/transaction/resource_harness.rb
+++ b/lib/puppet/transaction/resource_harness.rb
@@ -1,178 +1,178 @@
 require 'puppet/resource/status'
 
 class Puppet::Transaction::ResourceHarness
   extend Forwardable
   def_delegators :@transaction, :relationship_graph
 
   attr_reader :transaction
 
   def allow_changes?(resource)
     if resource.purging? and resource.deleting? and deps = relationship_graph.dependents(resource) \
             and ! deps.empty? and deps.detect { |d| ! d.deleting? }
       deplabel = deps.collect { |r| r.ref }.join(",")
       plurality = deps.length > 1 ? "":"s"
       resource.warning "#{deplabel} still depend#{plurality} on me -- not purging"
       false
     else
       true
     end
   end
 
   # Used mostly for scheduling and auditing at this point.
   def cached(resource, name)
     Puppet::Util::Storage.cache(resource)[name]
   end
 
   # Used mostly for scheduling and auditing at this point.
   def cache(resource, name, value)
     Puppet::Util::Storage.cache(resource)[name] = value
   end
 
   def perform_changes(resource)
     current = resource.retrieve_resource
 
     cache resource, :checked, Time.now
 
     return [] if ! allow_changes?(resource)
 
     current_values = current.to_hash
     historical_values = Puppet::Util::Storage.cache(resource).dup
     desired_values = {}
     resource.properties.each do |property|
       desired_values[property.name] = property.should
     end
     audited_params = (resource[:audit] || []).map { |p| p.to_sym }
     synced_params = []
 
     # Record the current state in state.yml.
     audited_params.each do |param|
       cache(resource, param, current_values[param])
     end
 
     # Update the machine state & create logs/events
     events = []
     ensure_param = resource.parameter(:ensure)
-    if desired_values[:ensure] && !ensure_param.insync?(current_values[:ensure])
+    if desired_values[:ensure] && !ensure_param.safe_insync?(current_values[:ensure])
       events << apply_parameter(ensure_param, current_values[:ensure], audited_params.include?(:ensure), historical_values[:ensure])
       synced_params << :ensure
     elsif current_values[:ensure] != :absent
       work_order = resource.properties # Note: only the resource knows what order to apply changes in
       work_order.each do |param|
-        if desired_values[param.name] && !param.insync?(current_values[param.name])
+        if desired_values[param.name] && !param.safe_insync?(current_values[param.name])
           events << apply_parameter(param, current_values[param.name], audited_params.include?(param.name), historical_values[param.name])
           synced_params << param.name
         end
       end
     end
 
     # Add more events to capture audit results
     audited_params.each do |param_name|
       if historical_values.include?(param_name)
         if historical_values[param_name] != current_values[param_name] && !synced_params.include?(param_name)
           event = create_change_event(resource.parameter(param_name), current_values[param_name], true, historical_values[param_name])
           event.send_log
           events << event
         end
       else
         resource.property(param_name).notice "audit change: newly-recorded value #{current_values[param_name]}"
       end
     end
 
     events
   end
 
   def create_change_event(property, current_value, do_audit, historical_value)
     event = property.event
     event.previous_value = current_value
     event.desired_value = property.should
     event.historical_value = historical_value
 
     if do_audit
       event.audited = true
       event.status = "audit"
       if historical_value != current_value
         event.message = "audit change: previously recorded value #{property.is_to_s(historical_value)} has been changed to #{property.is_to_s(current_value)}"
       end
     end
 
     event
   end
 
   def apply_parameter(property, current_value, do_audit, historical_value)
     event = create_change_event(property, current_value, do_audit, historical_value)
 
     if do_audit && historical_value && historical_value != current_value
       brief_audit_message = " (previously recorded value was #{property.is_to_s(historical_value)})"
     else
       brief_audit_message = ""
     end
 
     if property.noop
       event.message = "current_value #{property.is_to_s(current_value)}, should be #{property.should_to_s(property.should)} (noop)#{brief_audit_message}"
       event.status = "noop"
     else
       property.sync
       event.message = [ property.change_to_s(current_value, property.should), brief_audit_message ].join
       event.status = "success"
     end
     event
   rescue => detail
     puts detail.backtrace if Puppet[:trace]
     event.status = "failure"
 
     event.message = "change from #{property.is_to_s(current_value)} to #{property.should_to_s(property.should)} failed: #{detail}"
     event
   ensure
     event.send_log
   end
 
   def evaluate(resource)
     start = Time.now
     status = Puppet::Resource::Status.new(resource)
 
     perform_changes(resource).each do |event|
       status << event
     end
 
     if status.changed? && ! resource.noop?
       cache(resource, :synced, Time.now)
       resource.flush if resource.respond_to?(:flush)
     end
 
     return status
   rescue => detail
     resource.fail "Could not create resource status: #{detail}" unless status
     puts detail.backtrace if Puppet[:trace]
     resource.err "Could not evaluate: #{detail}"
     status.failed = true
     return status
   ensure
     (status.evaluation_time = Time.now - start) if status
   end
 
   def initialize(transaction)
     @transaction = transaction
   end
 
   def scheduled?(status, resource)
     return true if Puppet[:ignoreschedules]
     return true unless schedule = schedule(resource)
 
     # We use 'checked' here instead of 'synced' because otherwise we'll
     # end up checking most resources most times, because they will generally
     # have been synced a long time ago (e.g., a file only gets updated
     # once a month on the server and its schedule is daily; the last sync time
     # will have been a month ago, so we'd end up checking every run).
     schedule.match?(cached(resource, :checked).to_i)
   end
 
   def schedule(resource)
     unless resource.catalog
       resource.warning "Cannot schedule without a schedule-containing catalog"
       return nil
     end
 
     return nil unless name = resource[:schedule]
     resource.catalog.resource(:schedule, name) || resource.fail("Could not find schedule #{name}")
   end
 end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index ea3944b4e..e03650b54 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -1,1897 +1,1897 @@
 require 'puppet'
 require 'puppet/util/log'
 require 'puppet/util/metric'
 require 'puppet/property'
 require 'puppet/parameter'
 require 'puppet/util'
 require 'puppet/util/autoload'
 require 'puppet/metatype/manager'
 require 'puppet/util/errors'
 require 'puppet/util/log_paths'
 require 'puppet/util/logging'
 require 'puppet/util/cacher'
 require 'puppet/file_collection/lookup'
 require 'puppet/util/tagging'
 
 # see the bottom of the file for the rest of the inclusions
 
 module Puppet
 class Type
   include Puppet::Util
   include Puppet::Util::Errors
   include Puppet::Util::LogPaths
   include Puppet::Util::Logging
   include Puppet::Util::Cacher
   include Puppet::FileCollection::Lookup
   include Puppet::Util::Tagging
 
   ###############################
   # Code related to resource type attributes.
   class << self
     include Puppet::Util::ClassGen
     include Puppet::Util::Warnings
     attr_reader :properties
   end
 
   def self.states
     warnonce "The states method is deprecated; use properties"
     properties
   end
 
   # All parameters, in the appropriate order.  The key_attributes come first, then
   # the provider, then the properties, and finally the params and metaparams
   # in the order they were specified in the files.
   def self.allattrs
     key_attributes | (parameters & [:provider]) | properties.collect { |property| property.name } | parameters | metaparams
   end
 
   # Retrieve an attribute alias, if there is one.
   def self.attr_alias(param)
     @attr_aliases[symbolize(param)]
   end
 
   # Create an alias to an existing attribute.  This will cause the aliased
   # attribute to be valid when setting and retrieving values on the instance.
   def self.set_attr_alias(hash)
     hash.each do |new, old|
       @attr_aliases[symbolize(new)] = symbolize(old)
     end
   end
 
   # Find the class associated with any given attribute.
   def self.attrclass(name)
     @attrclasses ||= {}
 
     # We cache the value, since this method gets called such a huge number
     # of times (as in, hundreds of thousands in a given run).
     unless @attrclasses.include?(name)
       @attrclasses[name] = case self.attrtype(name)
       when :property; @validproperties[name]
       when :meta; @@metaparamhash[name]
       when :param; @paramhash[name]
       end
     end
     @attrclasses[name]
   end
 
   # What type of parameter are we dealing with? Cache the results, because
   # this method gets called so many times.
   def self.attrtype(attr)
     @attrtypes ||= {}
     unless @attrtypes.include?(attr)
       @attrtypes[attr] = case
         when @validproperties.include?(attr); :property
         when @paramhash.include?(attr); :param
         when @@metaparamhash.include?(attr); :meta
         end
     end
 
     @attrtypes[attr]
   end
 
   def self.eachmetaparam
     @@metaparams.each { |p| yield p.name }
   end
 
   # Create the 'ensure' class.  This is a separate method so other types
   # can easily call it and create their own 'ensure' values.
   def self.ensurable(&block)
     if block_given?
       self.newproperty(:ensure, :parent => Puppet::Property::Ensure, &block)
     else
       self.newproperty(:ensure, :parent => Puppet::Property::Ensure) do
         self.defaultvalues
       end
     end
   end
 
   # Should we add the 'ensure' property to this class?
   def self.ensurable?
     # If the class has all three of these methods defined, then it's
     # ensurable.
     ens = [:exists?, :create, :destroy].inject { |set, method|
       set &&= self.public_method_defined?(method)
     }
 
     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
   end
 
   # Is the parameter in question a meta-parameter?
   def self.metaparam?(param)
     @@metaparamhash.include?(symbolize(param))
   end
 
   # Find the metaparameter class associated with a given metaparameter name.
   def self.metaparamclass(name)
     @@metaparamhash[symbolize(name)]
   end
 
   def self.metaparams
     @@metaparams.collect { |param| param.name }
   end
 
   def self.metaparamdoc(metaparam)
     @@metaparamhash[metaparam].doc
   end
 
   # 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, options = {}, &block)
     @@metaparams ||= []
     @@metaparamhash ||= {}
     name = symbolize(name)
 
 
       param = genclass(
         name,
       :parent => options[:parent] || Puppet::Parameter,
       :prefix => "MetaParam",
       :hash => @@metaparamhash,
       :array => @@metaparams,
       :attributes => options[:attributes],
 
       &block
     )
 
     # Grr.
     param.required_features = options[:required_features] if options[:required_features]
 
     handle_param_options(name, options)
 
     param.metaparam = true
 
     param
   end
 
   def self.key_attribute_parameters
     @key_attribute_parameters ||= (
       params = @parameters.find_all { |param|
         param.isnamevar? or param.name == :name
       }
     )
   end
 
   def self.key_attributes
     key_attribute_parameters.collect { |p| p.name }
   end
 
   def self.title_patterns
     case key_attributes.length
     when 0; []
     when 1;
       identity = lambda {|x| x}
       [ [ /(.*)/m, [ [key_attributes.first, identity ] ] ] ]
     else
       raise Puppet::DevError,"you must specify title patterns when there are two or more key attributes"
     end
   end
 
   def uniqueness_key
     to_resource.uniqueness_key
   end
 
   # 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] ||= {}
 
       param = genclass(
         name,
       :parent => options[:parent] || Puppet::Parameter,
       :attributes => options[:attributes],
       :block => block,
       :prefix => "Parameter",
       :array => @parameters,
 
       :hash => @paramhash
     )
 
     handle_param_options(name, options)
 
     # Grr.
     param.required_features = options[:required_features] if options[:required_features]
 
     param.isnamevar if options[:namevar]
 
     param
   end
 
   def self.newstate(name, options = {}, &block)
     Puppet.warning "newstate() has been deprecrated; use newproperty(#{name})"
     newproperty(name, options, &block)
   end
 
   # Create a new property. The first parameter must be the name of the property;
   # this is how users will refer to the property when creating new instances.
   # The second parameter is a hash of options; the options are:
   # * <tt>:parent</tt>: The parent class for the property.  Defaults to Puppet::Property.
   # * <tt>:retrieve</tt>: The method to call on the provider or @parent object (if
   #   the provider is not set) to retrieve the current value.
   def self.newproperty(name, options = {}, &block)
     name = symbolize(name)
 
     # This is here for types that might still have the old method of defining
     # a parent class.
     unless options.is_a? Hash
       raise Puppet::DevError,
         "Options must be a hash, not #{options.inspect}"
     end
 
     raise Puppet::DevError, "Class #{self.name} already has a property named #{name}" if @validproperties.include?(name)
 
     if parent = options[:parent]
       options.delete(:parent)
     else
       parent = Puppet::Property
     end
 
     # We have to create our own, new block here because we want to define
     # an initial :retrieve method, if told to, and then eval the passed
     # block if available.
     prop = genclass(name, :parent => parent, :hash => @validproperties, :attributes => options) do
       # If they've passed a retrieve method, then override the retrieve
       # method on the class.
       if options[:retrieve]
         define_method(:retrieve) do
           provider.send(options[:retrieve])
         end
       end
 
       class_eval(&block) if block
     end
 
     # If it's the 'ensure' property, always put it first.
     if name == :ensure
       @properties.unshift prop
     else
       @properties << prop
     end
 
     prop
   end
 
   def self.paramdoc(param)
     @paramhash[param].doc
   end
 
   # Return the parameter names
   def self.parameters
     return [] unless defined?(@parameters)
     @parameters.collect { |klass| klass.name }
   end
 
   # Find the parameter class associated with a given parameter name.
   def self.paramclass(name)
     @paramhash[name]
   end
 
   # Return the property class associated with a name
   def self.propertybyname(name)
     @validproperties[name]
   end
 
   def self.validattr?(name)
     name = symbolize(name)
     return true if name == :name
     @validattrs ||= {}
 
     unless @validattrs.include?(name)
       @validattrs[name] = !!(self.validproperty?(name) or self.validparameter?(name) or self.metaparam?(name))
     end
 
     @validattrs[name]
   end
 
   # does the name reflect a valid property?
   def self.validproperty?(name)
     name = symbolize(name)
     @validproperties.include?(name) && @validproperties[name]
   end
 
   # Return the list of validproperties
   def self.validproperties
     return {} unless defined?(@parameters)
 
     @validproperties.keys
   end
 
   # does the name reflect a valid parameter?
   def self.validparameter?(name)
     raise Puppet::DevError, "Class #{self} has not defined parameters" unless defined?(@parameters)
     !!(@paramhash.include?(name) or @@metaparamhash.include?(name))
   end
 
   # This is a forward-compatibility method - it's the validity interface we'll use in Puppet::Resource.
   def self.valid_parameter?(name)
     validattr?(name)
   end
 
   # Return either the attribute alias or the attribute.
   def attr_alias(name)
     name = symbolize(name)
     if synonym = self.class.attr_alias(name)
       return synonym
     else
       return name
     end
   end
 
   # Are we deleting this resource?
   def deleting?
     obj = @parameters[:ensure] and obj.should == :absent
   end
 
   # Create a new property if it is valid but doesn't exist
   # Returns: true if a new parameter was added, false otherwise
   def add_property_parameter(prop_name)
     if self.class.validproperty?(prop_name) && !@parameters[prop_name]
       self.newattr(prop_name)
       return true
     end
     false
   end
 
   #
   # The name_var is the key_attribute in the case that there is only one.
   #
   def name_var
     key_attributes = self.class.key_attributes
     (key_attributes.length == 1) && key_attributes.first
   end
 
   # abstract accessing parameters and properties, and normalize
   # access to always be symbols, not strings
   # This returns a value, not an object.  It returns the 'is'
   # value, but you can also specifically return 'is' and 'should'
   # values using 'object.is(:property)' or 'object.should(:property)'.
   def [](name)
     name = attr_alias(name)
 
     fail("Invalid parameter #{name}(#{name.inspect})") unless self.class.validattr?(name)
 
     if name == :name
       name = name_var
     end
 
     if obj = @parameters[name]
       # Note that if this is a property, then the value is the "should" value,
       # not the current value.
       obj.value
     else
       return nil
     end
   end
 
   # Abstract setting parameters and properties, and normalize
   # access to always be symbols, not strings.  This sets the 'should'
   # value on properties, and otherwise just sets the appropriate parameter.
   def []=(name,value)
     name = attr_alias(name)
 
     fail("Invalid parameter #{name}") unless self.class.validattr?(name)
 
     if name == :name
       name = name_var
     end
     raise Puppet::Error.new("Got nil value for #{name}") if value.nil?
 
     property = self.newattr(name)
 
     if property
       begin
         # make sure the parameter doesn't have any errors
         property.value = value
       rescue => detail
         error = Puppet::Error.new("Parameter #{name} failed: #{detail}")
         error.set_backtrace(detail.backtrace)
         raise error
       end
     end
 
     nil
   end
 
   # remove a property from the object; useful in testing or in cleanup
   # when an error has been encountered
   def delete(attr)
     attr = symbolize(attr)
     if @parameters.has_key?(attr)
       @parameters.delete(attr)
     else
       raise Puppet::DevError.new("Undefined attribute '#{attr}' in #{self}")
     end
   end
 
   # iterate across the existing properties
   def eachproperty
     # properties is a private method
     properties.each { |property|
       yield property
     }
   end
 
   # Create a transaction event.  Called by Transaction or by
   # a property.
   def event(options = {})
     Puppet::Transaction::Event.new({:resource => self, :file => file, :line => line, :tags => tags}.merge(options))
   end
 
   # Let the catalog determine whether a given cached value is
   # still valid or has expired.
   def expirer
     catalog
   end
 
   # retrieve the 'should' value for a specified property
   def should(name)
     name = attr_alias(name)
     (prop = @parameters[name] and prop.is_a?(Puppet::Property)) ? prop.should : nil
   end
 
   # Create the actual attribute instance.  Requires either the attribute
   # name or class as the first argument, then an optional hash of
   # attributes to set during initialization.
   def newattr(name)
     if name.is_a?(Class)
       klass = name
       name = klass.name
     end
 
     unless klass = self.class.attrclass(name)
       raise Puppet::Error, "Resource type #{self.class.name} does not support parameter #{name}"
     end
 
     if provider and ! provider.class.supports_parameter?(klass)
       missing = klass.required_features.find_all { |f| ! provider.class.feature?(f) }
       info "Provider %s does not support features %s; not managing attribute %s" % [provider.class.name, missing.join(", "), name]
       return nil
     end
 
     return @parameters[name] if @parameters.include?(name)
 
     @parameters[name] = klass.new(:resource => self)
   end
 
   # return the value of a parameter
   def parameter(name)
     @parameters[name.to_sym]
   end
 
   def parameters
     @parameters.dup
   end
 
   # Is the named property defined?
   def propertydefined?(name)
     name = name.intern unless name.is_a? Symbol
     @parameters.include?(name)
   end
 
   # Return an actual property instance by name; to return the value, use 'resource[param]'
   # LAK:NOTE(20081028) Since the 'parameter' method is now a superset of this method,
   # this one should probably go away at some point.
   def property(name)
     (obj = @parameters[symbolize(name)] and obj.is_a?(Puppet::Property)) ? obj : nil
   end
 
   # For any parameters or properties that have defaults and have not yet been
   # set, set them now.  This method can be handed a list of attributes,
   # and if so it will only set defaults for those attributes.
   def set_default(attr)
     return unless klass = self.class.attrclass(attr)
     return unless klass.method_defined?(:default)
     return if @parameters.include?(klass.name)
 
     return unless parameter = newattr(klass.name)
 
     if value = parameter.default and ! value.nil?
       parameter.value = value
     else
       @parameters.delete(parameter.name)
     end
   end
 
   # Convert our object to a hash.  This just includes properties.
   def to_hash
     rethash = {}
 
     @parameters.each do |name, obj|
       rethash[name] = obj.value
     end
 
     rethash
   end
 
   def type
     self.class.name
   end
 
   # Return a specific value for an attribute.
   def value(name)
     name = attr_alias(name)
 
     (obj = @parameters[name] and obj.respond_to?(:value)) ? obj.value : nil
   end
 
   def version
     return 0 unless catalog
     catalog.version
   end
 
   # Return all of the property objects, in the order specified in the
   # class.
   def properties
     self.class.properties.collect { |prop| @parameters[prop.name] }.compact
   end
 
   # Is this type's name isomorphic with the object?  That is, if the
   # name conflicts, does it necessarily mean that the objects conflict?
   # Defaults to true.
   def self.isomorphic?
     if defined?(@isomorphic)
       return @isomorphic
     else
       return true
     end
   end
 
   def isomorphic?
     self.class.isomorphic?
   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
   # in a list
   def managed?
     # Once an object is managed, it always stays managed; but an object
     # that is listed as unmanaged might become managed later in the process,
     # so we have to check that every time
     if @managed
       return @managed
     else
       @managed = false
       properties.each { |property|
         s = property.should
         if s and ! property.class.unmanaged
           @managed = true
           break
         end
       }
       return @managed
     end
   end
 
   ###############################
   # Code related to the container behaviour.
 
   # this is a retarded hack method to get around the difference between
   # component children and file children
   def self.depthfirst?
     @depthfirst
   end
 
   def depthfirst?
     self.class.depthfirst?
   end
 
   # Remove an object.  The argument determines whether the object's
   # subscriptions get eliminated, too.
   def remove(rmdeps = true)
     # This is hackish (mmm, cut and paste), but it works for now, and it's
     # better than warnings.
     @parameters.each do |name, obj|
       obj.remove
     end
     @parameters.clear
 
     @parent = nil
 
     # Remove the reference to the provider.
     if self.provider
       @provider.clear
       @provider = nil
     end
   end
 
   ###############################
   # Code related to evaluating the resources.
 
   # Flush the provider, if it supports it.  This is called by the
   # transaction.
   def flush
     self.provider.flush if self.provider and self.provider.respond_to?(:flush)
   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,
   # it's really only used for testing
   def insync?(is)
     insync = true
 
     if property = @parameters[:ensure]
       unless is.include? property
         raise Puppet::DevError,
           "The is value is not in the is array for '#{property.name}'"
       end
       ensureis = is[property]
-      if property.insync?(ensureis) and property.should == :absent
+      if property.safe_insync?(ensureis) and property.should == :absent
         return true
       end
     end
 
     properties.each { |property|
       unless is.include? property
         raise Puppet::DevError,
           "The is value is not in the is array for '#{property.name}'"
       end
 
       propis = is[property]
-      unless property.insync?(propis)
+      unless property.safe_insync?(propis)
         property.debug("Not in sync: #{propis.inspect} vs #{property.should.inspect}")
         insync = false
       #else
       #    property.debug("In sync")
       end
     }
 
     #self.debug("#{self} sync status is #{insync}")
     insync
   end
 
   # retrieve the current value of all contained properties
   def retrieve
     fail "Provider #{provider.class.name} is not functional on this host" if self.provider.is_a?(Puppet::Provider) and ! provider.class.suitable?
 
     result = Puppet::Resource.new(type, title)
 
     # Provide the name, so we know we'll always refer to a real thing
     result[:name] = self[:name] unless self[:name] == title
 
     if ensure_prop = property(:ensure) or (self.class.validattr?(:ensure) and ensure_prop = newattr(:ensure))
       result[:ensure] = ensure_state = ensure_prop.retrieve
     else
       ensure_state = nil
     end
 
     properties.each do |property|
       next if property.name == :ensure
       if ensure_state == :absent
         result[property] = :absent
       else
         result[property] = property.retrieve
       end
     end
 
     result
   end
 
   def retrieve_resource
     resource = retrieve
     resource = Resource.new(type, title, :parameters => resource) if resource.is_a? Hash
     resource
   end
 
   # Get a hash of the current properties.  Returns a hash with
   # the actual property instance as the key and the current value
   # as the, um, value.
   def currentpropvalues
     # It's important to use the 'properties' method here, as it follows the order
     # in which they're defined in the class.  It also guarantees that 'ensure'
     # is the first property, which is important for skipping 'retrieve' on
     # all the properties if the resource is absent.
     ensure_state = false
     return properties.inject({}) do | prophash, property|
       if property.name == :ensure
         ensure_state = property.retrieve
         prophash[property] = ensure_state
       else
         if ensure_state == :absent
           prophash[property] = :absent
         else
           prophash[property] = property.retrieve
         end
       end
       prophash
     end
   end
 
   # Are we running in noop mode?
   def noop?
     # If we're not a host_config, we're almost certainly part of
     # Settings, and we want to ignore 'noop'
     return false if catalog and ! catalog.host_config?
 
     if defined?(@noop)
       @noop
     else
       Puppet[:noop]
     end
   end
 
   def noop
     noop?
   end
 
   ###############################
   # Code related to managing resource instances.
   require 'puppet/transportable'
 
   # retrieve a named instance of the current type
   def self.[](name)
     raise "Global resource access is deprecated"
     @objects[name] || @aliases[name]
   end
 
   # add an instance by name to the class list of instances
   def self.[]=(name,object)
     raise "Global resource storage is deprecated"
     newobj = nil
     if object.is_a?(Puppet::Type)
       newobj = object
     else
       raise Puppet::DevError, "must pass a Puppet::Type object"
     end
 
     if exobj = @objects[name] and self.isomorphic?
       msg = "Object '#{newobj.class.name}[#{name}]' already exists"
 
       msg += ("in file #{object.file} at line #{object.line}") if exobj.file and exobj.line
       msg += ("and cannot be redefined in file #{object.file} at line #{object.line}") if object.file and object.line
       error = Puppet::Error.new(msg)
       raise error
     else
       #Puppet.info("adding %s of type %s to class list" %
       #    [name,object.class])
       @objects[name] = newobj
     end
   end
 
   # Create an alias.  We keep these in a separate hash so that we don't encounter
   # the objects multiple times when iterating over them.
   def self.alias(name, obj)
     raise "Global resource aliasing is deprecated"
     if @objects.include?(name)
       unless @objects[name] == obj
         raise Puppet::Error.new(
           "Cannot create alias #{name}: object already exists"
         )
       end
     end
 
     if @aliases.include?(name)
       unless @aliases[name] == obj
         raise Puppet::Error.new(
           "Object #{@aliases[name].name} already has alias #{name}"
         )
       end
     end
 
     @aliases[name] = obj
   end
 
   # remove all of the instances of a single type
   def self.clear
     raise "Global resource removal is deprecated"
     if defined?(@objects)
       @objects.each do |name, obj|
         obj.remove(true)
       end
       @objects.clear
     end
     @aliases.clear if defined?(@aliases)
   end
 
   # Force users to call this, so that we can merge objects if
   # necessary.
   def self.create(args)
     # LAK:DEP Deprecation notice added 12/17/2008
     Puppet.warning "Puppet::Type.create is deprecated; use Puppet::Type.new"
     new(args)
   end
 
   # remove a specified object
   def self.delete(resource)
     raise "Global resource removal is deprecated"
     return unless defined?(@objects)
     @objects.delete(resource.title) if @objects.include?(resource.title)
     @aliases.delete(resource.title) if @aliases.include?(resource.title)
     if @aliases.has_value?(resource)
       names = []
       @aliases.each do |name, otherres|
         if otherres == resource
           names << name
         end
       end
       names.each { |name| @aliases.delete(name) }
     end
   end
 
   # iterate across each of the type's instances
   def self.each
     raise "Global resource iteration is deprecated"
     return unless defined?(@objects)
     @objects.each { |name,instance|
       yield instance
     }
   end
 
   # does the type have an object with the given name?
   def self.has_key?(name)
     raise "Global resource access is deprecated"
     @objects.has_key?(name)
   end
 
   # Retrieve all known instances.  Either requires providers or must be overridden.
   def self.instances
     raise Puppet::DevError, "#{self.name} has no providers and has not overridden 'instances'" if provider_hash.empty?
 
     # Put the default provider first, then the rest of the suitable providers.
     provider_instances = {}
     providers_by_source.collect do |provider|
       provider.instances.collect do |instance|
         # We always want to use the "first" provider instance we find, unless the resource
         # is already managed and has a different provider set
         if other = provider_instances[instance.name]
           Puppet.warning "%s %s found in both %s and %s; skipping the %s version" %
             [self.name.to_s.capitalize, instance.name, other.class.name, instance.class.name, instance.class.name]
           next
         end
         provider_instances[instance.name] = instance
 
         new(:name => instance.name, :provider => instance, :audit => :all)
       end
     end.flatten.compact
   end
 
   # Return a list of one suitable provider per source, with the default provider first.
   def self.providers_by_source
     # Put the default provider first, then the rest of the suitable providers.
     sources = []
     [defaultprovider, suitableprovider].flatten.uniq.collect do |provider|
       next if sources.include?(provider.source)
 
       sources << provider.source
       provider
     end.compact
   end
 
   # Convert a simple hash into a Resource instance.
   def self.hash2resource(hash)
     hash = hash.inject({}) { |result, ary| result[ary[0].to_sym] = ary[1]; result }
 
     title = hash.delete(:title)
     title ||= hash[:name]
     title ||= hash[key_attributes.first] if key_attributes.length == 1
 
     raise Puppet::Error, "Title or name must be provided" unless title
 
     # Now create our resource.
     resource = Puppet::Resource.new(self.name, title)
     [:catalog].each do |attribute|
       if value = hash[attribute]
         hash.delete(attribute)
         resource.send(attribute.to_s + "=", value)
       end
     end
 
     hash.each do |param, value|
       resource[param] = value
     end
     resource
   end
 
   # Create the path for logging and such.
   def pathbuilder
     if p = parent
       [p.pathbuilder, self.ref].flatten
     else
       [self.ref]
     end
   end
 
   ###############################
   # Add all of the meta parameters.
   newmetaparam(:noop) do
     desc "Boolean flag indicating whether work should actually
       be done."
 
     newvalues(:true, :false)
     munge do |value|
       case value
       when true, :true, "true"; @resource.noop = true
       when false, :false, "false"; @resource.noop = false
       end
     end
   end
 
   newmetaparam(:schedule) do
     desc "On what schedule the object should be managed.  You must create a
       schedule object, and then reference the name of that object to use
       that for your schedule:
 
           schedule { daily:
             period => daily,
             range => \"2-4\"
           }
 
           exec { \"/usr/bin/apt-get update\":
             schedule => daily
           }
 
       The creation of the schedule object does not need to appear in the
       configuration before objects that use it."
   end
 
   newmetaparam(:audit) do
     desc "Audit specified attributes of resources over time, and report if any have changed.
       This attribute can be used to track changes to any resource over time, and can
       provide an audit trail of every change that happens on any given machine.
 
       Note that you cannot both audit and manage an attribute - managing it guarantees
       the value, and any changes already get logged."
 
     validate do |list|
       list = Array(list).collect {|p| p.to_sym}
       unless list == [:all]
         list.each do |param|
           next if @resource.class.validattr?(param)
           fail "Cannot audit #{param}: not a valid attribute for #{resource}"
         end
       end
     end
 
     munge do |args|
       properties_to_audit(args).each do |param|
         next unless resource.class.validproperty?(param)
         resource.newattr(param)
       end
     end
 
     def all_properties
       resource.class.properties.find_all do |property|
         resource.provider.nil? or resource.provider.class.supports_parameter?(property)
       end.collect do |property|
         property.name
       end
     end
 
     def properties_to_audit(list)
       if !list.kind_of?(Array) && list.to_sym == :all
         list = all_properties
       else
         list = Array(list).collect { |p| p.to_sym }
       end
     end
   end
 
   newmetaparam(:check) do
     desc "Audit specified attributes of resources over time, and report if any have changed.
       This parameter has been deprecated in favor of 'audit'."
 
     munge do |args|
       resource.warning "'check' attribute is deprecated; use 'audit' instead"
       resource[:audit] = args
     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
       syslog (which is currently the default)."
     defaultto :notice
 
     newvalues(*Puppet::Util::Log.levels)
     newvalues(:verbose)
 
     munge do |loglevel|
       val = super(loglevel)
       if val == :verbose
         val = :info
       end
       val
     end
   end
 
   newmetaparam(:alias) do
     desc "Creates an alias for the object.  Puppet uses this internally when you
       provide a symbolic name:
 
           file { sshdconfig:
             path => $operatingsystem ? {
               solaris => \"/usr/local/etc/ssh/sshd_config\",
               default => \"/etc/ssh/sshd_config\"
             },
             source => \"...\"
           }
 
           service { sshd:
             subscribe => File[sshdconfig]
           }
 
       When you use this feature, the parser sets `sshdconfig` as the name,
       and the library sets that as an alias for the file so the dependency
       lookup for `sshd` works.  You can use this parameter yourself,
       but note that only the library can use these aliases; for instance,
       the following code will not work:
 
           file { \"/etc/ssh/sshd_config\":
             owner => root,
             group => root,
             alias => sshdconfig
           }
 
           file { sshdconfig:
             mode => 644
           }
 
       There's no way here for the Puppet parser to know that these two stanzas
       should be affecting the same file.
 
       See the [Language Tutorial](http://docs.puppetlabs.com/guides/language_tutorial.html) for more information.
 
       "
 
     munge do |aliases|
       aliases = [aliases] unless aliases.is_a?(Array)
 
       raise(ArgumentError, "Cannot add aliases without a catalog") unless @resource.catalog
 
       aliases.each do |other|
         if obj = @resource.catalog.resource(@resource.class.name, other)
           unless obj.object_id == @resource.object_id
             self.fail("#{@resource.title} can not create alias #{other}: object already exists")
           end
           next
         end
 
         # Newschool, add it to the catalog.
         @resource.catalog.alias(@resource, other)
       end
     end
   end
 
   newmetaparam(:tag) do
     desc "Add the specified tags to the associated resource.  While all resources
       are automatically tagged with as much information as possible
       (e.g., each class and definition containing the resource), it can
       be useful to add your own tags to a given resource.
 
       Tags are currently useful for things like applying a subset of a
       host's configuration:
 
           puppet agent --test --tags mytag
 
       This way, when you're testing a configuration you can run just the
       portion you're testing."
 
     munge do |tags|
       tags = [tags] unless tags.is_a? Array
 
       tags.each do |tag|
         @resource.tag(tag)
       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(references)
       references = [references] unless references.is_a?(Array)
       references.collect do |ref|
         if ref.is_a?(Puppet::Resource)
           ref
         else
           Puppet::Resource.new(ref)
         end
       end
     end
 
     def validate_relationship
       @value.each do |ref|
         unless @resource.catalog.resource(ref.to_s)
           description = self.class.direction == :in ? "dependency" : "dependent"
           fail "Could not find #{description} #{ref} for #{resource.ref}"
         end
       end
     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 |reference|
         reference.catalog = resource.catalog
 
         # Either of the two retrieval attempts could have returned
         # nil.
         unless related_resource = reference.resolve
           self.fail "Could not retrieve dependency '#{reference}' of #{@resource.ref}"
         end
 
         # Are we requiring them, or vice versa?  See the method docs
         # for futher info on this.
         if self.class.direction == :in
           source = related_resource
           target = @resource
         else
           source = @resource
           target = related_resource
         end
 
         if method = self.class.callback
           subargs = {
             :event => self.class.events,
             :callback => method
           }
           self.debug("subscribes to #{related_resource.ref}")
         else
           # If there's no callback, there's no point in even adding
           # a label.
           subargs = nil
           self.debug("requires #{related_resource.ref}")
         end
 
         rel = Puppet::Relationship.new(source, target, subargs)
       end
     end
   end
 
   def self.relationship_params
     RelationshipMetaparam.subclasses
   end
 
 
   # Note that the order in which the relationships params is defined
   # matters.  The labelled params (notify and subcribe) must be later,
   # so that if both params are used, those ones win.  It's a hackish
   # solution, but it works.
 
   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\"]
           }
 
       Multiple dependencies can be specified by providing a comma-seperated list
       of resources, enclosed in square brackets:
 
           require => [ File[\"/usr/local\"], 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 resources 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 resources 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
 
   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:
               ensure => running,
               subscribe => File[nagconf]
             }
           }
 
       Currently the `exec`, `mount` and `service` type support
       refreshing.
       "
   end
 
   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:
 
           file { "/var/nagios/configuration":
             source  => "...",
             recurse => true,
             before => Exec["nagios-rebuid"]
           }
 
           exec { "nagios-rebuild":
             command => "/usr/bin/make",
             cwd => "/var/nagios/configuration"
           }
 
       This will make sure all of the files are up to date before the
       make command is run.}
   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:
 
           file { "/etc/sshd_config":
             source => "....",
             notify => Service[sshd]
           }
 
           service { sshd:
             ensure => running
           }
 
       This will restart the sshd service if the sshd config file changes.}
   end
 
   newmetaparam(:stage) do
     desc %{Which run stage a given resource should reside in.  This just creates
       a dependency on or from the named milestone.  For instance, saying that
       this is in the 'bootstrap' stage creates a dependency on the 'bootstrap'
       milestone.
 
       By default, all classes get directly added to the
       'main' stage.  You can create new stages as resources:
 
           stage { [pre, post]: }
 
       To order stages, use standard relationships:
 
           stage { pre: before => Stage[main] }
 
       Or use the new relationship syntax:
 
           Stage[pre] -> Stage[main] -> Stage[post]
 
       Then use the new class parameters to specify a stage:
 
           class { foo: stage => pre }
 
       Stages can only be set on classes, not individual resources.  This will
       fail:
 
           file { '/foo': stage => pre, ensure => file }
     }
   end
 
   ###############################
   # All of the provider plumbing for the resource types.
   require 'puppet/provider'
   require 'puppet/util/provider_features'
 
   # Add the feature handling module.
   extend Puppet::Util::ProviderFeatures
 
   attr_reader :provider
 
   # the Type class attribute accessors
   class << self
     attr_accessor :providerloader
     attr_writer :defaultprovider
   end
 
   # Find the default provider.
   def self.defaultprovider
     unless @defaultprovider
       suitable = suitableprovider
 
       # Find which providers are a default for this system.
       defaults = suitable.find_all { |provider| provider.default? }
 
       # If we don't have any default we use suitable providers
       defaults = suitable if defaults.empty?
       max = defaults.collect { |provider| provider.specificity }.max
       defaults = defaults.find_all { |provider| provider.specificity == max }
 
       retval = nil
       if defaults.length > 1
         Puppet.warning(
           "Found multiple default providers for #{self.name}: #{defaults.collect { |i| i.name.to_s }.join(", ")}; using #{defaults[0].name}"
         )
         retval = defaults.shift
       elsif defaults.length == 1
         retval = defaults.shift
       else
         raise Puppet::DevError, "Could not find a default provider for #{self.name}"
       end
 
       @defaultprovider = retval
     end
 
     @defaultprovider
   end
 
   def self.provider_hash_by_type(type)
     @provider_hashes ||= {}
     @provider_hashes[type] ||= {}
   end
 
   def self.provider_hash
     Puppet::Type.provider_hash_by_type(self.name)
   end
 
   # Retrieve a provider by name.
   def self.provider(name)
     name = Puppet::Util.symbolize(name)
 
     # If we don't have it yet, try loading it.
     @providerloader.load(name) unless provider_hash.has_key?(name)
     provider_hash[name]
   end
 
   # Just list all of the providers.
   def self.providers
     provider_hash.keys
   end
 
   def self.validprovider?(name)
     name = Puppet::Util.symbolize(name)
 
     (provider_hash.has_key?(name) && provider_hash[name].suitable?)
   end
 
   # Create a new provider of a type.  This method must be called
   # directly on the type that it's implementing.
   def self.provide(name, options = {}, &block)
     name = Puppet::Util.symbolize(name)
 
     if obj = provider_hash[name]
       Puppet.debug "Reloading #{name} #{self.name} provider"
       unprovide(name)
     end
 
     parent = if pname = options[:parent]
       options.delete(:parent)
       if pname.is_a? Class
         pname
       else
         if provider = self.provider(pname)
           provider
         else
           raise Puppet::DevError,
             "Could not find parent provider #{pname} of #{name}"
         end
       end
     else
       Puppet::Provider
     end
 
     options[:resource_type] ||= self
 
     self.providify
 
 
       provider = genclass(
         name,
       :parent => parent,
       :hash => provider_hash,
       :prefix => "Provider",
       :block => block,
       :include => feature_module,
       :extend => feature_module,
 
       :attributes => options
     )
 
     provider
   end
 
   # Make sure we have a :provider parameter defined.  Only gets called if there
   # are providers.
   def self.providify
     return if @paramhash.has_key? :provider
 
     newparam(:provider) do
       desc "The specific backend for #{self.name.to_s} to use. You will
         seldom need to specify this -- Puppet will usually discover the
         appropriate provider for your platform."
 
       # This is so we can refer back to the type to get a list of
       # providers for documentation.
       class << self
         attr_accessor :parenttype
       end
 
       # We need to add documentation for each provider.
       def self.doc
         @doc + "  Available providers are:\n\n" + parenttype.providers.sort { |a,b|
           a.to_s <=> b.to_s
         }.collect { |i|
           "* **#{i}**: #{parenttype().provider(i).doc}"
         }.join("\n")
       end
 
       defaultto {
         @resource.class.defaultprovider.name
       }
 
       validate do |provider_class|
         provider_class = provider_class[0] if provider_class.is_a? Array
         provider_class = provider_class.class.name if provider_class.is_a?(Puppet::Provider)
 
         unless provider = @resource.class.provider(provider_class)
           raise ArgumentError, "Invalid #{@resource.class.name} provider '#{provider_class}'"
         end
       end
 
       munge do |provider|
         provider = provider[0] if provider.is_a? Array
         provider = provider.intern if provider.is_a? String
         @resource.provider = provider
 
         if provider.is_a?(Puppet::Provider)
           provider.class.name
         else
           provider
         end
       end
     end.parenttype = self
   end
 
   def self.unprovide(name)
     if provider_hash.has_key? name
 
       rmclass(
         name,
         :hash => provider_hash,
 
         :prefix => "Provider"
       )
       if @defaultprovider and @defaultprovider.name == name
         @defaultprovider = nil
       end
     end
   end
 
   # Return an array of all of the suitable providers.
   def self.suitableprovider
     providerloader.loadall if provider_hash.empty?
     provider_hash.find_all { |name, provider|
       provider.suitable?
     }.collect { |name, provider|
       provider
     }.reject { |p| p.name == :fake } # For testing
   end
 
   def provider=(name)
     if name.is_a?(Puppet::Provider)
       @provider = name
       @provider.resource = self
     elsif klass = self.class.provider(name)
       @provider = klass.new(self)
     else
       raise ArgumentError, "Could not find #{name} provider of #{self.class.name}"
     end
   end
 
   ###############################
   # All of the relationship code.
 
   # Specify a block for generating a list of objects to autorequire.  This
   # makes it so that you don't have to manually specify things that you clearly
   # require.
   def self.autorequire(name, &block)
     @autorequires ||= {}
     @autorequires[name] = block
   end
 
   # Yield each of those autorequires in turn, yo.
   def self.eachautorequire
     @autorequires ||= {}
     @autorequires.each { |type, block|
       yield(type, block)
     }
   end
 
   # Figure out of there are any objects we can automatically add as
   # dependencies.
   def autorequire(rel_catalog = nil)
     rel_catalog ||= catalog
     raise(Puppet::DevError, "You cannot add relationships without a catalog") unless rel_catalog
 
     reqs = []
     self.class.eachautorequire { |type, block|
       # Ignore any types we can't find, although that would be a bit odd.
       next unless typeobj = Puppet::Type.type(type)
 
       # Retrieve the list of names from the block.
       next unless list = self.instance_eval(&block)
       list = [list] unless list.is_a?(Array)
 
       # Collect the current prereqs
       list.each { |dep|
         obj = nil
         # Support them passing objects directly, to save some effort.
         unless dep.is_a? Puppet::Type
           # Skip autorequires that we aren't managing
           unless dep = rel_catalog.resource(type, dep)
             next
           end
         end
 
         reqs << Puppet::Relationship.new(dep, self)
       }
     }
 
     reqs
   end
 
   # Build the dependencies associated with an individual object.
   def builddepends
     # Handle the requires
     self.class.relationship_params.collect do |klass|
       if param = @parameters[klass.name]
         param.to_edges
       end
     end.flatten.reject { |r| r.nil? }
   end
 
   # Define the initial list of tags.
   def tags=(list)
     tag(self.class.name)
     tag(*list)
   end
 
   # Types (which map to resources in the languages) are entirely composed of
   # attribute value pairs.  Generally, Puppet calls any of these things an
   # 'attribute', but these attributes always take one of three specific
   # forms:  parameters, metaparams, or properties.
 
   # In naming methods, I have tried to consistently name the method so
   # that it is clear whether it operates on all attributes (thus has 'attr' in
   # the method name, or whether it operates on a specific type of attributes.
   attr_writer :title
   attr_writer :noop
 
   include Enumerable
 
   # class methods dealing with Type management
 
   public
 
   # the Type class attribute accessors
   class << self
     attr_reader :name
     attr_accessor :self_refresh
     include Enumerable, Puppet::Util::ClassGen
     include Puppet::MetaType::Manager
 
     include Puppet::Util
     include Puppet::Util::Logging
   end
 
   # all of the variables that must be initialized for each subclass
   def self.initvars
     # all of the instances of this class
     @objects = Hash.new
     @aliases = Hash.new
 
     @defaults = {}
 
     @parameters ||= []
 
     @validproperties = {}
     @properties = []
     @parameters = []
     @paramhash = {}
 
     @attr_aliases = {}
 
     @paramdoc = Hash.new { |hash,key|
       key = key.intern if key.is_a?(String)
       if hash.include?(key)
         hash[key]
       else
         "Param Documentation for #{key} not found"
       end
     }
 
     @doc ||= ""
 
   end
 
   def self.to_s
     if defined?(@name)
       "Puppet::Type::#{@name.to_s.capitalize}"
     else
       super
     end
   end
 
   # Create a block to validate that our object is set up entirely.  This will
   # be run before the object is operated on.
   def self.validate(&block)
     define_method(:validate, &block)
     #@validate = block
   end
 
   # The catalog that this resource is stored in.
   attr_accessor :catalog
 
   # is the resource exported
   attr_accessor :exported
 
   # is the resource virtual (it should not :-))
   attr_accessor :virtual
 
   # create a log at specified level
   def log(msg)
 
     Puppet::Util::Log.create(
 
       :level => @parameters[:loglevel].value,
       :message => msg,
 
       :source => self
     )
   end
 
 
   # instance methods related to instance intrinsics
   # e.g., initialize and name
 
   public
 
   attr_reader :original_parameters
 
   # initialize the type instance
   def initialize(resource)
     raise Puppet::DevError, "Got TransObject instead of Resource or hash" if resource.is_a?(Puppet::TransObject)
     resource = self.class.hash2resource(resource) unless resource.is_a?(Puppet::Resource)
 
     # The list of parameter/property instances.
     @parameters = {}
 
     # Set the title first, so any failures print correctly.
     if resource.type.to_s.downcase.to_sym == self.class.name
       self.title = resource.title
     else
       # This should only ever happen for components
       self.title = resource.ref
     end
 
     [:file, :line, :catalog, :exported, :virtual].each do |getter|
       setter = getter.to_s + "="
       if val = resource.send(getter)
         self.send(setter, val)
       end
     end
 
     @tags = resource.tags
 
     @original_parameters = resource.to_hash
 
     set_name(@original_parameters)
 
     set_default(:provider)
 
     set_parameters(@original_parameters)
 
     self.validate if self.respond_to?(:validate)
   end
 
   private
 
   # Set our resource's name.
   def set_name(hash)
     self[name_var] = hash.delete(name_var) if name_var
   end
 
   # Set all of the parameters from a hash, in the appropriate order.
   def set_parameters(hash)
     # Use the order provided by allattrs, but add in any
     # extra attributes from the resource so we get failures
     # on invalid attributes.
     no_values = []
     (self.class.allattrs + hash.keys).uniq.each do |attr|
       begin
         # Set any defaults immediately.  This is mostly done so
         # that the default provider is available for any other
         # property validation.
         if hash.has_key?(attr)
           self[attr] = hash[attr]
         else
           no_values << attr
         end
       rescue ArgumentError, Puppet::Error, TypeError
         raise
       rescue => detail
         error = Puppet::DevError.new( "Could not set #{attr} on #{self.class.name}: #{detail}")
         error.set_backtrace(detail.backtrace)
         raise error
       end
     end
     no_values.each do |attr|
       set_default(attr)
     end
   end
 
   public
 
   # Set up all of our autorequires.
   def finish
     # Make sure all of our relationships are valid.  Again, must be done
     # when the entire catalog is instantiated.
     self.class.relationship_params.collect do |klass|
       if param = @parameters[klass.name]
         param.validate_relationship
       end
     end.flatten.reject { |r| r.nil? }
   end
 
   # For now, leave the 'name' method functioning like it used to.  Once 'title'
   # works everywhere, I'll switch it.
   def name
     self[:name]
   end
 
   # Look up our parent in the catalog, if we have one.
   def parent
     return nil unless catalog
 
     unless defined?(@parent)
       if parents = catalog.adjacent(self, :direction => :in)
         # We should never have more than one parent, so let's just ignore
         # it if we happen to.
         @parent = parents.shift
       else
         @parent = nil
       end
     end
     @parent
   end
 
   # Return the "type[name]" style reference.
   def ref
     "#{self.class.name.to_s.capitalize}[#{self.title}]"
   end
 
   def self_refresh?
     self.class.self_refresh
   end
 
   # Mark that we're purging.
   def purging
     @purging = true
   end
 
   # Is this resource being purged?  Used by transactions to forbid
   # deletion when there are dependencies.
   def purging?
     if defined?(@purging)
       @purging
     else
       false
     end
   end
 
   # Retrieve the title of an object.  If no title was set separately,
   # then use the object's name.
   def title
     unless @title
       if self.class.validparameter?(name_var)
         @title = self[:name]
       elsif self.class.validproperty?(name_var)
         @title = self.should(name_var)
       else
         self.devfail "Could not find namevar #{name_var} for #{self.class.name}"
       end
     end
 
     @title
   end
 
   # convert to a string
   def to_s
     self.ref
   end
 
   # Convert to a transportable object
   def to_trans(ret = true)
     trans = TransObject.new(self.title, self.class.name)
 
     values = retrieve_resource
     values.each do |name, value|
       name = name.name if name.respond_to? :name
       trans[name] = value
     end
 
     @parameters.each do |name, param|
       # Avoid adding each instance name twice
       next if param.class.isnamevar? and param.value == self.title
 
       # We've already got property values
       next if param.is_a?(Puppet::Property)
       trans[name] = param.value
     end
 
     trans.tags = self.tags
 
     # FIXME I'm currently ignoring 'parent' and 'path'
 
     trans
   end
 
   def to_resource
     # this 'type instance' versus 'resource' distinction seems artificial
     # I'd like to see it collapsed someday ~JW
     self.to_trans.to_resource
   end
 
   def virtual?;  !!@virtual;  end
   def exported?; !!@exported; end
 end
 end
 
 require 'puppet/provider'
 
 # Always load these types.
 require 'puppet/type/component'
diff --git a/lib/puppet/type/cron.rb b/lib/puppet/type/cron.rb
index 76399d693..4b18e71f9 100755
--- a/lib/puppet/type/cron.rb
+++ b/lib/puppet/type/cron.rb
@@ -1,414 +1,410 @@
 require 'etc'
 require 'facter'
 require 'puppet/util/filetype'
 
 Puppet::Type.newtype(:cron) do
   @doc = "Installs and manages cron jobs.  All fields except the command
     and the user are optional, although specifying no periodic
     fields would result in the command being executed every
     minute.  While the name of the cron job is not part of the actual
     job, it is used by Puppet to store and retrieve it.
 
     If you specify a cron job that matches an existing job in every way
     except name, then the jobs will be considered equivalent and the
     new name will be permanently associated with that job.  Once this
     association is made and synced to disk, you can then manage the job
     normally (e.g., change the schedule of the job).
 
     Example:
 
         cron { logrotate:
           command => \"/usr/sbin/logrotate\",
           user => root,
           hour => 2,
           minute => 0
         }
 
     Note that all cron values can be specified as an array of values:
 
         cron { logrotate:
           command => \"/usr/sbin/logrotate\",
           user => root,
           hour => [2, 4]
         }
 
     Or using ranges, or the step syntax `*/2` (although there's no guarantee that
     your `cron` daemon supports it):
 
         cron { logrotate:
           command => \"/usr/sbin/logrotate\",
           user => root,
           hour => ['2-4'],
           minute => '*/10'
         }
     "
   ensurable
 
   # A base class for all of the Cron parameters, since they all have
   # similar argument checking going on.
   class CronParam < Puppet::Property
     class << self
       attr_accessor :boundaries, :default
     end
 
     # We have to override the parent method, because we consume the entire
     # "should" array
     def insync?(is)
-      if @should
-        self.is_to_s(is) == self.should_to_s
-      else
-        true
-      end
+      self.is_to_s(is) == self.should_to_s
     end
 
     # A method used to do parameter input handling.  Converts integers
     # in string form to actual integers, and returns the value if it's
     # an integer or false if it's just a normal string.
     def numfix(num)
       if num =~ /^\d+$/
         return num.to_i
       elsif num.is_a?(Integer)
         return num
       else
         return false
       end
     end
 
     # Verify that a number is within the specified limits.  Return the
     # number if it is, or false if it is not.
     def limitcheck(num, lower, upper)
       (num >= lower and num <= upper) && num
     end
 
     # Verify that a value falls within the specified array.  Does case
     # insensitive matching, and supports matching either the entire word
     # or the first three letters of the word.
     def alphacheck(value, ary)
       tmp = value.downcase
 
       # If they specified a shortened version of the name, then see
       # if we can lengthen it (e.g., mon => monday).
       if tmp.length == 3
         ary.each_with_index { |name, index|
           if name =~ /#{tmp}/i
             return index
           end
         }
       else
         return ary.index(tmp) if ary.include?(tmp)
       end
 
       false
     end
 
     def should_to_s(newvalue = @should)
       if newvalue
         newvalue = [newvalue] unless newvalue.is_a?(Array)
         if self.name == :command or newvalue[0].is_a? Symbol
           newvalue[0]
         else
           newvalue.join(",")
         end
       else
         nil
       end
     end
 
     def is_to_s(currentvalue = @is)
       if currentvalue
         return currentvalue unless currentvalue.is_a?(Array)
 
         if self.name == :command or currentvalue[0].is_a? Symbol
           currentvalue[0]
         else
           currentvalue.join(",")
         end
       else
         nil
       end
     end
 
     def should
       if @should and @should[0] == :absent
         :absent
       else
         @should
       end
     end
 
     def should=(ary)
       super
       @should.flatten!
     end
 
     # The method that does all of the actual parameter value
     # checking; called by all of the +param<name>=+ methods.
     # Requires the value, type, and bounds, and optionally supports
     # a boolean of whether to do alpha checking, and if so requires
     # the ary against which to do the checking.
     munge do |value|
       # Support 'absent' as a value, so that they can remove
       # a value
       if value == "absent" or value == :absent
         return :absent
       end
 
       # Allow the */2 syntax
       if value =~ /^\*\/[0-9]+$/
         return value
       end
 
       # Allow ranges
       if value =~ /^[0-9]+-[0-9]+$/
         return value
       end
 
       # Allow ranges + */2
       if value =~ /^[0-9]+-[0-9]+\/[0-9]+$/
         return value
       end
 
       if value == "*"
         return :absent
       end
 
       return value unless self.class.boundaries
       lower, upper = self.class.boundaries
       retval = nil
       if num = numfix(value)
         retval = limitcheck(num, lower, upper)
       elsif respond_to?(:alpha)
         # If it has an alpha method defined, then we check
         # to see if our value is in that list and if so we turn
         # it into a number
         retval = alphacheck(value, alpha)
       end
 
       if retval
         return retval.to_s
       else
         self.fail "#{value} is not a valid #{self.class.name}"
       end
     end
   end
 
   # Somewhat uniquely, this property does not actually change anything -- it
   # just calls +@resource.sync+, which writes out the whole cron tab for
   # the user in question.  There is no real way to change individual cron
   # jobs without rewriting the entire cron file.
   #
   # Note that this means that managing many cron jobs for a given user
   # could currently result in multiple write sessions for that user.
   newproperty(:command, :parent => CronParam) do
     desc "The command to execute in the cron job.  The environment
       provided to the command varies by local system rules, and it is
       best to always provide a fully qualified command.  The user's
       profile is not sourced when the command is run, so if the
       user's environment is desired it should be sourced manually.
 
       All cron parameters support `absent` as a value; this will
       remove any existing values for that field."
 
     def retrieve
       return_value = super
       return_value = return_value[0] if return_value && return_value.is_a?(Array)
 
       return_value
     end
 
     def should
       if @should
         if @should.is_a? Array
           @should[0]
         else
           devfail "command is not an array"
         end
       else
         nil
       end
     end
   end
 
   newproperty(:special) do
     desc "Special schedules"
 
     def specials
       %w{reboot yearly annually monthly weekly daily midnight hourly}
     end
 
     validate do |value|
       raise ArgumentError, "Invalid special schedule #{value.inspect}" unless specials.include?(value)
     end
   end
 
   newproperty(:minute, :parent => CronParam) do
     self.boundaries = [0, 59]
     desc "The minute at which to run the cron job.
       Optional; if specified, must be between 0 and 59, inclusive."
   end
 
   newproperty(:hour, :parent => CronParam) do
     self.boundaries = [0, 23]
     desc "The hour at which to run the cron job. Optional;
       if specified, must be between 0 and 23, inclusive."
   end
 
   newproperty(:weekday, :parent => CronParam) do
     def alpha
       %w{sunday monday tuesday wednesday thursday friday saturday}
     end
     self.boundaries = [0, 7]
     desc "The weekday on which to run the command.
       Optional; if specified, must be between 0 and 7, inclusive, with
       0 (or 7) being Sunday, or must be the name of the day (e.g., Tuesday)."
   end
 
   newproperty(:month, :parent => CronParam) do
     def alpha
       %w{january february march april may june july
         august september october november december}
     end
     self.boundaries = [1, 12]
     desc "The month of the year.  Optional; if specified
       must be between 1 and 12 or the month name (e.g., December)."
   end
 
   newproperty(:monthday, :parent => CronParam) do
     self.boundaries = [1, 31]
     desc "The day of the month on which to run the
       command.  Optional; if specified, must be between 1 and 31."
   end
 
   newproperty(:environment) do
     desc "Any environment settings associated with this cron job.  They
       will be stored between the header and the job in the crontab.  There
       can be no guarantees that other, earlier settings will not also
       affect a given cron job.
 
 
       Also, Puppet cannot automatically determine whether an existing,
       unmanaged environment setting is associated with a given cron
       job.  If you already have cron jobs with environment settings,
       then Puppet will keep those settings in the same place in the file,
       but will not associate them with a specific job.
 
       Settings should be specified exactly as they should appear in
       the crontab, e.g., `PATH=/bin:/usr/bin:/usr/sbin`."
 
     validate do |value|
       unless value =~ /^\s*(\w+)\s*=\s*(.*)\s*$/ or value == :absent or value == "absent"
         raise ArgumentError, "Invalid environment setting #{value.inspect}"
       end
     end
 
     def insync?(is)
       if is.is_a? Array
         return is.sort == @should.sort
       else
         return is == @should
       end
     end
 
     def is_to_s(newvalue)
       if newvalue
         if newvalue.is_a?(Array)
           newvalue.join(",")
         else
           newvalue
         end
       else
         nil
       end
     end
 
     def should
       @should
     end
 
     def should_to_s(newvalue = @should)
       if newvalue
         newvalue.join(",")
       else
         nil
       end
     end
   end
 
   newparam(:name) do
     desc "The symbolic name of the cron job.  This name
       is used for human reference only and is generated automatically
       for cron jobs found on the system.  This generally won't
       matter, as Puppet will do its best to match existing cron jobs
       against specified jobs (and Puppet adds a comment to cron jobs it adds), but it is at least possible that converting from
       unmanaged jobs to managed jobs might require manual
       intervention."
 
     isnamevar
   end
 
   newproperty(:user) do
     desc "The user to run the command as.  This user must
       be allowed to run cron jobs, which is not currently checked by
       Puppet.
 
       The user defaults to whomever Puppet is running as."
 
     defaultto { Etc.getpwuid(Process.uid).name || "root" }
   end
 
   newproperty(:target) do
     desc "Where the cron job should be stored.  For crontab-style
       entries this is the same as the user and defaults that way.
       Other providers default accordingly."
 
     defaultto {
       if provider.is_a?(@resource.class.provider(:crontab))
         if val = @resource.should(:user)
           val
         else
           raise ArgumentError,
             "You must provide a user with crontab entries"
         end
       elsif provider.class.ancestors.include?(Puppet::Provider::ParsedFile)
         provider.class.default_target
       else
         nil
       end
     }
   end
 
   # We have to reorder things so that :provide is before :target
 
   attr_accessor :uid
 
   def value(name)
     name = symbolize(name)
     ret = nil
     if obj = @parameters[name]
       ret = obj.should
 
       ret ||= obj.retrieve
 
       if ret == :absent
         ret = nil
       end
     end
 
     unless ret
       case name
       when :command
         devfail "No command, somehow"
       when :special
         # nothing
       else
         #ret = (self.class.validproperty?(name).default || "*").to_s
         ret = "*"
       end
     end
 
     ret
   end
 end
 
 
 
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index eee948cd5..0d69446b4 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -1,801 +1,801 @@
 require 'digest/md5'
 require 'cgi'
 require 'etc'
 require 'uri'
 require 'fileutils'
 require 'puppet/network/handler'
 require 'puppet/util/diff'
 require 'puppet/util/checksums'
 require 'puppet/network/client'
 require 'puppet/util/backups'
 
 Puppet::Type.newtype(:file) do
   include Puppet::Util::MethodHelper
   include Puppet::Util::Checksums
   include Puppet::Util::Backups
   @doc = "Manages local files, including setting ownership and
     permissions, creation of both files and directories, and
     retrieving entire files from remote servers.  As Puppet matures, it
     expected that the `file` resource will be used less and less to
     manage content, and instead native resources will be used to do so.
 
     If you find that you are often copying files in from a central
     location, rather than using native resources, please contact
     Puppet Labs and we can hopefully work with you to develop a
     native resource to support what you are doing."
 
   def self.title_patterns
     [ [ /^(.*?)\/*\Z/m, [ [ :path, lambda{|x| x} ] ] ] ]
   end
 
   newparam(:path) do
     desc "The path to the file to manage.  Must be fully qualified."
     isnamevar
 
     validate do |value|
       # accept various path syntaxes: lone slash, posix, win32, unc
       unless (Puppet.features.posix? and (value =~ /^\/$/ or value =~ /^\/[^\/]/)) or (Puppet.features.microsoft_windows? and (value =~ /^.:\// or value =~ /^\/\/[^\/]+\/[^\/]+/))
         fail Puppet::Error, "File paths must be fully qualified, not '#{value}'"
       end
     end
 
     # convert the current path in an index into the collection and the last
     # path name. The aim is to use less storage for all common paths in a hierarchy
     munge do |value|
       path, name = File.split(value.gsub(/\/+/,'/'))
       { :index => Puppet::FileCollection.collection.index(path), :name => name }
     end
 
     # and the reverse
     unmunge do |value|
       basedir = Puppet::FileCollection.collection.path(value[:index])
       # a lone slash as :name indicates a root dir on windows
       if value[:name] == '/'
         basedir
       else
         File.join( basedir, value[:name] )
       end
     end
   end
 
   newparam(:backup) do
     desc "Whether files should be backed up before
       being replaced.  The preferred method of backing files up is via
       a `filebucket`, which stores files by their MD5 sums and allows
       easy retrieval without littering directories with backups.  You
       can specify a local filebucket or a network-accessible
       server-based filebucket by setting `backup => bucket-name`.
       Alternatively, if you specify any value that begins with a `.`
       (e.g., `.puppet-bak`), then Puppet will use copy the file in
       the same directory with that value as the extension of the
       backup. Setting `backup => false` disables all backups of the
       file in question.
 
       Puppet automatically creates a local filebucket named `puppet` and
       defaults to backing up there.  To use a server-based filebucket,
       you must specify one in your configuration
 
             filebucket { main:
               server => puppet
             }
 
       The `puppet master` daemon creates a filebucket by default,
       so you can usually back up to your main server with this
       configuration.  Once you've described the bucket in your
       configuration, you can use it in any file
 
             file { \"/my/file\":
               source => \"/path/in/nfs/or/something\",
               backup => main
             }
 
       This will back the file up to the central server.
 
       At this point, the benefits of using a filebucket are that you do not
       have backup files lying around on each of your machines, a given
       version of a file is only backed up once, and you can restore
       any given file manually, no matter how old.  Eventually,
       transactional support will be able to automatically restore
       filebucketed files.
       "
 
     defaultto "puppet"
 
     munge do |value|
       # I don't really know how this is happening.
       value = value.shift if value.is_a?(Array)
 
       case value
       when false, "false", :false
         false
       when true, "true", ".puppet-bak", :true
         ".puppet-bak"
       when String
         value
       else
         self.fail "Invalid backup type #{value.inspect}"
       end
     end
   end
 
   newparam(:recurse) do
     desc "Whether and how deeply to do recursive
       management."
 
     newvalues(:true, :false, :inf, :remote, /^[0-9]+$/)
 
     # Replace the validation so that we allow numbers in
     # addition to string representations of them.
     validate { |arg| }
     munge do |value|
       newval = super(value)
       case newval
       when :true, :inf; true
       when :false; false
       when :remote; :remote
       when Integer, Fixnum, Bignum
         self.warning "Setting recursion depth with the recurse parameter is now deprecated, please use recurselimit"
 
         # recurse == 0 means no recursion
         return false if value == 0
 
         resource[:recurselimit] = value
         true
       when /^\d+$/
         self.warning "Setting recursion depth with the recurse parameter is now deprecated, please use recurselimit"
         value = Integer(value)
 
         # recurse == 0 means no recursion
         return false if value == 0
 
         resource[:recurselimit] = value
         true
       else
         self.fail "Invalid recurse value #{value.inspect}"
       end
     end
   end
 
   newparam(:recurselimit) do
     desc "How deeply to do recursive management."
 
     newvalues(/^[0-9]+$/)
 
     munge do |value|
       newval = super(value)
       case newval
       when Integer, Fixnum, Bignum; value
       when /^\d+$/; Integer(value)
       else
         self.fail "Invalid recurselimit value #{value.inspect}"
       end
     end
   end
 
   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."
     newvalues(:true, :false)
     aliasvalue(:yes, :true)
     aliasvalue(:no, :false)
     defaultto :true
   end
 
   newparam(:force, :boolean => true) do
     desc "Force the file operation.  Currently only used when replacing
       directories with links."
     newvalues(:true, :false)
     defaultto false
   end
 
   newparam(:ignore) do
     desc "A parameter which omits action on files matching
       specified patterns during recursion.  Uses Ruby's builtin globbing
       engine, so shell metacharacters are fully supported, e.g. `[a-z]*`.
       Matches that would descend into the directory structure are ignored,
       e.g., `*/*`."
 
     validate do |value|
       unless value.is_a?(Array) or value.is_a?(String) or value == false
         self.devfail "Ignore must be a string or an Array"
       end
     end
   end
 
   newparam(:links) do
     desc "How to handle links during file actions.  During file copying,
       `follow` will copy the target file instead of the link, `manage`
       will copy the link itself, and `ignore` will just pass it by.
       When not copying, `manage` and `ignore` behave equivalently
       (because you cannot really ignore links entirely during local recursion), and `follow` will manage the file to which the
       link points."
 
     newvalues(:follow, :manage)
 
     defaultto :manage
   end
 
   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
       files unless you really know what you are doing.  This option only
       makes sense when recursively managing directories.
 
       Note that when using `purge` with `source`, Puppet will purge any files
       that are not on the remote system."
 
     defaultto :false
 
     newvalues(:true, :false)
   end
 
   newparam(:sourceselect) do
     desc "Whether to copy all valid sources, or just the first one.  This parameter
       is only used in recursive copies; by default, the first valid source is the
       only one used as a recursive source, but if this parameter is set to `all`,
       then all valid sources will have all of their contents copied to the local host,
       and for sources that have the same file, the source earlier in the list will
       be used."
 
     defaultto :first
 
     newvalues(:first, :all)
   end
 
   # Autorequire any parent directories.
   autorequire(:file) do
     basedir = File.dirname(self[:path])
     if basedir != self[:path]
       basedir
     else
       nil
     end
   end
 
   # Autorequire the owner and group of the file.
   {:user => :owner, :group => :group}.each do |type, property|
     autorequire(type) do
       if @parameters.include?(property)
         # The user/group property automatically converts to IDs
         next unless should = @parameters[property].shouldorig
         val = should[0]
         if val.is_a?(Integer) or val =~ /^\d+$/
           nil
         else
           val
         end
       end
     end
   end
 
   CREATORS = [:content, :source, :target]
 
   validate do
     count = 0
     CREATORS.each do |param|
       count += 1 if self.should(param)
     end
     count += 1 if @parameters.include?(:source)
     self.fail "You cannot specify more than one of #{CREATORS.collect { |p| p.to_s}.join(", ")}" if count > 1
 
     self.fail "You cannot specify a remote recursion without a source" if !self[:source] and self[:recurse] == :remote
 
     self.warning "Possible error: recurselimit is set but not recurse, no recursion will happen" if !self[:recurse] and self[:recurselimit]
   end
 
   def self.[](path)
     return nil unless path
     super(path.gsub(/\/+/, '/').sub(/\/$/, ''))
   end
 
   # List files, but only one level deep.
   def self.instances(base = "/")
     return [] unless FileTest.directory?(base)
 
     files = []
     Dir.entries(base).reject { |e|
       e == "." or e == ".."
     }.each do |name|
       path = File.join(base, name)
       if obj = self[path]
         obj[:audit] = :all
         files << obj
       else
         files << self.new(
           :name => path, :audit => :all
         )
       end
     end
     files
   end
 
   @depthfirst = false
 
   # Determine the user to write files as.
   def asuser
     if self.should(:owner) and ! self.should(:owner).is_a?(Symbol)
       writeable = Puppet::Util::SUIDManager.asuser(self.should(:owner)) {
         FileTest.writable?(File.dirname(self[:path]))
       }
 
       # If the parent directory is writeable, then we execute
       # as the user in question.  Otherwise we'll rely on
       # the 'owner' property to do things.
       asuser = self.should(:owner) if writeable
     end
 
     asuser
   end
 
   def bucket
     return @bucket if @bucket
 
     backup = self[:backup]
     return nil unless backup
     return nil if backup =~ /^\./
 
     unless catalog or backup == "puppet"
       fail "Can not find filebucket for backups without a catalog"
     end
 
     unless catalog and filebucket = catalog.resource(:filebucket, backup) or backup == "puppet"
       fail "Could not find filebucket #{backup} specified in backup"
     end
 
     return default_bucket unless filebucket
 
     @bucket = filebucket.bucket
 
     @bucket
   end
 
   def default_bucket
     Puppet::Type.type(:filebucket).mkdefaultbucket.bucket
   end
 
   # Does the file currently exist?  Just checks for whether
   # we have a stat
   def exist?
     stat ? true : false
   end
 
   # We have to do some extra finishing, to retrieve our bucket if
   # there is one.
   def finish
     # Look up our bucket, if there is one
     bucket
     super
   end
 
   # Create any children via recursion or whatever.
   def eval_generate
     return [] unless self.recurse?
 
     recurse
     #recurse.reject do |resource|
     #    catalog.resource(:file, resource[:path])
     #end.each do |child|
     #    catalog.add_resource child
     #    catalog.relationship_graph.add_edge self, child
     #end
   end
 
   def flush
     # We want to make sure we retrieve metadata anew on each transaction.
     @parameters.each do |name, param|
       param.flush if param.respond_to?(:flush)
     end
     @stat = nil
   end
 
   def initialize(hash)
     # Used for caching clients
     @clients = {}
 
     super
 
     # If they've specified a source, we get our 'should' values
     # from it.
     unless self[:ensure]
       if self[:target]
         self[:ensure] = :symlink
       elsif self[:content]
         self[:ensure] = :file
       end
     end
 
     @stat = nil
   end
 
   # Configure discovered resources to be purged.
   def mark_children_for_purging(children)
     children.each do |name, child|
       next if child[:source]
       child[:ensure] = :absent
     end
   end
 
   # Create a new file or directory object as a child to the current
   # object.
   def newchild(path)
     full_path = File.join(self[:path], path)
 
     # Add some new values to our original arguments -- these are the ones
     # set at initialization.  We specifically want to exclude any param
     # values set by the :source property or any default values.
     # LAK:NOTE This is kind of silly, because the whole point here is that
     # the values set at initialization should live as long as the resource
     # but values set by default or by :source should only live for the transaction
     # or so.  Unfortunately, we don't have a straightforward way to manage
     # the different lifetimes of this data, so we kludge it like this.
     # The right-side hash wins in the merge.
     options = @original_parameters.merge(:path => full_path).reject { |param, value| value.nil? }
 
     # These should never be passed to our children.
     [:parent, :ensure, :recurse, :recurselimit, :target, :alias, :source].each do |param|
       options.delete(param) if options.include?(param)
     end
 
     self.class.new(options)
   end
 
   # Files handle paths specially, because they just lengthen their
   # path names, rather than including the full parent's title each
   # time.
   def pathbuilder
     # We specifically need to call the method here, so it looks
     # up our parent in the catalog graph.
     if parent = parent()
       # We only need to behave specially when our parent is also
       # a file
       if parent.is_a?(self.class)
         # Remove the parent file name
         list = parent.pathbuilder
         list.pop # remove the parent's path info
         return list << self.ref
       else
         return super
       end
     else
       return [self.ref]
     end
   end
 
   # Should we be purging?
   def purge?
     @parameters.include?(:purge) and (self[:purge] == :true or self[:purge] == "true")
   end
 
   # Recursively generate a list of file resources, which will
   # be used to copy remote files, manage local files, and/or make links
   # to map to another directory.
   def recurse
     children = {}
     children = recurse_local if self[:recurse] != :remote
 
     if self[:target]
       recurse_link(children)
     elsif self[:source]
       recurse_remote(children)
     end
 
     # If we're purging resources, then delete any resource that isn't on the
     # remote system.
     mark_children_for_purging(children) if self.purge?
 
     result = children.values.sort { |a, b| a[:path] <=> b[:path] }
     remove_less_specific_files(result)
   end
 
   # This is to fix bug #2296, where two files recurse over the same
   # set of files.  It's a rare case, and when it does happen you're
   # not likely to have many actual conflicts, which is good, because
   # this is a pretty inefficient implementation.
   def remove_less_specific_files(files)
     mypath = self[:path].split(File::Separator)
     other_paths = catalog.vertices.
       select  { |r| r.is_a?(self.class) and r[:path] != self[:path] }.
       collect { |r| r[:path].split(File::Separator) }.
       select  { |p| p[0,mypath.length]  == mypath }
 
     return files if other_paths.empty?
 
     files.reject { |file|
       path = file[:path].split(File::Separator)
       other_paths.any? { |p| path[0,p.length] == p }
       }
   end
 
   # A simple method for determining whether we should be recursing.
   def recurse?
     return false unless @parameters.include?(:recurse)
 
     val = @parameters[:recurse].value
 
     !!(val and (val == true or val == :remote))
   end
 
   # Recurse the target of the link.
   def recurse_link(children)
     perform_recursion(self[:target]).each do |meta|
       if meta.relative_path == "."
         self[:ensure] = :directory
         next
       end
 
       children[meta.relative_path] ||= newchild(meta.relative_path)
       if meta.ftype == "directory"
         children[meta.relative_path][:ensure] = :directory
       else
         children[meta.relative_path][:ensure] = :link
         children[meta.relative_path][:target] = meta.full_path
       end
     end
     children
   end
 
   # Recurse the file itself, returning a Metadata instance for every found file.
   def recurse_local
     result = perform_recursion(self[:path])
     return {} unless result
     result.inject({}) do |hash, meta|
       next hash if meta.relative_path == "."
 
       hash[meta.relative_path] = newchild(meta.relative_path)
       hash
     end
   end
 
   # Recurse against our remote file.
   def recurse_remote(children)
     sourceselect = self[:sourceselect]
 
     total = self[:source].collect do |source|
       next unless result = perform_recursion(source)
       return if top = result.find { |r| r.relative_path == "." } and top.ftype != "directory"
       result.each { |data| data.source = "#{source}/#{data.relative_path}" }
       break result if result and ! result.empty? and sourceselect == :first
       result
     end.flatten
 
     # This only happens if we have sourceselect == :all
     unless sourceselect == :first
       found = []
       total.reject! do |data|
         result = found.include?(data.relative_path)
         found << data.relative_path unless found.include?(data.relative_path)
         result
       end
     end
 
     total.each do |meta|
       if meta.relative_path == "."
         parameter(:source).metadata = meta
         next
       end
       children[meta.relative_path] ||= newchild(meta.relative_path)
       children[meta.relative_path][:source] = meta.source
       children[meta.relative_path][:checksum] = :md5 if meta.ftype == "file"
 
       children[meta.relative_path].parameter(:source).metadata = meta
     end
 
     children
   end
 
   def perform_recursion(path)
 
     Puppet::FileServing::Metadata.search(
 
       path,
       :links => self[:links],
       :recurse => (self[:recurse] == :remote ? true : self[:recurse]),
 
       :recurselimit => self[:recurselimit],
       :ignore => self[:ignore],
       :checksum_type => (self[:source] || self[:content]) ? self[:checksum] : :none
     )
   end
 
   # Remove any existing data.  This is only used when dealing with
   # links or directories.
   def remove_existing(should)
     return unless s = stat
 
     self.fail "Could not back up; will not replace" unless perform_backup
 
     unless should.to_s == "link"
       return if s.ftype.to_s == should.to_s
     end
 
     case s.ftype
     when "directory"
       if self[:force] == :true
         debug "Removing existing directory for replacement with #{should}"
         FileUtils.rmtree(self[:path])
       else
         notice "Not removing directory; use 'force' to override"
       end
     when "link", "file"
       debug "Removing existing #{s.ftype} for replacement with #{should}"
       File.unlink(self[:path])
     else
       self.fail "Could not back up files of type #{s.ftype}"
     end
     expire
   end
 
   def retrieve
     if source = parameter(:source)
       source.copy_source_values
     end
     super
   end
 
   # Set the checksum, from another property.  There are multiple
   # properties that modify the contents of a file, and they need the
   # ability to make sure that the checksum value is in sync.
   def setchecksum(sum = nil)
     if @parameters.include? :checksum
       if sum
         @parameters[:checksum].checksum = sum
       else
         # If they didn't pass in a sum, then tell checksum to
         # figure it out.
         currentvalue = @parameters[:checksum].retrieve
         @parameters[:checksum].checksum = currentvalue
       end
     end
   end
 
   # Should this thing be a normal file?  This is a relatively complex
   # way of determining whether we're trying to create a normal file,
   # and it's here so that the logic isn't visible in the content property.
   def should_be_file?
     return true if self[:ensure] == :file
 
     # I.e., it's set to something like "directory"
     return false if e = self[:ensure] and e != :present
 
     # The user doesn't really care, apparently
     if self[:ensure] == :present
       return true unless s = stat
       return(s.ftype == "file" ? true : false)
     end
 
     # If we've gotten here, then :ensure isn't set
     return true if self[:content]
     return true if stat and stat.ftype == "file"
     false
   end
 
   # Stat our file.  Depending on the value of the 'links' attribute, we
   # use either 'stat' or 'lstat', and we expect the properties to use the
   # resulting stat object accordingly (mostly by testing the 'ftype'
   # value).
   cached_attr(:stat) do
     method = :stat
 
     # Files are the only types that support links
     if (self.class.name == :file and self[:links] != :follow) or self.class.name == :tidy
       method = :lstat
     end
     path = self[:path]
 
     begin
       File.send(method, self[:path])
     rescue Errno::ENOENT => error
       return nil
     rescue Errno::EACCES => error
       warning "Could not stat; permission denied"
       return nil
     end
   end
 
   # We have to hack this just a little bit, because otherwise we'll get
   # an error when the target and the contents are created as properties on
   # the far side.
   def to_trans(retrieve = true)
     obj = super
     obj.delete(:target) if obj[:target] == :notlink
     obj
   end
 
   # Write out the file.  Requires the property name for logging.
   # Write will be done by the content property, along with checksum computation
   def write(property)
     remove_existing(:file)
 
     use_temporary_file = write_temporary_file?
     if use_temporary_file
       path = "#{self[:path]}.puppettmp_#{rand(10000)}"
       path = "#{self[:path]}.puppettmp_#{rand(10000)}" while File.exists?(path) or File.symlink?(path)
     else
       path = self[:path]
     end
 
     mode = self.should(:mode) # might be nil
     umask = mode ? 000 : 022
     mode_int = mode ? mode.to_i(8) : nil
 
     content_checksum = Puppet::Util.withumask(umask) { File.open(path, 'w', mode_int ) { |f| write_content(f) } }
 
     # And put our new file in place
     if use_temporary_file # This is only not true when our file is empty.
       begin
         fail_if_checksum_is_wrong(path, content_checksum) if validate_checksum?
         File.rename(path, self[:path])
       rescue => detail
         fail "Could not rename temporary file #{path} to #{self[:path]}: #{detail}"
       ensure
         # Make sure the created file gets removed
         File.unlink(path) if FileTest.exists?(path)
       end
     end
 
     # make sure all of the modes are actually correct
     property_fix
 
   end
 
   private
 
   # Should we validate the checksum of the file we're writing?
   def validate_checksum?
     self[:checksum] !~ /time/
   end
 
   # Make sure the file we wrote out is what we think it is.
   def fail_if_checksum_is_wrong(path, content_checksum)
     newsum = parameter(:checksum).sum_file(path)
     return if [:absent, nil, content_checksum].include?(newsum)
 
     self.fail "File written to disk did not match checksum; discarding changes (#{content_checksum} vs #{newsum})"
   end
 
   # write the current content. Note that if there is no content property
   # simply opening the file with 'w' as done in write is enough to truncate
   # or write an empty length file.
   def write_content(file)
     (content = property(:content)) && content.write(file)
   end
 
   private
 
   def write_temporary_file?
     # unfortunately we don't know the source file size before fetching it
     # so let's assume the file won't be empty
     (c = property(:content) and c.length) || (s = @parameters[:source] and 1)
   end
 
   # There are some cases where all of the work does not get done on
   # file creation/modification, so we have to do some extra checking.
   def property_fix
     properties.each do |thing|
       next unless [:mode, :owner, :group, :seluser, :selrole, :seltype, :selrange].include?(thing.name)
 
       # Make sure we get a new stat objct
       expire
       currentvalue = thing.retrieve
-      thing.sync unless thing.insync?(currentvalue)
+      thing.sync unless thing.safe_insync?(currentvalue)
     end
   end
 end
 
 # We put all of the properties in separate files, because there are so many
 # of them.  The order these are loaded is important, because it determines
 # the order they are in the property lit.
 require 'puppet/type/file/checksum'
 require 'puppet/type/file/content'     # can create the file
 require 'puppet/type/file/source'      # can create the file
 require 'puppet/type/file/target'      # creates a different type of file
 require 'puppet/type/file/ensure'      # can create the file
 require 'puppet/type/file/owner'
 require 'puppet/type/file/group'
 require 'puppet/type/file/mode'
 require 'puppet/type/file/type'
 require 'puppet/type/file/selcontext'  # SELinux file context
 require 'puppet/type/file/ctime'
 require 'puppet/type/file/mtime'
diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb
index b8f30a9c7..04b917a7e 100755
--- a/lib/puppet/type/file/content.rb
+++ b/lib/puppet/type/file/content.rb
@@ -1,216 +1,215 @@
 require 'net/http'
 require 'uri'
 require 'tempfile'
 
 require 'puppet/util/checksums'
 require 'puppet/network/http/api/v1'
 require 'puppet/network/http/compression'
 
 module Puppet
   Puppet::Type.type(:file).newproperty(:content) do
     include Puppet::Util::Diff
     include Puppet::Util::Checksums
     include Puppet::Network::HTTP::API::V1
     include Puppet::Network::HTTP::Compression.module
 
     attr_reader :actual_content
 
     desc "Specify the contents of a file as a string.  Newlines, tabs, and
       spaces can be specified using the escaped syntax (e.g., \\n for a newline).  The primary purpose of this parameter is to provide a
       kind of limited templating::
 
         define resolve(nameserver1, nameserver2, domain, search) {
           $str = \"search $search
         domain $domain
         nameserver $nameserver1
         nameserver $nameserver2
         \"
 
           file { \"/etc/resolv.conf\":
             content => $str
           }
         }
 
       This attribute is especially useful when used with
       `PuppetTemplating templating`:trac:."
 
     # Store a checksum as the value, rather than the actual content.
     # Simplifies everything.
     munge do |value|
       if value == :absent
         value
       elsif checksum?(value)
         # XXX This is potentially dangerous because it means users can't write a file whose
         # entire contents are a plain checksum
         value
       else
         @actual_content = value
         resource.parameter(:checksum).sum(value)
       end
     end
 
     # Checksums need to invert how changes are printed.
     def change_to_s(currentvalue, newvalue)
       # Our "new" checksum value is provided by the source.
       if source = resource.parameter(:source) and tmp = source.checksum
         newvalue = tmp
       end
       if currentvalue == :absent
         return "defined content as '#{newvalue}'"
       elsif newvalue == :absent
         return "undefined content from '#{currentvalue}'"
       else
         return "content changed '#{currentvalue}' to '#{newvalue}'"
       end
     end
 
     def checksum_type
       if source = resource.parameter(:source)
         result = source.checksum
       else checksum = resource.parameter(:checksum)
         result = resource[:checksum]
       end
       if result =~ /^\{(\w+)\}.+/
         return $1.to_sym
       else
         return result
       end
     end
 
     def length
       (actual_content and actual_content.length) || 0
     end
 
     def content
       self.should
     end
 
     # Override this method to provide diffs if asked for.
     # Also, fix #872: when content is used, and replace is true, the file
     # should be insync when it exists
     def insync?(is)
       if resource.should_be_file?
         return false if is == :absent
       else
         return true
       end
 
       return true if ! @resource.replace?
-      return true unless self.should
 
       result = super
 
       if ! result and Puppet[:show_diff]
         write_temporarily do |path|
           print diff(@resource[:path], path)
         end
       end
       result
     end
 
     def retrieve
       return :absent unless stat = @resource.stat
       ftype = stat.ftype
       # Don't even try to manage the content on directories or links
       return nil if ["directory","link"].include?(ftype)
 
       begin
         resource.parameter(:checksum).sum_file(resource[:path])
       rescue => detail
         raise Puppet::Error, "Could not read #{ftype} #{@resource.title}: #{detail}"
       end
     end
 
     # Make sure we're also managing the checksum property.
     def should=(value)
       @resource.newattr(:checksum) unless @resource.parameter(:checksum)
       super
     end
 
     # Just write our content out to disk.
     def sync
       return_event = @resource.stat ? :file_changed : :file_created
 
       # We're safe not testing for the 'source' if there's no 'should'
       # because we wouldn't have gotten this far if there weren't at least
       # one valid value somewhere.
       @resource.write(:content)
 
       return_event
     end
 
     def write_temporarily
       tempfile = Tempfile.new("puppet-file")
       tempfile.open
 
       write(tempfile)
 
       tempfile.close
 
       yield tempfile.path
 
       tempfile.delete
     end
 
     def write(file)
       resource.parameter(:checksum).sum_stream { |sum|
         each_chunk_from(actual_content || resource.parameter(:source)) { |chunk|
           sum << chunk
           file.print chunk
         }
       }
     end
 
     def self.standalone?
       Puppet.settings[:name] == "apply"
     end
 
     def each_chunk_from(source_or_content)
       if source_or_content.is_a?(String)
         yield source_or_content
       elsif source_or_content.nil?
         yield read_file_from_filebucket
       elsif self.class.standalone?
         yield source_or_content.content
       elsif source_or_content.local?
         chunk_file_from_disk(source_or_content) { |chunk| yield chunk }
       else
         chunk_file_from_source(source_or_content) { |chunk| yield chunk }
       end
     end
 
     private
 
     def chunk_file_from_disk(source_or_content)
       File.open(source_or_content.full_path, "r") do |src|
         while chunk = src.read(8192)
           yield chunk
         end
       end
     end
 
     def chunk_file_from_source(source_or_content)
       request = Puppet::Indirector::Request.new(:file_content, :find, source_or_content.full_path.sub(/^\//,''))
       connection = Puppet::Network::HttpPool.http_instance(source_or_content.server, source_or_content.port)
       connection.request_get(indirection2uri(request), add_accept_encoding({"Accept" => "raw"})) do |response|
         case response.code
         when "404"; nil
         when /^2/;  uncompress(response) { |uncompressor| response.read_body { |chunk| yield uncompressor.uncompress(chunk) } }
         else
           # Raise the http error if we didn't get a 'success' of some kind.
           message = "Error #{response.code} on SERVER: #{(response.body||'').empty? ? response.message : uncompress_body(response)}"
           raise Net::HTTPError.new(message, response)
         end
       end
     end
 
     def read_file_from_filebucket
       raise "Could not get filebucket from file" unless dipper = resource.bucket
       sum = should.sub(/\{\w+\}/, '')
 
       dipper.getfile(sum)
     rescue => detail
       fail "Could not retrieve content for #{should} from filebucket: #{detail}"
     end
   end
 end
diff --git a/lib/puppet/type/file/target.rb b/lib/puppet/type/file/target.rb
index 9e7229dda..b9fe9213b 100644
--- a/lib/puppet/type/file/target.rb
+++ b/lib/puppet/type/file/target.rb
@@ -1,74 +1,74 @@
 module Puppet
   Puppet::Type.type(:file).newproperty(:target) do
     desc "The target for creating a link.  Currently, symlinks are the
       only type supported."
 
     newvalue(:notlink) do
       # We do nothing if the value is absent
       return :nochange
     end
 
     # Anything else, basically
     newvalue(/./) do
       @resource[:ensure] = :link if ! @resource.should(:ensure)
 
       # Only call mklink if ensure didn't call us in the first place.
       currentensure  = @resource.property(:ensure).retrieve
-      mklink if @resource.property(:ensure).insync?(currentensure)
+      mklink if @resource.property(:ensure).safe_insync?(currentensure)
     end
 
     # Create our link.
     def mklink
       raise Puppet::Error, "Cannot symlink on Microsoft Windows" if Puppet.features.microsoft_windows?
 
       target = self.should
 
       # Clean up any existing objects.  The argument is just for logging,
       # it doesn't determine what's removed.
       @resource.remove_existing(target)
 
       raise Puppet::Error, "Could not remove existing file" if FileTest.exists?(@resource[:path])
 
       Dir.chdir(File.dirname(@resource[:path])) do
         Puppet::Util::SUIDManager.asuser(@resource.asuser) do
           mode = @resource.should(:mode)
           if mode
             Puppet::Util.withumask(000) do
               File.symlink(target, @resource[:path])
             end
           else
             File.symlink(target, @resource[:path])
           end
         end
 
         @resource.send(:property_fix)
 
         :link_created
       end
     end
 
     def insync?(currentvalue)
       if [:nochange, :notlink].include?(self.should) or @resource.recurse?
         return true
       elsif ! @resource.replace? and File.exists?(@resource[:path])
         return true
       else
         return super(currentvalue)
       end
     end
 
 
     def retrieve
       if stat = @resource.stat
         if stat.ftype == "link"
           return File.readlink(@resource[:path])
         else
           return :notlink
         end
       else
         return :absent
       end
     end
   end
 end
 
diff --git a/lib/puppet/type/mount.rb b/lib/puppet/type/mount.rb
index d048c90f1..36fb553f5 100755
--- a/lib/puppet/type/mount.rb
+++ b/lib/puppet/type/mount.rb
@@ -1,224 +1,224 @@
 module Puppet
   # We want the mount to refresh when it changes.
   newtype(:mount, :self_refresh => true) do
     @doc = "Manages mounted filesystems, including putting mount
       information into the mount table. The actual behavior depends
       on the value of the 'ensure' parameter.
 
       Note that if a `mount` receives an event from another resource,
       it will try to remount the filesystems if `ensure` is set to `mounted`."
 
     feature :refreshable, "The provider can remount the filesystem.",
       :methods => [:remount]
 
     # Use the normal parent class, because we actually want to
     # call code when sync is called.
     newproperty(:ensure) do
       desc "Control what to do with this mount. Set this attribute to
         `umounted` to make sure the filesystem is in the filesystem table
         but not mounted (if the filesystem is currently mounted, it will be unmounted).  Set it to `absent` to unmount (if necessary) and remove
         the filesystem from the fstab.  Set to `mounted` to add it to the
         fstab and mount it. Set to `present` to add to fstab but not change
         mount/unmount status"
 
       newvalue(:defined) do
         provider.create
         return :mount_created
       end
 
       aliasvalue :present, :defined
 
       newvalue(:unmounted) do
         if provider.mounted?
           syncothers
           provider.unmount
           return :mount_unmounted
         else
           provider.create
           return :mount_created
         end
       end
 
       newvalue(:absent, :event => :mount_deleted) do
         provider.unmount if provider.mounted?
 
         provider.destroy
       end
 
       newvalue(:mounted, :event => :mount_mounted) do
         # Create the mount point if it does not already exist.
         current_value = self.retrieve
         provider.create if current_value.nil? or current_value == :absent
 
         syncothers
 
         # The fs can be already mounted if it was absent but mounted
         provider.mount unless provider.mounted?
       end
 
       def insync?(is)
         if should == :defined and is != :absent
           true
         else
           super
         end
       end
 
       def retrieve
         # We need to special case :mounted; if we're absent, we still
         # want
         curval = super()
         if curval == :absent
           return :absent
         elsif provider.mounted?
           return :mounted
         else
           return :unmounted
         end
       end
 
       def syncothers
         # We have to flush any changes to disk.
         currentvalues = @resource.retrieve_resource
 
         # Determine if there are any out-of-sync properties.
         oos = @resource.send(:properties).find_all do |prop|
           unless currentvalues.include?(prop)
             raise Puppet::DevError, "Parent has property %s but it doesn't appear in the current values", [prop.name]
           end
           if prop.name == :ensure
             false
           else
-            ! prop.insync?(currentvalues[prop])
+            ! prop.safe_insync?(currentvalues[prop])
           end
         end.each { |prop| prop.sync }.length
         @resource.flush if oos > 0
       end
     end
 
     newproperty(:device) do
       desc "The device providing the mount.  This can be whatever
         device is supporting by the mount, including network
         devices or devices specified by UUID rather than device
         path, depending on the operating system."
     end
 
     # Solaris specifies two devices, not just one.
     newproperty(:blockdevice) do
       desc "The device to fsck.  This is property is only valid
         on Solaris, and in most cases will default to the correct
         value."
 
       # Default to the device but with "dsk" replaced with "rdsk".
       defaultto do
         if Facter["operatingsystem"].value == "Solaris"
           device = @resource.value(:device)
           if device =~ %r{/dsk/}
             device.sub(%r{/dsk/}, "/rdsk/")
           else
             nil
           end
         else
           nil
         end
       end
     end
 
     newproperty(:fstype) do
       desc "The mount type.  Valid values depend on the
         operating system.  This is a required option."
     end
 
     newproperty(:options) do
       desc "Mount options for the mounts, as they would
         appear in the fstab."
     end
 
     newproperty(:pass) do
       desc "The pass in which the mount is checked."
 
       defaultto {
         0 if @resource.managed?
       }
     end
 
     newproperty(:atboot) do
       desc "Whether to mount the mount at boot.  Not all platforms
         support this."
     end
 
     newproperty(:dump) do
       desc "Whether to dump the mount.  Not all platform support this.
         Valid values are `1` or `0`. or `2` on FreeBSD, Default is `0`."
 
       if Facter["operatingsystem"].value == "FreeBSD"
         newvalue(%r{(0|1|2)})
       else
         newvalue(%r{(0|1)})
       end
 
       newvalue(%r{(0|1)})
 
       defaultto {
         0 if @resource.managed?
       }
     end
 
     newproperty(:target) do
       desc "The file in which to store the mount table.  Only used by
         those providers that write to disk."
 
       defaultto { if @resource.class.defaultprovider.ancestors.include?(Puppet::Provider::ParsedFile)
           @resource.class.defaultprovider.default_target
         else
           nil
         end
       }
     end
 
     newparam(:name) do
       desc "The mount path for the mount."
 
       isnamevar
     end
 
     newparam(:path) do
       desc "The deprecated name for the mount point.  Please use `name` now."
 
       def value=(value)
         warning "'path' is deprecated for mounts.  Please use 'name'."
         @resource[:name] = value
         super
       end
     end
 
     newparam(:remounts) do
       desc "Whether the mount can be remounted  `mount -o remount`.  If
         this is false, then the filesystem will be unmounted and remounted
         manually, which is prone to failure."
 
       newvalues(:true, :false)
       defaultto do
         case Facter.value(:operatingsystem)
         when "FreeBSD", "Darwin"
           false
         else
           true
         end
       end
     end
 
     def refresh
       # Only remount if we're supposed to be mounted.
       provider.remount if self.should(:fstype) != "swap" and provider.mounted?
     end
 
     def value(name)
       name = symbolize(name)
       ret = nil
       if property = @parameters[name]
         return property.value
       end
     end
   end
 end
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index 51a866332..d73d90dff 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -1,321 +1,319 @@
 # Define the different packaging systems.  Each package system is implemented
 # in a module, which then gets used to individually extend each package object.
 # This allows packages to exist on the same machine using different packaging
 # systems.
 
 module Puppet
   newtype(:package) do
     @doc = "Manage packages.  There is a basic dichotomy in package
       support right now:  Some package types (e.g., yum and apt) can
       retrieve their own package files, while others (e.g., rpm and sun) cannot.  For those package formats that cannot retrieve
       their own files, you can use the `source` parameter to point to
       the correct file.
 
       Puppet will automatically guess the packaging format that you are
       using based on the platform you are on, but you can override it
       using the `provider` parameter; each provider defines what it
       requires in order to function, and you must meet those requirements
       to use a given provider."
 
     feature :installable, "The provider can install packages.",
       :methods => [:install]
     feature :uninstallable, "The provider can uninstall packages.",
       :methods => [:uninstall]
     feature :upgradeable, "The provider can upgrade to the latest version of a
         package.  This feature is used by specifying `latest` as the
         desired value for the package.",
       :methods => [:update, :latest]
     feature :purgeable, "The provider can purge packages.  This generally means
         that all traces of the package are removed, including
         existing configuration files.  This feature is thus destructive
         and should be used with the utmost care.",
       :methods => [:purge]
     feature :versionable, "The provider is capable of interrogating the
         package database for installed version(s), and can select
         which out of a set of available versions of a package to
         install if asked."
     feature :holdable, "The provider is capable of placing packages on hold
         such that they are not automatically upgraded as a result of
         other package dependencies unless explicit action is taken by
         a user or another package. Held is considered a superset of
         installed.",
       :methods => [:hold]
 
     ensurable do
       desc "What state the package should be in.
         *latest* only makes sense for those packaging formats that can
         retrieve new packages on their own and will throw an error on
         those that cannot.  For those packaging systems that allow you
         to specify package versions, specify them here.  Similarly,
         *purged* is only useful for packaging systems that support
         the notion of managing configuration files separately from
         'normal' system files."
 
       attr_accessor :latest
 
       newvalue(:present, :event => :package_installed) do
         provider.install
       end
 
       newvalue(:absent, :event => :package_removed) do
         provider.uninstall
       end
 
       newvalue(:purged, :event => :package_purged, :required_features => :purgeable) do
         provider.purge
       end
 
       newvalue(:held, :event => :package_held, :required_features => :holdable) do
         provider.hold
       end
 
       # Alias the 'present' value.
       aliasvalue(:installed, :present)
 
       newvalue(:latest, :required_features => :upgradeable) do
         # Because yum always exits with a 0 exit code, there's a retrieve
         # in the "install" method.  So, check the current state now,
         # to compare against later.
         current = self.retrieve
         begin
           provider.update
         rescue => detail
           self.fail "Could not update: #{detail}"
         end
 
         if current == :absent
           :package_installed
         else
           :package_changed
         end
       end
 
       newvalue(/./, :required_features => :versionable) do
         begin
           provider.install
         rescue => detail
           self.fail "Could not update: #{detail}"
         end
 
         if self.retrieve == :absent
           :package_installed
         else
           :package_changed
         end
       end
 
       defaultto :installed
 
       # Override the parent method, because we've got all kinds of
       # funky definitions of 'in sync'.
       def insync?(is)
-        @should ||= []
-
         @latest ||= nil
         @lateststamp ||= (Time.now.to_i - 1000)
         # Iterate across all of the should values, and see how they
         # turn out.
 
         @should.each { |should|
           case should
           when :present
             return true unless [:absent, :purged, :held].include?(is)
           when :latest
             # Short-circuit packages that are not present
             return false if is == :absent or is == :purged
 
             # Don't run 'latest' more than about every 5 minutes
             if @latest and ((Time.now.to_i - @lateststamp) / 60) < 5
               #self.debug "Skipping latest check"
             else
               begin
                 @latest = provider.latest
                 @lateststamp = Time.now.to_i
               rescue => detail
                 error = Puppet::Error.new("Could not get latest version: #{detail}")
                 error.set_backtrace(detail.backtrace)
                 raise error
               end
             end
 
             case is
             when @latest
               return true
             when :present
               # This will only happen on retarded packaging systems
               # that can't query versions.
               return true
             else
               self.debug "#{@resource.name} #{is.inspect} is installed, latest is #{@latest.inspect}"
             end
           when :absent
             return true if is == :absent or is == :purged
           when :purged
             return true if is == :purged
           when is
             return true
           end
         }
 
         false
       end
 
       # This retrieves the current state. LAK: I think this method is unused.
       def retrieve
         provider.properties[:ensure]
       end
 
       # Provide a bit more information when logging upgrades.
       def should_to_s(newvalue = @should)
         if @latest
           @latest.to_s
         else
           super(newvalue)
         end
       end
     end
 
     newparam(:name) do
       desc "The package name.  This is the name that the packaging
       system uses internally, which is sometimes (especially on Solaris)
       a name that is basically useless to humans.  If you want to
       abstract package installation, then you can use aliases to provide
       a common name to packages:
 
           # In the 'openssl' class
           $ssl = $operatingsystem ? {
             solaris => SMCossl,
             default => openssl
           }
 
           # It is not an error to set an alias to the same value as the
           # object name.
           package { $ssl:
             ensure => installed,
             alias => openssl
           }
 
           . etc. .
 
           $ssh = $operatingsystem ? {
             solaris => SMCossh,
             default => openssh
           }
 
           # Use the alias to specify a dependency, rather than
           # having another selector to figure it out again.
           package { $ssh:
             ensure => installed,
             alias => openssh,
             require => Package[openssl]
           }
 
       "
       isnamevar
     end
 
     newparam(:source) do
       desc "Where to find the actual package.  This must be a local file
         (or on a network file system) or a URL that your specific
         packaging type understands; Puppet will not retrieve files for you."
     end
     newparam(:instance) do
       desc "A read-only parameter set by the package."
     end
     newparam(:status) do
       desc "A read-only parameter set by the package."
     end
 
     newparam(:type) do
       desc "Deprecated form of `provider`."
 
       munge do |value|
         warning "'type' is deprecated; use 'provider' instead"
         @resource[:provider] = value
 
         @resource[:provider]
       end
     end
 
     newparam(:adminfile) do
       desc "A file containing package defaults for installing packages.
         This is currently only used on Solaris.  The value will be
         validated according to system rules, which in the case of
         Solaris means that it should either be a fully qualified path
         or it should be in `/var/sadm/install/admin`."
     end
 
     newparam(:responsefile) do
       desc "A file containing any necessary answers to questions asked by
         the package.  This is currently used on Solaris and Debian.  The
         value will be validated according to system rules, but it should
         generally be a fully qualified path."
     end
 
     newparam(:configfiles) do
       desc "Whether configfiles should be kept or replaced.  Most packages
         types do not support this parameter."
 
       defaultto :keep
 
       newvalues(:keep, :replace)
     end
 
     newparam(:category) do
       desc "A read-only parameter set by the package."
     end
     newparam(:platform) do
       desc "A read-only parameter set by the package."
     end
     newparam(:root) do
       desc "A read-only parameter set by the package."
     end
     newparam(:vendor) do
       desc "A read-only parameter set by the package."
     end
     newparam(:description) do
       desc "A read-only parameter set by the package."
     end
 
     newparam(:allowcdrom) do
       desc "Tells apt to allow cdrom sources in the sources.list file.
         Normally apt will bail if you try this."
 
       newvalues(:true, :false)
     end
 
     newparam(:flavor) do
       desc "Newer versions of OpenBSD support 'flavors', which are
         further specifications for which type of package you want."
     end
 
     autorequire(:file) do
       autos = []
       [:responsefile, :adminfile].each { |param|
         if val = self[param]
           autos << val
         end
       }
 
       if source = self[:source]
         if source =~ /^#{File::SEPARATOR}/
           autos << source
         end
       end
       autos
     end
 
     # This only exists for testing.
     def clear
       if obj = @parameters[:ensure]
         obj.latest = nil
       end
     end
 
     # The 'query' method returns a hash of info if the package
     # exists and returns nil if it does not.
     def exists?
       @provider.get(:ensure) != :absent
     end
   end
 end
diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb
index c00f02789..973a76c2d 100644
--- a/lib/puppet/type/service.rb
+++ b/lib/puppet/type/service.rb
@@ -1,185 +1,185 @@
 # This is our main way of managing processes right now.
 #
 # a service is distinct from a process in that services
 # can only be managed through the interface of an init script
 # which is why they have a search path for initscripts and such
 
 module Puppet
 
   newtype(:service) do
     @doc = "Manage running services.  Service support unfortunately varies
       widely by platform -- some platforms have very little if any
       concept of a running service, and some have a very codified and
       powerful concept.  Puppet's service support will generally be able
       to make up for any inherent shortcomings (e.g., if there is no
       'status' command, then Puppet will look in the process table for a
       command matching the service name), but the more information you
       can provide the better behaviour you will get.  Or, you can just
       use a platform that has very good service support.
 
       Note that if a `service` receives an event from another resource,
       the service will get restarted. The actual command to restart the
       service depends on the platform. You can provide a special command
       for restarting with the `restart` attribute."
 
     feature :refreshable, "The provider can restart the service.",
       :methods => [:restart]
 
     feature :enableable, "The provider can enable and disable the service",
       :methods => [:disable, :enable, :enabled?]
 
     feature :controllable, "The provider uses a control variable."
 
     newproperty(:enable, :required_features => :enableable) do
       desc "Whether a service should be enabled to start at boot.
         This property behaves quite differently depending on the platform;
         wherever possible, it relies on local tools to enable or disable
         a given service."
 
       newvalue(:true, :event => :service_enabled) do
         provider.enable
       end
 
       newvalue(:false, :event => :service_disabled) do
         provider.disable
       end
 
       def retrieve
         provider.enabled?
       end
     end
 
     # Handle whether the service should actually be running right now.
     newproperty(:ensure) do
       desc "Whether a service should be running."
 
       newvalue(:stopped, :event => :service_stopped) do
         provider.stop
       end
 
       newvalue(:running, :event => :service_started) do
         provider.start
       end
 
       aliasvalue(:false, :stopped)
       aliasvalue(:true, :running)
 
       def retrieve
         provider.status
       end
 
       def sync
         event = super()
 
         if property = @resource.property(:enable)
           val = property.retrieve
-          property.sync unless property.insync?(val)
+          property.sync unless property.safe_insync?(val)
         end
 
         event
       end
     end
 
     newparam(:binary) do
       desc "The path to the daemon.  This is only used for
         systems that do not support init scripts.  This binary will be
         used to start the service if no `start` parameter is
         provided."
     end
 
     newparam(:hasstatus) do
       desc "Declare the the service's init script has a
         functional status command.  Based on testing, it was found
         that a large number of init scripts on different platforms do
         not support any kind of status command; thus, you must specify
         manually whether the service you are running has such a
         command (or you can specify a specific command using the
         `status` parameter).
 
         If you do not specify anything, then the service name will be
         looked for in the process table."
 
       newvalues(:true, :false)
     end
     newparam(:name) do
       desc "The name of the service to run.  This name is used to find
         the service in whatever service subsystem it is in."
       isnamevar
     end
 
     newparam(:path) do
       desc "The search path for finding init scripts.  Multiple values should
         be separated by colons or provided as an array."
 
       munge do |value|
         value = [value] unless value.is_a?(Array)
         # LAK:NOTE See http://snurl.com/21zf8  [groups_google_com]
         # It affects stand-alone blocks, too.
         paths = value.flatten.collect { |p| x = p.split(":") }.flatten
       end
 
       defaultto { provider.class.defpath if provider.class.respond_to?(:defpath) }
     end
     newparam(:pattern) do
       desc "The pattern to search for in the process table.
         This is used for stopping services on platforms that do not
         support init scripts, and is also used for determining service
         status on those service whose init scripts do not include a status
         command.
 
         If this is left unspecified and is needed to check the status
         of a service, then the service name will be used instead.
 
         The pattern can be a simple string or any legal Ruby pattern."
 
       defaultto { @resource[:binary] || @resource[:name] }
     end
     newparam(:restart) do
       desc "Specify a *restart* command manually.  If left
         unspecified, the service will be stopped and then started."
     end
     newparam(:start) do
       desc "Specify a *start* command manually.  Most service subsystems
         support a `start` command, so this will not need to be
         specified."
     end
     newparam(:status) do
       desc "Specify a *status* command manually.  If left
         unspecified, the status method will be determined
         automatically, usually by looking for the service in the
         process table."
     end
 
     newparam(:stop) do
       desc "Specify a *stop* command manually."
     end
 
     newparam(:control) do
       desc "The control variable used to manage services (originally for HP-UX).
         Defaults to the upcased service name plus `START` replacing dots with
         underscores, for those providers that support the `controllable` feature."
       defaultto { resource.name.gsub(".","_").upcase + "_START" if resource.provider.controllable? }
     end
 
     newparam :hasrestart do
       desc "Specify that an init script has a `restart` option.  Otherwise,
         the init script's `stop` and `start` methods are used."
       newvalues(:true, :false)
     end
 
     newparam(:manifest) do
       desc "Specify a command to config a service, or a path to a manifest to do so."
     end
 
     # Basically just a synonym for restarting.  Used to respond
     # to events.
     def refresh
       # Only restart if we're actually running
       if (@parameters[:ensure] || newattr(:ensure)).retrieve == :running
         provider.restart
       else
         debug "Skipping restart; service is not running"
       end
     end
   end
 end
diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb
index 761d5d71b..5de73e3dd 100755
--- a/lib/puppet/type/user.rb
+++ b/lib/puppet/type/user.rb
@@ -1,438 +1,436 @@
 require 'etc'
 require 'facter'
 require 'puppet/property/list'
 require 'puppet/property/ordered_list'
 require 'puppet/property/keyvalue'
 
 module Puppet
   newtype(:user) do
     @doc = "Manage users.  This type is mostly built to manage system
       users, so it is lacking some features useful for managing normal
       users.
 
       This resource type uses the prescribed native tools for creating
       groups and generally uses POSIX APIs for retrieving information
       about them.  It does not directly modify `/etc/passwd` or anything."
 
     feature :allows_duplicates,
       "The provider supports duplicate users with the same UID."
 
     feature :manages_homedir,
       "The provider can create and remove home directories."
 
     feature :manages_passwords,
       "The provider can modify user passwords, by accepting a password
       hash."
 
     feature :manages_password_age,
       "The provider can set age requirements and restrictions for
       passwords."
 
     feature :manages_solaris_rbac,
       "The provider can manage roles and normal users"
 
     feature :manages_expiry,
       "The provider can manage the expiry date for a user."
 
     newproperty(:ensure, :parent => Puppet::Property::Ensure) do
       newvalue(:present, :event => :user_created) do
         provider.create
       end
 
       newvalue(:absent, :event => :user_removed) do
         provider.delete
       end
 
       newvalue(:role, :event => :role_created, :required_features => :manages_solaris_rbac) do
         provider.create_role
       end
 
       desc "The basic state that the object should be in."
 
       # If they're talking about the thing at all, they generally want to
       # say it should exist.
       defaultto do
         if @resource.managed?
           :present
         else
           nil
         end
       end
 
       def retrieve
         if provider.exists?
           if provider.respond_to?(:is_role?) and provider.is_role?
             return :role
           else
             return :present
           end
         else
           return :absent
         end
       end
     end
 
     newproperty(:home) do
       desc "The home directory of the user.  The directory must be created
         separately and is not currently checked for existence."
     end
 
     newproperty(:uid) do
       desc "The user ID.  Must be specified numerically.  For new users
         being created, if no user ID is specified then one will be
         chosen automatically, which will likely result in the same user
         having different IDs on different systems, which is not
         recommended.  This is especially noteworthy if you use Puppet
         to manage the same user on both Darwin and other platforms,
         since Puppet does the ID generation for you on Darwin, but the
         tools do so on other platforms."
 
       munge do |value|
         case value
         when String
           if value =~ /^[-0-9]+$/
             value = Integer(value)
           end
         end
 
         return value
       end
     end
 
     newproperty(:gid) do
       desc "The user's primary group.  Can be specified numerically or
         by name."
 
       munge do |value|
         if value.is_a?(String) and value =~ /^[-0-9]+$/
           Integer(value)
         else
           value
         end
       end
 
       def insync?(is)
-        return true unless self.should
-
         # We know the 'is' is a number, so we need to convert the 'should' to a number,
         # too.
         @should.each do |value|
           return true if number = Puppet::Util.gid(value) and is == number
         end
 
         false
       end
 
       def sync
         found = false
         @should.each do |value|
           if number = Puppet::Util.gid(value)
             provider.gid = number
             found = true
             break
           end
         end
 
         fail "Could not find group(s) #{@should.join(",")}" unless found
 
         # Use the default event.
       end
     end
 
     newproperty(:comment) do
       desc "A description of the user.  Generally is a user's full name."
     end
 
     newproperty(:shell) do
       desc "The user's login shell.  The shell must exist and be
         executable."
     end
 
     newproperty(:password, :required_features => :manages_passwords) do
       desc "The user's password, in whatever encrypted format the local machine requires. Be sure to enclose any value that includes a dollar sign ($) in single quotes (\')."
 
       validate do |value|
         raise ArgumentError, "Passwords cannot include ':'" if value.is_a?(String) and value.include?(":")
       end
 
       def change_to_s(currentvalue, newvalue)
         if currentvalue == :absent
           return "created password"
         else
           return "changed password"
         end
       end
     end
 
     newproperty(:password_min_age, :required_features => :manages_password_age) do
       desc "The minimum amount of time in days a password must be used before it may be changed"
 
       munge do |value|
         case value
         when String
           Integer(value)
         else
           value
         end
       end
 
       validate do |value|
         if value.to_s !~ /^\d+$/
           raise ArgumentError, "Password minimum age must be provided as a number"
         end
       end
     end
 
     newproperty(:password_max_age, :required_features => :manages_password_age) do
       desc "The maximum amount of time in days a password may be used before it must be changed"
 
       munge do |value|
         case value
         when String
           Integer(value)
         else
           value
         end
       end
 
       validate do |value|
         if value.to_s !~ /^\d+$/
           raise ArgumentError, "Password maximum age must be provided as a number"
         end
       end
     end
 
     newproperty(:groups, :parent => Puppet::Property::List) do
       desc "The groups of which the user is a member.  The primary
         group should not be listed.  Multiple groups should be
         specified as an array."
 
       validate do |value|
         if value =~ /^\d+$/
           raise ArgumentError, "Group names must be provided, not numbers"
         end
         raise ArgumentError, "Group names must be provided as an array, not a comma-separated list" if value.include?(",")
       end
     end
 
     newparam(:name) do
       desc "User name.  While limitations are determined for
         each operating system, it is generally a good idea to keep to
         the degenerate 8 characters, beginning with a letter."
       isnamevar
     end
 
     newparam(:membership) do
       desc "Whether specified groups should be treated as the only groups
         of which the user is a member or whether they should merely
         be treated as the minimum membership list."
 
       newvalues(:inclusive, :minimum)
 
       defaultto :minimum
     end
 
     newparam(:allowdupe, :boolean => true) do
       desc "Whether to allow duplicate UIDs."
 
       newvalues(:true, :false)
 
       defaultto false
     end
 
     newparam(:managehome, :boolean => true) do
       desc "Whether to manage the home directory when managing the user."
 
       newvalues(:true, :false)
 
       defaultto false
 
       validate do |val|
         if val.to_s == "true"
           raise ArgumentError, "User provider #{provider.class.name} can not manage home directories" unless provider.class.manages_homedir?
         end
       end
     end
 
     newproperty(:expiry, :required_features => :manages_expiry) do
       desc "The expiry date for this user. Must be provided in
            a zero padded YYYY-MM-DD format - e.g 2010-02-19."
 
       validate do |value|
         if value !~ /^\d{4}-\d{2}-\d{2}$/
           raise ArgumentError, "Expiry dates must be YYYY-MM-DD"
         end
       end
     end
 
     # Autorequire the group, if it's around
     autorequire(:group) do
       autos = []
 
       if obj = @parameters[:gid] and groups = obj.shouldorig
         groups = groups.collect { |group|
           if group =~ /^\d+$/
             Integer(group)
           else
             group
           end
         }
         groups.each { |group|
           case group
           when Integer
             if resource = catalog.resources.find { |r| r.is_a?(Puppet::Type.type(:group)) and r.should(:gid) == group }
               autos << resource
             end
           else
             autos << group
           end
         }
       end
 
       if obj = @parameters[:groups] and groups = obj.should
         autos += groups.split(",")
       end
 
       autos
     end
 
     # Provide an external hook.  Yay breaking out of APIs.
     def exists?
       provider.exists?
     end
 
     def retrieve
       absent = false
       properties.inject({}) { |prophash, property|
         current_value = :absent
 
         if absent
           prophash[property] = :absent
         else
           current_value = property.retrieve
           prophash[property] = current_value
         end
 
         if property.name == :ensure and current_value == :absent
           absent = true
         end
         prophash
       }
     end
 
     newproperty(:roles, :parent => Puppet::Property::List, :required_features => :manages_solaris_rbac) do
       desc "The roles the user has.  Multiple roles should be
         specified as an array."
 
       def membership
         :role_membership
       end
 
       validate do |value|
         if value =~ /^\d+$/
           raise ArgumentError, "Role names must be provided, not numbers"
         end
         raise ArgumentError, "Role names must be provided as an array, not a comma-separated list" if value.include?(",")
       end
     end
 
     #autorequire the roles that the user has
     autorequire(:user) do
       reqs = []
 
       if roles_property = @parameters[:roles] and roles = roles_property.should
         reqs += roles.split(',')
       end
 
       reqs
     end
 
     newparam(:role_membership) do
       desc "Whether specified roles should be treated as the only roles
         of which the user is a member or whether they should merely
         be treated as the minimum membership list."
 
       newvalues(:inclusive, :minimum)
 
       defaultto :minimum
     end
 
     newproperty(:auths, :parent => Puppet::Property::List, :required_features => :manages_solaris_rbac) do
       desc "The auths the user has.  Multiple auths should be
         specified as an array."
 
       def membership
         :auth_membership
       end
 
       validate do |value|
         if value =~ /^\d+$/
           raise ArgumentError, "Auth names must be provided, not numbers"
         end
         raise ArgumentError, "Auth names must be provided as an array, not a comma-separated list" if value.include?(",")
       end
     end
 
     newparam(:auth_membership) do
       desc "Whether specified auths should be treated as the only auths
         of which the user is a member or whether they should merely
         be treated as the minimum membership list."
 
       newvalues(:inclusive, :minimum)
 
       defaultto :minimum
     end
 
     newproperty(:profiles, :parent => Puppet::Property::OrderedList, :required_features => :manages_solaris_rbac) do
       desc "The profiles the user has.  Multiple profiles should be
         specified as an array."
 
       def membership
         :profile_membership
       end
 
       validate do |value|
         if value =~ /^\d+$/
           raise ArgumentError, "Profile names must be provided, not numbers"
         end
         raise ArgumentError, "Profile names must be provided as an array, not a comma-separated list" if value.include?(",")
       end
     end
 
     newparam(:profile_membership) do
       desc "Whether specified roles should be treated as the only roles
         of which the user is a member or whether they should merely
         be treated as the minimum membership list."
 
       newvalues(:inclusive, :minimum)
 
       defaultto :minimum
     end
 
     newproperty(:keys, :parent => Puppet::Property::KeyValue, :required_features => :manages_solaris_rbac) do
       desc "Specify user attributes in an array of keyvalue pairs"
 
       def membership
         :key_membership
       end
 
       validate do |value|
         raise ArgumentError, "key value pairs must be seperated by an =" unless value.include?("=")
       end
     end
 
     newparam(:key_membership) do
       desc "Whether specified key value pairs should be treated as the only attributes
         of the user or whether they should merely
         be treated as the minimum list."
 
       newvalues(:inclusive, :minimum)
 
       defaultto :minimum
     end
 
     newproperty(:project, :required_features => :manages_solaris_rbac) do
       desc "The name of the project associated with a user"
     end
   end
 end
diff --git a/lib/puppet/type/yumrepo.rb b/lib/puppet/type/yumrepo.rb
index 160b2164d..9b4c79428 100644
--- a/lib/puppet/type/yumrepo.rb
+++ b/lib/puppet/type/yumrepo.rb
@@ -1,359 +1,359 @@
 # Description of yum repositories
 
 require 'puppet/util/inifile'
 
 module Puppet
   # A property for one entry in a .ini-style file
   class IniProperty < Puppet::Property
     def insync?(is)
       # A should property of :absent is the same as nil
-      if is.nil? && (should.nil? || should == :absent)
+      if is.nil? && should == :absent
         return true
       end
       super(is)
     end
 
     def sync
-      if insync?(retrieve)
+      if safe_insync?(retrieve)
         result = nil
       else
         result = set(self.should)
         if should == :absent
           resource.section[inikey] = nil
         else
           resource.section[inikey] = should
         end
       end
       result
     end
 
     def retrieve
       resource.section[inikey]
     end
 
     def inikey
       name.to_s
     end
 
     # Set the key associated with this property to KEY, instead
     # of using the property's NAME
     def self.inikey(key)
       # Override the inikey instance method
       # Is there a way to do this without resorting to strings ?
       # Using a block fails because the block can't access
       # the variable 'key' in the outer scope
       self.class_eval("def inikey ; \"#{key.to_s}\" ; end")
     end
 
   end
 
   # Doc string for properties that can be made 'absent'
   ABSENT_DOC="Set this to 'absent' to remove it from the file completely"
 
   newtype(:yumrepo) do
     @doc = "The client-side description of a yum repository. Repository
       configurations are found by parsing `/etc/yum.conf` and
       the files indicated by the `reposdir` option in that file 
       (see yum.conf(5) for details)
 
       Most parameters are identical to the ones documented
       in yum.conf(5)
 
       Continuation lines that yum supports for example for the
       baseurl are not supported. No attempt is made to access
       files included with the **include** directive"
 
     class << self
       attr_accessor :filetype
       # The writer is only used for testing, there should be no need
       # to change yumconf or inifile in any other context
       attr_accessor :yumconf
       attr_writer :inifile
     end
 
     self.filetype = Puppet::Util::FileType.filetype(:flat)
 
     @inifile = nil
 
     @yumconf = "/etc/yum.conf"
 
     # Where to put files for brand new sections
     @defaultrepodir = nil
 
     def self.instances
       l = []
       check = validproperties
       clear
       inifile.each_section do |s|
         next if s.name == "main"
         obj = new(:name => s.name, :audit => check)
         current_values = obj.retrieve
         obj.eachproperty do |property|
           if current_values[property].nil?
             obj.delete(property.name)
           else
             property.should = current_values[property]
           end
         end
         obj.delete(:audit)
         l << obj
       end
       l
     end
 
     # Return the Puppet::Util::IniConfig::File for the whole yum config
     def self.inifile
       if @inifile.nil?
         @inifile = read
         main = @inifile['main']
         raise Puppet::Error, "File #{yumconf} does not contain a main section" if main.nil?
         reposdir = main['reposdir']
         reposdir ||= "/etc/yum.repos.d, /etc/yum/repos.d"
         reposdir.gsub!(/[\n,]/, " ")
         reposdir.split.each do |dir|
           Dir::glob("#{dir}/*.repo").each do |file|
             @inifile.read(file) if File.file?(file)
           end
         end
         reposdir.split.each do |dir|
           if File::directory?(dir) && File::writable?(dir)
             @defaultrepodir = dir
             break
           end
         end
       end
       @inifile
     end
 
     # Parse the yum config files. Only exposed for the tests
     # Non-test code should use self.inifile to get at the
     # underlying file
     def self.read
       result = Puppet::Util::IniConfig::File.new
       result.read(yumconf)
       main = result['main']
       raise Puppet::Error, "File #{yumconf} does not contain a main section" if main.nil?
       reposdir = main['reposdir']
       reposdir ||= "/etc/yum.repos.d, /etc/yum/repos.d"
       reposdir.gsub!(/[\n,]/, " ")
       reposdir.split.each do |dir|
         Dir::glob("#{dir}/*.repo").each do |file|
           result.read(file) if File.file?(file)
         end
       end
       if @defaultrepodir.nil?
         reposdir.split.each do |dir|
           if File::directory?(dir) && File::writable?(dir)
             @defaultrepodir = dir
             break
           end
         end
       end
       result
     end
 
     # Return the Puppet::Util::IniConfig::Section with name NAME
     # from the yum config
     def self.section(name)
       result = inifile[name]
       if result.nil?
         # Brand new section
         path = yumconf
         path = File::join(@defaultrepodir, "#{name}.repo") unless @defaultrepodir.nil?
         Puppet::info "create new repo #{name} in file #{path}"
         result = inifile.add_section(name, path)
       end
       result
     end
 
     # Store all modifications back to disk
     def self.store
       inifile.store
       unless Puppet[:noop]
         target_mode = 0644 # FIXME: should be configurable
         inifile.each_file do |file|
           current_mode = File.stat(file).mode & 0777
           unless current_mode == target_mode
             Puppet::info "changing mode of #{file} from %03o to %03o" % [current_mode, target_mode]
             File.chmod(target_mode, file)
           end
         end
       end
     end
 
     # This is only used during testing.
     def self.clear
       @inifile = nil
       @yumconf = "/etc/yum.conf"
       @defaultrepodir = nil
     end
 
     # Return the Puppet::Util::IniConfig::Section for this yumrepo resource
     def section
       self.class.section(self[:name])
     end
 
     # Store modifications to this yumrepo resource back to disk
     def flush
       self.class.store
     end
 
     newparam(:name) do
       desc "The name of the repository.  This corresponds to the
         repositoryid parameter in yum.conf(5)."
       isnamevar
     end
 
     newproperty(:descr, :parent => Puppet::IniProperty) do
       desc "A human readable description of the repository.
         This corresponds to the name parameter in yum.conf(5).
         #{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(/.*/) { }
       inikey "name"
     end
 
     newproperty(:mirrorlist, :parent => Puppet::IniProperty) do
       desc "The URL that holds the list of mirrors for this repository.
         #{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       # Should really check that it's a valid URL
       newvalue(/.*/) { }
     end
 
     newproperty(:baseurl, :parent => Puppet::IniProperty) do
       desc "The URL for this repository.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       # Should really check that it's a valid URL
       newvalue(/.*/) { }
     end
 
     newproperty(:enabled, :parent => Puppet::IniProperty) do
       desc "Whether this repository is enabled or disabled. Possible
         values are '0', and '1'.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(%r{(0|1)}) { }
     end
 
     newproperty(:gpgcheck, :parent => Puppet::IniProperty) do
       desc "Whether to check the GPG signature on packages installed
         from this repository. Possible values are '0', and '1'.
         \n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(%r{(0|1)}) { }
     end
 
     newproperty(:gpgkey, :parent => Puppet::IniProperty) do
       desc "The URL for the GPG key with which packages from this
         repository are signed.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       # Should really check that it's a valid URL
       newvalue(/.*/) { }
     end
 
     newproperty(:include, :parent => Puppet::IniProperty) do
       desc "A URL from which to include the config.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       # Should really check that it's a valid URL
       newvalue(/.*/) { }
     end
 
     newproperty(:exclude, :parent => Puppet::IniProperty) do
       desc "List of shell globs. Matching packages will never be
         considered in updates or installs for this repo.
         #{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(/.*/) { }
     end
 
     newproperty(:includepkgs, :parent => Puppet::IniProperty) do
       desc "List of shell globs. If this is set, only packages
         matching one of the globs will be considered for
         update or install.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(/.*/) { }
     end
 
     newproperty(:enablegroups, :parent => Puppet::IniProperty) do
       desc "Determines whether yum will allow the use of
         package groups for this  repository. Possible
         values are '0', and '1'.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(%r{(0|1)}) { }
     end
 
     newproperty(:failovermethod, :parent => Puppet::IniProperty) do
       desc "Either 'roundrobin' or 'priority'.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(%r{roundrobin|priority}) { }
     end
 
     newproperty(:keepalive, :parent => Puppet::IniProperty) do
       desc "Either '1' or '0'. This tells yum whether or not HTTP/1.1
         keepalive  should  be  used with this repository.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(%r{(0|1)}) { }
     end
 
      newproperty(:http_caching, :parent => Puppet::IniProperty) do
        desc "Either 'packages' or 'all' or 'none'.\n#{ABSENT_DOC}" 
        newvalue(:absent) { self.should = :absent }
        newvalue(%r(packages|all|none)) { }
      end
 
     newproperty(:timeout, :parent => Puppet::IniProperty) do
       desc "Number of seconds to wait for a connection before timing
         out.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(%r{[0-9]+}) { }
     end
 
     newproperty(:metadata_expire, :parent => Puppet::IniProperty) do
       desc "Number of seconds after which the metadata will expire.
         #{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(%r{[0-9]+}) { }
     end
 
     newproperty(:protect, :parent => Puppet::IniProperty) do
       desc "Enable or disable protection for this repository. Requires
         that the protectbase plugin is installed and enabled.
         #{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(%r{(0|1)}) { }
     end
 
     newproperty(:priority, :parent => Puppet::IniProperty) do
       desc "Priority of this repository from 1-99. Requires that
         the priorities plugin is installed and enabled.
         #{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(%r{[1-9][0-9]?}) { }
     end
 
     newproperty(:cost, :parent => Puppet::IniProperty) do
       desc "Cost of this repository.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(%r{\d+}) { }
     end
 
     newproperty(:proxy, :parent => Puppet::IniProperty) do
       desc "URL to the proxy server for this repository.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       # Should really check that it's a valid URL
       newvalue(/.*/) { }
     end
 
     newproperty(:proxy_username, :parent => Puppet::IniProperty) do
       desc "Username for this proxy.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(/.*/) { }
     end
 
     newproperty(:proxy_password, :parent => Puppet::IniProperty) do
       desc "Password for this proxy.\n#{ABSENT_DOC}"
       newvalue(:absent) { self.should = :absent }
       newvalue(/.*/) { }
     end
   end
 end
diff --git a/lib/puppet/type/zpool.rb b/lib/puppet/type/zpool.rb
index 49cce552a..df06522e8 100755
--- a/lib/puppet/type/zpool.rb
+++ b/lib/puppet/type/zpool.rb
@@ -1,92 +1,88 @@
 module Puppet
   class Property
 
     class VDev < Property
 
       def flatten_and_sort(array)
         array.collect { |a| a.split(' ') }.flatten.sort
       end
 
       def insync?(is)
-        return true unless self.should
-
         return @should == [:absent] if is == :absent
 
         flatten_and_sort(is) == flatten_and_sort(@should)
       end
     end
 
     class MultiVDev < VDev
       def insync?(is)
-        return true unless self.should
-
         return @should == [:absent] if is == :absent
 
         return false unless is.length == @should.length
 
         is.each_with_index { |list, i| return false unless flatten_and_sort(list) == flatten_and_sort(@should[i]) }
 
         #if we made it this far we are in sync
         true
       end
     end
   end
 
   newtype(:zpool) do
     @doc = "Manage zpools. Create and delete zpools. The provider WILL NOT SYNC, only report differences.
 
       Supports vdevs with mirrors, raidz, logs and spares."
 
     ensurable
 
     newproperty(:disk, :array_matching => :all, :parent => Puppet::Property::VDev) do
       desc "The disk(s) for this pool. Can be an array or space separated string"
     end
 
     newproperty(:mirror, :array_matching => :all, :parent => Puppet::Property::MultiVDev) do
       desc "List of all the devices to mirror for this pool. Each mirror should be a space separated string:
 
     mirror => [\"disk1 disk2\", \"disk3 disk4\"]
 
       "
 
       validate do |value|
         raise ArgumentError, "mirror names must be provided as string separated, not a comma-separated list" if value.include?(",")
       end
     end
 
     newproperty(:raidz, :array_matching => :all, :parent => Puppet::Property::MultiVDev) do
       desc "List of all the devices to raid for this pool. Should be an array of space separated strings:
       
           raidz => [\"disk1 disk2\", \"disk3 disk4\"]
 
       "
 
       validate do |value|
         raise ArgumentError, "raid names must be provided as string separated, not a comma-separated list" if value.include?(",")
       end
     end
 
     newproperty(:spare, :array_matching => :all, :parent => Puppet::Property::VDev) do
       desc "Spare disk(s) for this pool."
     end
 
     newproperty(:log, :array_matching => :all, :parent => Puppet::Property::VDev) do
       desc "Log disks for this pool. (doesn't support mirroring yet)"
     end
 
     newparam(:pool) do
       desc "The name for this pool."
       isnamevar
     end
 
     newparam(:raid_parity) do
       desc "Determines parity when using raidz property."
     end
 
     validate do
       has_should = [:disk, :mirror, :raidz].select { |prop| self.should(prop) }
       self.fail "You cannot specify #{has_should.join(" and ")} on this type (only one)" if has_should.length > 1
     end
   end
 end
diff --git a/spec/unit/property/keyvalue_spec.rb b/spec/unit/property/keyvalue_spec.rb
index 7666def56..a44d891d7 100644
--- a/spec/unit/property/keyvalue_spec.rb
+++ b/spec/unit/property/keyvalue_spec.rb
@@ -1,168 +1,168 @@
 #!/usr/bin/env ruby
 
 Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
 
 require 'puppet/property/keyvalue'
 
 klass = Puppet::Property::KeyValue
 
 describe klass do
 
   it "should be a subclass of Property" do
     klass.superclass.must == Puppet::Property
   end
 
   describe "as an instance" do
     before do
       # Wow that's a messy interface to the resource.
       klass.initvars
       @resource = stub 'resource', :[]= => nil, :property => nil
       @property = klass.new(:resource => @resource)
     end
 
     it "should have a , as default delimiter" do
       @property.delimiter.should == ";"
     end
 
     it "should have a = as default separator" do
       @property.separator.should == "="
     end
 
     it "should have a :membership as default membership" do
       @property.membership.should == :key_value_membership
     end
 
     it "should return the same value passed into should_to_s" do
       @property.should_to_s({:foo => "baz", :bar => "boo"}) == "foo=baz;bar=boo"
     end
 
     it "should return the passed in array values joined with the delimiter from is_to_s" do
       @property.is_to_s({"foo" => "baz" , "bar" => "boo"}).should == "foo=baz;bar=boo"
     end
 
     describe "when calling inclusive?" do
       it "should use the membership method to look up on the @resource" do
         @property.expects(:membership).returns(:key_value_membership)
         @resource.expects(:[]).with(:key_value_membership)
         @property.inclusive?
       end
 
       it "should return true when @resource[membership] == inclusive" do
         @property.stubs(:membership).returns(:key_value_membership)
         @resource.stubs(:[]).with(:key_value_membership).returns(:inclusive)
         @property.inclusive?.must == true
       end
 
       it "should return false when @resource[membership] != inclusive" do
         @property.stubs(:membership).returns(:key_value_membership)
         @resource.stubs(:[]).with(:key_value_membership).returns(:minimum)
         @property.inclusive?.must == false
       end
     end
 
     describe "when calling process_current_hash" do
       it "should return {} if hash is :absent" do
         @property.process_current_hash(:absent).must == {}
       end
 
       it "should set every key to nil if inclusive?" do
         @property.stubs(:inclusive?).returns(true)
         @property.process_current_hash({:foo => "bar", :do => "re"}).must == { :foo => nil, :do => nil }
       end
 
       it "should return the hash if !inclusive?" do
         @property.stubs(:inclusive?).returns(false)
         @property.process_current_hash({:foo => "bar", :do => "re"}).must == {:foo => "bar", :do => "re"}
       end
     end
 
     describe "when calling should" do
       it "should return nil if @should is nil" do
         @property.should.must == nil
       end
 
       it "should call process_current_hash" do
         @property.should = ["foo=baz", "bar=boo"]
         @property.stubs(:retrieve).returns({:do => "re", :mi => "fa" })
         @property.expects(:process_current_hash).returns({})
         @property.should
       end
 
       it "should return the hashed values of @should and the nilled values of retrieve if inclusive" do
         @property.should = ["foo=baz", "bar=boo"]
         @property.expects(:retrieve).returns({:do => "re", :mi => "fa" })
         @property.expects(:inclusive?).returns(true)
         @property.should.must == { :foo => "baz", :bar => "boo", :do => nil, :mi => nil }
       end
 
       it "should return the hashed @should + the unique values of retrieve if !inclusive" do
         @property.should = ["foo=baz", "bar=boo"]
         @property.expects(:retrieve).returns({:foo => "diff", :do => "re", :mi => "fa"})
         @property.expects(:inclusive?).returns(false)
         @property.should.must == { :foo => "baz", :bar => "boo", :do => "re", :mi => "fa" }
       end
     end
 
     describe "when calling retrieve" do
       before do
         @provider = mock("provider")
         @property.stubs(:provider).returns(@provider)
       end
 
       it "should send 'name' to the provider" do
         @provider.expects(:send).with(:keys)
         @property.expects(:name).returns(:keys)
         @property.retrieve
       end
 
       it "should return a hash with the provider returned info" do
         @provider.stubs(:send).with(:keys).returns({"do" => "re", "mi" => "fa" })
         @property.stubs(:name).returns(:keys)
         @property.retrieve == {"do" => "re", "mi" => "fa" }
       end
 
       it "should return :absent when the provider returns :absent" do
         @provider.stubs(:send).with(:keys).returns(:absent)
         @property.stubs(:name).returns(:keys)
         @property.retrieve == :absent
       end
     end
 
     describe "when calling hashify" do
       it "should return the array hashified" do
         @property.hashify(["foo=baz", "bar=boo"]).must == { :foo => "baz", :bar => "boo" }
       end
     end
 
-    describe "when calling insync?" do
+    describe "when calling safe_insync?" do
       before do
         @provider = mock("provider")
         @property.stubs(:provider).returns(@provider)
         @property.stubs(:name).returns(:prop_name)
       end
 
       it "should return true unless @should is defined and not nil" do
-        @property.insync?("foo") == true
+        @property.safe_insync?("foo") == true
       end
 
       it "should return true if the passed in values is nil" do
         @property.should = "foo"
-        @property.insync?(nil) == true
+        @property.safe_insync?(nil) == true
       end
 
       it "should return true if hashified should value == (retrieved) value passed in" do
         @provider.stubs(:prop_name).returns({ :foo => "baz", :bar => "boo" })
         @property.should = ["foo=baz", "bar=boo"]
         @property.expects(:inclusive?).returns(true)
-        @property.insync?({ :foo => "baz", :bar => "boo" }).must == true
+        @property.safe_insync?({ :foo => "baz", :bar => "boo" }).must == true
       end
 
       it "should return false if prepared value != should value" do
         @provider.stubs(:prop_name).returns({ "foo" => "bee", "bar" => "boo" })
         @property.should = ["foo=baz", "bar=boo"]
         @property.expects(:inclusive?).returns(true)
-        @property.insync?({ "foo" => "bee", "bar" => "boo" }).must == false
+        @property.safe_insync?({ "foo" => "bee", "bar" => "boo" }).must == false
       end
     end
   end
 end
diff --git a/spec/unit/property/list_spec.rb b/spec/unit/property/list_spec.rb
index 3e8cc5402..c6c5db10e 100644
--- a/spec/unit/property/list_spec.rb
+++ b/spec/unit/property/list_spec.rb
@@ -1,166 +1,166 @@
 #!/usr/bin/env ruby
 
 Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
 
 require 'puppet/property/list'
 
 list_class = Puppet::Property::List
 
 describe list_class do
 
   it "should be a subclass of Property" do
     list_class.superclass.must == Puppet::Property
   end
 
   describe "as an instance" do
     before do
       # Wow that's a messy interface to the resource.
       list_class.initvars
       @resource = stub 'resource', :[]= => nil, :property => nil
       @property = list_class.new(:resource => @resource)
     end
 
     it "should have a , as default delimiter" do
       @property.delimiter.should == ","
     end
 
     it "should have a :membership as default membership" do
       @property.membership.should == :membership
     end
 
     it "should return the same value passed into should_to_s" do
       @property.should_to_s("foo") == "foo"
     end
 
     it "should return the passed in array values joined with the delimiter from is_to_s" do
       @property.is_to_s(["foo","bar"]).should == "foo,bar"
     end
 
     it "should be able to correctly convert ':absent' to a string" do
       @property.is_to_s(:absent).should == "absent"
     end
 
     describe "when adding should to current" do
       it "should add the arrays when current is an array" do
         @property.add_should_with_current(["foo"], ["bar"]).should == ["foo", "bar"]
       end
 
       it "should return should if current is not a array" do
         @property.add_should_with_current(["foo"], :absent).should == ["foo"]
       end
 
       it "should return only the uniq elements" do
         @property.add_should_with_current(["foo", "bar"], ["foo", "baz"]).should == ["foo", "bar", "baz"]
       end
     end
 
     describe "when calling inclusive?" do
       it "should use the membership method to look up on the @resource" do
         @property.expects(:membership).returns(:membership)
         @resource.expects(:[]).with(:membership)
         @property.inclusive?
       end
 
       it "should return true when @resource[membership] == inclusive" do
         @property.stubs(:membership).returns(:membership)
         @resource.stubs(:[]).with(:membership).returns(:inclusive)
         @property.inclusive?.must == true
       end
 
       it "should return false when @resource[membership] != inclusive" do
         @property.stubs(:membership).returns(:membership)
         @resource.stubs(:[]).with(:membership).returns(:minimum)
         @property.inclusive?.must == false
       end
     end
 
     describe "when calling should" do
       it "should return nil if @should is nil" do
         @property.should.must == nil
       end
 
       it "should return the sorted values of @should as a string if inclusive" do
         @property.should = ["foo", "bar"]
         @property.expects(:inclusive?).returns(true)
         @property.should.must == "bar,foo"
       end
 
       it "should return the uniq sorted values of @should + retrieve as a string if !inclusive" do
         @property.should = ["foo", "bar"]
         @property.expects(:inclusive?).returns(false)
         @property.expects(:retrieve).returns(["foo","baz"])
         @property.should.must == "bar,baz,foo"
       end
     end
 
     describe "when calling retrieve" do
       before do
         @provider = mock("provider")
         @property.stubs(:provider).returns(@provider)
       end
 
       it "should send 'name' to the provider" do
         @provider.expects(:send).with(:group)
         @property.expects(:name).returns(:group)
         @property.retrieve
       end
 
       it "should return an array with the provider returned info" do
         @provider.stubs(:send).with(:group).returns("foo,bar,baz")
         @property.stubs(:name).returns(:group)
         @property.retrieve == ["foo", "bar", "baz"]
       end
 
       it "should return :absent when the provider returns :absent" do
         @provider.stubs(:send).with(:group).returns(:absent)
         @property.stubs(:name).returns(:group)
         @property.retrieve == :absent
       end
     end
 
-    describe "when calling insync?" do
+    describe "when calling safe_insync?" do
       it "should return true unless @should is defined and not nil" do
-        @property.must be_insync("foo")
+        @property.must be_safe_insync("foo")
       end
 
       it "should return true unless the passed in values is not nil" do
         @property.should = "foo"
-        @property.must be_insync(nil)
+        @property.must be_safe_insync(nil)
       end
 
       it "should call prepare_is_for_comparison with value passed in and should" do
         @property.should = "foo"
         @property.expects(:prepare_is_for_comparison).with("bar")
         @property.expects(:should)
-        @property.insync?("bar")
+        @property.safe_insync?("bar")
       end
 
       it "should return true if 'is' value is array of comma delimited should values" do
         @property.should = "bar,foo"
         @property.expects(:inclusive?).returns(true)
-        @property.must be_insync(["bar","foo"])
+        @property.must be_safe_insync(["bar","foo"])
       end
 
       it "should return true if 'is' value is :absent and should value is empty string" do
         @property.should = ""
         @property.expects(:inclusive?).returns(true)
-        @property.must be_insync([])
+        @property.must be_safe_insync([])
       end
 
       it "should return false if prepared value != should value" do
         @property.should = "bar,baz,foo"
         @property.expects(:inclusive?).returns(true)
-        @property.must_not be_insync(["bar","foo"])
+        @property.must_not be_safe_insync(["bar","foo"])
       end
     end
 
     describe "when calling dearrayify" do
       it "should sort and join the array with 'delimiter'" do
         array = mock "array"
         array.expects(:sort).returns(array)
         array.expects(:join).with(@property.delimiter)
         @property.dearrayify(array)
       end
     end
   end
 end
diff --git a/spec/unit/type/file/content_spec.rb b/spec/unit/type/file/content_spec.rb
index cde643fc8..3ec57f00e 100755
--- a/spec/unit/type/file/content_spec.rb
+++ b/spec/unit/type/file/content_spec.rb
@@ -1,461 +1,465 @@
 #!/usr/bin/env ruby
 
 Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
 
 content = Puppet::Type.type(:file).attrclass(:content)
 describe content do
   before do
     @resource = Puppet::Type.type(:file).new :path => "/foo/bar"
     content.stubs(:standalone?).returns(false)
   end
 
   it "should be a subclass of Property" do
     content.superclass.must == Puppet::Property
   end
 
   describe "when determining the checksum type" do
     it "should use the type specified in the source checksum if a source is set" do
       @resource[:source] = "/foo"
       @resource.parameter(:source).expects(:checksum).returns "{md5lite}eh"
 
       @content = content.new(:resource => @resource)
       @content.checksum_type.should == :md5lite
     end
 
     it "should use the type specified by the checksum parameter if no source is set" do
       @resource[:checksum] = :md5lite
 
       @content = content.new(:resource => @resource)
       @content.checksum_type.should == :md5lite
     end
   end
 
   describe "when determining the actual content to write" do
     it "should use the set content if available" do
       @content = content.new(:resource => @resource)
       @content.should = "ehness"
       @content.actual_content.should == "ehness"
     end
 
     it "should not use the content from the source if the source is set" do
       source = mock 'source'
 
       @resource.expects(:parameter).never.with(:source).returns source
 
       @content = content.new(:resource => @resource)
       @content.actual_content.should be_nil
     end
   end
 
   describe "when setting the desired content" do
     it "should make the actual content available via an attribute" do
       @content = content.new(:resource => @resource)
       @content.stubs(:checksum_type).returns "md5"
       @content.should = "this is some content"
 
       @content.actual_content.should == "this is some content"
     end
 
     it "should store the checksum as the desired content" do
       @content = content.new(:resource => @resource)
       digest = Digest::MD5.hexdigest("this is some content")
 
       @content.stubs(:checksum_type).returns "md5"
       @content.should = "this is some content"
 
       @content.should.must == "{md5}#{digest}"
     end
 
     it "should not checksum 'absent'" do
       @content = content.new(:resource => @resource)
       @content.should = :absent
 
       @content.should.must == :absent
     end
 
     it "should accept a checksum as the desired content" do
       @content = content.new(:resource => @resource)
       digest = Digest::MD5.hexdigest("this is some content")
 
       string = "{md5}#{digest}"
       @content.should = string
 
       @content.should.must == string
     end
   end
 
   describe "when retrieving the current content" do
     it "should return :absent if the file does not exist" do
       @content = content.new(:resource => @resource)
       @resource.expects(:stat).returns nil
 
       @content.retrieve.should == :absent
     end
 
     it "should not manage content on directories" do
       @content = content.new(:resource => @resource)
 
       stat = mock 'stat', :ftype => "directory"
       @resource.expects(:stat).returns stat
 
       @content.retrieve.should be_nil
     end
 
     it "should not manage content on links" do
       @content = content.new(:resource => @resource)
 
       stat = mock 'stat', :ftype => "link"
       @resource.expects(:stat).returns stat
 
       @content.retrieve.should be_nil
     end
 
     it "should always return the checksum as a string" do
       @content = content.new(:resource => @resource)
       @resource[:checksum] = :mtime
 
       stat = mock 'stat', :ftype => "file"
       @resource.expects(:stat).returns stat
 
       time = Time.now
       @resource.parameter(:checksum).expects(:mtime_file).with(@resource[:path]).returns time
 
       @content.retrieve.should == "{mtime}#{time}"
     end
 
     it "should return the checksum of the file if it exists and is a normal file" do
       @content = content.new(:resource => @resource)
       stat = mock 'stat', :ftype => "file"
       @resource.expects(:stat).returns stat
       @resource.parameter(:checksum).expects(:md5_file).with(@resource[:path]).returns "mysum"
 
       @content.retrieve.should == "{md5}mysum"
     end
   end
 
   describe "when testing whether the content is in sync" do
     before do
       @resource[:ensure] = :file
       @content = content.new(:resource => @resource)
     end
 
     it "should return true if the resource shouldn't be a regular file" do
       @resource.expects(:should_be_file?).returns false
-      @content.must be_insync("whatever")
+      @content.should = "foo"
+      @content.must be_safe_insync("whatever")
     end
 
     it "should return false if the current content is :absent" do
-      @content.should_not be_insync(:absent)
+      @content.should = "foo"
+      @content.should_not be_safe_insync(:absent)
     end
 
     it "should return false if the file should be a file but is not present" do
       @resource.expects(:should_be_file?).returns true
+      @content.should = "foo"
 
-      @content.should_not be_insync(:absent)
+      @content.should_not be_safe_insync(:absent)
     end
 
     describe "and the file exists" do
       before do
         @resource.stubs(:stat).returns mock("stat")
       end
 
       it "should return false if the current contents are different from the desired content" do
         @content.should = "some content"
-        @content.should_not be_insync("other content")
+        @content.should_not be_safe_insync("other content")
       end
 
       it "should return true if the sum for the current contents is the same as the sum for the desired content" do
         @content.should = "some content"
-        @content.must be_insync("{md5}" + Digest::MD5.hexdigest("some content"))
+        @content.must be_safe_insync("{md5}" + Digest::MD5.hexdigest("some content"))
       end
 
       describe "and Puppet[:show_diff] is set" do
         before do
           Puppet[:show_diff] = true
         end
 
         it "should display a diff if the current contents are different from the desired content" do
           @content.should = "some content"
           @content.expects(:diff).returns("my diff").once
           @content.expects(:print).with("my diff").once
 
-          @content.insync?("other content")
+          @content.safe_insync?("other content")
         end
 
         it "should not display a diff if the sum for the current contents is the same as the sum for the desired content" do
           @content.should = "some content"
           @content.expects(:diff).never
 
-          @content.insync?("{md5}" + Digest::MD5.hexdigest("some content"))
+          @content.safe_insync?("{md5}" + Digest::MD5.hexdigest("some content"))
         end
       end
     end
 
     describe "and :replace is false" do
       before do
         @resource.stubs(:replace?).returns false
       end
 
       it "should be insync if the file exists and the content is different" do
         @resource.stubs(:stat).returns mock('stat')
 
-        @content.must be_insync("whatever")
+        @content.must be_safe_insync("whatever")
       end
 
       it "should be insync if the file exists and the content is right" do
         @resource.stubs(:stat).returns mock('stat')
 
-        @content.must be_insync("something")
+        @content.must be_safe_insync("something")
       end
 
       it "should not be insync if the file does not exist" do
-        @content.should_not be_insync(:absent)
+        @content.should = "foo"
+        @content.should_not be_safe_insync(:absent)
       end
     end
   end
 
   describe "when changing the content" do
     before do
       @content = content.new(:resource => @resource)
       @content.should = "some content"
 
       @resource.stubs(:[]).with(:path).returns "/boo"
       @resource.stubs(:stat).returns "eh"
     end
 
     it "should use the file's :write method to write the content" do
       @resource.expects(:write).with(:content)
 
       @content.sync
     end
 
     it "should return :file_changed if the file already existed" do
       @resource.expects(:stat).returns "something"
       @resource.stubs(:write)
       @content.sync.should == :file_changed
     end
 
     it "should return :file_created if the file did not exist" do
       @resource.expects(:stat).returns nil
       @resource.stubs(:write)
       @content.sync.should == :file_created
     end
   end
 
   describe "when writing" do
     before do
       @content = content.new(:resource => @resource)
       @fh = stub_everything
     end
 
     it "should attempt to read from the filebucket if no actual content nor source exists" do
       @content.should = "{md5}foo"
       @content.resource.bucket.class.any_instance.stubs(:getfile).returns "foo"
       @content.write(@fh)
     end
 
     describe "from actual content" do
       before(:each) do
         @content.stubs(:actual_content).returns("this is content")
       end
 
       it "should write to the given file handle" do
         @fh.expects(:print).with("this is content")
         @content.write(@fh)
       end
 
       it "should return the current checksum value" do
         @resource.parameter(:checksum).expects(:sum_stream).returns "checksum"
         @content.write(@fh).should == "checksum"
       end
     end
 
     describe "from a file bucket" do
       it "should fail if a file bucket cannot be retrieved" do
         @content.should = "{md5}foo"
         @content.resource.expects(:bucket).returns nil
         lambda { @content.write(@fh) }.should raise_error(Puppet::Error)
       end
 
       it "should fail if the file bucket cannot find any content" do
         @content.should = "{md5}foo"
         bucket = stub 'bucket'
         @content.resource.expects(:bucket).returns bucket
         bucket.expects(:getfile).with("foo").raises "foobar"
         lambda { @content.write(@fh) }.should raise_error(Puppet::Error)
       end
 
       it "should write the returned content to the file" do
         @content.should = "{md5}foo"
         bucket = stub 'bucket'
         @content.resource.expects(:bucket).returns bucket
         bucket.expects(:getfile).with("foo").returns "mycontent"
 
         @fh.expects(:print).with("mycontent")
         @content.write(@fh)
       end
     end
 
     describe "from local source" do
       before(:each) do
         @content.stubs(:actual_content).returns(nil)
         @source = stub_everything 'source', :local? => true, :full_path => "/path/to/source"
         @resource.stubs(:parameter).with(:source).returns @source
 
         @sum = stub_everything 'sum'
         @resource.stubs(:parameter).with(:checksum).returns(@sum)
 
         @digest = stub_everything 'digest'
         @sum.stubs(:sum_stream).yields(@digest)
 
         @file = stub_everything 'file'
         File.stubs(:open).yields(@file)
         @file.stubs(:read).with(8192).returns("chunk1").then.returns("chunk2").then.returns(nil)
       end
 
       it "should open the local file" do
         File.expects(:open).with("/path/to/source", "r")
         @content.write(@fh)
       end
 
       it "should read the local file by chunks" do
         @file.expects(:read).with(8192).returns("chunk1").then.returns(nil)
         @content.write(@fh)
       end
 
       it "should write each chunk to the file" do
         @fh.expects(:print).with("chunk1").then.with("chunk2")
         @content.write(@fh)
       end
 
       it "should pass each chunk to the current sum stream" do
         @digest.expects(:<<).with("chunk1").then.with("chunk2")
         @content.write(@fh)
       end
 
       it "should return the checksum computed" do
         @sum.stubs(:sum_stream).yields(@digest).returns("checksum")
         @content.write(@fh).should == "checksum"
       end
     end
 
     describe "from remote source" do
       before(:each) do
         @response = stub_everything 'mock response', :code => "404"
         @conn = stub_everything 'connection'
         @conn.stubs(:request_get).yields(@response)
         Puppet::Network::HttpPool.stubs(:http_instance).returns @conn
 
         @content.stubs(:actual_content).returns(nil)
         @source = stub_everything 'source', :local? => false, :full_path => "/path/to/source", :server => "server", :port => 1234
         @resource.stubs(:parameter).with(:source).returns @source
 
         @sum = stub_everything 'sum'
         @resource.stubs(:parameter).with(:checksum).returns(@sum)
 
         @digest = stub_everything 'digest'
         @sum.stubs(:sum_stream).yields(@digest)
       end
 
       it "should open a network connection to source server and port" do
         Puppet::Network::HttpPool.expects(:http_instance).with("server", 1234).returns @conn
         @content.write(@fh)
       end
 
       it "should send the correct indirection uri" do
         @conn.expects(:request_get).with { |uri,headers| uri == "/production/file_content/path/to/source" }.yields(@response)
         @content.write(@fh)
       end
 
       it "should return nil if source is not found" do
         @response.expects(:code).returns("404")
         @content.write(@fh).should == nil
       end
 
       it "should not write anything if source is not found" do
         @response.expects(:code).returns("404")
         @fh.expects(:print).never
         @content.write(@fh).should == nil
       end
 
       it "should raise an HTTP error in case of server error" do
         @response.expects(:code).returns("500")
         lambda { @content.write(@fh) }.should raise_error
       end
 
       it "should write content by chunks" do
         @response.expects(:code).returns("200")
         @response.expects(:read_body).multiple_yields("chunk1","chunk2")
         @fh.expects(:print).with("chunk1").then.with("chunk2")
         @content.write(@fh)
       end
 
       it "should pass each chunk to the current sum stream" do
         @response.expects(:code).returns("200")
         @response.expects(:read_body).multiple_yields("chunk1","chunk2")
         @digest.expects(:<<).with("chunk1").then.with("chunk2")
         @content.write(@fh)
       end
 
       it "should return the checksum computed" do
         @response.expects(:code).returns("200")
         @response.expects(:read_body).multiple_yields("chunk1","chunk2")
         @sum.expects(:sum_stream).yields(@digest).returns("checksum")
         @content.write(@fh).should == "checksum"
       end
 
       it "should get the current accept encoding header value" do
         @content.expects(:add_accept_encoding)
         @content.write(@fh)
       end
 
       it "should uncompress body on error" do
         @response.expects(:code).returns("500")
         @response.expects(:body).returns("compressed body")
         @content.expects(:uncompress_body).with(@response).returns("uncompressed")
         lambda { @content.write(@fh) }.should raise_error { |e| e.message =~ /uncompressed/ }
       end
 
       it "should uncompress chunk by chunk" do
         uncompressor = stub_everything 'uncompressor'
         @content.expects(:uncompress).with(@response).yields(uncompressor)
         @response.expects(:code).returns("200")
         @response.expects(:read_body).multiple_yields("chunk1","chunk2")
 
         uncompressor.expects(:uncompress).with("chunk1").then.with("chunk2")
         @content.write(@fh)
       end
 
       it "should write uncompressed chunks to the file" do
         uncompressor = stub_everything 'uncompressor'
         @content.expects(:uncompress).with(@response).yields(uncompressor)
         @response.expects(:code).returns("200")
         @response.expects(:read_body).multiple_yields("chunk1","chunk2")
 
         uncompressor.expects(:uncompress).with("chunk1").returns("uncompressed1")
         uncompressor.expects(:uncompress).with("chunk2").returns("uncompressed2")
 
         @fh.expects(:print).with("uncompressed1")
         @fh.expects(:print).with("uncompressed2")
 
         @content.write(@fh)
       end
 
       it "should pass each uncompressed chunk to the current sum stream" do
         uncompressor = stub_everything 'uncompressor'
         @content.expects(:uncompress).with(@response).yields(uncompressor)
         @response.expects(:code).returns("200")
         @response.expects(:read_body).multiple_yields("chunk1","chunk2")
 
         uncompressor.expects(:uncompress).with("chunk1").returns("uncompressed1")
         uncompressor.expects(:uncompress).with("chunk2").returns("uncompressed2")
 
         @digest.expects(:<<).with("uncompressed1").then.with("uncompressed2")
         @content.write(@fh)
       end
     end
 
     describe "from a filebucket" do
     end
   end
 end
diff --git a/spec/unit/type/file/ensure_spec.rb b/spec/unit/type/file/ensure_spec.rb
index ec53ed85a..dbb3a1053 100755
--- a/spec/unit/type/file/ensure_spec.rb
+++ b/spec/unit/type/file/ensure_spec.rb
@@ -1,84 +1,85 @@
 #!/usr/bin/env ruby
 
 Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
 
 property = Puppet::Type.type(:file).attrclass(:ensure)
 
 describe property do
   before do
     # Wow that's a messy interface to the resource.
     @resource = stub 'resource', :[] => nil, :[]= => nil, :property => nil, :newattr => nil, :parameter => nil, :replace? => true
     @resource.stubs(:[]).returns "foo"
     @resource.stubs(:[]).with(:path).returns "/my/file"
     @ensure = property.new :resource => @resource
   end
 
   it "should be a subclass of Ensure" do
     property.superclass.must == Puppet::Property::Ensure
   end
 
   describe "when retrieving the current state" do
     it "should return :absent if the file does not exist" do
       @ensure = property.new(:resource => @resource)
       @resource.expects(:stat).returns nil
 
       @ensure.retrieve.should == :absent
     end
 
     it "should return the current file type if the file exists" do
       @ensure = property.new(:resource => @resource)
       stat = mock 'stat', :ftype => "directory"
       @resource.expects(:stat).returns stat
 
       @ensure.retrieve.should == :directory
     end
   end
 
   describe "when testing whether :ensure is in sync" do
     before do
       @ensure = property.new(:resource => @resource)
       @stat = stub 'stat', :ftype => "file"
     end
 
     it "should always be in sync if replace is 'false' unless the file is missing" do
+      @ensure.should = :file
       @resource.expects(:replace?).returns false
-      @ensure.insync?(:link).should be_true
+      @ensure.safe_insync?(:link).should be_true
     end
 
     it "should be in sync if :ensure is set to :absent and the file does not exist" do
       @ensure.should = :absent
 
-      @ensure.must be_insync(:absent)
+      @ensure.must be_safe_insync(:absent)
     end
 
     it "should not be in sync if :ensure is set to :absent and the file exists" do
       @ensure.should = :absent
 
-      @ensure.should_not be_insync(:file)
+      @ensure.should_not be_safe_insync(:file)
     end
 
     it "should be in sync if a normal file exists and :ensure is set to :present" do
       @ensure.should = :present
 
-      @ensure.must be_insync(:file)
+      @ensure.must be_safe_insync(:file)
     end
 
     it "should be in sync if a directory exists and :ensure is set to :present" do
       @ensure.should = :present
 
-      @ensure.must be_insync(:directory)
+      @ensure.must be_safe_insync(:directory)
     end
 
     it "should be in sync if a symlink exists and :ensure is set to :present" do
       @ensure.should = :present
 
-      @ensure.must be_insync(:link)
+      @ensure.must be_safe_insync(:link)
     end
 
     it "should not be in sync if :ensure is set to :file and a directory exists" do
       @ensure.should = :file
 
-      @ensure.should_not be_insync(:directory)
+      @ensure.should_not be_safe_insync(:directory)
     end
   end
 end
diff --git a/spec/unit/type/file/group_spec.rb b/spec/unit/type/file/group_spec.rb
index 2283b57fa..956cd57e7 100755
--- a/spec/unit/type/file/group_spec.rb
+++ b/spec/unit/type/file/group_spec.rb
@@ -1,123 +1,123 @@
 #!/usr/bin/env ruby
 
 Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
 
 property = Puppet::Type.type(:file).attrclass(:group)
 
 describe property do
   before do
     @resource = stub 'resource', :line => "foo", :file => "bar"
     @resource.stubs(:[]).returns "foo"
     @resource.stubs(:[]).with(:path).returns "/my/file"
     @group = property.new :resource => @resource
   end
 
   it "should have a method for testing whether a group is valid" do
     @group.must respond_to(:validgroup?)
   end
 
   it "should return the found gid if a group is valid" do
     @group.expects(:gid).with("foo").returns 500
     @group.validgroup?("foo").should == 500
   end
 
   it "should return false if a group is not valid" do
     @group.expects(:gid).with("foo").returns nil
     @group.validgroup?("foo").should be_false
   end
 
   describe "when retrieving the current value" do
     it "should return :absent if the file cannot stat" do
       @resource.expects(:stat).returns nil
 
       @group.retrieve.should == :absent
     end
 
     it "should get the gid from the stat instance from the file" do
       stat = stub 'stat', :ftype => "foo"
       @resource.expects(:stat).returns stat
       stat.expects(:gid).returns 500
 
       @group.retrieve.should == 500
     end
 
     it "should warn and return :silly if the found value is higher than the maximum uid value" do
       Puppet.settings.expects(:value).with(:maximum_uid).returns 500
 
       stat = stub 'stat', :ftype => "foo"
       @resource.expects(:stat).returns stat
       stat.expects(:gid).returns 1000
 
       @group.expects(:warning)
       @group.retrieve.should == :silly
     end
   end
 
   describe "when determining if the file is in sync" do
     it "should directly compare the group values if the desired group is an integer" do
       @group.should = [10]
-      @group.must be_insync(10)
+      @group.must be_safe_insync(10)
     end
 
     it "should treat numeric strings as integers" do
       @group.should = ["10"]
-      @group.must be_insync(10)
+      @group.must be_safe_insync(10)
     end
 
     it "should convert the group name to an integer if the desired group is a string" do
       @group.expects(:gid).with("foo").returns 10
       @group.should = %w{foo}
 
-      @group.must be_insync(10)
+      @group.must be_safe_insync(10)
     end
 
     it "should not validate that groups exist when a group is specified as an integer" do
       @group.expects(:gid).never
       @group.validgroup?(10)
     end
 
     it "should fail if it cannot convert a group name to an integer" do
       @group.expects(:gid).with("foo").returns nil
       @group.should = %w{foo}
 
-      lambda { @group.insync?(10) }.should raise_error(Puppet::Error)
+      lambda { @group.safe_insync?(10) }.should raise_error(Puppet::Error)
     end
 
     it "should return false if the groups are not equal" do
       @group.should = [10]
-      @group.should_not be_insync(20)
+      @group.should_not be_safe_insync(20)
     end
   end
 
   describe "when changing the group" do
     before do
       @group.should = %w{one}
       @group.stubs(:gid).returns 500
     end
 
     it "should chown the file if :links is set to :follow" do
       @resource.expects(:[]).with(:links).returns :follow
       File.expects(:chown)
 
       @group.sync
     end
 
     it "should lchown the file if :links is set to :manage" do
       @resource.expects(:[]).with(:links).returns :manage
       File.expects(:lchown)
 
       @group.sync
     end
 
     it "should use the first valid group in its 'should' list" do
       @group.should = %w{one two three}
       @group.expects(:validgroup?).with("one").returns nil
       @group.expects(:validgroup?).with("two").returns 500
       @group.expects(:validgroup?).with("three").never
 
       File.expects(:chown).with(nil, 500, "/my/file")
 
       @group.sync
     end
   end
 end
diff --git a/spec/unit/type/file/owner_spec.rb b/spec/unit/type/file/owner_spec.rb
index 8e136a187..bcb8e07d6 100755
--- a/spec/unit/type/file/owner_spec.rb
+++ b/spec/unit/type/file/owner_spec.rb
@@ -1,150 +1,150 @@
 #!/usr/bin/env ruby
 
 Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
 
 property = Puppet::Type.type(:file).attrclass(:owner)
 
 describe property do
   before do
     # FIXME: many of these tests exercise the provider rather than `owner`
     # and should be moved into provider tests. ~JW
     @provider = Puppet::Type.type(:file).provider(:posix).new
     @provider.stubs(:uid).with("one").returns(1)
 
     @resource = stub 'resource', :line => "foo", :file => "bar"
     @resource.stubs(:[]).returns "foo"
     @resource.stubs(:[]).with(:path).returns "/my/file"
     @resource.stubs(:provider).returns @provider
 
     @owner = property.new :resource => @resource
   end
 
   it "should have a method for testing whether an owner is valid" do
     @provider.must respond_to(:validuser?)
   end
 
   it "should return the found uid if an owner is valid" do
     @provider.expects(:uid).with("foo").returns 500
     @provider.validuser?("foo").should == 500
   end
 
   it "should return false if an owner is not valid" do
     @provider.expects(:uid).with("foo").returns nil
     @provider.validuser?("foo").should be_false
   end
 
   describe "when retrieving the current value" do
     it "should return :absent if the file cannot stat" do
       @resource.expects(:stat).returns nil
 
       @owner.retrieve.should == :absent
     end
 
     it "should get the uid from the stat instance from the file" do
       stat = stub 'stat', :ftype => "foo"
       @resource.expects(:stat).returns stat
       stat.expects(:uid).returns 500
 
       @owner.retrieve.should == 500
     end
 
     it "should warn and return :silly if the found value is higher than the maximum uid value" do
       Puppet.settings.expects(:value).with(:maximum_uid).returns 500
 
       stat = stub 'stat', :ftype => "foo"
       @resource.expects(:stat).returns stat
       stat.expects(:uid).returns 1000
 
       @provider.expects(:warning)
       @owner.retrieve.should == :silly
     end
   end
 
   describe "when determining if the file is in sync" do
     describe "and not running as root" do
       it "should warn once and return true" do
         Puppet.features.expects(:root?).returns false
 
         @provider.expects(:warnonce)
 
         @owner.should = [10]
-        @owner.must be_insync(20)
+        @owner.must be_safe_insync(20)
       end
     end
 
     before do
       Puppet.features.stubs(:root?).returns true
     end
 
     it "should be in sync if 'should' is not provided" do
-      @owner.must be_insync(10)
+      @owner.must be_safe_insync(10)
     end
 
     it "should directly compare the owner values if the desired owner is an integer" do
       @owner.should = [10]
-      @owner.must be_insync(10)
+      @owner.must be_safe_insync(10)
     end
 
     it "should treat numeric strings as integers" do
       @owner.should = ["10"]
-      @owner.must be_insync(10)
+      @owner.must be_safe_insync(10)
     end
 
     it "should convert the owner name to an integer if the desired owner is a string" do
       @provider.expects(:uid).with("foo").returns 10
       @owner.should = %w{foo}
 
-      @owner.must be_insync(10)
+      @owner.must be_safe_insync(10)
     end
 
     it "should not validate that users exist when a user is specified as an integer" do
       @provider.expects(:uid).never
       @provider.validuser?(10)
     end
 
     it "should fail if it cannot convert an owner name to an integer" do
       @provider.expects(:uid).with("foo").returns nil
       @owner.should = %w{foo}
 
-      lambda { @owner.insync?(10) }.should raise_error(Puppet::Error)
+      lambda { @owner.safe_insync?(10) }.should raise_error(Puppet::Error)
     end
 
     it "should return false if the owners are not equal" do
       @owner.should = [10]
-      @owner.should_not be_insync(20)
+      @owner.should_not be_safe_insync(20)
     end
   end
 
   describe "when changing the owner" do
     before do
       @owner.should = %w{one}
       @owner.stubs(:path).returns "path"
       @owner.stubs(:uid).returns 500
     end
 
     it "should chown the file if :links is set to :follow" do
       @resource.expects(:[]).with(:links).returns :follow
       File.expects(:chown)
 
       @owner.sync
     end
 
     it "should lchown the file if :links is set to :manage" do
       @resource.expects(:[]).with(:links).returns :manage
       File.expects(:lchown)
 
       @owner.sync
     end
 
     it "should use the first valid owner in its 'should' list" do
       @owner.should = %w{one two three}
       @provider.expects(:validuser?).with("one").returns nil
       @provider.expects(:validuser?).with("two").returns 500
       @provider.expects(:validuser?).with("three").never
 
       File.expects(:chown).with(500, nil, "/my/file")
 
       @owner.sync
     end
   end
 end
diff --git a/spec/unit/type/file/selinux_spec.rb b/spec/unit/type/file/selinux_spec.rb
index 1ca59e9e7..043471dec 100644
--- a/spec/unit/type/file/selinux_spec.rb
+++ b/spec/unit/type/file/selinux_spec.rb
@@ -1,83 +1,83 @@
 #!/usr/bin/env ruby
 
 Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
 
 
 [:seluser, :selrole, :seltype, :selrange].each do |param|
   property = Puppet::Type.type(:file).attrclass(param)
   describe property do
     before do
       @resource = Puppet::Type.type(:file).new :path => "/my/file"
       @sel = property.new :resource => @resource
     end
 
     it "retrieve on #{param} should return :absent if the file isn't statable" do
       @resource.expects(:stat).returns nil
       @sel.retrieve.should == :absent
     end
 
     it "should retrieve nil for #{param} if there is no SELinux support" do
       stat = stub 'stat', :ftype => "foo"
       @resource.expects(:stat).returns stat
       @sel.expects(:get_selinux_current_context).with("/my/file").returns nil
       @sel.retrieve.should be_nil
     end
 
     it "should retrieve #{param} if a SELinux context is found with a range" do
       stat = stub 'stat', :ftype => "foo"
       @resource.expects(:stat).returns stat
       @sel.expects(:get_selinux_current_context).with("/my/file").returns "user_u:role_r:type_t:s0"
       expectedresult = case param
         when :seluser; "user_u"
         when :selrole; "role_r"
         when :seltype; "type_t"
         when :selrange; "s0"
       end
       @sel.retrieve.should == expectedresult
     end
 
     it "should retrieve #{param} if a SELinux context is found without a range" do
       stat = stub 'stat', :ftype => "foo"
       @resource.expects(:stat).returns stat
       @sel.expects(:get_selinux_current_context).with("/my/file").returns "user_u:role_r:type_t"
       expectedresult = case param
         when :seluser; "user_u"
         when :selrole; "role_r"
         when :seltype; "type_t"
         when :selrange; nil
       end
       @sel.retrieve.should == expectedresult
     end
 
     it "should handle no default gracefully" do
       @sel.expects(:get_selinux_default_context).with("/my/file").returns nil
       @sel.default.must be_nil
     end
 
     it "should be able to detect matchpathcon defaults" do
       @sel.stubs(:debug)
       @sel.expects(:get_selinux_default_context).with("/my/file").returns "user_u:role_r:type_t:s0"
       expectedresult = case param
         when :seluser; "user_u"
         when :selrole; "role_r"
         when :seltype; "type_t"
         when :selrange; "s0"
       end
       @sel.default.must == expectedresult
     end
 
     it "should be able to set a new context" do
       stat = stub 'stat', :ftype => "foo"
       @sel.should = %w{newone}
       @sel.expects(:set_selinux_context).with("/my/file", ["newone"], param)
       @sel.sync
     end
 
-    it "should do nothing for insync? if no SELinux support" do
+    it "should do nothing for safe_insync? if no SELinux support" do
       @sel.should = %{newcontext}
       @sel.expects(:selinux_support?).returns false
-      @sel.insync?("oldcontext").should == true
+      @sel.safe_insync?("oldcontext").should == true
     end
   end
 end
 
diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb
index 22921d85a..51c27c0e6 100755
--- a/spec/unit/type/file_spec.rb
+++ b/spec/unit/type/file_spec.rb
@@ -1,1068 +1,1082 @@
 #!/usr/bin/env ruby
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
 describe Puppet::Type.type(:file) do
   before do
     Puppet.settings.stubs(:use)
     @real_posix = Puppet.features.posix?
     Puppet.features.stubs("posix?").returns(true)
 
     @path = Tempfile.new("puppetspec")
     pathname = @path.path
     @path.close!()
     @path = pathname
     @file = Puppet::Type::File.new(:name => @path)
 
     @catalog = Puppet::Resource::Catalog.new
     @file.catalog = @catalog
   end
 
   describe "when determining if recursion is enabled" do
     it "should default to recursion being disabled" do
       @file.should_not be_recurse
     end
     [true, "true", 10, "inf", "remote"].each do |value|
       it "should consider #{value} to enable recursion" do
         @file[:recurse] = value
         @file.must be_recurse
       end
     end
 
     [false, "false", 0].each do |value|
       it "should consider #{value} to disable recursion" do
         @file[:recurse] = value
         @file.should_not be_recurse
       end
     end
   end
 
   describe "#write" do
 
     it "should propagate failures encountered when renaming the temporary file" do
       File.stubs(:open)
 
       File.expects(:rename).raises ArgumentError
       file = Puppet::Type::File.new(:name => "/my/file", :backup => "puppet")
 
       file.stubs(:validate_checksum?).returns(false)
 
       property = stub('content_property', :actual_content => "something", :length => "something".length)
       file.stubs(:property).with(:content).returns(property)
 
       lambda { file.write(:content) }.should raise_error(Puppet::Error)
     end
 
     it "should delegate writing to the content property" do
       filehandle = stub_everything 'fh'
       File.stubs(:open).yields(filehandle)
       File.stubs(:rename)
       property = stub('content_property', :actual_content => "something", :length => "something".length)
       file = Puppet::Type::File.new(:name => "/my/file", :backup => "puppet")
       file.stubs(:validate_checksum?).returns(false)
       file.stubs(:property).with(:content).returns(property)
 
       property.expects(:write).with(filehandle)
 
       file.write(:content)
     end
 
     describe "when validating the checksum" do
       before { @file.stubs(:validate_checksum?).returns(true) }
 
       it "should fail if the checksum parameter and content checksums do not match" do
         checksum = stub('checksum_parameter',  :sum => 'checksum_b', :sum_file => 'checksum_b')
         @file.stubs(:parameter).with(:checksum).returns(checksum)
 
         property = stub('content_property', :actual_content => "something", :length => "something".length, :write => 'checksum_a')
         @file.stubs(:property).with(:content).returns(property)
 
         lambda { @file.write :NOTUSED }.should raise_error(Puppet::Error)
       end
     end
 
     describe "when not validating the checksum" do
       before { @file.stubs(:validate_checksum?).returns(false) }
 
       it "should not fail if the checksum property and content checksums do not match" do
         checksum = stub('checksum_parameter',  :sum => 'checksum_b')
         @file.stubs(:parameter).with(:checksum).returns(checksum)
 
         property = stub('content_property', :actual_content => "something", :length => "something".length, :write => 'checksum_a')
         @file.stubs(:property).with(:content).returns(property)
 
         lambda { @file.write :NOTUSED }.should_not raise_error(Puppet::Error)
       end
 
     end
   end
 
   it "should have a method for determining if the file is present" do
     @file.must respond_to(:exist?)
   end
 
   it "should be considered existent if it can be stat'ed" do
     @file.expects(:stat).returns mock('stat')
     @file.must be_exist
   end
 
   it "should be considered nonexistent if it can not be stat'ed" do
     @file.expects(:stat).returns nil
     @file.must_not be_exist
   end
 
   it "should have a method for determining if the file should be a normal file" do
     @file.must respond_to(:should_be_file?)
   end
 
   it "should be a file if :ensure is set to :file" do
     @file[:ensure] = :file
     @file.must be_should_be_file
   end
 
   it "should be a file if :ensure is set to :present and the file exists as a normal file" do
     @file.stubs(:stat).returns(mock('stat', :ftype => "file"))
     @file[:ensure] = :present
     @file.must be_should_be_file
   end
 
   it "should not be a file if :ensure is set to something other than :file" do
     @file[:ensure] = :directory
     @file.must_not be_should_be_file
   end
 
   it "should not be a file if :ensure is set to :present and the file exists but is not a normal file" do
     @file.stubs(:stat).returns(mock('stat', :ftype => "directory"))
     @file[:ensure] = :present
     @file.must_not be_should_be_file
   end
 
   it "should be a file if :ensure is not set and :content is" do
     @file[:content] = "foo"
     @file.must be_should_be_file
   end
 
   it "should be a file if neither :ensure nor :content is set but the file exists as a normal file" do
     @file.stubs(:stat).returns(mock("stat", :ftype => "file"))
     @file.must be_should_be_file
   end
 
   it "should not be a file if neither :ensure nor :content is set but the file exists but not as a normal file" do
     @file.stubs(:stat).returns(mock("stat", :ftype => "directory"))
     @file.must_not be_should_be_file
   end
 
   describe "when using POSIX filenames" do
     describe "on POSIX systems" do
       before do
         Puppet.features.stubs(:posix?).returns(true)
         Puppet.features.stubs(:microsoft_windows?).returns(false)
       end
 
       it "should autorequire its parent directory" do
         file = Puppet::Type::File.new(:path => "/foo/bar")
         dir = Puppet::Type::File.new(:path => "/foo")
         @catalog.add_resource file
         @catalog.add_resource dir
         reqs = file.autorequire
         reqs[0].source.must == dir
         reqs[0].target.must == file
       end
 
       it "should not autorequire its parent dir if its parent dir is itself" do
         file = Puppet::Type::File.new(:path => "/")
         @catalog.add_resource file
         file.autorequire.should be_empty
       end
 
       it "should remove trailing slashes" do
         file = Puppet::Type::File.new(:path => "/foo/bar/baz/")
         file[:path].should == "/foo/bar/baz"
       end
 
       it "should remove double slashes" do
         file = Puppet::Type::File.new(:path => "/foo/bar//baz")
         file[:path].should == "/foo/bar/baz"
       end
 
       it "should remove trailing double slashes" do
         file = Puppet::Type::File.new(:path => "/foo/bar/baz//")
         file[:path].should == "/foo/bar/baz"
       end
 
       it "should leave a single slash alone" do
         file = Puppet::Type::File.new(:path => "/")
         file[:path].should == "/"
       end
     end
 
     describe "on Microsoft Windows systems" do
       before do
         Puppet.features.stubs(:posix?).returns(false)
         Puppet.features.stubs(:microsoft_windows?).returns(true)
       end
 
       it "should refuse to work" do
         lambda { Puppet::Type::File.new(:path => "/foo/bar") }.should raise_error(Puppet::Error)
       end
     end
   end
 
   describe "when using Microsoft Windows filenames", :if => Puppet.features.microsoft_windows? do
     describe "on Microsoft Windows systems" do
       before do
         Puppet.features.stubs(:posix?).returns(false)
         Puppet.features.stubs(:microsoft_windows?).returns(true)
       end
 
       it "should autorequire its parent directory" do
         file = Puppet::Type::File.new(:path => "X:/foo/bar")
         dir = Puppet::Type::File.new(:path => "X:/foo")
         @catalog.add_resource file
         @catalog.add_resource dir
         reqs = file.autorequire
         reqs[0].source.must == dir
         reqs[0].target.must == file
       end
 
       it "should not autorequire its parent dir if its parent dir is itself" do
         file = Puppet::Type::File.new(:path => "X:/")
         @catalog.add_resource file
         file.autorequire.should be_empty
       end
 
       it "should remove trailing slashes" do
         file = Puppet::Type::File.new(:path => "X:/foo/bar/baz/")
         file[:path].should == "X:/foo/bar/baz"
       end
 
       it "should remove double slashes" do
         file = Puppet::Type::File.new(:path => "X:/foo/bar//baz")
         file[:path].should == "X:/foo/bar/baz"
       end
 
       it "should remove trailing double slashes" do
         file = Puppet::Type::File.new(:path => "X:/foo/bar/baz//")
         file[:path].should == "X:/foo/bar/baz"
       end
 
       it "should leave a drive letter with a slash alone" do
         file = Puppet::Type::File.new(:path => "X:/")
         file[:path].should == "X:/"
       end
 
       it "should add a slash to a drive letter" do
         file = Puppet::Type::File.new(:path => "X:")
         file[:path].should == "X:/"
       end
     end
 
     describe "on POSIX systems" do
       before do
         Puppet.features.stubs(:posix?).returns(true)
         Puppet.features.stubs(:microsoft_windows?).returns(false)
       end
 
       it "should refuse to work" do
         lambda { Puppet::Type::File.new(:path => "X:/foo/bar") }.should raise_error(Puppet::Error)
       end
     end
   end
 
   describe "when using UNC filenames" do
     describe "on Microsoft Windows systems", :if => Puppet.features.microsoft_windows? do
       before do
         Puppet.features.stubs(:posix?).returns(false)
         Puppet.features.stubs(:microsoft_windows?).returns(true)
       end
 
       it "should autorequire its parent directory" do
         file = Puppet::Type::File.new(:path => "//server/foo/bar")
         dir = Puppet::Type::File.new(:path => "//server/foo")
         @catalog.add_resource file
         @catalog.add_resource dir
         reqs = file.autorequire
         reqs[0].source.must == dir
         reqs[0].target.must == file
       end
 
       it "should not autorequire its parent dir if its parent dir is itself" do
         file = Puppet::Type::File.new(:path => "//server/foo")
         @catalog.add_resource file
         puts file.autorequire
         file.autorequire.should be_empty
       end
 
       it "should remove trailing slashes" do
         file = Puppet::Type::File.new(:path => "//server/foo/bar/baz/")
         file[:path].should == "//server/foo/bar/baz"
       end
 
       it "should remove double slashes" do
         file = Puppet::Type::File.new(:path => "//server/foo/bar//baz")
         file[:path].should == "//server/foo/bar/baz"
       end
 
       it "should remove trailing double slashes" do
         file = Puppet::Type::File.new(:path => "//server/foo/bar/baz//")
         file[:path].should == "//server/foo/bar/baz"
       end
 
       it "should remove a trailing slash from a sharename" do
         file = Puppet::Type::File.new(:path => "//server/foo/")
         file[:path].should == "//server/foo"
       end
 
       it "should not modify a sharename" do
         file = Puppet::Type::File.new(:path => "//server/foo")
         file[:path].should == "//server/foo"
       end
     end
 
     describe "on POSIX systems" do
       before do
         Puppet.features.stubs(:posix?).returns(true)
         Puppet.features.stubs(:microsoft_windows?).returns(false)
       end
 
       it "should refuse to work" do
         lambda { Puppet::Type::File.new(:path => "X:/foo/bar") }.should raise_error(Puppet::Error)
       end
     end
   end
 
   describe "when initializing" do
     it "should set a desired 'ensure' value if none is set and 'content' is set" do
       file = Puppet::Type::File.new(:name => "/my/file", :content => "/foo/bar")
       file[:ensure].should == :file
     end
 
     it "should set a desired 'ensure' value if none is set and 'target' is set" do
       file = Puppet::Type::File.new(:name => "/my/file", :target => "/foo/bar")
       file[:ensure].should == :symlink
     end
   end
 
   describe "when validating attributes" do
     %w{path checksum backup recurse recurselimit source replace force ignore links purge sourceselect}.each do |attr|
       it "should have a '#{attr}' parameter" do
         Puppet::Type.type(:file).attrtype(attr.intern).should == :param
       end
     end
 
     %w{content target ensure owner group mode type}.each do |attr|
       it "should have a '#{attr}' property" do
         Puppet::Type.type(:file).attrtype(attr.intern).should == :property
       end
     end
 
     it "should have its 'path' attribute set as its namevar" do
       Puppet::Type.type(:file).key_attributes.should == [:path]
     end
   end
 
   describe "when managing links" do
     require 'puppettest/support/assertions'
     include PuppetTest
     require 'tempfile'
 
     if @real_posix
       describe "on POSIX systems" do
         before do
           @basedir = tempfile
           Dir.mkdir(@basedir)
           @file = File.join(@basedir, "file")
           @link = File.join(@basedir, "link")
 
           File.open(@file, "w", 0644) { |f| f.puts "yayness"; f.flush }
           File.symlink(@file, @link)
 
 
                 @resource = Puppet::Type.type(:file).new(
 
             :path => @link,
 
             :mode => "755"
           )
           @catalog.add_resource @resource
         end
 
         after do
           remove_tmp_files
         end
 
         it "should default to managing the link" do
           @catalog.apply
           # I convert them to strings so they display correctly if there's an error.
           ("%o" % (File.stat(@file).mode & 007777)).should == "%o" % 0644
         end
 
         it "should be able to follow links" do
           @resource[:links] = :follow
           @catalog.apply
 
           ("%o" % (File.stat(@file).mode & 007777)).should == "%o" % 0755
         end
       end
     else # @real_posix
       # should recode tests using expectations instead of using the filesystem
     end
 
     describe "on Microsoft Windows systems" do
       before do
         Puppet.features.stubs(:posix?).returns(false)
         Puppet.features.stubs(:microsoft_windows?).returns(true)
       end
 
       it "should refuse to work with links"
     end
   end
 
   it "should be able to retrieve a stat instance for the file it is managing" do
     Puppet::Type.type(:file).new(:path => "/foo/bar", :source => "/bar/foo").should respond_to(:stat)
   end
 
   describe "when stat'ing its file" do
     before do
       @resource = Puppet::Type.type(:file).new(:path => "/foo/bar")
       @resource[:links] = :manage # so we always use :lstat
     end
 
     it "should use :stat if it is following links" do
       @resource[:links] = :follow
       File.expects(:stat)
 
       @resource.stat
     end
 
     it "should use :lstat if is it not following links" do
       @resource[:links] = :manage
       File.expects(:lstat)
 
       @resource.stat
     end
 
     it "should stat the path of the file" do
       File.expects(:lstat).with("/foo/bar")
 
       @resource.stat
     end
 
     # This only happens in testing.
     it "should return nil if the stat does not exist" do
       File.expects(:lstat).returns nil
 
       @resource.stat.should be_nil
     end
 
     it "should return nil if the file does not exist" do
       File.expects(:lstat).raises(Errno::ENOENT)
 
       @resource.stat.should be_nil
     end
 
     it "should return nil if the file cannot be stat'ed" do
       File.expects(:lstat).raises(Errno::EACCES)
 
       @resource.stat.should be_nil
     end
 
     it "should return the stat instance" do
       File.expects(:lstat).returns "mystat"
 
       @resource.stat.should == "mystat"
     end
 
     it "should cache the stat instance if it has a catalog and is applying" do
       stat = mock 'stat'
       File.expects(:lstat).returns stat
 
       catalog = Puppet::Resource::Catalog.new
       @resource.catalog = catalog
 
       catalog.stubs(:applying?).returns true
 
       @resource.stat.should equal(@resource.stat)
     end
   end
 
   describe "when flushing" do
     it "should flush all properties that respond to :flush" do
       @resource = Puppet::Type.type(:file).new(:path => "/foo/bar", :source => "/bar/foo")
       @resource.parameter(:source).expects(:flush)
       @resource.flush
     end
 
     it "should reset its stat reference" do
       @resource = Puppet::Type.type(:file).new(:path => "/foo/bar")
       File.expects(:lstat).times(2).returns("stat1").then.returns("stat2")
       @resource.stat.should == "stat1"
       @resource.flush
       @resource.stat.should == "stat2"
     end
   end
 
   it "should have a method for performing recursion" do
     @file.must respond_to(:perform_recursion)
   end
 
   describe "when executing a recursive search" do
     it "should use Metadata to do its recursion" do
       Puppet::FileServing::Metadata.expects(:search)
       @file.perform_recursion(@file[:path])
     end
 
     it "should use the provided path as the key to the search" do
       Puppet::FileServing::Metadata.expects(:search).with { |key, options| key == "/foo" }
       @file.perform_recursion("/foo")
     end
 
     it "should return the results of the metadata search" do
       Puppet::FileServing::Metadata.expects(:search).returns "foobar"
       @file.perform_recursion(@file[:path]).should == "foobar"
     end
 
     it "should pass its recursion value to the search" do
       @file[:recurse] = true
       Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurse] == true }
       @file.perform_recursion(@file[:path])
     end
 
     it "should pass true if recursion is remote" do
       @file[:recurse] = :remote
       Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurse] == true }
       @file.perform_recursion(@file[:path])
     end
 
     it "should pass its recursion limit value to the search" do
       @file[:recurselimit] = 10
       Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurselimit] == 10 }
       @file.perform_recursion(@file[:path])
     end
 
     it "should configure the search to ignore or manage links" do
       @file[:links] = :manage
       Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:links] == :manage }
       @file.perform_recursion(@file[:path])
     end
 
     it "should pass its 'ignore' setting to the search if it has one" do
       @file[:ignore] = %w{.svn CVS}
       Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:ignore] == %w{.svn CVS} }
       @file.perform_recursion(@file[:path])
     end
   end
 
   it "should have a method for performing local recursion" do
     @file.must respond_to(:recurse_local)
   end
 
   describe "when doing local recursion" do
     before do
       @metadata = stub 'metadata', :relative_path => "my/file"
     end
 
     it "should pass its path to the :perform_recursion method" do
       @file.expects(:perform_recursion).with(@file[:path]).returns [@metadata]
       @file.stubs(:newchild)
       @file.recurse_local
     end
 
     it "should return an empty hash if the recursion returns nothing" do
       @file.expects(:perform_recursion).returns nil
       @file.recurse_local.should == {}
     end
 
     it "should create a new child resource with each generated metadata instance's relative path" do
       @file.expects(:perform_recursion).returns [@metadata]
       @file.expects(:newchild).with(@metadata.relative_path).returns "fiebar"
       @file.recurse_local
     end
 
     it "should not create a new child resource for the '.' directory" do
       @metadata.stubs(:relative_path).returns "."
 
       @file.expects(:perform_recursion).returns [@metadata]
       @file.expects(:newchild).never
       @file.recurse_local
     end
 
     it "should return a hash of the created resources with the relative paths as the hash keys" do
       @file.expects(:perform_recursion).returns [@metadata]
       @file.expects(:newchild).with("my/file").returns "fiebar"
       @file.recurse_local.should == {"my/file" => "fiebar"}
     end
 
     it "should set checksum_type to none if this file checksum is none" do
       @file[:checksum] = :none
       Puppet::FileServing::Metadata.expects(:search).with { |path,params| params[:checksum_type] == :none }.returns [@metadata]
       @file.expects(:newchild).with("my/file").returns "fiebar"
       @file.recurse_local
     end
   end
 
   it "should have a method for performing link recursion" do
     @file.must respond_to(:recurse_link)
   end
 
   describe "when doing link recursion" do
     before do
       @first = stub 'first', :relative_path => "first", :full_path => "/my/first", :ftype => "directory"
       @second = stub 'second', :relative_path => "second", :full_path => "/my/second", :ftype => "file"
 
       @resource = stub 'file', :[]= => nil
     end
 
     it "should pass its target to the :perform_recursion method" do
       @file[:target] = "mylinks"
       @file.expects(:perform_recursion).with("mylinks").returns [@first]
       @file.stubs(:newchild).returns @resource
       @file.recurse_link({})
     end
 
     it "should ignore the recursively-found '.' file and configure the top-level file to create a directory" do
       @first.stubs(:relative_path).returns "."
       @file[:target] = "mylinks"
       @file.expects(:perform_recursion).with("mylinks").returns [@first]
       @file.stubs(:newchild).never
       @file.expects(:[]=).with(:ensure, :directory)
       @file.recurse_link({})
     end
 
     it "should create a new child resource for each generated metadata instance's relative path that doesn't already exist in the children hash" do
       @file.expects(:perform_recursion).returns [@first, @second]
       @file.expects(:newchild).with(@first.relative_path).returns @resource
       @file.recurse_link("second" => @resource)
     end
 
     it "should not create a new child resource for paths that already exist in the children hash" do
       @file.expects(:perform_recursion).returns [@first]
       @file.expects(:newchild).never
       @file.recurse_link("first" => @resource)
     end
 
     it "should set the target to the full path of discovered file and set :ensure to :link if the file is not a directory" do
       file = stub 'file'
       file.expects(:[]=).with(:target, "/my/second")
       file.expects(:[]=).with(:ensure, :link)
 
       @file.stubs(:perform_recursion).returns [@first, @second]
       @file.recurse_link("first" => @resource, "second" => file)
     end
 
     it "should :ensure to :directory if the file is a directory" do
       file = stub 'file'
       file.expects(:[]=).with(:ensure, :directory)
 
       @file.stubs(:perform_recursion).returns [@first, @second]
       @file.recurse_link("first" => file, "second" => @resource)
     end
 
     it "should return a hash with both created and existing resources with the relative paths as the hash keys" do
       file = stub 'file', :[]= => nil
 
       @file.expects(:perform_recursion).returns [@first, @second]
       @file.stubs(:newchild).returns file
       @file.recurse_link("second" => @resource).should == {"second" => @resource, "first" => file}
     end
   end
 
   it "should have a method for performing remote recursion" do
     @file.must respond_to(:recurse_remote)
   end
 
   describe "when doing remote recursion" do
     before do
       @file[:source] = "puppet://foo/bar"
 
       @first = Puppet::FileServing::Metadata.new("/my", :relative_path => "first")
       @second = Puppet::FileServing::Metadata.new("/my", :relative_path => "second")
       @first.stubs(:ftype).returns "directory"
       @second.stubs(:ftype).returns "directory"
 
       @parameter = stub 'property', :metadata= => nil
       @resource = stub 'file', :[]= => nil, :parameter => @parameter
     end
 
     it "should pass its source to the :perform_recursion method" do
       data = Puppet::FileServing::Metadata.new("/whatever", :relative_path => "foobar")
       @file.expects(:perform_recursion).with("puppet://foo/bar").returns [data]
       @file.stubs(:newchild).returns @resource
       @file.recurse_remote({})
     end
 
     it "should not recurse when the remote file is not a directory" do
       data = Puppet::FileServing::Metadata.new("/whatever", :relative_path => ".")
       data.stubs(:ftype).returns "file"
       @file.expects(:perform_recursion).with("puppet://foo/bar").returns [data]
       @file.expects(:newchild).never
       @file.recurse_remote({})
     end
 
     it "should set the source of each returned file to the searched-for URI plus the found relative path" do
       @first.expects(:source=).with File.join("puppet://foo/bar", @first.relative_path)
       @file.expects(:perform_recursion).returns [@first]
       @file.stubs(:newchild).returns @resource
       @file.recurse_remote({})
     end
 
     it "should create a new resource for any relative file paths that do not already have a resource" do
       @file.stubs(:perform_recursion).returns [@first]
       @file.expects(:newchild).with("first").returns @resource
       @file.recurse_remote({}).should == {"first" => @resource}
     end
 
     it "should not create a new resource for any relative file paths that do already have a resource" do
       @file.stubs(:perform_recursion).returns [@first]
       @file.expects(:newchild).never
       @file.recurse_remote("first" => @resource)
     end
 
     it "should set the source of each resource to the source of the metadata" do
       @file.stubs(:perform_recursion).returns [@first]
       @resource.stubs(:[]=)
       @resource.expects(:[]=).with(:source, File.join("puppet://foo/bar", @first.relative_path))
       @file.recurse_remote("first" => @resource)
     end
 
     # LAK:FIXME This is a bug, but I can't think of a fix for it.  Fortunately it's already
     # filed, and when it's fixed, we'll just fix the whole flow.
     it "should set the checksum type to :md5 if the remote file is a file" do
       @first.stubs(:ftype).returns "file"
       @file.stubs(:perform_recursion).returns [@first]
       @resource.stubs(:[]=)
       @resource.expects(:[]=).with(:checksum, :md5)
       @file.recurse_remote("first" => @resource)
     end
 
     it "should store the metadata in the source property for each resource so the source does not have to requery the metadata" do
       @file.stubs(:perform_recursion).returns [@first]
       @resource.expects(:parameter).with(:source).returns @parameter
 
       @parameter.expects(:metadata=).with(@first)
 
       @file.recurse_remote("first" => @resource)
     end
 
     it "should not create a new resource for the '.' file" do
       @first.stubs(:relative_path).returns "."
       @file.stubs(:perform_recursion).returns [@first]
 
       @file.expects(:newchild).never
 
       @file.recurse_remote({})
     end
 
     it "should store the metadata in the main file's source property if the relative path is '.'" do
       @first.stubs(:relative_path).returns "."
       @file.stubs(:perform_recursion).returns [@first]
 
       @file.parameter(:source).expects(:metadata=).with @first
 
       @file.recurse_remote("first" => @resource)
     end
 
     describe "and multiple sources are provided" do
       describe "and :sourceselect is set to :first" do
         it "should create file instances for the results for the first source to return any values" do
           data = Puppet::FileServing::Metadata.new("/whatever", :relative_path => "foobar")
           @file[:source] = %w{/one /two /three /four}
           @file.expects(:perform_recursion).with("/one").returns nil
           @file.expects(:perform_recursion).with("/two").returns []
           @file.expects(:perform_recursion).with("/three").returns [data]
           @file.expects(:perform_recursion).with("/four").never
           @file.expects(:newchild).with("foobar").returns @resource
           @file.recurse_remote({})
         end
       end
 
       describe "and :sourceselect is set to :all" do
         before do
           @file[:sourceselect] = :all
         end
 
         it "should return every found file that is not in a previous source" do
           klass = Puppet::FileServing::Metadata
           @file[:source] = %w{/one /two /three /four}
           @file.stubs(:newchild).returns @resource
 
           one = [klass.new("/one", :relative_path => "a")]
           @file.expects(:perform_recursion).with("/one").returns one
           @file.expects(:newchild).with("a").returns @resource
 
           two = [klass.new("/two", :relative_path => "a"), klass.new("/two", :relative_path => "b")]
           @file.expects(:perform_recursion).with("/two").returns two
           @file.expects(:newchild).with("b").returns @resource
 
           three = [klass.new("/three", :relative_path => "a"), klass.new("/three", :relative_path => "c")]
           @file.expects(:perform_recursion).with("/three").returns three
           @file.expects(:newchild).with("c").returns @resource
 
           @file.expects(:perform_recursion).with("/four").returns []
 
           @file.recurse_remote({})
         end
       end
     end
   end
 
   describe "when returning resources with :eval_generate" do
     before do
       @graph = stub 'graph', :add_edge => nil
       @catalog.stubs(:relationship_graph).returns @graph
 
       @file.catalog = @catalog
       @file[:recurse] = true
     end
 
     it "should recurse if recursion is enabled" do
       resource = stub('resource', :[] => "resource")
       @file.expects(:recurse?).returns true
       @file.expects(:recurse).returns [resource]
       @file.eval_generate.should == [resource]
     end
 
     it "should not recurse if recursion is disabled" do
       @file.expects(:recurse?).returns false
       @file.expects(:recurse).never
       @file.eval_generate.should == []
     end
 
     it "should return each resource found through recursion" do
       foo = stub 'foo', :[] => "/foo"
       bar = stub 'bar', :[] => "/bar"
       bar2 = stub 'bar2', :[] => "/bar"
 
       @file.expects(:recurse).returns [foo, bar]
 
       @file.eval_generate.should == [foo, bar]
     end
   end
 
   describe "when recursing" do
     before do
       @file[:recurse] = true
       @metadata = Puppet::FileServing::Metadata
     end
 
     describe "and a source is set" do
       before { @file[:source] = "/my/source" }
 
       it "should pass the already-discovered resources to recurse_remote" do
         @file.stubs(:recurse_local).returns(:foo => "bar")
         @file.expects(:recurse_remote).with(:foo => "bar").returns []
         @file.recurse
       end
     end
 
     describe "and a target is set" do
       before { @file[:target] = "/link/target" }
 
       it "should use recurse_link" do
         @file.stubs(:recurse_local).returns(:foo => "bar")
         @file.expects(:recurse_link).with(:foo => "bar").returns []
         @file.recurse
       end
     end
 
     it "should use recurse_local if recurse is not remote" do
       @file.expects(:recurse_local).returns({})
       @file.recurse
     end
 
     it "should not use recurse_local if recurse remote" do
       @file[:recurse] = :remote
       @file.expects(:recurse_local).never
       @file.recurse
     end
 
     it "should return the generated resources as an array sorted by file path" do
       one = stub 'one', :[] => "/one"
       two = stub 'two', :[] => "/one/two"
       three = stub 'three', :[] => "/three"
       @file.expects(:recurse_local).returns(:one => one, :two => two, :three => three)
       @file.recurse.should == [one, two, three]
     end
 
     describe "and purging is enabled" do
       before do
         @file[:purge] = true
       end
 
       it "should configure each file to be removed" do
         local = stub 'local'
         local.stubs(:[]).with(:source).returns nil # Thus, a local file
         local.stubs(:[]).with(:path).returns "foo"
         @file.expects(:recurse_local).returns("local" => local)
         local.expects(:[]=).with(:ensure, :absent)
 
         @file.recurse
       end
 
       it "should not remove files that exist in the remote repository" do
         @file["source"] = "/my/file"
         @file.expects(:recurse_local).returns({})
 
         remote = stub 'remote'
         remote.stubs(:[]).with(:source).returns "/whatever" # Thus, a remote file
         remote.stubs(:[]).with(:path).returns "foo"
 
         @file.expects(:recurse_remote).with { |hash| hash["remote"] = remote }
         remote.expects(:[]=).with(:ensure, :absent).never
 
         @file.recurse
       end
     end
 
     describe "and making a new child resource" do
       it "should not copy the parent resource's parent" do
         Puppet::Type.type(:file).expects(:new).with { |options| ! options.include?(:parent) }
         @file.newchild("my/path")
       end
 
       {:recurse => true, :target => "/foo/bar", :ensure => :present, :alias => "yay", :source => "/foo/bar"}.each do |param, value|
         it "should not pass on #{param} to the sub resource" do
           @file = Puppet::Type::File.new(:name => @path, param => value, :catalog => @catalog)
 
           @file.class.expects(:new).with { |params| params[param].nil? }
 
           @file.newchild("sub/file")
         end
       end
 
       it "should copy all of the parent resource's 'should' values that were set at initialization" do
         file = @file.class.new(:path => "/foo/bar", :owner => "root", :group => "wheel")
         @catalog.add_resource(file)
         file.class.expects(:new).with { |options| options[:owner] == "root" and options[:group] == "wheel" }
         file.newchild("my/path")
       end
 
       it "should not copy default values to the new child" do
         @file.class.expects(:new).with { |params| params[:backup].nil? }
         @file.newchild("my/path")
       end
 
       it "should not copy values to the child which were set by the source" do
         @file[:source] = "/foo/bar"
         metadata = stub 'metadata', :owner => "root", :group => "root", :mode => 0755, :ftype => "file", :checksum => "{md5}whatever"
         @file.parameter(:source).stubs(:metadata).returns metadata
 
         @file.parameter(:source).copy_source_values
 
         @file.class.expects(:new).with { |params| params[:group].nil? }
         @file.newchild("my/path")
       end
     end
   end
 
   describe "when setting the backup" do
     it "should default to 'puppet'" do
       Puppet::Type::File.new(:name => "/my/file")[:backup].should == "puppet"
     end
 
     it "should allow setting backup to 'false'" do
       (!Puppet::Type::File.new(:name => "/my/file", :backup => false)[:backup]).should be_true
     end
 
     it "should set the backup to '.puppet-bak' if it is set to true" do
       Puppet::Type::File.new(:name => "/my/file", :backup => true)[:backup].should == ".puppet-bak"
     end
 
     it "should support any other backup extension" do
       Puppet::Type::File.new(:name => "/my/file", :backup => ".bak")[:backup].should == ".bak"
     end
 
     it "should set the filebucket when backup is set to a string matching the name of a filebucket in the catalog" do
       catalog = Puppet::Resource::Catalog.new
       bucket_resource = Puppet::Type.type(:filebucket).new :name => "foo", :path => "/my/file/bucket"
       catalog.add_resource bucket_resource
 
       file = Puppet::Type::File.new(:name => "/my/file")
       catalog.add_resource file
 
       file[:backup] = "foo"
       file.bucket.should == bucket_resource.bucket
     end
 
     it "should find filebuckets added to the catalog after the file resource was created" do
       catalog = Puppet::Resource::Catalog.new
 
       file = Puppet::Type::File.new(:name => "/my/file", :backup => "foo")
       catalog.add_resource file
 
       bucket_resource = Puppet::Type.type(:filebucket).new :name => "foo", :path => "/my/file/bucket"
       catalog.add_resource bucket_resource
 
       file.bucket.should == bucket_resource.bucket
     end
 
     it "should have a nil filebucket if backup is false" do
       catalog = Puppet::Resource::Catalog.new
       bucket_resource = Puppet::Type.type(:filebucket).new :name => "foo", :path => "/my/file/bucket"
       catalog.add_resource bucket_resource
 
       file = Puppet::Type::File.new(:name => "/my/file", :backup => false)
       catalog.add_resource file
 
       file.bucket.should be_nil
     end
 
     it "should have a nil filebucket if backup is set to a string starting with '.'" do
       catalog = Puppet::Resource::Catalog.new
       bucket_resource = Puppet::Type.type(:filebucket).new :name => "foo", :path => "/my/file/bucket"
       catalog.add_resource bucket_resource
 
       file = Puppet::Type::File.new(:name => "/my/file", :backup => ".foo")
       catalog.add_resource file
 
       file.bucket.should be_nil
     end
 
     it "should fail if there's no catalog and backup is not false" do
       file = Puppet::Type::File.new(:name => "/my/file", :backup => "foo")
 
       lambda { file.bucket }.should raise_error(Puppet::Error)
     end
 
     it "should fail if a non-existent catalog is specified" do
       file = Puppet::Type::File.new(:name => "/my/file", :backup => "foo")
       catalog = Puppet::Resource::Catalog.new
       catalog.add_resource file
 
       lambda { file.bucket }.should raise_error(Puppet::Error)
     end
 
     it "should be able to use the default filebucket without a catalog" do
       file = Puppet::Type::File.new(:name => "/my/file", :backup => "puppet")
       file.bucket.should be_instance_of(Puppet::FileBucket::Dipper)
     end
 
     it "should look up the filebucket during finish()" do
       file = Puppet::Type::File.new(:name => "/my/file", :backup => ".foo")
       file.expects(:bucket)
       file.finish
     end
   end
 
   describe "when retrieving the current file state" do
     it "should copy the source values if the 'source' parameter is set" do
       file = Puppet::Type::File.new(:name => "/my/file", :source => "/foo/bar")
       file.parameter(:source).expects(:copy_source_values)
       file.retrieve
     end
   end
 
   describe ".title_patterns" do
     before do
       @type_class = Puppet::Type.type(:file)
     end
 
     it "should have a regexp that captures the entire string, except for a terminating slash" do
       patterns = @type_class.title_patterns
       string = "abc/\n\tdef/"
       patterns[0][0] =~ string
       $1.should == "abc/\n\tdef"
     end
   end
 
+  describe "when auditing" do
+    it "should not fail if creating a new file if group is not set" do
+      File.exists?(@path).should == false
+      file = Puppet::Type::File.new(:name => @path, :audit => "all", :content => "content")
+      catalog = Puppet::Resource::Catalog.new
+      catalog.add_resource(file)
+
+      Puppet::Util::Storage.stubs(:store) # to prevent the catalog from trying to write state.yaml
+      transaction = catalog.apply
+
+      transaction.report.resource_statuses["File[#{@path}]"].failed.should == false
+      File.exists?(@path).should == true
+    end
+  end
 end
diff --git a/spec/unit/type/mount_spec.rb b/spec/unit/type/mount_spec.rb
index ce82cb516..0d74042e3 100755
--- a/spec/unit/type/mount_spec.rb
+++ b/spec/unit/type/mount_spec.rb
@@ -1,261 +1,261 @@
 #!/usr/bin/env ruby
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
 describe Puppet::Type.type(:mount) do
   it "should have a :refreshable feature that requires the :remount method" do
     Puppet::Type.type(:mount).provider_feature(:refreshable).methods.should == [:remount]
   end
 
   it "should have no default value for :ensure" do
     mount = Puppet::Type.type(:mount).new(:name => "yay")
     mount.should(:ensure).should be_nil
   end
 end
 
 describe Puppet::Type.type(:mount), "when validating attributes" do
   [:name, :remounts].each do |param|
     it "should have a #{param} parameter" do
       Puppet::Type.type(:mount).attrtype(param).should == :param
     end
   end
 
   [:ensure, :device, :blockdevice, :fstype, :options, :pass, :dump, :atboot, :target].each do |param|
     it "should have a #{param} property" do
       Puppet::Type.type(:mount).attrtype(param).should == :property
     end
   end
 end
 
 describe Puppet::Type.type(:mount)::Ensure, "when validating values" do
   before do
     @provider = stub 'provider', :class => Puppet::Type.type(:mount).defaultprovider, :clear => nil
     Puppet::Type.type(:mount).defaultprovider.expects(:new).returns(@provider)
   end
 
   it "should alias :present to :defined as a value to :ensure" do
     mount = Puppet::Type.type(:mount).new(:name => "yay", :ensure => :present)
     mount.should(:ensure).should == :defined
   end
 
   it "should support :unmounted as a value to :ensure" do
     mount = Puppet::Type.type(:mount).new(:name => "yay", :ensure => :unmounted)
     mount.should(:ensure).should == :unmounted
   end
 
   it "should support :absent as a value to :ensure" do
     Puppet::Type.type(:mount).new(:name => "yay", :ensure => :absent)
   end
 
   it "should support :mounted as a value to :ensure" do
     Puppet::Type.type(:mount).new(:name => "yay", :ensure => :mounted)
   end
 end
 
 describe Puppet::Type.type(:mount)::Ensure do
   before :each do
     @provider = stub 'provider', :class => Puppet::Type.type(:mount).defaultprovider, :clear => nil, :satisfies? => true, :name => :mock
     Puppet::Type.type(:mount).defaultprovider.stubs(:new).returns(@provider)
     @mount = Puppet::Type.type(:mount).new(:name => "yay", :check => :ensure)
 
     @ensure = @mount.property(:ensure)
   end
 
   def mount_stub(params)
     Puppet::Type.type(:mount).validproperties.each do |prop|
       unless params[prop]
         params[prop] = :absent
         @mount[prop] = :absent
       end
     end
 
     params.each do |param, value|
       @provider.stubs(param).returns(value)
     end
   end
 
   describe Puppet::Type.type(:mount)::Ensure, "when retrieving its current state" do
 
     it "should return the provider's value if it is :absent" do
       @provider.expects(:ensure).returns(:absent)
       @ensure.retrieve.should == :absent
     end
 
     it "should return :mounted if the provider indicates it is mounted and the value is not :absent" do
       @provider.expects(:ensure).returns(:present)
       @provider.expects(:mounted?).returns(true)
       @ensure.retrieve.should == :mounted
     end
 
     it "should return :unmounted if the provider indicates it is not mounted and the value is not :absent" do
       @provider.expects(:ensure).returns(:present)
       @provider.expects(:mounted?).returns(false)
       @ensure.retrieve.should == :unmounted
     end
   end
 
   describe Puppet::Type.type(:mount)::Ensure, "when changing the host" do
 
     it "should destroy itself if it should be absent" do
       @provider.stubs(:mounted?).returns(false)
       @provider.expects(:destroy)
       @ensure.should = :absent
       @ensure.sync
     end
 
     it "should unmount itself before destroying if it is mounted and should be absent" do
       @provider.expects(:mounted?).returns(true)
       @provider.expects(:unmount)
       @provider.expects(:destroy)
       @ensure.should = :absent
       @ensure.sync
     end
 
     it "should create itself if it is absent and should be defined" do
       @provider.stubs(:ensure).returns(:absent)
       @provider.stubs(:mounted?).returns(true)
 
       @provider.stubs(:mounted?).returns(false)
       @provider.expects(:create)
       @ensure.should = :defined
       @ensure.sync
     end
 
     it "should not unmount itself if it is mounted and should be defined" do
       @provider.stubs(:ensure).returns(:mounted)
       @provider.stubs(:mounted?).returns(true)
 
       @provider.stubs(:create)
       @provider.expects(:mount).never
       @provider.expects(:unmount).never
       @ensure.should = :defined
       @ensure.sync
     end
 
     it "should not mount itself if it is unmounted and should be defined" do
       @provider.stubs(:ensure).returns(:unmounted)
       @provider.stubs(:mounted?).returns(false)
 
       @ensure.stubs(:syncothers)
       @provider.stubs(:create)
       @provider.expects(:mount).never
       @provider.expects(:unmount).never
       @ensure.should = :present
       @ensure.sync
     end
 
     it "should unmount itself if it is mounted and should be unmounted" do
       @provider.stubs(:ensure).returns(:present)
       @provider.stubs(:mounted?).returns(true)
 
       @ensure.stubs(:syncothers)
       @provider.expects(:unmount)
       @ensure.should = :unmounted
       @ensure.sync
     end
 
     it "should create and mount itself if it does not exist and should be mounted" do
       @provider.stubs(:ensure).returns(:absent)
       @provider.stubs(:mounted?).returns(false)
       @provider.expects(:create)
       @ensure.stubs(:syncothers)
       @provider.expects(:mount)
       @ensure.should = :mounted
       @ensure.sync
     end
 
     it "should mount itself if it is present and should be mounted" do
       @provider.stubs(:ensure).returns(:present)
       @provider.stubs(:mounted?).returns(false)
       @ensure.stubs(:syncothers)
       @provider.expects(:mount)
       @ensure.should = :mounted
       @ensure.sync
     end
 
     it "should create but not mount itself if it is absent and mounted and should be mounted" do
       @provider.stubs(:ensure).returns(:absent)
       @provider.stubs(:mounted?).returns(true)
       @ensure.stubs(:syncothers)
       @provider.expects(:create)
       @ensure.should = :mounted
       @ensure.sync
     end
 
     it "should be insync if it is mounted and should be defined" do
       @ensure.should = :defined
-      @ensure.insync?(:mounted).should == true
+      @ensure.safe_insync?(:mounted).should == true
     end
 
     it "should be insync if it is unmounted and should be defined" do
       @ensure.should = :defined
-      @ensure.insync?(:unmounted).should == true
+      @ensure.safe_insync?(:unmounted).should == true
     end
 
     it "should be insync if it is mounted and should be present" do
       @ensure.should = :present
-      @ensure.insync?(:mounted).should == true
+      @ensure.safe_insync?(:mounted).should == true
     end
 
     it "should be insync if it is unmounted and should be present" do
       @ensure.should = :present
-      @ensure.insync?(:unmounted).should == true
+      @ensure.safe_insync?(:unmounted).should == true
     end
   end
 
   describe Puppet::Type.type(:mount), "when responding to events" do
 
     it "should remount if it is currently mounted" do
       @provider.expects(:mounted?).returns(true)
       @provider.expects(:remount)
 
       @mount.refresh
     end
 
     it "should not remount if it is not currently mounted" do
       @provider.expects(:mounted?).returns(false)
       @provider.expects(:remount).never
 
       @mount.refresh
     end
 
     it "should not remount swap filesystems" do
       @mount[:fstype] = "swap"
       @provider.expects(:remount).never
 
       @mount.refresh
     end
   end
 end
 
 describe Puppet::Type.type(:mount), "when modifying an existing mount entry" do
   before do
     @provider = stub 'provider', :class => Puppet::Type.type(:mount).defaultprovider, :clear => nil, :satisfies? => true, :name => :mock, :remount => nil
     Puppet::Type.type(:mount).defaultprovider.stubs(:new).returns(@provider)
     @mount = Puppet::Type.type(:mount).new(:name => "yay", :ensure => :mounted)
 
     {:device => "/foo/bar", :blockdevice => "/other/bar", :target => "/what/ever", :fstype => 'eh', :options => "", :pass => 0, :dump => 0, :atboot => 0,
       :ensure => :mounted}.each do
       |param, value|
       @mount.provider.stubs(param).returns value
       @mount[param] = value
     end
 
     @mount.provider.stubs(:mounted?).returns true
 
     # stub this to not try to create state.yaml
     Puppet::Util::Storage.stubs(:store)
 
     @catalog = Puppet::Resource::Catalog.new
     @catalog.add_resource @mount
   end
 
   it "should use the provider to change the dump value" do
     @mount.provider.expects(:dump).returns 0
     @mount.provider.expects(:dump=).with(1)
 
     @mount[:dump] = 1
 
     @catalog.apply
   end
 end
diff --git a/spec/unit/type/ssh_authorized_key_spec.rb b/spec/unit/type/ssh_authorized_key_spec.rb
index a0b435f80..666616c03 100755
--- a/spec/unit/type/ssh_authorized_key_spec.rb
+++ b/spec/unit/type/ssh_authorized_key_spec.rb
@@ -1,152 +1,152 @@
 #!/usr/bin/env ruby
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
 ssh_authorized_key = Puppet::Type.type(:ssh_authorized_key)
 
 describe ssh_authorized_key do
   before do
     @class = Puppet::Type.type(:ssh_authorized_key)
 
     @provider_class = stub 'provider_class', :name => "fake", :suitable? => true, :supports_parameter? => true
     @class.stubs(:defaultprovider).returns(@provider_class)
     @class.stubs(:provider).returns(@provider_class)
 
     @provider = stub 'provider', :class => @provider_class, :file_path => "/tmp/whatever", :clear => nil
     @provider_class.stubs(:new).returns(@provider)
     @catalog = Puppet::Resource::Catalog.new
   end
 
   it "should have a name parameter" do
     @class.attrtype(:name).should == :param
   end
 
   it "should have :name be its namevar" do
     @class.key_attributes.should == [:name]
   end
 
   it "should have a :provider parameter" do
     @class.attrtype(:provider).should == :param
   end
 
   it "should have an ensure property" do
     @class.attrtype(:ensure).should == :property
   end
 
   it "should support :present as a value for :ensure" do
     proc { @class.new(:name => "whev", :ensure => :present, :user => "nobody") }.should_not raise_error
   end
 
   it "should support :absent as a value for :ensure" do
     proc { @class.new(:name => "whev", :ensure => :absent, :user => "nobody") }.should_not raise_error
   end
 
   it "should have an type property" do
     @class.attrtype(:type).should == :property
   end
   it "should support ssh-dss as an type value" do
     proc { @class.new(:name => "whev", :type => "ssh-dss", :user => "nobody") }.should_not raise_error
   end
   it "should support ssh-rsa as an type value" do
     proc { @class.new(:name => "whev", :type => "ssh-rsa", :user => "nobody") }.should_not raise_error
   end
   it "should support :dsa as an type value" do
     proc { @class.new(:name => "whev", :type => :dsa, :user => "nobody") }.should_not raise_error
   end
   it "should support :rsa as an type value" do
     proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody") }.should_not raise_error
   end
 
   it "should not support values other than ssh-dss, ssh-rsa, dsa, rsa in the ssh_authorized_key_type" do
     proc { @class.new(:name => "whev", :type => :something) }.should raise_error(Puppet::Error)
   end
 
   it "should have an key property" do
     @class.attrtype(:key).should == :property
   end
 
   it "should have an user property" do
     @class.attrtype(:user).should == :property
   end
 
   it "should have an options property" do
     @class.attrtype(:options).should == :property
   end
 
   it "'s options property should return well formed string of arrays from is_to_s" do
     resource = @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => ["a","b","c"])
 
     resource.property(:options).is_to_s(["a","b","c"]).should == "a,b,c"
   end
 
   it "'s options property should return well formed string of arrays from is_to_s" do
     resource = @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => ["a","b","c"])
 
     resource.property(:options).should_to_s(["a","b","c"]).should == "a,b,c"
   end
 
   it "should have a target property" do
     @class.attrtype(:target).should == :property
   end
 
   describe "when neither user nor target is specified" do
     it "should raise an error" do
       proc do
 
         @class.create(
 
           :name   => "Test",
           :key    => "AAA",
           :type   => "ssh-rsa",
 
           :ensure => :present)
       end.should raise_error(Puppet::Error)
     end
   end
 
   describe "when both target and user are specified" do
     it "should use target" do
 
       resource = @class.create(
 
         :name => "Test",
         :user => "root",
 
         :target => "/tmp/blah")
       resource.should(:target).should == "/tmp/blah"
     end
   end
 
 
   describe "when user is specified" do
     it "should determine target" do
 
       resource = @class.create(
 
         :name   => "Test",
 
         :user   => "root")
       target = File.expand_path("~root/.ssh/authorized_keys")
       resource.should(:target).should == target
     end
 
     # Bug #2124 - ssh_authorized_key always changes target if target is not defined
     it "should not raise spurious change events" do
       resource = @class.new(:name => "Test", :user => "root")
       target = File.expand_path("~root/.ssh/authorized_keys")
-      resource.property(:target).insync?(target).should == true
+      resource.property(:target).safe_insync?(target).should == true
     end
   end
 
   describe "when calling validate" do
     it "should not crash on a non-existant user" do
 
       resource = @class.create(
 
         :name   => "Test",
 
         :user   => "ihopesuchuserdoesnotexist")
       proc { resource.validate }.should_not raise_error
     end
   end
 end
diff --git a/spec/unit/type/user_spec.rb b/spec/unit/type/user_spec.rb
index ccea9ee4c..0c3e2ab2d 100755
--- a/spec/unit/type/user_spec.rb
+++ b/spec/unit/type/user_spec.rb
@@ -1,304 +1,304 @@
 #!/usr/bin/env ruby
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
 user = Puppet::Type.type(:user)
 
 describe user do
   before do
     ENV["PATH"] += File::PATH_SEPARATOR + "/usr/sbin" unless ENV["PATH"].split(File::PATH_SEPARATOR).include?("/usr/sbin")
     @provider = stub 'provider'
     @resource = stub 'resource', :resource => nil, :provider => @provider, :line => nil, :file => nil
   end
 
   it "should have a default provider inheriting from Puppet::Provider" do
     user.defaultprovider.ancestors.should be_include(Puppet::Provider)
   end
 
   it "should be able to create a instance" do
     user.new(:name => "foo").should_not be_nil
   end
 
   it "should have an allows_duplicates feature" do
     user.provider_feature(:allows_duplicates).should_not be_nil
   end
 
   it "should have an manages_homedir feature" do
     user.provider_feature(:manages_homedir).should_not be_nil
   end
 
   it "should have an manages_passwords feature" do
     user.provider_feature(:manages_passwords).should_not be_nil
   end
 
   it "should have a manages_solaris_rbac feature" do
     user.provider_feature(:manages_solaris_rbac).should_not be_nil
   end
 
   it "should have a manages_expiry feature" do
     user.provider_feature(:manages_expiry).should_not be_nil
   end
 
   it "should have a manages_password_age feature" do
     user.provider_feature(:manages_password_age).should_not be_nil
   end
 
   describe "instances" do
     it "should have a valid provider" do
       user.new(:name => "foo").provider.class.ancestors.should be_include(Puppet::Provider)
     end
 
     it "should delegate existence questions to its provider" do
       instance = user.new(:name => "foo")
       instance.provider.expects(:exists?).returns "eh"
       instance.exists?.should == "eh"
     end
   end
 
   properties = [:ensure, :uid, :gid, :home, :comment, :shell, :password, :password_min_age, :password_max_age, :groups, :roles, :auths, :profiles, :project, :keys, :expiry]
 
   properties.each do |property|
     it "should have a #{property} property" do
       user.attrclass(property).ancestors.should be_include(Puppet::Property)
     end
 
     it "should have documentation for its #{property} property" do
       user.attrclass(property).doc.should be_instance_of(String)
     end
   end
 
   list_properties = [:groups, :roles, :auths]
 
   list_properties.each do |property|
     it "should have a list '#{property}'" do
       user.attrclass(property).ancestors.should be_include(Puppet::Property::List)
     end
   end
 
   it "should have an ordered list 'profiles'" do
     user.attrclass(:profiles).ancestors.should be_include(Puppet::Property::OrderedList)
   end
 
   it "should have key values 'keys'" do
     user.attrclass(:keys).ancestors.should be_include(Puppet::Property::KeyValue)
   end
 
   describe "when retrieving all current values" do
     before do
       @user = user.new(:name => "foo", :uid => 10)
     end
 
     it "should return a hash containing values for all set properties" do
       @user[:gid] = 10
       @user.property(:ensure).expects(:retrieve).returns :present
       @user.property(:uid).expects(:retrieve).returns 15
       @user.property(:gid).expects(:retrieve).returns 15
       values = @user.retrieve
       [@user.property(:uid), @user.property(:gid)].each { |property| values.should be_include(property) }
     end
 
     it "should set all values to :absent if the user is absent" do
       @user.property(:ensure).expects(:retrieve).returns :absent
       @user.property(:uid).expects(:retrieve).never
       @user.retrieve[@user.property(:uid)].should == :absent
     end
 
     it "should include the result of retrieving each property's current value if the user is present" do
       @user.property(:ensure).expects(:retrieve).returns :present
       @user.property(:uid).expects(:retrieve).returns 15
       @user.retrieve[@user.property(:uid)].should == 15
     end
   end
 
   describe "when managing the ensure property" do
     before do
       @ensure = user.attrclass(:ensure).new(:resource => @resource)
     end
 
     it "should support a :present value" do
       lambda { @ensure.should = :present }.should_not raise_error
     end
 
     it "should support an :absent value" do
       lambda { @ensure.should = :absent }.should_not raise_error
     end
 
     it "should call :create on the provider when asked to sync to the :present state" do
       @provider.expects(:create)
       @ensure.should = :present
       @ensure.sync
     end
 
     it "should call :delete on the provider when asked to sync to the :absent state" do
       @provider.expects(:delete)
       @ensure.should = :absent
       @ensure.sync
     end
 
     describe "and determining the current state" do
       it "should return :present when the provider indicates the user exists" do
         @provider.expects(:exists?).returns true
         @ensure.retrieve.should == :present
       end
 
       it "should return :absent when the provider indicates the user does not exist" do
         @provider.expects(:exists?).returns false
         @ensure.retrieve.should == :absent
       end
     end
   end
 
   describe "when managing the uid property" do
     it "should convert number-looking strings into actual numbers" do
       uid = user.attrclass(:uid).new(:resource => @resource)
       uid.should = "50"
       uid.should.must == 50
     end
 
     it "should support UIDs as numbers" do
       uid = user.attrclass(:uid).new(:resource => @resource)
       uid.should = 50
       uid.should.must == 50
     end
 
     it "should :absent as a value" do
       uid = user.attrclass(:uid).new(:resource => @resource)
       uid.should = :absent
       uid.should.must == :absent
     end
   end
 
   describe "when managing the gid" do
     it "should :absent as a value" do
       gid = user.attrclass(:gid).new(:resource => @resource)
       gid.should = :absent
       gid.should.must == :absent
     end
 
     it "should convert number-looking strings into actual numbers" do
       gid = user.attrclass(:gid).new(:resource => @resource)
       gid.should = "50"
       gid.should.must == 50
     end
 
     it "should support GIDs specified as integers" do
       gid = user.attrclass(:gid).new(:resource => @resource)
       gid.should = 50
       gid.should.must == 50
     end
 
     it "should support groups specified by name" do
       gid = user.attrclass(:gid).new(:resource => @resource)
       gid.should = "foo"
       gid.should.must == "foo"
     end
 
     describe "when testing whether in sync" do
       before do
         @gid = user.attrclass(:gid).new(:resource => @resource, :should => %w{foo bar})
       end
 
       it "should return true if no 'should' values are set" do
         @gid = user.attrclass(:gid).new(:resource => @resource)
 
-        @gid.must be_insync(500)
+        @gid.must be_safe_insync(500)
       end
 
       it "should return true if any of the specified groups are equal to the current integer" do
         Puppet::Util.expects(:gid).with("foo").returns 300
         Puppet::Util.expects(:gid).with("bar").returns 500
 
-        @gid.must be_insync(500)
+        @gid.must be_safe_insync(500)
       end
 
       it "should return false if none of the specified groups are equal to the current integer" do
         Puppet::Util.expects(:gid).with("foo").returns 300
         Puppet::Util.expects(:gid).with("bar").returns 500
 
-        @gid.should_not be_insync(700)
+        @gid.should_not be_safe_insync(700)
       end
     end
 
     describe "when syncing" do
       before do
         @gid = user.attrclass(:gid).new(:resource => @resource, :should => %w{foo bar})
       end
 
       it "should use the first found, specified group as the desired value and send it to the provider" do
         Puppet::Util.expects(:gid).with("foo").returns nil
         Puppet::Util.expects(:gid).with("bar").returns 500
 
         @provider.expects(:gid=).with 500
 
         @gid.sync
       end
     end
   end
 
   describe "when managing expiry" do
     before do
       @expiry = user.attrclass(:expiry).new(:resource => @resource)
     end
 
     it "should fail if given an invalid date" do
       lambda { @expiry.should = "200-20-20" }.should raise_error(Puppet::Error)
     end
   end
 
   describe "when managing passwords" do
     before do
       @password = user.attrclass(:password).new(:resource => @resource, :should => "mypass")
     end
 
     it "should not include the password in the change log when adding the password" do
       @password.change_to_s(:absent, "mypass").should_not be_include("mypass")
     end
 
     it "should not include the password in the change log when changing the password" do
       @password.change_to_s("other", "mypass").should_not be_include("mypass")
     end
 
     it "should fail if a ':' is included in the password" do
       lambda { @password.should = "some:thing" }.should raise_error(Puppet::Error)
     end
 
     it "should allow the value to be set to :absent" do
       lambda { @password.should = :absent }.should_not raise_error
     end
   end
 
   describe "when manages_solaris_rbac is enabled" do
     before do
       @provider.stubs(:satisfies?).returns(false)
       @provider.expects(:satisfies?).with([:manages_solaris_rbac]).returns(true)
     end
 
     it "should support a :role value for ensure" do
       @ensure = user.attrclass(:ensure).new(:resource => @resource)
       lambda { @ensure.should = :role }.should_not raise_error
     end
   end
 
   describe "when user has roles" do
     before do
       # To test this feature, we have to support it.
       user.new(:name => "foo").provider.class.stubs(:feature?).returns(true)
     end
 
     it "should autorequire roles" do
       testuser = Puppet::Type.type(:user).new(:name => "testuser")
       testuser[:roles] = "testrole"
 
       testrole = Puppet::Type.type(:user).new(:name => "testrole")
 
       config = Puppet::Resource::Catalog.new :testing do |conf|
         [testuser, testrole].each { |resource| conf.add_resource resource }
       end
       Puppet::Type::User::ProviderDirectoryservice.stubs(:get_macosx_version_major).returns "10.5"
 
       rel = testuser.autorequire[0]
       rel.source.ref.should == testrole.ref
       rel.target.ref.should == testuser.ref
     end
   end
 end
diff --git a/spec/unit/type/zpool_spec.rb b/spec/unit/type/zpool_spec.rb
index db12459ab..be8cb12ba 100755
--- a/spec/unit/type/zpool_spec.rb
+++ b/spec/unit/type/zpool_spec.rb
@@ -1,110 +1,110 @@
 #!/usr/bin/env ruby
 
 Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
 
 zpool = Puppet::Type.type(:zpool)
 
 describe zpool do
   before do
     @provider = stub 'provider'
     @resource = stub 'resource', :resource => nil, :provider => @provider, :line => nil, :file => nil
   end
 
   properties = [:ensure, :disk, :mirror, :raidz, :spare, :log]
 
   properties.each do |property|
     it "should have a #{property} property" do
       zpool.attrclass(property).ancestors.should be_include(Puppet::Property)
     end
   end
 
   parameters = [:pool, :raid_parity]
 
   parameters.each do |parameter|
     it "should have a #{parameter} parameter" do
       zpool.attrclass(parameter).ancestors.should be_include(Puppet::Parameter)
     end
   end
 end
 
 vdev_property = Puppet::Property::VDev
 
 describe vdev_property do
   before do
     vdev_property.initvars
     @resource = stub 'resource', :[]= => nil, :property => nil
     @property = vdev_property.new(:resource => @resource)
   end
 
   it "should be insync if the devices are the same" do
     @property.should = ["dev1 dev2"]
-    @property.insync?(["dev2 dev1"]).must be_true
+    @property.safe_insync?(["dev2 dev1"]).must be_true
   end
 
   it "should be out of sync if the devices are not the same" do
     @property.should = ["dev1 dev3"]
-    @property.insync?(["dev2 dev1"]).must be_false
+    @property.safe_insync?(["dev2 dev1"]).must be_false
   end
 
   it "should be insync if the devices are the same and the should values are comma seperated" do
     @property.should = ["dev1", "dev2"]
-    @property.insync?(["dev2 dev1"]).must be_true
+    @property.safe_insync?(["dev2 dev1"]).must be_true
   end
 
   it "should be out of sync if the device is absent and should has a value" do
     @property.should = ["dev1", "dev2"]
-    @property.insync?(:absent).must be_false
+    @property.safe_insync?(:absent).must be_false
   end
 
   it "should be insync if the device is absent and should is absent" do
     @property.should = [:absent]
-    @property.insync?(:absent).must be_true
+    @property.safe_insync?(:absent).must be_true
   end
 end
 
 multi_vdev_property = Puppet::Property::MultiVDev
 
 describe multi_vdev_property do
   before do
     multi_vdev_property.initvars
     @resource = stub 'resource', :[]= => nil, :property => nil
     @property = multi_vdev_property.new(:resource => @resource)
   end
 
   it "should be insync if the devices are the same" do
     @property.should = ["dev1 dev2"]
-    @property.insync?(["dev2 dev1"]).must be_true
+    @property.safe_insync?(["dev2 dev1"]).must be_true
   end
 
   it "should be out of sync if the devices are not the same" do
     @property.should = ["dev1 dev3"]
-    @property.insync?(["dev2 dev1"]).must be_false
+    @property.safe_insync?(["dev2 dev1"]).must be_false
   end
 
   it "should be out of sync if the device is absent and should has a value" do
     @property.should = ["dev1", "dev2"]
-    @property.insync?(:absent).must be_false
+    @property.safe_insync?(:absent).must be_false
   end
 
   it "should be insync if the device is absent and should is absent" do
     @property.should = [:absent]
-    @property.insync?(:absent).must be_true
+    @property.safe_insync?(:absent).must be_true
   end
 
   describe "when there are multiple lists of devices" do
     it "should be in sync if each group has the same devices" do
       @property.should = ["dev1 dev2", "dev3 dev4"]
-      @property.insync?(["dev2 dev1", "dev3 dev4"]).must be_true
+      @property.safe_insync?(["dev2 dev1", "dev3 dev4"]).must be_true
     end
 
     it "should be out of sync if any group has the different devices" do
       @property.should = ["dev1 devX", "dev3 dev4"]
-      @property.insync?(["dev2 dev1", "dev3 dev4"]).must be_false
+      @property.safe_insync?(["dev2 dev1", "dev3 dev4"]).must be_false
     end
 
     it "should be out of sync if devices are in the wrong group" do
       @property.should = ["dev1 dev2", "dev3 dev4"]
-      @property.insync?(["dev2 dev3", "dev1 dev4"]).must be_false
+      @property.safe_insync?(["dev2 dev3", "dev1 dev4"]).must be_false
     end
   end
 end