Another round of bug-fixes, prompted by test logs from David Schmitt

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

7 files changed, 79 insertions, 112 deletions

diff --git a/lib/puppet/metatype/attributes.rb b/lib/puppet/metatype/attributes.rb
index a96533964..b4cc5f687 100644
--- a/lib/puppet/metatype/attributes.rb
+++ b/lib/puppet/metatype/attributes.rb
@@ -102,26 +102,28 @@ class Puppet::Type
         end
     end
 
-    # A similar function but one that yields the name, type, and class.
+    # A similar function but one that yields the class and type.
     # This is mainly so that setdefaults doesn't call quite so many functions.
     def self.eachattr(*ary)
-        # now get all of the arguments, in a specific order
-        # Cache this, since it gets called so many times
-
         if ary.empty?
             ary = nil
         end
-        self.properties.each { |property|
-            yield(property, :property) if ary.nil? or ary.include?(property.name)
-        }
-
-        @parameters.each { |param|
-            yield(param, :param) if ary.nil? or ary.include?(param.name)
-        }
 
-        @@metaparams.each { |param|
-            yield(param, :meta) if ary.nil? or ary.include?(param.name)
-        }
+        # We have to do this in a specific order, so that defaults are
+        # created in that order (e.g., providers should be set up before
+        # anything else).
+        allattrs.each do |name|
+            next unless ary.nil? or ary.include?(name)
+            if obj = @properties.find { |p| p.name == name }
+                yield obj, :property
+            elsif obj = @parameters.find { |p| p.name == name }
+                yield obj, :param
+            elsif obj = @@metaparams.find { |p| p.name == name }
+                yield obj, :meta
+            else
+                raise Puppet::DevError, "Could not find parameter %s" % name
+            end
+        end
     end
 
     def self.eachmetaparam
@@ -627,6 +629,7 @@ class Puppet::Type
     # 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 setdefaults(*ary)
+        #self.class.eachattr(*ary) { |klass, type|
         self.class.eachattr(*ary) { |klass, type|
             # not many attributes will have defaults defined, so we short-circuit
             # those away
diff --git a/lib/puppet/metatype/providers.rb b/lib/puppet/metatype/providers.rb
index c078ced24..98b5caf43 100644
--- a/lib/puppet/metatype/providers.rb
+++ b/lib/puppet/metatype/providers.rb
@@ -45,74 +45,6 @@ class Puppet::Type
         return @defaultprovider
     end
 
-    # Define one or more features.  Currently, features are just a list of
-    # methods; if all methods are defined as instance methods on the provider,
-    # then the provider has that feature, otherwise it does not.
-    def self.dis_features(hash)
-        @features ||= {}
-        hash.each do |name, methods|
-            name = symbolize(name)
-            methods = methods.collect { |m| symbolize(m) }
-            if @features.include?(name)
-                raise Puppet::DevError, "Feature %s is already defined" % name
-            end
-            @features[name] = methods
-        end
-    end
-
-    # Generate a module that sets up the boolean methods to test for given
-    # features.
-    def self.dis_feature_module
-        unless defined? @feature_module
-            @features ||= {}
-            @feature_module = ::Module.new
-            const_set("FeatureModule", @feature_module)
-            features = @features
-            @feature_module.send(:define_method, :feature?) do |name|
-                method = name.to_s + "?"
-                if respond_to?(method) and send(method)
-                    return true
-                else
-                    return false
-                end
-            end
-            @feature_module.send(:define_method, :features) do
-                return false unless defined?(features)
-                features.keys.find_all { |n| feature?(n) }.sort { |a,b| 
-                    a.to_s <=> b.to_s 
-                }
-            end
-            #if defined?(@features)
-                @features.each do |name, methods|
-                    method = name.to_s + "?"
-                    @feature_module.send(:define_method, method) do
-                        set = nil
-                        methods.each do |m|
-                            if is_a?(Class)
-                                unless public_method_defined?(m)
-                                    set = false
-                                    break
-                                end
-                            else
-                                unless respond_to?(m)
-                                    set = false
-                                    break
-                                end
-                            end
-                        end
-
-                        if set.nil?
-                            true
-                        else
-                            false
-                        end
-                    end
-                end
-            #end
-        end
-        @feature_module
-    end
-
     # Convert a hash, as provided by, um, a provider, into an instance of self.
     def self.hash2obj(hash)
         obj = nil
@@ -267,7 +199,9 @@ class Puppet::Type
                 }.join("\n")
             end
 
-            defaultto { @parent.class.defaultprovider.name }
+            defaultto {
+                @parent.class.defaultprovider.name
+            }
 
             validate do |value|
                 value = value[0] if value.is_a? Array
diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb
index 6ae7d55b3..d568b8ae7 100644
--- a/lib/puppet/network/client/master.rb
+++ b/lib/puppet/network/client/master.rb
@@ -128,27 +128,27 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
         end
     end
 
-    # Have the facts changed since we last compiled?
-    def facts_changed?(facts)
-        oldfacts = Puppet::Util::Storage.cache(:configuration)[:facts]
-        newfacts = self.class.facts
-        if oldfacts == newfacts
-            return false
-        else
-            return true
-        end
-    end
-
     # Check whether our configuration is up to date
     def fresh?(facts)
-        return false if Puppet[:ignorecache]
-        return false unless self.compile_time
-        return false if self.facts_changed?(facts)
+        if Puppet[:ignorecache]
+            Puppet.notice "Ignoring cache"
+            return false
+        end
+        unless self.compile_time
+            Puppet.debug "No cached compile time"
+            return false
+        end
+        if facts_changed?(facts)
+            Puppet.info "Facts have changed; recompiling"
+            return false
+        end
 
         # We're willing to give a 2 second drift
-        if @driver.freshness - @compile_time.to_i < 1
+        newcompile = @driver.freshness
+        if newcompile - @compile_time.to_i < 1
             return true
         else
+            Puppet.debug "Server compile time is %s vs %s" % [newcompile, @compile_time]
             return false
         end
     end
@@ -174,12 +174,15 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
         if self.objects or FileTest.exists?(self.cachefile)
             if self.fresh?(facts)
                 Puppet.info "Config is up to date"
-                begin
-                    @objects = YAML.load(self.retrievecache).to_type
-                rescue => detail
-                    Puppet.warning "Could not load cached configuration: %s" % detail
+                unless self.objects
+                    oldtext = self.retrievecache
+                    begin
+                        @objects = YAML.load(oldtext).to_type
+                    rescue => detail
+                        Puppet.warning "Could not load cached configuration: %s" % detail
+                    end
+                    return
                 end
-                return
             end
         end
         Puppet.debug("getting config")
@@ -513,6 +516,26 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
     loadfacts()
     
     private
+
+    # Have the facts changed since we last compiled?
+    def facts_changed?(facts)
+        oldfacts = Puppet::Util::Storage.cache(:configuration)[:facts]
+        newfacts = facts
+        if oldfacts == newfacts
+            return false
+        else
+#            unless oldfacts
+#                puts "no old facts"
+#                return true
+#            end
+#            newfacts.keys.each do |k|
+#                unless newfacts[k] == oldfacts[k]
+#                    puts "%s: %s vs %s" % [k, newfacts[k], oldfacts[k]]
+#                end
+#            end
+            return true
+        end
+    end
     
     # Actually retrieve the configuration, either from the server or from a
     # local master.
diff --git a/lib/puppet/network/handler/filebucket.rb b/lib/puppet/network/handler/filebucket.rb
index 96b9a1e9a..f0d389aea 100755
--- a/lib/puppet/network/handler/filebucket.rb
+++ b/lib/puppet/network/handler/filebucket.rb
@@ -98,9 +98,11 @@ class Puppet::Network::Handler # :nodoc:
             self.info msg
 
             # ...then just create the file
-            File.open(bfile, File::WRONLY|File::CREAT, 0440) { |of|
-                of.print contents
-            }
+            Puppet::Util.withumask(0007) do
+                File.open(bfile, File::WRONLY|File::CREAT, 0440) { |of|
+                    of.print contents
+                }
+            end
 
             # Write the path to the paths file.
             add_path(path, pathpath)
@@ -132,6 +134,10 @@ class Puppet::Network::Handler # :nodoc:
             end
         end
 
+        def paths(md5)
+            self.class(@path, md5)
+        end
+
         def to_s
             self.name
         end
diff --git a/lib/puppet/provider/cron/crontab.rb b/lib/puppet/provider/cron/crontab.rb
index b7caa4a21..bd03c21e3 100755
--- a/lib/puppet/provider/cron/crontab.rb
+++ b/lib/puppet/provider/cron/crontab.rb
@@ -114,10 +114,11 @@ Puppet::Type.type(:cron).provide(:crontab,
                     break
                 end
 
-                # FIXME It'd be great if I could somehow reuse how the
-                # fields are turned into text, but....
+                # Yay differing definitions of absent.
                 next if (hash[field] == :absent and obj.value(field) == "*")
-                next if (hash[field].join(",") == obj.value(field))
+
+                # Everything should be in the form of arrays, not the normal text.
+                next if (hash[field] == obj.value(field))
                 Puppet.info "Did not match %s: %s vs %s" %
                     [field, obj.value(field).inspect, hash[field].inspect]
                 matched = false 
diff --git a/lib/puppet/provider/package/apt.rb b/lib/puppet/provider/package/apt.rb
index d19276cc1..034a406f1 100755
--- a/lib/puppet/provider/package/apt.rb
+++ b/lib/puppet/provider/package/apt.rb
@@ -69,7 +69,7 @@ Puppet::Type.type(:package).provide :apt, :parent => :dpkg do
             end
         end
 
-        cmd << 'install' << str
+        cmd << :install << str
         
         aptget(*cmd)
     end
diff --git a/lib/puppet/provider/package/aptitude.rb b/lib/puppet/provider/package/aptitude.rb
index 240c3fd82..c4f8023d2 100755
--- a/lib/puppet/provider/package/aptitude.rb
+++ b/lib/puppet/provider/package/aptitude.rb
@@ -15,7 +15,7 @@ Puppet::Type.type(:package).provide :aptitude, :parent => :apt do
         output = aptitude(*args)
 
         # Yay, stupid aptitude doesn't throw an error when the package is missing.
-        if args.include?(:install) and output =~ /0 newly installed/
+        if args.include?(:install) and output =~ /Couldn't find any package/
             raise Puppet::Error.new(
                 "Could not find package %s" % self.name
             )
@@ -24,7 +24,7 @@ Puppet::Type.type(:package).provide :aptitude, :parent => :apt do
 
     def purge
         aptitude '-y', 'purge', @model[:name]
-	 end
+	end
 end
 
 # $Id$