Fixed merging of state values, but I have not yet solved merging of parameter or metaparam values, since they are quite different.

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

3 files changed, 90 insertions, 66 deletions

diff --git a/lib/puppet/log.rb b/lib/puppet/log.rb
index 62fe98fdf..7c30e88b5 100644
--- a/lib/puppet/log.rb
+++ b/lib/puppet/log.rb
@@ -167,10 +167,10 @@ module Puppet # :nodoc:
                     # cannot log a message with a '%' in it.  So, we get rid
                     # of them.
                     if msg.source == "Puppet"
-                        dest.send(msg.level, msg.to_s.gsub("%", ''))
+                        dest.send(msg.level, msg.to_s.gsub("%", '%%'))
                     else
                         dest.send(msg.level, "(%s) %s" %
-                            [msg.source.to_s.gsub("%", ""), msg.to_s.gsub("%", '')]
+                            [msg.source.to_s.gsub("%", ""), msg.to_s.gsub("%", '%%')]
                         )
                     end
                 when File:
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 8a08152f8..9d728c07c 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -808,36 +808,46 @@ class Type < Puppet::Element
         self.class[self.name] = self
     end
 
-    # merge new information with an existing object, checking for conflicts
-    # and such
+    # Merge new information with an existing object, checking for conflicts
+    # and such.  This allows for two specifications of the same object and
+    # the same values, but it's pretty limited right now.  The result of merging
+    # states is very different from the result of merging parameters or metaparams.
     def merge(hash)
         hash.each { |param, value|
             if param.is_a?(String)
                 param = param.intern
             end
+            
+            # Of course names are the same, duh.
             next if param == :name or param == self.class.namevar
 
             unless value.is_a?(Array)
                 value = [value]
             end
 
-            # This needs some way to retrieve the original values specified to
-            # 'should'.
-            if oldvals = self.should(param)
-                if oldvals.is_a?(Array)
-                    # take the intersection
-                    newvals = oldvals & value
-                    if newvals.empty?
-                        raise Puppet::Error, "No common values for %s on %s(%s)" %
-                            [param, self.class.name, self.name]
-                    else
-                        self.debug "Reduced old values %s and new values %s to %s" %
-                            [oldvals.inspect, value.inspect, newvals.inspect]
-                        self.should = newvals
-                    end
-                else
-                    raise Puppet::Error, "Cannot override %s on %s(%s)" %
+            if oldvals = @states[param].shouldorig
+                unless oldvals.is_a?(Array)
+                    oldvals = [oldvals]
+                end
+                # If the values are exactly the same, order and everything,
+                # then it's okay.
+                if oldvals == value
+                    return true
+                end
+                # take the intersection
+                newvals = oldvals & value
+                if newvals.empty?
+                    raise Puppet::Error, "No common values for %s on %s(%s)" %
+                        [param, self.class.name, self.name]
+                elsif newvals.length > 1
+                    raise Puppet::Error, "Too many values for %s on %s(%s)" %
                         [param, self.class.name, self.name]
+                else
+                    self.debug "Reduced old values %s and new values %s to %s" %
+                        [oldvals.inspect, value.inspect, newvals.inspect]
+                    @states[param].should = newvals
+                    #self.should = newvals
+                    return true
                 end
             else
                 self[param] = value
diff --git a/lib/puppet/type/pfile/uid.rb b/lib/puppet/type/pfile/uid.rb
index b61fa8af7..299d92756 100755
--- a/lib/puppet/type/pfile/uid.rb
+++ b/lib/puppet/type/pfile/uid.rb
@@ -20,6 +20,40 @@ module Puppet
                 end
             end
 
+            def name2id(value)
+                begin
+                    user = Etc.getpwnam(value)
+                    if user.uid == ""
+                        return nil
+                    end
+                    return user.uid
+                rescue ArgumentError => detail
+                    return nil
+                end
+            end
+
+            # Determine if the user is valid, and if so, return the UID
+            def validuser?(value)
+                if value =~ /^\d+$/
+                    value = value.to_i
+                end
+
+                if value.is_a?(Integer)
+                    # verify the user is a valid user
+                    if tmp = id2name(value)
+                        return value
+                    else
+                        return false
+                    end
+                else
+                    if tmp = name2id(value)
+                        return tmp
+                    else
+                        return false
+                    end
+                end
+            end
+
             # We want to print names, not numbers
             def is_to_s
                 id2name(@is) || @is
@@ -38,49 +72,17 @@ module Puppet
                 self.is = stat.uid
             end
 
-            # If we're not root, we can check the values but we cannot change them
+            # If we're not root, we can check the values but we cannot change them.
+            # We can't really do any processing here, because users might
+            # not exist yet.  FIXME There's still a bit of a problem here if
+            # the user's UID changes at run time, but we're just going to have
+            # to be okay with that for now, unfortunately.
             def shouldprocess(value)
-                if value.is_a?(Integer)
-                    # verify the user is a valid user
-                    begin
-                        user = Etc.getpwuid(value)
-                        if user.uid == ""
-                            error = Puppet::Error.new(
-                                "Could not retrieve uid for '%s'" %
-                                    @parent.name)
-                            raise error
-                        end
-                    rescue ArgumentError => detail
-                        raise Puppet::Error.new("User ID %s does not exist" %
-                            value
-                        )
-                    rescue => detail
-                        raise Puppet::Error.new(
-                            "Could not find user '%s': %s" % [value, detail])
-                        raise error
-                    end
+                if tmp = self.validuser?(value)
+                    return tmp
                 else
-                    begin
-                        user = Etc.getpwnam(value)
-                        if user.uid == ""
-                            error = Puppet::Error.new(
-                                "Could not retrieve uid for '%s'" %
-                                    @parent.name)
-                            raise error
-                        end
-                        value = user.uid
-                    rescue ArgumentError => detail
-                        raise Puppet::Error.new("User %s does not exist" %
-                            value
-                        )
-                    rescue => detail
-                        error = Puppet::Error.new(
-                            "Could not find user '%s': %s" % [value, detail])
-                        raise error
-                    end
+                    return value
                 end
-
-                return value
             end
 
             def sync
@@ -90,13 +92,25 @@ module Puppet
                         #@parent.delete(self.name)
                         @@notifieduid = true
                     end
-                    # there's a possibility that we never got retrieve() called
-                    # e.g., if the file didn't exist
-                    # thus, just delete ourselves now and don't do any work
-                    #@parent.delete(self.name)
                     return nil
                 end
 
+                user = nil
+                unless user = self.validuser?(self.should)
+                    tmp = self.should
+                    unless defined? @@usermissing
+                        @@usermissing = {}
+                    end
+
+                    if @@usermissing.include?(tmp)
+                        @@usermissing[tmp] += 1
+                    else
+                        self.notice "user %s does not exist" % tmp
+                        @@usermissing[tmp] = 1
+                        return nil
+                    end
+                end
+
                 if @is == :notfound
                     @parent.stat(true)
                     self.retrieve
@@ -111,10 +125,10 @@ module Puppet
                 end
 
                 begin
-                    File.chown(self.should,nil,@parent[:path])
+                    File.chown(user, nil, @parent[:path])
                 rescue => detail
                     raise Puppet::Error, "Failed to set owner to '%s': %s" %
-                        [self.should,detail]
+                        [user, detail]
                 end
 
                 return :inode_changed