Merge branch '0.24.x' of git://github.com/lak/puppet into 0.24.x

30 files changed, 770 insertions, 153 deletions

diff --git a/ext/ldap/puppet.schema b/ext/ldap/puppet.schema
index d8dc4260d..a7a5f46ff 100644
--- a/ext/ldap/puppet.schema
+++ b/ext/ldap/puppet.schema
@@ -17,6 +17,11 @@ attributetype ( 1.1.3.11 NAME 'environment'
 	EQUALITY caseIgnoreIA5Match
 	SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 
+attributetype ( 1.1.3.12 NAME 'puppetvar'
+	DESC 'A variable setting for puppet'
+	EQUALITY caseIgnoreIA5Match
+	SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+
 objectclass ( 1.1.1.2 NAME 'puppetClient' SUP top AUXILIARY
 	DESC 'Puppet Client objectclass'
-	MAY ( puppetclass $ parentnode $ environment ))
+	MAY ( puppetclass $ parentnode $ environment $ puppetvar ))
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index eed1a00f3..df2fb9425 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -622,6 +622,10 @@ module Puppet
         :ldapclassattrs => ["puppetclass",
             "The LDAP attributes to use to define Puppet classes.  Values
             should be comma-separated."],
+        :ldapstackedattrs => ["puppetvar",
+            "The LDAP attributes that should be stacked to arrays by adding
+            the values in all hierarchy elements of the tree.  Values
+            should be comma-separated."],
         :ldapattrs => ["all",
             "The LDAP attributes to include when querying LDAP for nodes.  All
             returned attributes are set as variables in the top-level scope.
diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb
index 6c41c18d4..bc58908fd 100644
--- a/lib/puppet/indirector/node/ldap.rb
+++ b/lib/puppet/indirector/node/ldap.rb
@@ -19,6 +19,8 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
 
         node = Puppet::Node.new(name)
 
+        information[:stacked_parameters] = {}
+
         parent_info = nil
         parent = information[:parent]
         parents = [name]
@@ -34,6 +36,10 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
                 raise Puppet::Error.new("Could not find parent node '%s'" % parent)
             end
             information[:classes] += parent_info[:classes]
+            parent_info[:stacked].each do |value|
+                param = value.split('=', 2)
+                information[:stacked_parameters][param[0]] = param[1]
+            end
             parent_info[:parameters].each do |param, value|
                 # Specifically test for whether it's set, so false values are handled
                 # correctly.
@@ -45,6 +51,15 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
             parent = parent_info[:parent]
         end
 
+        information[:stacked].each do |value|
+            param = value.split('=', 2)
+            information[:stacked_parameters][param[0]] = param[1]
+        end
+
+        information[:stacked_parameters].each do |param, value|
+            information[:parameters][param] = value unless information[:parameters].include?(param)
+        end
+
         node.classes = information[:classes].uniq unless information[:classes].empty?
         node.parameters = information[:parameters] unless information[:parameters].empty?
         node.environment = information[:environment] if information[:environment]
@@ -62,6 +77,12 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         end
     end
 
+    # The attributes that Puppet will stack as array over the full
+    # hierarchy.
+    def stacked_attributes
+        Puppet[:ldapstackedattrs].split(/\s*,\s*/)
+    end
+
     # Process the found entry.  We assume that we don't just want the
     # ldap object.
     def process(name, entry)
@@ -85,6 +106,14 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
             end
         }
 
+        result[:stacked] = []
+        stacked_attributes.each { |attr|
+            if values = entry.vals(attr)
+                result[:stacked] = result[:stacked] + values
+            end
+        }
+        
+
         result[:parameters] = entry.to_hash.inject({}) do |hash, ary|
             if ary[1].length == 1
                 hash[ary[0]] = ary[1].shift
diff --git a/lib/puppet/provider.rb b/lib/puppet/provider.rb
index e73bb0cb6..c02e15029 100644
--- a/lib/puppet/provider.rb
+++ b/lib/puppet/provider.rb
@@ -5,6 +5,10 @@ class Puppet::Provider
     include Puppet::Util::Warnings
     extend Puppet::Util::Warnings
 
+    require 'puppet/provider/confiner'
+
+    extend Puppet::Provider::Confiner
+
     Puppet::Util.logmethods(self, true)
 
     class << self
@@ -40,27 +44,13 @@ class Puppet::Provider
                 [name, self.name]
         end
 
-        if command == :missing
-            return nil
-        end
-
-        command
+        return binary(command)
     end
 
     # Define commands that are not optional.
     def self.commands(hash)
         optional_commands(hash) do |name, path|
-            confine :exists => path
-        end
-    end
-
-    def self.confine(hash)
-        hash.each do |p,v|
-            if v.is_a? Array
-                @confines[p] += v
-            else
-                @confines[p] << v
-            end
+            confine :exists => path, :for_binary => true
         end
     end
 
@@ -108,10 +98,6 @@ class Puppet::Provider
     def self.initvars
         @defaults = {}
         @commands = {}
-        @origcommands = {}
-        @confines = Hash.new do |hash, key|
-            hash[key] = []
-        end
     end
 
     # The method for returning a list of provider instances.  Note that it returns providers, preferably with values already
@@ -180,16 +166,7 @@ class Puppet::Provider
     def self.optional_commands(hash)
         hash.each do |name, path|
             name = symbolize(name)
-            @origcommands[name] = path
-
-            # Try to find the full path (or verify already-full paths); otherwise
-            # store that the command is missing so we know it's defined but absent.
-            if tmp = binary(path)
-                path = tmp
-                @commands[name] = path
-            else
-                @commands[name] = :missing
-            end
+            @commands[name] = path
 
             if block_given?
                 yield(name, path)
@@ -208,69 +185,6 @@ class Puppet::Provider
         @source
     end
 
-    # Check whether this implementation is suitable for our platform.
-    def self.suitable?(short = true)
-        # A single false result is sufficient to turn the whole thing down.
-        # We don't return 'true' until the very end, though, so that every
-        # confine is tested.
-        missing = {}
-        @confines.each do |check, values|
-            case check
-            when :exists:
-                values.each do |value|
-                    unless value and FileTest.exists? value
-                        debug "Not suitable: missing %s" % value
-                        return false if short
-                        missing[:exists] ||= []
-                        missing[:exists] << value
-                    end
-                end
-            when :true:
-                values.each do |v|
-                    debug "Not suitable: false value"
-                    unless v
-                        return false if short
-                        missing[:true] ||= 0
-                        missing[:true] += 1
-                    end
-                end
-            when :false:
-                values.each do |v|
-                    debug "Not suitable: true value"
-                    if v and short
-                        return false if short
-                        missing[:false] ||= 0
-                        missing[:false] += 1
-                    end
-                end
-            else # Just delegate everything else to facter
-                if result = Facter.value(check)
-                    result = result.to_s.downcase.intern
-
-                    found = values.find do |v|
-                        result == v.to_s.downcase.intern
-                    end
-                    unless found
-                        debug "Not suitable: %s not in %s" % [check, values]
-                        return false if short
-                        missing[:facter] ||= {}
-                        missing[:facter][check] = values
-                    end
-                else
-                    return false if short
-                    missing[:facter] ||= {}
-                    missing[:facter][check] = values
-                end
-            end
-        end
-
-        if short
-            return true
-        else
-            return missing
-        end
-    end
-
     # Does this provider support the specified parameter?
     def self.supports_parameter?(param)
         if param.is_a?(Class)
@@ -309,8 +223,8 @@ class Puppet::Provider
     end
 
     dochook(:commands) do
-        if @origcommands.length > 0
-            return "  Required binaries: " + @origcommands.collect do |n, c|
+        if @commands.length > 0
+            return "  Required binaries: " + @commands.collect do |n, c|
                 "``#{c}``"
             end.join(", ") + "."
         end
diff --git a/lib/puppet/provider/confine.rb b/lib/puppet/provider/confine.rb
new file mode 100644
index 000000000..227c923e6
--- /dev/null
+++ b/lib/puppet/provider/confine.rb
@@ -0,0 +1,89 @@
+# The class that handles testing whether our providers
+# actually work or not.
+require 'puppet/util'
+
+class Puppet::Provider::Confine
+    include Puppet::Util
+
+    attr_reader :test, :values, :fact
+
+    # Mark that this confine is used for testing binary existence.
+    attr_accessor :for_binary
+    def for_binary?
+        for_binary
+    end
+
+    def exists?(value)
+        if for_binary?
+            return false unless value = binary(value)
+        end
+        value and FileTest.exist?(value)
+    end
+
+    # Are we a facter comparison?
+    def facter?
+        defined?(@facter)
+    end
+
+    # Retrieve the value from facter
+    def facter_value
+        unless defined?(@facter_value) and @facter_value
+            @facter_value = Facter.value(@fact).to_s.downcase
+        end
+        @facter_value
+    end
+
+    def false?(value)
+        ! value
+    end
+
+    def initialize(test, values)
+        values = [values] unless values.is_a?(Array)
+        @values = values
+
+        if %w{exists false true}.include?(test.to_s)
+            @test = test
+            @method = @test.to_s + "?"
+        else
+            @fact = test
+            @test = :facter
+            @method = "match?"
+        end
+    end
+
+    def match?(value)
+        facter_value == value.to_s.downcase
+    end
+
+    # Collect the results of all of them.
+    def result
+        values.collect { |value| send(@method, value) }
+    end
+
+    def true?(value)
+        # Double negate, so we only get true or false.
+        ! ! value
+    end
+
+    # Test whether our confine matches.
+    def valid?
+        values.each do |value|
+            unless send(@method, value)
+                msg = case test
+                      when :false:  "false value when expecting true"
+                      when :true:  "true value when expecting false"
+                      when :exists:  "file %s does not exist" % value
+                      when :facter:  "facter value '%s' for '%s' not in required list '%s'" % [value, @fact, values.join(",")]
+                      end
+                Puppet.debug msg
+                return false
+            end
+        end
+
+        return true
+    ensure
+        # Reset the cache.  We want to cache it during a given
+        # run, but across runs.
+        @facter_value = nil
+    end
+end
diff --git a/lib/puppet/provider/confine_collection.rb b/lib/puppet/provider/confine_collection.rb
new file mode 100644
index 000000000..f38035521
--- /dev/null
+++ b/lib/puppet/provider/confine_collection.rb
@@ -0,0 +1,48 @@
+# Manage a collection of confines, returning a boolean or
+# helpful information.
+require 'puppet/provider/confine'
+
+class Puppet::Provider::ConfineCollection
+    def confine(hash)
+        if hash.include?(:for_binary)
+            for_binary = true
+            hash.delete(:for_binary)
+        else
+            for_binary = false
+        end
+        hash.each do |test, values|
+            @confines << Puppet::Provider::Confine.new(test, values)
+            @confines[-1].for_binary = true if for_binary
+        end
+    end
+
+    def initialize
+        @confines = []
+    end
+
+    # Return a hash of the whole confine set, used for the Provider
+    # reference.
+    def result
+        defaults = {
+            :false => 0,
+            :true => 0,
+            :exists => [],
+            :facter => {}
+        }
+        missing = Hash.new { |hash, key| hash[key] = defaults[key] }
+        @confines.each do |confine|
+            case confine.test
+            when :false: missing[confine.test] += confine.result.find_all { |v| v == false }.length
+            when :true: missing[confine.test] += confine.result.find_all { |v| v == true }.length
+            when :exists: confine.result.zip(confine.values).each { |val, f| missing[:exists] << f unless val }
+            when :facter: missing[:facter][confine.fact] = confine.values if confine.result.include?(false)
+            end
+        end
+
+        missing
+    end
+
+    def valid?
+        ! @confines.detect { |c| ! c.valid? }
+    end
+end
diff --git a/lib/puppet/provider/confiner.rb b/lib/puppet/provider/confiner.rb
new file mode 100644
index 000000000..3e406873b
--- /dev/null
+++ b/lib/puppet/provider/confiner.rb
@@ -0,0 +1,20 @@
+require 'puppet/provider/confine_collection'
+
+module Puppet::Provider::Confiner
+    def confine(hash)
+        confine_collection.confine(hash)
+    end
+
+    def confine_collection
+        unless defined?(@confine_collection)
+            @confine_collection = Puppet::Provider::ConfineCollection.new
+        end
+        @confine_collection
+    end
+
+    # Check whether this implementation is suitable for our platform.
+    def suitable?(short = true)
+        return confine_collection.valid? if short
+        return confine_collection.result
+    end
+end
diff --git a/spec/integration/ral/types/package.rb b/spec/integration/type/package.rb
index 20567629d..c244fa1cd 100755
--- a/spec/integration/ral/types/package.rb
+++ b/spec/integration/type/package.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/type/package'
 
diff --git a/spec/unit/indirector/node/ldap.rb b/spec/unit/indirector/node/ldap.rb
index a40698662..878039c7c 100755
--- a/spec/unit/indirector/node/ldap.rb
+++ b/spec/unit/indirector/node/ldap.rb
@@ -17,6 +17,7 @@ describe Puppet::Node::Ldap do
             @searcher.stubs(:connection).returns(@connection)
             @searcher.stubs(:class_attributes).returns([])
             @searcher.stubs(:parent_attribute).returns(nil)
+            @searcher.stubs(:stacked_attributes).returns([])
             @searcher.stubs(:search_base).returns(:yay)
             @searcher.stubs(:search_filter).returns(:filter)
 
@@ -195,6 +196,96 @@ describe Puppet::Node::Ldap do
                 proc { @searcher.find(@request) }.should raise_error(ArgumentError)
             end
         end
+
+        describe "and a puppet variable is specified" do
+            before do
+                @searcher.stubs(:stacked_attributes).returns(['puppetvar'])
+            end
+
+            it "should add the variable to the node parameters" do
+                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two})
+                @entry.stubs(:to_hash).returns({})
+                @node.expects(:parameters=).with("one" => "two")
+                @searcher.find(@request)
+            end
+
+            it "should not overwrite node parameters specified as ldap object attribute" do
+                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two})
+                @entry.stubs(:to_hash).returns("one" => "three")
+                @node.expects(:parameters=).with("one" => "three")
+                @searcher.find(@request)
+            end
+
+            it "should set entries without an equal sign to nil" do
+                @entry.stubs(:vals).with("puppetvar").returns(%w{one})
+                @entry.stubs(:to_hash).returns({})
+                @node.expects(:parameters=).with("one" => nil)
+                @searcher.find(@request)
+            end
+
+            it "should ignore empty entries" do
+                @entry.stubs(:vals).with("puppetvar").returns(%w{})
+                @entry.stubs(:to_hash).returns({})
+                @searcher.find(@request)
+            end
+        end
+        describe "and a puppet variable as well as a parent node are specified" do
+            before do
+                @parent = mock 'parent'
+
+                @searcher.meta_def(:search_filter) do |name|
+                    return name
+                end
+                @connection.stubs(:search).with { |*args| args[2] == @name              }.yields(@entry)
+                @connection.stubs(:search).with { |*args| args[2] == 'parent'           }.yields(@parent)
+
+                @searcher.stubs(:stacked_attributes).returns(['puppetvar'])
+                @searcher.stubs(:parent_attribute).returns(:parent)
+            end
+
+            it "should add parent node variables to the child node parameters" do
+                @parent.stubs(:to_hash).returns({})
+                @parent.stubs(:vals).with("puppetvar").returns(%w{one=two})
+                @parent.stubs(:vals).with(:parent).returns(nil)
+
+                @entry.stubs(:to_hash).returns({})
+                @entry.stubs(:vals).with("puppetvar").returns(%w{})
+                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+
+                @node.expects(:parameters=).with("one" => "two")
+
+                @searcher.find(@request)
+            end
+
+            it "should overwrite parent node variables with child node parameters" do
+                @parent.stubs(:to_hash).returns({})
+                @parent.stubs(:vals).with("puppetvar").returns(%w{one=two})
+                @parent.stubs(:vals).with(:parent).returns(nil)
+
+                @entry.stubs(:to_hash).returns({})
+                @entry.stubs(:vals).with("puppetvar").returns(%w{one=three})
+                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+
+                @node.expects(:parameters=).with("one" => "three")
+
+                @searcher.find(@request)
+            end
+
+            it "should not overwrite parent node parameters specified as ldap object attribute" do
+                @parent.stubs(:to_hash).returns("one" => "three")
+                @parent.stubs(:vals).with("puppetvar").returns(%w{})
+                @parent.stubs(:vals).with(:parent).returns(nil)
+
+                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two})
+                @entry.stubs(:to_hash).returns({})
+                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+
+                @node.expects(:parameters=).with("one" => "three")
+
+                @searcher.find(@request)
+            end
+
+        end
     end
 end
 
diff --git a/spec/unit/provider/confine.rb b/spec/unit/provider/confine.rb
new file mode 100755
index 000000000..bb2e751d6
--- /dev/null
+++ b/spec/unit/provider/confine.rb
@@ -0,0 +1,225 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/provider/confine'
+
+describe Puppet::Provider::Confine do
+    it "should require a test" do
+        lambda { Puppet::Provider::Confine.new }.should raise_error(ArgumentError)
+    end
+
+    it "should require a value" do
+        lambda { Puppet::Provider::Confine.new(:exists) }.should raise_error(ArgumentError)
+    end
+
+    it "should have a test" do
+        Puppet::Provider::Confine.new(:exists, "/some/file").test.should == :exists
+    end
+
+    it "should always convert values to an array" do
+        Puppet::Provider::Confine.new(:exists, "/some/file").values.should be_instance_of(Array)
+    end
+
+    it "should have an accessor for its fact" do
+        Puppet::Provider::Confine.new(:foo, :bar).should respond_to(:fact)
+    end
+
+    it "should be possible to mark the confine as a binary test" do
+        Puppet::Provider::Confine.new(:foo, :bar).should respond_to(:for_binary=)
+    end
+
+    it "should have a boolean method to indicate it's a binary confine" do
+        Puppet::Provider::Confine.new(:foo, :bar).should respond_to(:for_binary?)
+    end
+
+    it "should indicate it's a boolean confine if it has been marked that way" do
+        confine = Puppet::Provider::Confine.new(:foo, :bar)
+        confine.for_binary = true
+        confine.should be_for_binary
+    end
+
+    it "should have a method for returning a binary's path" do
+        Puppet::Provider::Confine.new(:foo, :bar).private_methods.should be_include("binary")
+    end
+
+    describe "when testing values" do
+        before { @confine = Puppet::Provider::Confine.new("eh", "foo") }
+
+        describe "and the test is 'false'" do
+            it "should use the 'false?' method to test validity" do
+                @confine = Puppet::Provider::Confine.new(:false, "foo")
+                @confine.expects(:false?).with("foo")
+                @confine.valid?
+            end
+
+            it "should return true if the value is false" do
+                @confine.false?(false).should be_true
+            end
+
+            it "should return false if the value is not false" do
+                @confine.false?("else").should be_false
+            end
+
+            it "should log that a value is false" do
+                @confine = Puppet::Provider::Confine.new(:false, "foo")
+                Puppet.expects(:debug).with { |l| l.include?("false") }
+                @confine.valid?
+            end
+        end
+
+        describe "and the test is 'true'" do
+            it "should use the 'true?' method to test validity" do
+                @confine = Puppet::Provider::Confine.new(:true, "foo")
+                @confine.expects(:true?).with("foo")
+                @confine.valid?
+            end
+
+            it "should return true if the value is not false" do
+                @confine.true?("else").should be_true
+            end
+
+            it "should return false if the value is false" do
+                @confine.true?(nil).should be_false
+            end
+        end
+
+        describe "and the test is 'exists'" do
+            it "should use the 'exists?' method to test validity" do
+                @confine = Puppet::Provider::Confine.new(:exists, "foo")
+                @confine.expects(:exists?).with("foo")
+                @confine.valid?
+            end
+
+            it "should return false if the value is false" do
+                @confine.exists?(false).should be_false
+            end
+
+            it "should return false if the value does not point to a file" do
+                FileTest.expects(:exist?).with("/my/file").returns false
+                @confine.exists?("/my/file").should be_false
+            end
+
+            it "should return true if the value points to a file" do
+                FileTest.expects(:exist?).with("/my/file").returns true
+                @confine.exists?("/my/file").should be_true
+            end
+
+            it "should log that a value is true" do
+                @confine = Puppet::Provider::Confine.new(:true, nil)
+                Puppet.expects(:debug).with { |l| l.include?("true") }
+                @confine.valid?
+            end
+
+            describe "and the confine is for binaries" do
+                before { @confine.stubs(:for_binary).returns true }
+                it "should use its 'binary' method to look up the full path of the file" do
+                    @confine.expects(:binary).returns nil
+                    @confine.exists?("/my/file")
+                end
+
+                it "should return false if no binary can be found" do
+                    @confine.expects(:binary).with("/my/file").returns nil
+                    @confine.exists?("/my/file").should be_false
+                end
+
+                it "should return true if the binary can be found and the file exists" do
+                    @confine.expects(:binary).with("/my/file").returns "/my/file"
+                    FileTest.expects(:exist?).with("/my/file").returns true
+                    @confine.exists?("/my/file").should be_true
+                end
+
+                it "should return false if the binary can be found but the file does not exist" do
+                    @confine.expects(:binary).with("/my/file").returns "/my/file"
+                    FileTest.expects(:exist?).with("/my/file").returns true
+                    @confine.exists?("/my/file").should be_true
+                end
+            end
+        end
+
+        describe "and the test is not 'true', 'false', or 'exists'" do
+            it "should use the 'match?' method to test validity" do
+                @confine = Puppet::Provider::Confine.new("yay", "foo")
+                @confine.expects(:match?).with("foo")
+                @confine.valid?
+            end
+
+            it "should return true if the value matches the facter value" do
+                Facter.expects(:value).returns("foo")
+
+                @confine.match?("foo").should be_true
+            end
+
+            it "should return false if the value does not match the facter value" do
+                Facter.expects(:value).returns("boo")
+
+                @confine.match?("foo").should be_false
+            end
+
+            it "should be case insensitive" do
+                Facter.expects(:value).returns("FOO")
+
+                @confine.match?("foo").should be_true
+            end
+
+            it "should not care whether the value is a string or symbol" do
+                Facter.expects(:value).returns("FOO")
+
+                @confine.match?(:foo).should be_true
+            end
+
+            it "should cache the fact during testing" do
+                Facter.expects(:value).once.returns("FOO")
+
+                @confine.match?(:foo)
+                @confine.match?(:foo)
+            end
+
+            it "should log that the fact value is not correct" do
+                @confine = Puppet::Provider::Confine.new("foo", ["bar", "bee"])
+                Facter.expects(:value).with("foo").returns "yayness"
+                Puppet.expects(:debug).with { |l| l.include?("facter") and l.include?("bar,bee") }
+                @confine.valid?
+            end
+        end
+    end
+
+    describe "when testing all values" do
+        before { @confine = Puppet::Provider::Confine.new(:true, %w{a b c}) }
+
+        it "should be invalid if any values fail" do
+            @confine.stubs(:true?).returns true
+            @confine.expects(:true?).with("b").returns false
+            @confine.should_not be_valid
+        end
+
+        it "should be valid if all values pass" do
+            @confine.stubs(:true?).returns true
+            @confine.should be_valid
+        end
+
+        it "should short-cut at the first failing value" do
+            @confine.expects(:true?).once.returns false
+            @confine.valid?
+        end
+
+        it "should remove the cached facter value" do
+            @confine = Puppet::Provider::Confine.new(:foo, :bar)
+            Facter.expects(:value).with(:foo).times(2).returns "eh"
+            @confine.valid?
+            @confine.valid?
+        end
+    end
+
+    describe "when testing the result of the values" do
+        before { @confine = Puppet::Provider::Confine.new(:true, %w{a b c d}) }
+
+        it "should return an array with the result of the test for each value" do
+            @confine.stubs(:true?).returns true
+            @confine.expects(:true?).with("b").returns false
+            @confine.expects(:true?).with("d").returns false
+
+            @confine.result.should == [true, false, true, false]
+        end
+    end
+end
diff --git a/spec/unit/provider/confine_collection.rb b/spec/unit/provider/confine_collection.rb
new file mode 100755
index 000000000..3430d604f
--- /dev/null
+++ b/spec/unit/provider/confine_collection.rb
@@ -0,0 +1,113 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/provider/confine_collection'
+
+describe Puppet::Provider::ConfineCollection do
+    it "should be able to add confines" do
+        Puppet::Provider::ConfineCollection.new.should respond_to(:confine)
+    end
+
+    it "should create a Confine instance for every confine call" do
+        Puppet::Provider::Confine.expects(:new).with(:foo, :bar).returns "eh"
+        Puppet::Provider::Confine.expects(:new).with(:baz, :bee).returns "eh"
+        Puppet::Provider::ConfineCollection.new.confine :foo => :bar, :baz => :bee
+    end
+
+    it "should mark each confine as a binary confine if :for_binary => true is included" do
+        confine = mock 'confine'
+        confine.expects(:for_binary=).with true
+        Puppet::Provider::Confine.expects(:new).with(:foo, :bar).returns confine
+        Puppet::Provider::ConfineCollection.new.confine :foo => :bar, :for_binary => true
+    end
+
+    it "should be valid if no confines are present" do
+        Puppet::Provider::ConfineCollection.new.should be_valid
+    end
+
+    it "should be valid if all confines are valid" do
+        c1 = mock 'c1', :valid? => true
+        c2 = mock 'c2', :valid? => true
+
+        Puppet::Provider::Confine.expects(:new).times(2).returns(c1).then.returns(c2)
+
+        confiner = Puppet::Provider::ConfineCollection.new
+        confiner.confine :foo => :bar, :baz => :bee
+
+        confiner.should be_valid
+    end
+
+    it "should not be valid if any confines are valid" do
+        c1 = mock 'c1', :valid? => true
+        c2 = mock 'c2', :valid? => false
+
+        Puppet::Provider::Confine.expects(:new).times(2).returns(c1).then.returns(c2)
+
+        confiner = Puppet::Provider::ConfineCollection.new
+        confiner.confine :foo => :bar, :baz => :bee
+
+        confiner.should_not be_valid
+    end
+
+    describe "when providing a complete result" do
+        before do
+            @confiner = Puppet::Provider::ConfineCollection.new
+        end
+
+        it "should return a hash" do
+            @confiner.result.should be_instance_of(Hash)
+        end
+
+        it "should return an empty hash if the confiner is valid" do
+            @confiner.result.should == {}
+        end
+
+        it "should contain the number of incorrectly false values" do
+            c1 = stub 'c1', :result => [true, false, true], :test => :true
+            c2 = stub 'c2', :result => [false, true, false], :test => :true
+
+            Puppet::Provider::Confine.expects(:new).times(2).returns(c1).then.returns(c2)
+
+            confiner = Puppet::Provider::ConfineCollection.new
+            confiner.confine :foo => :bar, :baz => :bee
+
+            confiner.result[:true].should == 3
+        end
+
+        it "should contain the number of incorrectly true values" do
+            c1 = stub 'c1', :result => [true, false, true], :test => :false
+            c2 = stub 'c2', :result => [false, true, false], :test => :false
+
+            Puppet::Provider::Confine.expects(:new).times(2).returns(c1).then.returns(c2)
+
+            confiner = Puppet::Provider::ConfineCollection.new
+            confiner.confine :foo => :bar, :baz => :bee
+
+            confiner.result[:false].should == 3
+        end
+
+        it "should contain the missing files" do
+            FileTest.stubs(:exist?).returns true
+            FileTest.expects(:exist?).with("/two").returns false
+            FileTest.expects(:exist?).with("/four").returns false
+
+            confiner = Puppet::Provider::ConfineCollection.new
+            confiner.confine :exists => %w{/one /two}
+            confiner.confine :exists => %w{/three /four}
+
+            confiner.result[:exists].should == %w{/two /four}
+        end
+
+        it "should contain a hash of facts and the allowed values" do
+            Facter.expects(:value).with(:foo).returns "yay"
+            Facter.expects(:value).with(:bar).returns "boo"
+            confiner = Puppet::Provider::ConfineCollection.new
+            confiner.confine :foo => "yes", :bar => "boo"
+
+            result = confiner.result
+            result[:facter][:foo].should == %w{yes}
+            result[:facter][:bar].should be_nil
+        end
+    end
+end
diff --git a/spec/unit/provider/confiner.rb b/spec/unit/provider/confiner.rb
new file mode 100755
index 000000000..38fffc102
--- /dev/null
+++ b/spec/unit/provider/confiner.rb
@@ -0,0 +1,62 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/provider/confiner'
+
+describe Puppet::Provider::Confiner do
+    before do
+        @object = Object.new
+        @object.extend(Puppet::Provider::Confiner)
+    end
+
+    it "should have a method for defining confines" do
+        @object.should respond_to(:confine)
+    end
+
+    it "should have a method for returning its confine collection" do
+        @object.should respond_to(:confine_collection)
+    end
+
+    it "should have a method for testing suitability" do
+        @object.should respond_to(:suitable?)
+    end
+
+    it "should delegate its confine method to its confine collection" do
+        coll = mock 'collection'
+        @object.stubs(:confine_collection).returns coll
+        coll.expects(:confine).with(:foo => :bar, :bee => :baz)
+        @object.confine(:foo => :bar, :bee => :baz)
+    end
+
+    it "should create a new confine collection if one does not exist" do
+        Puppet::Provider::ConfineCollection.expects(:new).returns "mycoll"
+        @object.confine_collection.should == "mycoll"
+    end
+
+    it "should reuse the confine collection" do
+        @object.confine_collection.should equal(@object.confine_collection)
+    end
+
+    describe "when testing suitability" do
+        before do
+            @coll = mock 'collection'
+            @object.stubs(:confine_collection).returns @coll
+        end
+
+        it "should return true if the confine collection is valid" do
+            @coll.expects(:valid?).returns true
+            @object.should be_suitable
+        end
+
+        it "should return false if the confine collection is invalid" do
+            @coll.expects(:valid?).returns false
+            @object.should_not be_suitable
+        end
+
+        it "should return the result of the confine collection if a long result is asked for" do
+            @coll.expects(:result).returns "myresult"
+            @object.suitable?(false).should == "myresult"
+        end
+    end
+end
diff --git a/spec/unit/ral/provider/interface/redhat.rb b/spec/unit/provider/interface/redhat.rb
index 9bf1b9722..5a7a8dfcd 100755
--- a/spec/unit/ral/provider/interface/redhat.rb
+++ b/spec/unit/provider/interface/redhat.rb
@@ -3,7 +3,7 @@
 #  Created by Luke Kanies on 2007-11-20.
 #  Copyright (c) 2006. All rights reserved.
 
-require File.dirname(__FILE__) + '/../../../../spec_helper'
+require File.dirname(__FILE__) + '/../../../spec_helper'
 
 provider_class = Puppet::Type.type(:interface).provider(:redhat)
 
diff --git a/spec/unit/ral/provider/interface/sunos.rb b/spec/unit/provider/interface/sunos.rb
index 7b9f462e6..6a7bd19c1 100755
--- a/spec/unit/ral/provider/interface/sunos.rb
+++ b/spec/unit/provider/interface/sunos.rb
@@ -3,7 +3,7 @@
 #  Created by Luke Kanies on 2007-11-25.
 #  Copyright (c) 2006. All rights reserved.
 
-require File.dirname(__FILE__) + '/../../../../spec_helper'
+require File.dirname(__FILE__) + '/../../../spec_helper'
 
 require 'puppet/provider/interface/sunos'
 
diff --git a/spec/unit/ral/provider/mount.rb b/spec/unit/provider/mount.rb
index 0b90d53c9..41abcd424 100755
--- a/spec/unit/ral/provider/mount.rb
+++ b/spec/unit/provider/mount.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/provider/mount'
 
diff --git a/spec/unit/ral/provider/mount/parsed.rb b/spec/unit/provider/mount/parsed.rb
index 21276d911..ba65b70a4 100755
--- a/spec/unit/ral/provider/mount/parsed.rb
+++ b/spec/unit/provider/mount/parsed.rb
@@ -3,7 +3,7 @@
 #  Created by Luke Kanies on 2007-9-12.
 #  Copyright (c) 2006. All rights reserved.
 
-require File.dirname(__FILE__) + '/../../../../spec_helper'
+require File.dirname(__FILE__) + '/../../../spec_helper'
 
 require 'puppettest/support/utils'
 require 'puppettest/fileparsing'
diff --git a/spec/unit/ral/provider/ssh_authorized_key/parsed.rb b/spec/unit/provider/ssh_authorized_key/parsed.rb
index 459001cb5..c35ddc513 100755
--- a/spec/unit/ral/provider/ssh_authorized_key/parsed.rb
+++ b/spec/unit/provider/ssh_authorized_key/parsed.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../../spec_helper'
+require File.dirname(__FILE__) + '/../../../spec_helper'
 
 require 'puppettest'
 require 'puppettest/support/utils'
diff --git a/spec/unit/ral/type.rb b/spec/unit/type.rb
index 5980167d6..9815ed32d 100755
--- a/spec/unit/ral/type.rb
+++ b/spec/unit/type.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../spec_helper'
+require File.dirname(__FILE__) + '/../spec_helper'
 
 describe Puppet::Type, " when in a configuration" do
     before do
diff --git a/spec/unit/ral/type/exec.rb b/spec/unit/type/exec.rb
index 260804227..cf0e02929 100755
--- a/spec/unit/ral/type/exec.rb
+++ b/spec/unit/type/exec.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/type/exec'
 
diff --git a/spec/unit/ral/type/file.rb b/spec/unit/type/file.rb
index e1a597434..12b806d88 100755
--- a/spec/unit/ral/type/file.rb
+++ b/spec/unit/type/file.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 describe Puppet::Type.type(:file) do
     before do
diff --git a/spec/unit/ral/type/interface.rb b/spec/unit/type/interface.rb
index 2e0176152..27f34b7e0 100755
--- a/spec/unit/ral/type/interface.rb
+++ b/spec/unit/type/interface.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 interface = Puppet::Type.type(:interface)
 
diff --git a/spec/unit/ral/type/mount.rb b/spec/unit/type/mount.rb
index 8fa2e6f7c..a9b78672e 100755
--- a/spec/unit/ral/type/mount.rb
+++ b/spec/unit/type/mount.rb
@@ -1,77 +1,75 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
-require 'puppet/type/mount'
-
-describe Puppet::Type::Mount do
+describe Puppet::Type.type(:mount) do
     it "should have a :refreshable feature that requires the :remount method" do
-        Puppet::Type::Mount.provider_feature(:refreshable).methods.should == [:remount]
+        Puppet::Type.type(:mount).provider_feature(:refreshable).methods.should == [:remount]
     end
 
     it "should have no default value for :ensure" do
-        mount = Puppet::Type::Mount.create(:name => "yay")
+        mount = Puppet::Type.type(:mount).create(:name => "yay")
         mount.should(:ensure).should be_nil
     end
 
-    after { Puppet::Type::Mount.clear }
+    after { Puppet::Type.type(:mount).clear }
 end
 
-describe Puppet::Type::Mount, "when validating attributes" do
+describe Puppet::Type.type(:mount), "when validating attributes" do
     [:name, :remounts].each do |param|
         it "should have a #{param} parameter" do
-            Puppet::Type::Mount.attrtype(param).should == :param
+            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::Mount.attrtype(param).should == :property
+            Puppet::Type.type(:mount).attrtype(param).should == :property
         end
     end
 end
 
-describe Puppet::Type::Mount::Ensure, "when validating values" do
+describe Puppet::Type.type(:mount)::Ensure, "when validating values" do
     before do
-        @provider = stub 'provider', :class => Puppet::Type::Mount.defaultprovider, :clear => nil
-        Puppet::Type::Mount.defaultprovider.expects(:new).returns(@provider)
+        @provider = stub 'provider', :class => Puppet::Type.type(:mount).defaultprovider, :clear => nil
+        Puppet::Type.type(:mount).defaultprovider.expects(:new).returns(@provider)
     end
 
     it "should support :present as a value to :ensure" do
-        Puppet::Type::Mount.create(:name => "yay", :ensure => :present)
+        Puppet::Type.type(:mount).create(:name => "yay", :ensure => :present)
     end
 
     it "should alias :unmounted to :present as a value to :ensure" do
-        mount = Puppet::Type::Mount.create(:name => "yay", :ensure => :unmounted)
+        mount = Puppet::Type.type(:mount).create(:name => "yay", :ensure => :unmounted)
         mount.should(:ensure).should == :present
     end
 
     it "should support :absent as a value to :ensure" do
-        Puppet::Type::Mount.create(:name => "yay", :ensure => :absent)
+        Puppet::Type.type(:mount).create(:name => "yay", :ensure => :absent)
     end
 
     it "should support :mounted as a value to :ensure" do
-        Puppet::Type::Mount.create(:name => "yay", :ensure => :mounted)
+        Puppet::Type.type(:mount).create(:name => "yay", :ensure => :mounted)
     end
 
-    after { Puppet::Type::Mount.clear }
+    after { Puppet::Type.type(:mount).clear }
 end
 
-describe Puppet::Type::Mount::Ensure do
+describe Puppet::Type.type(:mount)::Ensure do
     before :each do
-        @provider = stub 'provider', :class => Puppet::Type::Mount.defaultprovider, :clear => nil, :satisfies? => true, :name => :mock
-        Puppet::Type::Mount.defaultprovider.stubs(:new).returns(@provider)
-        @mount = Puppet::Type::Mount.create(:name => "yay", :check => :ensure)
+        @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).create(:name => "yay", :check => :ensure)
 
         @ensure = @mount.property(:ensure)
     end
 
     after :each do
-        Puppet::Type::Mount.clear
+        Puppet::Type.type(:mount).clear
     end
     
     def mount_stub(params)
-        Puppet::Type::Mount.validproperties.each do |prop|
+        Puppet::Type.type(:mount).validproperties.each do |prop|
             unless params[prop]
                 params[prop] = :absent
                 @mount[prop] = :absent
@@ -83,7 +81,7 @@ describe Puppet::Type::Mount::Ensure do
         end
     end
 
-    describe Puppet::Type::Mount::Ensure, "when retrieving its current state" do
+    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)
@@ -103,7 +101,7 @@ describe Puppet::Type::Mount::Ensure do
         end
     end
 
-    describe Puppet::Type::Mount::Ensure, "when changing the host" do
+    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)
@@ -166,7 +164,7 @@ describe Puppet::Type::Mount::Ensure do
         end
     end
 
-    describe Puppet::Type::Mount, "when responding to events" do
+    describe Puppet::Type.type(:mount), "when responding to events" do
 
         it "should remount if it is currently mounted" do
             @provider.expects(:mounted?).returns(true)
diff --git a/spec/unit/ral/type/nagios.rb b/spec/unit/type/nagios.rb
index 35f00b0e5..563c82c2f 100755
--- a/spec/unit/ral/type/nagios.rb
+++ b/spec/unit/type/nagios.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/external/nagios'
 
diff --git a/spec/unit/ral/type/noop_metaparam.rb b/spec/unit/type/noop_metaparam.rb
index 0cbed3714..2a3e0160d 100755
--- a/spec/unit/ral/type/noop_metaparam.rb
+++ b/spec/unit/type/noop_metaparam.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/metatype/metaparams'
 
diff --git a/spec/unit/ral/type/package.rb b/spec/unit/type/package.rb
index 5d96dc4ae..335910c63 100755
--- a/spec/unit/ral/type/package.rb
+++ b/spec/unit/type/package.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/type/package'
 
diff --git a/spec/unit/ral/type/schedule.rb b/spec/unit/type/schedule.rb
index 4e9840c34..da38f68a9 100755
--- a/spec/unit/ral/type/schedule.rb
+++ b/spec/unit/type/schedule.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/type/schedule'
 
diff --git a/spec/unit/ral/type/service.rb b/spec/unit/type/service.rb
index 0f00992fa..e8358cb22 100755
--- a/spec/unit/ral/type/service.rb
+++ b/spec/unit/type/service.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/type/service'
 
diff --git a/spec/unit/ral/type/ssh_authorized_key.rb b/spec/unit/type/ssh_authorized_key.rb
index d27cb9f25..0e869747d 100755
--- a/spec/unit/ral/type/ssh_authorized_key.rb
+++ b/spec/unit/type/ssh_authorized_key.rb
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 ssh_authorized_key = Puppet::Type.type(:ssh_authorized_key)
 
diff --git a/spec/unit/ral/type/user.rb b/spec/unit/type/user.rb
index 4e43a8ceb..d16d752f9 100755
--- a/spec/unit/ral/type/user.rb
+++ b/spec/unit/type/user.rb
@@ -1,14 +1,12 @@
 #!/usr/bin/env ruby
 
-require File.dirname(__FILE__) + '/../../../spec_helper'
-
-require 'puppet/type/user'
+require File.dirname(__FILE__) + '/../../spec_helper'
 
 module UserTestFunctions
     def mkuser(name)
         user = nil;
         lambda {
-            user = Puppet::Type::User.create(
+            user = Puppet::Type.type(:user).create(
                 :name => name,
                 :comment => "Puppet Testing User",
                 :gid => Puppet::Util::SUIDManager.gid,
@@ -30,12 +28,12 @@ module UserTestFunctions
     end
 end
 
-describe Puppet::Type::User do
+describe Puppet::Type.type(:user) do
 
     include UserTestFunctions
 
     it "should have a default provider inheriting from Puppet::Provider" do
-        test_provider_class Puppet::Type::User.defaultprovider
+        test_provider_class Puppet::Type.type(:user).defaultprovider
     end
 
     it "should be able to create a instance" do
@@ -43,7 +41,7 @@ describe Puppet::Type::User do
     end
 end
 
-describe Puppet::Type::User, "instances" do
+describe Puppet::Type.type(:user), "instances" do
 
     include UserTestFunctions
 
diff --git a/test/ral/providers/provider.rb b/test/ral/providers/provider.rb
index 2196fafce..70f606a37 100755
--- a/test/ral/providers/provider.rb
+++ b/test/ral/providers/provider.rb
@@ -37,12 +37,13 @@ class TestProvider < Test::Unit::TestCase
         cleanup { Puppet::Type.rmtype(:provider_test) }
     end
 
-    def test_confine
-        provider = newprovider
+    def test_confine_defaults_to_suitable
 
-        assert(provider.suitable?,
-            "Marked unsuitable with no confines")
+        provider = newprovider
+        assert(provider.suitable?, "Marked unsuitable with no confines")
+    end
 
+    def test_confine_results
         {
             {:true => true} => true,
             {:true => false} => false,
@@ -54,6 +55,8 @@ class TestProvider < Test::Unit::TestCase
             {:exists => echo} => true,
             {:exists => "/this/file/does/not/exist"} => false,
         }.each do |hash, result|
+            provider = newprovider
+
             # First test :true
             hash.each do |test, val|
                 assert_nothing_raised do
@@ -61,19 +64,25 @@ class TestProvider < Test::Unit::TestCase
                 end
             end
 
-            assert_equal(result, provider.suitable?,
-                "Failed for %s" % [hash.inspect])
+            assert_equal(result, provider.suitable?, "Failed for %s" % [hash.inspect])
 
             provider.initvars
         end
+    end
+
+    def test_multiple_confines_do_not_override
+        provider = newprovider
 
         # Make sure multiple confines don't overwrite each other
         provider.confine :true => false
         assert(! provider.suitable?)
         provider.confine :true => true
         assert(! provider.suitable?)
+    end
 
-        provider.initvars
+    def test_one_failed_confine_is_sufficient
+
+        provider = newprovider
 
         # Make sure we test multiple of them, and that a single false wins
         provider.confine :true => true, :false => false
@@ -82,6 +91,18 @@ class TestProvider < Test::Unit::TestCase
         assert(! provider.suitable?)
     end
 
+    # #1197 - the binary should not be
+    def test_command_checks_for_binaries_each_time
+        provider = newprovider
+
+        provider.commands :testing => "/no/such/path"
+
+        provider.stubs(:binary).returns "/no/such/path"
+
+        provider.command(:testing)
+        assert_equal("/no/such/path", provider.command(:testing), "Did not return correct binary path")
+    end
+
     def test_command
         {:echo => "echo", :echo_with_path => echo, :missing => "nosuchcommand", :missing_qualified => "/path/to/nosuchcommand"}.each do |name, command|
             provider = newprovider