Fixing some sticky problems with checksums when just using the "check" metaparam.

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

4 files changed, 40 insertions, 19 deletions

diff --git a/lib/puppet/metatype/evaluation.rb b/lib/puppet/metatype/evaluation.rb
index 1ba9dee73..dde86a28d 100644
--- a/lib/puppet/metatype/evaluation.rb
+++ b/lib/puppet/metatype/evaluation.rb
@@ -52,8 +52,6 @@ class Puppet::Type
     # this returns any changes resulting from testing, thus 'collect'
     # rather than 'each'
     def evaluate
-        now = Time.now
-
         #Puppet.err "Evaluating %s" % self.path.join(":")
         unless defined? @evalcount
             self.err "No evalcount defined on '%s' of type '%s'" %
@@ -77,7 +75,7 @@ class Puppet::Type
 
         changes << @children.collect { |child|
             ch = child.evaluate
-            child.cache(:checked, now)
+            child.cache(:checked, Time.now)
             ch
         }
 
@@ -92,7 +90,7 @@ class Puppet::Type
             self.debug "%s change(s)" %
                 [changes.length]
         end
-        self.cache(:checked, now)
+        self.cache(:checked, Time.now)
         return changes.flatten
     end
 
diff --git a/lib/puppet/metatype/metaparams.rb b/lib/puppet/metatype/metaparams.rb
index b4030d3d0..e09bbc6d5 100644
--- a/lib/puppet/metatype/metaparams.rb
+++ b/lib/puppet/metatype/metaparams.rb
@@ -92,6 +92,7 @@ class Puppet::Type
             }
         end
     end
+
     # For each object we require, subscribe to all events that it generates. We
     # might reduce the level of subscription eventually, but for now...
     newmetaparam(:require) do
diff --git a/lib/puppet/type/pfile/checksum.rb b/lib/puppet/type/pfile/checksum.rb
index 355d211a5..a91f7e017 100755
--- a/lib/puppet/type/pfile/checksum.rb
+++ b/lib/puppet/type/pfile/checksum.rb
@@ -60,6 +60,9 @@ module Puppet
         # Store the checksum in the data cache, or retrieve it if only the
         # sum type is provided.
         def cache(type, sum = nil)
+            unless type
+                raise ArgumentError, "A type must be specified to cache a checksum"
+            end
             type = symbolize(type)
             unless state = @parent.cached(:checksums) 
                 self.debug "Initializing checksum hash"
@@ -85,6 +88,10 @@ module Puppet
             self.updatesum
         end
 
+        def checktype
+            self.should || :md5
+        end
+
         # Checksums need to invert how changes are printed.
         def change_to_s
             begin
@@ -95,8 +102,13 @@ module Puppet
                     return "undefined %s from '%s'" %
                         [self.name, self.is_to_s]
                 else
-                    return "%s changed '%s' to '%s'" %
-                        [self.name, self.currentsum, self.is_to_s]
+                    if defined? @cached and @cached
+                        return "%s changed '%s' to '%s'" %
+                            [self.name, @cached, self.is_to_s]
+                    else
+                        return "%s changed '%s' to '%s'" %
+                            [self.name, self.currentsum, self.is_to_s]
+                    end
                 end
             rescue Puppet::Error, Puppet::DevError
                 raise
@@ -108,7 +120,7 @@ module Puppet
 
         def currentsum
             #"{%s}%s" % [self.should, cache(self.should)]
-            cache(self.should)
+            cache(checktype())
         end
 
         # Retrieve the cached sum
@@ -236,7 +248,8 @@ module Puppet
         end
 
         def insync?
-            if cache(self.should)
+            @should = [checktype]
+            if cache(checktype())
                 return @is == currentsum()
             else
                 # If there's no cached sum, then we don't want to generate
@@ -268,16 +281,14 @@ module Puppet
                 return
             end
 
-            checktype = self.should || :md5
-
             # Just use the first allowed check type
-            @is = getsum(checktype)
+            @is = getsum(checktype())
 
             # If there is no sum defined, then store the current value
             # into the cache, so that we're not marked as being
             # out of sync.  We don't want to generate an event the first
             # time we get a sum.
-            unless cache(self.should)
+            unless cache(checktype())
                 # FIXME we should support an updatechecksums-like mechanism
                 self.updatesum
             end
@@ -294,7 +305,7 @@ module Puppet
             end
 
             # if we're replacing, vs. updating
-            if sum = cache(self.should)
+            if sum = cache(checktype())
                 unless defined? @should
                     raise Puppet::Error.new(
                         ("@should is not initialized for %s, even though we " +
@@ -318,7 +329,10 @@ module Puppet
                 @parent.debug "Creating checksum %s" % @is
                 result = false
             end
-            cache(self.should, @is)
+
+            # Cache the sum so the log message can be right if possible.
+            @cached = sum
+            cache(checktype(), @is)
             return result
         end
     end
diff --git a/lib/puppet/type/state.rb b/lib/puppet/type/state.rb
index d8578eb0a..cf400f613 100644
--- a/lib/puppet/type/state.rb
+++ b/lib/puppet/type/state.rb
@@ -247,6 +247,9 @@ class State < Puppet::Parameter
         end
 
         value = self.should
+        if value.nil?
+            self.devfail "Got a nil value for should"
+        end
         method = "set_" + value.to_s
         event = nil
         if self.respond_to?(method)
@@ -270,11 +273,16 @@ class State < Puppet::Parameter
             # the blocks could return values.
             event = self.instance_eval(&ary[1])
         else
-            begin
-                provider.send(self.class.name.to_s + "=", self.should)
-            rescue NoMethodError
-                self.fail "The %s provider can not handle attribute %s" %
-                    [provider.class.name, self.class.name]
+            if @parent.provider
+                begin
+                    provider.send(self.class.name.to_s + "=", self.should)
+                rescue NoMethodError
+                    self.fail "The %s provider can not handle attribute %s" %
+                        [provider.class.name, self.class.name]
+                end
+            else
+                self.fail "%s cannot handle values of type %s" %
+                    [self.class.name, self.should.inspect]
             end
         end