55 files changed, 321 insertions, 269 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 234b24393..825fad225 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,21 @@
+    Fixing #1062 by moving the yamldir setting to its own yaml
+    section.  This should keep the yamldir from being created
+    on clients.
+
+    Fixed #1047 -- Puppet's parser no longer changes the order
+    in which statements are evaluated, which means that case
+    statements can now set variables that are used by other
+    variables.
+
+    Fixed #1063 -- the master correctly logs syntax errors when
+    reparsing during a single run.
+
+    Removed the loglevels from the valid values for `logoutput`
+    in the Exec resource type -- the log levels are specified
+    using the `loglevel` parameter, not `logoutput`.  This never
+    worked, or at least hasn`t for ages, and now the docs are
+    just correct.
+
     Somewhat refactored fileserving so that it no longer caches
     any objects, nor does it use Puppet's RAL resources.  In the
     process, I fixed #894 (you can now copy links) and refactored
diff --git a/bin/puppetmasterd b/bin/puppetmasterd
index a00186b68..33e4f436d 100755
--- a/bin/puppetmasterd
+++ b/bin/puppetmasterd
@@ -81,6 +81,7 @@ end
 
 require 'getoptlong'
 require 'puppet'
+require 'puppet/network/handler'
 require 'puppet/sslcertificates'
 
 options = [
diff --git a/bin/puppetrun b/bin/puppetrun
index c92b59fc6..d0823b9c5 100755
--- a/bin/puppetrun
+++ b/bin/puppetrun
@@ -274,12 +274,7 @@ else
 end
 
 # Now parse the config
-config = File.join(Puppet[:confdir], "puppet.conf")
-Puppet.parse_config(config)
-
-if File.exists? config
-    Puppet.settings.parse(config)
-end
+Puppet.parse_config
 
 if Puppet[:node_terminus] = "ldap"
     if options[:all]
diff --git a/lib/puppet.rb b/lib/puppet.rb
index 18037cdc1..57f84d5f7 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -423,6 +423,7 @@ module Puppet
 end
 
 require 'puppet/type'
+require 'puppet/network'
 require 'puppet/module'
 require 'puppet/util/storage'
 require 'puppet/parser/interpreter'
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 520a18d1a..77792172f 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -140,8 +140,6 @@ module Puppet
         :show_diff => [false, "Whether to print a contextual diff when files are being replaced.  The diff
             is printed on stdout, so this option is meaningless unless you are running Puppet interactively.
             This feature currently requires the ``diff/lcs`` Ruby library."],
-        :yamldir => {:default => "$vardir/yaml", :owner => "$user", :group => "$user", :mode => "750",
-            :desc => "The directory in which YAML data is stored, usually in a subdirectory."},
         :daemonize => { :default => true,
             :desc => "Send the process into the background.  This is the default.",
             :short => "D"
@@ -672,5 +670,10 @@ module Puppet
         :rrdinterval => ["$runinterval", "How often RRD should expect data.
             This should match how often the hosts report back to the server."]
     )
+
+    Puppet.setdefaults(:yaml,
+        :yamldir => {:default => "$vardir/yaml", :owner => "$user", :group => "$user", :mode => "750",
+            :desc => "The directory in which YAML data is stored, usually in a subdirectory."}
+    )
 end
 
diff --git a/lib/puppet/file_serving/file_base.rb b/lib/puppet/file_serving/file_base.rb
index 06b3ad9ef..e87d683aa 100644
--- a/lib/puppet/file_serving/file_base.rb
+++ b/lib/puppet/file_serving/file_base.rb
@@ -46,6 +46,7 @@ class Puppet::FileServing::FileBase
     # Determine how we deal with links.
     attr_reader :links
     def links=(value)
+        value = :manage if value == :ignore
         raise(ArgumentError, ":links can only be set to :manage or :follow") unless [:manage, :follow].include?(value) 
         @links = value
     end
diff --git a/lib/puppet/network.rb b/lib/puppet/network.rb
new file mode 100644
index 000000000..8993b8869
--- /dev/null
+++ b/lib/puppet/network.rb
@@ -0,0 +1,3 @@
+# Just a stub, so we can correctly scope other classes.
+module Puppet::Network # :nodoc:
+end
diff --git a/lib/puppet/network/client.rb b/lib/puppet/network/client.rb
index 0a0a72345..cf1782f79 100644
--- a/lib/puppet/network/client.rb
+++ b/lib/puppet/network/client.rb
@@ -7,6 +7,8 @@ require 'puppet/util/subclass_loader'
 require 'puppet/util/methodhelper'
 require 'puppet/sslcertificates/support'
 
+require 'puppet/network/handler'
+
 require 'net/http'
 
 # Some versions of ruby don't have this method defined, which basically causes
diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb
index 390f3d4e2..913c51b3d 100644
--- a/lib/puppet/network/client/master.rb
+++ b/lib/puppet/network/client/master.rb
@@ -26,14 +26,15 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
         
         down = Puppet[:downcasefacts]
 
-        facts = {}
-        Facter.each { |name,fact|
+        facts = Facter.to_hash.inject({}) do |newhash, array|
+            name, fact = array
             if down
-                facts[name] = fact.to_s.downcase
+                newhash[name] = fact.to_s.downcase
             else
-                facts[name] = fact.to_s
+                newhash[name] = fact.to_s
             end
-        }
+            newhash
+        end
 
         # Add our client version to the list of facts, so people can use it
         # in their manifests
diff --git a/lib/puppet/parser/ast/astarray.rb b/lib/puppet/parser/ast/astarray.rb
index b66fd6bba..8f09aa922 100644
--- a/lib/puppet/parser/ast/astarray.rb
+++ b/lib/puppet/parser/ast/astarray.rb
@@ -16,16 +16,6 @@ class Puppet::Parser::AST
 
         # Evaluate our children.
         def evaluate(scope)
-            rets = nil
-            # We basically always operate declaratively, and when we
-            # do we need to evaluate the settor-like statements first.  This
-            # is basically variable and type-default declarations.
-            # This is such a stupid hack.  I've no real idea how to make a
-            # "real" declarative language, so I hack it so it looks like
-            # one, yay.
-            settors = []
-            others = []
-
             # Make a new array, so we don't have to deal with the details of
             # flattening and such
             items = []
@@ -34,22 +24,14 @@ class Puppet::Parser::AST
             @children.each { |child|
                 if child.instance_of?(AST::ASTArray)
                     child.each do |ac|
-                        if ac.class.settor?
-                            settors << ac
-                        else
-                            others << ac
-                        end
+                        items << ac
                     end
                 else
-                    if child.class.settor?
-                        settors << child
-                    else
-                        others << child
-                    end
+                    items << child
                 end
             }
 
-            rets = [settors, others].flatten.collect { |child|
+            rets = items.flatten.collect { |child|
                 child.safeevaluate(scope)
             }
             return rets.reject { |o| o.nil? }
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index 132ec15db..70cd6e11a 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -366,9 +366,8 @@ class Puppet::Parser::Compiler
     # Make sure all of our resources and such have done any last work
     # necessary.
     def finish
-        @catalog.resources.each do |name|
-            resource = @catalog.resource(name)
-
+        #@catalog.resources.each do |name|
+        @catalog.vertices.each do |resource|
             # Add in any resource overrides.
             if overrides = resource_overrides(resource)
                 overrides.each do |over|
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index d4655c403..f27c1c5c8 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -61,7 +61,11 @@ class Puppet::Parser::Interpreter
                 # If a parser already exists, than assume that we logged the
                 # exception elsewhere and reuse the parser.  If one doesn't
                 # exist, then reraise.
-                raise detail unless @parsers[environment]
+                if @parsers[environment]
+                    Puppet.err detail
+                else
+                    raise detail
+                end
             end
         end
         @parsers[environment]
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 46be89ca2..b001e165b 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -82,12 +82,19 @@ class Puppet::Parser::Resource
     # Do any finishing work on this object, called before evaluation or
     # before storage/translation.
     def finish
+        return if finished?
+        @finished = true
         add_defaults()
         add_metaparams()
         add_scope_tags()
         validate()
     end
 
+    # Has this resource already been finished?
+    def finished?
+        defined?(@finished) and @finished
+    end
+
     def initialize(options)
         # Set all of the options we can.
         options.each do |option, value|
diff --git a/lib/puppet/provider/interface/redhat.rb b/lib/puppet/provider/interface/redhat.rb
index aa217620e..4a9fcb491 100644
--- a/lib/puppet/provider/interface/redhat.rb
+++ b/lib/puppet/provider/interface/redhat.rb
@@ -5,7 +5,7 @@ Puppet::Type.type(:interface).provide(:redhat) do
     desc "Manage network interfaces on Red Hat operating systems.  This provider
         parses and generates configuration files in ``/etc/sysconfig/network-scripts``."
 
-	INTERFACE_DIR = "/etc/sysconfig/network-scripts"
+    INTERFACE_DIR = "/etc/sysconfig/network-scripts"
     confine :exists => INTERFACE_DIR
     defaultfor :operatingsystem => [:fedora, :centos, :redhat]
 
@@ -43,52 +43,52 @@ NETMASK=<%= self.netmask %>
 BROADCAST=
 LOOPBACKDUMMY
 
-	# maximum number of dummy interfaces
-	@max_dummies = 10
+    # maximum number of dummy interfaces
+    @max_dummies = 10
 
-	# maximum number of aliases per interface
-	@max_aliases_per_iface = 10
+    # maximum number of aliases per interface
+    @max_aliases_per_iface = 10
 
-	@@dummies = []
-	@@aliases = Hash.new { |hash, key| hash[key] = [] }
+    @@dummies = []
+    @@aliases = Hash.new { |hash, key| hash[key] = [] }
 
-	# calculate which dummy interfaces are currently already in
-	# use prior to needing to call self.next_dummy later on.
-	def self.instances
-		# parse all of the config files at once
-		Dir.glob("%s/ifcfg-*" % INTERFACE_DIR).collect do |file|
-			record = parse(file)
+    # calculate which dummy interfaces are currently already in
+    # use prior to needing to call self.next_dummy later on.
+    def self.instances
+        # parse all of the config files at once
+        Dir.glob("%s/ifcfg-*" % INTERFACE_DIR).collect do |file|
+            record = parse(file)
 
-			# store the existing dummy interfaces
+            # store the existing dummy interfaces
             @@dummies << record[:ifnum] if (record[:interface_type] == :dummy and ! @@dummies.include?(record[:ifnum]))
 
             @@aliases[record[:interface]] << record[:ifnum] if record[:interface_type] == :alias
 
             new(record)
-		end
-	end
-
-	# return the next avaliable dummy interface number, in the case where
-	# ifnum is not manually specified
-	def self.next_dummy
-		@max_dummies.times do |i|
-			unless @@dummies.include?(i.to_s)
-				@@dummies << i.to_s
-				return i.to_s
-			end
-		end
-	end
-
-	# return the next available alias on a given interface, in the case
-	# where ifnum if not manually specified
-	def self.next_alias(interface)
-		@max_aliases_per_iface.times do |i|
-			unless @@aliases[interface].include?(i.to_s)
-				@@aliases[interface] << i.to_s
-				return i.to_s
-			end
-		end
-	end
+        end
+    end
+
+    # return the next avaliable dummy interface number, in the case where
+    # ifnum is not manually specified
+    def self.next_dummy
+        @max_dummies.times do |i|
+            unless @@dummies.include?(i.to_s)
+                @@dummies << i.to_s
+                return i.to_s
+            end
+        end
+    end
+
+    # return the next available alias on a given interface, in the case
+    # where ifnum if not manually specified
+    def self.next_alias(interface)
+        @max_aliases_per_iface.times do |i|
+            unless @@aliases[interface].include?(i.to_s)
+                @@aliases[interface] << i.to_s
+                return i.to_s
+            end
+        end
+    end
 
     # base the ifnum, for dummy / loopback interface in linux
     # on the last octect of the IP address
@@ -139,14 +139,14 @@ LOOPBACKDUMMY
     # on whether we are adding an alias to a real interface, or a loopback
     # address (also dummy) on linux. For linux it's quite involved, and we
     # will use an ERB template
-	def generate
+    def generate
         itype = self.interface_type == :alias ? :alias : :normal
         self.class.template(itype).result(binding)
-	end
+    end
 
     # Where should the file be written out?
-	# This defaults to INTERFACE_DIR/ifcfg-<namevar>, but can have a
-	# more symbolic name by setting interface_desc in the type. 
+    # This defaults to INTERFACE_DIR/ifcfg-<namevar>, but can have a
+    # more symbolic name by setting interface_desc in the type. 
     def file_path
         if resource and val = resource[:interface_desc]
             desc = val
@@ -185,16 +185,16 @@ LOOPBACKDUMMY
         end
     end
 
-	# create the device name, so this based on the IP, and interface + type
-	def device
-		case @resource.should(:interface_type)
-		when :loopback
-			@property_hash[:ifnum] ||= self.class.next_dummy
-        	return "dummy" + @property_hash[:ifnum]
-		when :alias
-			@property_hash[:ifnum] ||= self.class.next_alias(@resource[:interface])
-        	return @resource[:interface] + ":" + @property_hash[:ifnum]
-		end
+    # create the device name, so this based on the IP, and interface + type
+    def device
+        case @resource.should(:interface_type)
+        when :loopback
+            @property_hash[:ifnum] ||= self.class.next_dummy
+            return "dummy" + @property_hash[:ifnum]
+        when :alias
+            @property_hash[:ifnum] ||= self.class.next_alias(@resource[:interface])
+            return @resource[:interface] + ":" + @property_hash[:ifnum]
+        end
     end
 
     # Set the name to our ip address.
@@ -202,19 +202,19 @@ LOOPBACKDUMMY
         @property_hash[:name] = value
     end
 
-	# whether the device is to be brought up on boot or not. converts
-	# the true / false of the type, into yes / no values respectively
-	# writing out the ifcfg-* files
-	def on_boot
-		case @property_hash[:onboot].to_s
-		when "true"
-			return "yes"
-		when "false"
-			return "no"
-		else
-			return "neither"
-		end
-	end
+    # whether the device is to be brought up on boot or not. converts
+    # the true / false of the type, into yes / no values respectively
+    # writing out the ifcfg-* files
+    def on_boot
+        case @property_hash[:onboot].to_s
+        when "true"
+            return "yes"
+        when "false"
+            return "no"
+        else
+            return "neither"
+        end
+    end
 
     # Mark whether the interface should be started on boot.
     def on_boot=(value)
diff --git a/lib/puppet/provider/package/yumhelper.py b/lib/puppet/provider/package/yumhelper.py
index 1142401b9..962b96ce4 100644
--- a/lib/puppet/provider/package/yumhelper.py
+++ b/lib/puppet/provider/package/yumhelper.py
@@ -13,8 +13,7 @@ OVERRIDE_OPTS = {
     'logfile': '/dev/null'
 }
 
-def pkg_lists():
-    my = yum.YumBase()
+def pkg_lists(my):
     my.doConfigSetup()
 
     for k in OVERRIDE_OPTS.keys():
@@ -28,10 +27,13 @@ def pkg_lists():
     return my.doPackageLists('updates')
 
 try:
-    ypl = pkg_lists()
+    try:
+        my = yum.YumBase()
+        ypl = pkg_lists(my)
+        for pkg in ypl.updates:
+            print "_pkg %s %s %s %s %s" % (pkg.name, pkg.epoch, pkg.version, pkg.release, pkg.arch)
+    finally:
+        my.closeRpmDB()
 except IOError, e:
     print "_err IOError %d %s" % (e.errno, e)
     sys.exit(1)
-    
-for pkg in ypl.updates:
-    print "_pkg %s %s %s %s %s" % (pkg.name, pkg.epoch, pkg.version, pkg.release, pkg.arch)
diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb
index f8049236f..2772b54a8 100755
--- a/lib/puppet/type/exec.rb
+++ b/lib/puppet/type/exec.rb
@@ -211,10 +211,7 @@ module Puppet
                 log the output when the command reports an error.  Values are
                 **true**, *false*, *on_failure*, and any legal log level."
 
-            values = [:true, :false, :on_failure]
-            # And all of the log levels
-            Puppet::Util::Log.eachlevel { |level| values << level }
-            newvalues(*values)
+            newvalues(:true, :false, :on_failure)
         end
 
         newparam(:refresh) do
diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb
index c41a7883b..1b625cc41 100644
--- a/lib/puppet/type/service.rb
+++ b/lib/puppet/type/service.rb
@@ -28,6 +28,8 @@ module Puppet
         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;
@@ -163,6 +165,13 @@ module Puppet
             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."
diff --git a/spec/integration/node.rb b/spec/integration/node.rb
index e4a311998..87ff448e4 100755
--- a/spec/integration/node.rb
+++ b/spec/integration/node.rb
@@ -10,10 +10,15 @@ require 'puppet/node'
 describe Puppet::Node, " when using the memory terminus" do
     before do
         @name = "me"
+        @old_terminus = Puppet::Node.indirection.terminus_class
         Puppet::Node.terminus_class = :memory
         @node = Puppet::Node.new(@name)
     end
 
+    after do
+        Puppet::Node.terminus_class = @old_terminus
+    end
+
     it "should find no nodes by default" do
         Puppet::Node.find(@name).should be_nil
     end
diff --git a/spec/unit/file_serving/file_base.rb b/spec/unit/file_serving/file_base.rb
index e1a61cd65..ded6ae4a8 100755
--- a/spec/unit/file_serving/file_base.rb
+++ b/spec/unit/file_serving/file_base.rb
@@ -13,7 +13,11 @@ describe Puppet::FileServing::FileBase do
         Puppet::FileServing::FileBase.new("puppet://host/module/dir/file", :links => :manage).links.should == :manage
     end
 
-    it "should fail if :links is set to anything other than :manage or :follow" do
+    it "should consider :ignore links equivalent to :manage links" do
+        Puppet::FileServing::FileBase.new("puppet://host/module/dir/file", :links => :ignore).links.should == :manage
+    end
+
+    it "should fail if :links is set to anything other than :manage, :follow, or :ignore" do
         proc { Puppet::FileServing::FileBase.new("puppet://host/module/dir/file", :links => :else) }.should raise_error(ArgumentError)
     end
 
diff --git a/spec/unit/parser/compiler.rb b/spec/unit/parser/compiler.rb
index 9980f2c6a..ab430da62 100755
--- a/spec/unit/parser/compiler.rb
+++ b/spec/unit/parser/compiler.rb
@@ -7,7 +7,7 @@ describe Puppet::Parser::Compiler do
         @node = Puppet::Node.new "testnode"
         @parser = Puppet::Parser::Parser.new :environment => "development"
 
-        @scope_resource = stub 'scope_resource', :builtin? => true
+        @scope_resource = stub 'scope_resource', :builtin? => true, :finish => nil, :ref => 'Class[main]'
         @scope = stub 'scope', :resource => @scope_resource, :source => mock("source")
         @compiler = Puppet::Parser::Compiler.new(@node, @parser)
     end
@@ -529,4 +529,4 @@ describe Puppet::Parser::Compiler do
             lambda { @compiler.class_set("one", @node) }.should raise_error(Puppet::ParseError)
         end
     end
-end
\ No newline at end of file
+end
diff --git a/spec/unit/parser/interpreter.rb b/spec/unit/parser/interpreter.rb
index eb5dd9aaf..f2526c73d 100755
--- a/spec/unit/parser/interpreter.rb
+++ b/spec/unit/parser/interpreter.rb
@@ -2,148 +2,158 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-describe Puppet::Parser::Interpreter, " when creating parser instances" do
+describe Puppet::Parser::Interpreter do
     before do
         @interp = Puppet::Parser::Interpreter.new
-        @parser = mock('parser')
-    end
-
-    it "should create a parser with code if there is code defined in the :code setting" do
-        Puppet.settings.stubs(:value).with(:code, :myenv).returns("mycode")
-        @parser.expects(:string=).with("mycode")
-        @parser.expects(:parse)
-        Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).returns(@parser)
-        @interp.send(:create_parser, :myenv).object_id.should equal(@parser.object_id)
-    end
-
-    it "should create a parser with the main manifest when the code setting is an empty string" do
-        Puppet.settings.stubs(:value).with(:code, :myenv).returns("")
-        Puppet.settings.stubs(:value).with(:manifest, :myenv).returns("/my/file")
-        @parser.expects(:parse)
-        @parser.expects(:file=).with("/my/file")
-        Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).returns(@parser)
-        @interp.send(:create_parser, :myenv).should equal(@parser)
-    end
-
-    it "should return nothing when new parsers fail" do
-        Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).raises(ArgumentError)
-        proc { @interp.send(:create_parser, :myenv) }.should raise_error(Puppet::Error)
-    end
-
-    it "should create parsers with environment-appropriate manifests" do
-        # Set our per-environment values.  We can't just stub :value, because
-        # it's called by too much of the rest of the code.
-        text = "[env1]\nmanifest = /t/env1.pp\n[env2]\nmanifest = /t/env2.pp"
-        file = mock 'file'
-        file.stubs(:changed?).returns(true)
-        file.stubs(:file).returns("/whatever")
-        Puppet.settings.stubs(:read_file).with(file).returns(text)
-        Puppet.settings.parse(file)
-
-        parser1 = mock 'parser1'
-        Puppet::Parser::Parser.expects(:new).with(:environment => :env1).returns(parser1)
-        parser1.expects(:file=).with("/t/env1.pp")
-        parser1.expects(:parse)
-        @interp.send(:create_parser, :env1)
-
-        parser2 = mock 'parser2'
-        Puppet::Parser::Parser.expects(:new).with(:environment => :env2).returns(parser2)
-        parser2.expects(:file=).with("/t/env2.pp")
-        parser2.expects(:parse)
-        @interp.send(:create_parser, :env2)
-    end
-end
-
-describe Puppet::Parser::Interpreter, " when managing parser instances" do
-    before do
-        @interp = Puppet::Parser::Interpreter.new
-        @parser = mock('parser')
-    end
-
-    it "should use the same parser when the parser does not need reparsing" do
-        @interp.expects(:create_parser).with(:myenv).returns(@parser)
-        @interp.send(:parser, :myenv).should equal(@parser)
-
-        @parser.expects(:reparse?).returns(false)
-        @interp.send(:parser, :myenv).should equal(@parser)
-    end
-
-    it "should create a new parser when reparse is true" do
-        oldparser = mock('oldparser')
-        newparser = mock('newparser')
-        oldparser.expects(:reparse?).returns(true)
-        oldparser.expects(:clear)
-
-        @interp.expects(:create_parser).with(:myenv).returns(oldparser)
-        @interp.send(:parser, :myenv).should equal(oldparser)
-        @interp.expects(:create_parser).with(:myenv).returns(newparser)
-        @interp.send(:parser, :myenv).should equal(newparser)
-    end
-
-    it "should fail intelligently if a parser cannot be created and one does not already exist" do
-        @interp.expects(:create_parser).with(:myenv).raises(ArgumentError)
-        proc { @interp.send(:parser, :myenv) }.should raise_error(ArgumentError)
-    end
-
-    it "should keep the old parser if a new parser cannot be created" do
-        # Get the first parser in the hash.
-        @interp.expects(:create_parser).with(:myenv).returns(@parser)
-        @interp.send(:parser, :myenv).should equal(@parser)
-
-        # Have it indicate something has changed
-        @parser.expects(:reparse?).returns(true)
-
-        # But fail to create a new parser
-        @interp.expects(:create_parser).with(:myenv).raises(ArgumentError)
-
-        # And make sure we still get the old valid parser
-        @interp.send(:parser, :myenv).should equal(@parser)
-    end
-
-    it "should use different parsers for different environments" do
-        # get one for the first env
-        @interp.expects(:create_parser).with(:first_env).returns(@parser)
-        @interp.send(:parser, :first_env).should equal(@parser)
-
-        other_parser = mock('otherparser')
-        @interp.expects(:create_parser).with(:second_env).returns(other_parser)
-        @interp.send(:parser, :second_env).should equal(other_parser)
-    end
-end
-
-describe Puppet::Parser::Interpreter, " when compiling catalog" do
-    before do
-        @interp = Puppet::Parser::Interpreter.new
-        @node = stub 'node', :environment => :myenv
-        @compiler = mock 'compile'
         @parser = mock 'parser'
     end
 
-    it "should create a compile with the node and parser" do
-        @compiler.expects(:compile).returns(:config)
-        @interp.expects(:parser).with(:myenv).returns(@parser)
-        Puppet::Parser::Compiler.expects(:new).with(@node, @parser).returns(@compiler)
-        @interp.compile(@node)
-    end
-
-    it "should fail intelligently when no parser can be found" do
-        @node.stubs(:name).returns("whatever")
-        @interp.expects(:parser).with(:myenv).returns(nil)
-        proc { @interp.compile(@node) }.should raise_error(Puppet::ParseError)
-    end
-end
-
-describe Puppet::Parser::Interpreter, " when returning catalog version" do
-    before do
-        @interp = Puppet::Parser::Interpreter.new
-    end
-
-    it "should ask the appropriate parser for the catalog version" do
-        node = mock 'node'
-        node.expects(:environment).returns(:myenv)
-        parser = mock 'parser'
-        parser.expects(:version).returns(:myvers)
-        @interp.expects(:parser).with(:myenv).returns(parser)
-        @interp.configuration_version(node).should equal(:myvers)
+    describe "when creating parser instances" do
+        it "should create a parser with code if there is code defined in the :code setting" do
+            Puppet.settings.stubs(:value).with(:code, :myenv).returns("mycode")
+            @parser.expects(:string=).with("mycode")
+            @parser.expects(:parse)
+            Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).returns(@parser)
+            @interp.send(:create_parser, :myenv).object_id.should equal(@parser.object_id)
+        end
+
+        it "should create a parser with the main manifest when the code setting is an empty string" do
+            Puppet.settings.stubs(:value).with(:code, :myenv).returns("")
+            Puppet.settings.stubs(:value).with(:manifest, :myenv).returns("/my/file")
+            @parser.expects(:parse)
+            @parser.expects(:file=).with("/my/file")
+            Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).returns(@parser)
+            @interp.send(:create_parser, :myenv).should equal(@parser)
+        end
+
+        it "should return nothing when new parsers fail" do
+            Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).raises(ArgumentError)
+            proc { @interp.send(:create_parser, :myenv) }.should raise_error(Puppet::Error)
+        end
+
+        it "should create parsers with environment-appropriate manifests" do
+            # Set our per-environment values.  We can't just stub :value, because
+            # it's called by too much of the rest of the code.
+            text = "[env1]\nmanifest = /t/env1.pp\n[env2]\nmanifest = /t/env2.pp"
+            file = mock 'file'
+            file.stubs(:changed?).returns(true)
+            file.stubs(:file).returns("/whatever")
+            Puppet.settings.stubs(:read_file).with(file).returns(text)
+            Puppet.settings.parse(file)
+
+            parser1 = mock 'parser1'
+            Puppet::Parser::Parser.expects(:new).with(:environment => :env1).returns(parser1)
+            parser1.expects(:file=).with("/t/env1.pp")
+            parser1.expects(:parse)
+            @interp.send(:create_parser, :env1)
+
+            parser2 = mock 'parser2'
+            Puppet::Parser::Parser.expects(:new).with(:environment => :env2).returns(parser2)
+            parser2.expects(:file=).with("/t/env2.pp")
+            parser2.expects(:parse)
+            @interp.send(:create_parser, :env2)
+        end
+    end
+
+    describe "when managing parser instances" do
+        it "should use the same parser when the parser does not need reparsing" do
+            @interp.expects(:create_parser).with(:myenv).returns(@parser)
+            @interp.send(:parser, :myenv).should equal(@parser)
+
+            @parser.expects(:reparse?).returns(false)
+            @interp.send(:parser, :myenv).should equal(@parser)
+        end
+
+        it "should fail intelligently if a parser cannot be created and one does not already exist" do
+            @interp.expects(:create_parser).with(:myenv).raises(ArgumentError)
+            proc { @interp.send(:parser, :myenv) }.should raise_error(ArgumentError)
+        end
+
+        it "should use different parsers for different environments" do
+            # get one for the first env
+            @interp.expects(:create_parser).with(:first_env).returns(@parser)
+            @interp.send(:parser, :first_env).should equal(@parser)
+
+            other_parser = mock('otherparser')
+            @interp.expects(:create_parser).with(:second_env).returns(other_parser)
+            @interp.send(:parser, :second_env).should equal(other_parser)
+        end
+
+        describe "when files need reparsing" do
+            it "should create a new parser" do
+                oldparser = mock('oldparser')
+                newparser = mock('newparser')
+                oldparser.expects(:reparse?).returns(true)
+                oldparser.expects(:clear)
+
+                @interp.expects(:create_parser).with(:myenv).returns(oldparser)
+                @interp.send(:parser, :myenv).should equal(oldparser)
+                @interp.expects(:create_parser).with(:myenv).returns(newparser)
+                @interp.send(:parser, :myenv).should equal(newparser)
+            end
+
+            it "should keep the old parser if a new parser cannot be created" do
+                # Get the first parser in the hash.
+                @interp.expects(:create_parser).with(:myenv).returns(@parser)
+                @interp.send(:parser, :myenv).should equal(@parser)
+
+                # Have it indicate something has changed
+                @parser.expects(:reparse?).returns(true)
+
+                # But fail to create a new parser
+                @interp.expects(:create_parser).with(:myenv).raises(ArgumentError)
+
+                # And make sure we still get the old valid parser
+                @interp.send(:parser, :myenv).should equal(@parser)
+            end
+
+            it "should log syntax errors when using the old parser" do
+                # Get the first parser in the hash.
+                @interp.stubs(:create_parser).with(:myenv).returns(@parser)
+                @interp.send(:parser, :myenv)
+
+                # Have it indicate something has changed
+                @parser.stubs(:reparse?).returns(true)
+
+                # But fail to create a new parser
+                @interp.stubs(:create_parser).with(:myenv).raises(ArgumentError)
+
+                Puppet.expects(:err)
+
+                # And make sure we still get the old valid parser
+                @interp.send(:parser, :myenv)
+            end
+        end
+    end
+
+    describe "when compiling a catalog" do
+        before do
+            @node = stub 'node', :environment => :myenv
+            @compiler = mock 'compile'
+        end
+
+        it "should create a compile with the node and parser" do
+            @compiler.expects(:compile).returns(:config)
+            @interp.expects(:parser).with(:myenv).returns(@parser)
+            Puppet::Parser::Compiler.expects(:new).with(@node, @parser).returns(@compiler)
+            @interp.compile(@node)
+        end
+
+        it "should fail intelligently when no parser can be found" do
+            @node.stubs(:name).returns("whatever")
+            @interp.expects(:parser).with(:myenv).returns(nil)
+            proc { @interp.compile(@node) }.should raise_error(Puppet::ParseError)
+        end
+    end
+
+    describe "when returning catalog version" do
+        it "should ask the appropriate parser for the catalog version" do
+            node = mock 'node'
+            node.expects(:environment).returns(:myenv)
+            parser = mock 'parser'
+            parser.expects(:version).returns(:myvers)
+            @interp.expects(:parser).with(:myenv).returns(parser)
+            @interp.configuration_version(node).should equal(:myvers)
+        end
     end
 end
diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb
index 035590341..9ce7b391b 100755
--- a/spec/unit/parser/resource.rb
+++ b/spec/unit/parser/resource.rb
@@ -67,6 +67,12 @@ describe Puppet::Parser::Resource do
             @resource = Puppet::Parser::Resource.new(:type => "mydefine", :title => "whatever", :scope => @scope, :source => @source)
         end
 
+        it "should do nothing if it has already been finished" do
+            @resource.finish
+            @resource.expects(:add_metaparams).never
+            @resource.finish
+        end
+
         it "should copy metaparams from its scope" do
             @scope.setvar("noop", "true")
 
diff --git a/spec/unit/ral/types/exec.rb b/spec/unit/ral/type/exec.rb
index 260804227..260804227 100755
--- a/spec/unit/ral/types/exec.rb
+++ b/spec/unit/ral/type/exec.rb
diff --git a/spec/unit/ral/types/file.rb b/spec/unit/ral/type/file.rb
index b213987bb..b213987bb 100755
--- a/spec/unit/ral/types/file.rb
+++ b/spec/unit/ral/type/file.rb
diff --git a/spec/unit/ral/types/interface.rb b/spec/unit/ral/type/interface.rb
index 2e0176152..2e0176152 100755
--- a/spec/unit/ral/types/interface.rb
+++ b/spec/unit/ral/type/interface.rb
diff --git a/spec/unit/ral/types/mount.rb b/spec/unit/ral/type/mount.rb
index 9247601e6..9247601e6 100755
--- a/spec/unit/ral/types/mount.rb
+++ b/spec/unit/ral/type/mount.rb
diff --git a/spec/unit/ral/types/nagios.rb b/spec/unit/ral/type/nagios.rb
index 8aca7d401..8aca7d401 100755
--- a/spec/unit/ral/types/nagios.rb
+++ b/spec/unit/ral/type/nagios.rb
diff --git a/spec/unit/ral/types/package.rb b/spec/unit/ral/type/package.rb
index 5d96dc4ae..5d96dc4ae 100755
--- a/spec/unit/ral/types/package.rb
+++ b/spec/unit/ral/type/package.rb
diff --git a/spec/unit/ral/types/schedule.rb b/spec/unit/ral/type/schedule.rb
index 4e9840c34..4e9840c34 100755
--- a/spec/unit/ral/types/schedule.rb
+++ b/spec/unit/ral/type/schedule.rb
diff --git a/spec/unit/ral/types/service.rb b/spec/unit/ral/type/service.rb
index 981d38a15..0f00992fa 100755
--- a/spec/unit/ral/types/service.rb
+++ b/spec/unit/ral/type/service.rb
@@ -15,7 +15,7 @@ describe Puppet::Type::Service do
 end
 
 describe Puppet::Type::Service, "when validating attributes" do
-    [:name, :binary, :hasstatus, :path, :pattern, :start, :restart, :stop, :status, :hasrestart].each do |param|
+    [:name, :binary, :hasstatus, :path, :pattern, :start, :restart, :stop, :status, :hasrestart, :control].each do |param|
         it "should have a #{param} parameter" do
             Puppet::Type::Service.attrtype(param).should == :param
         end
@@ -30,7 +30,7 @@ end
 
 describe Puppet::Type::Service, "when validating attribute values" do
     before do
-        @provider = stub 'provider', :class => Puppet::Type::Service.defaultprovider, :clear => nil
+        @provider = stub 'provider', :class => Puppet::Type::Service.defaultprovider, :clear => nil, :controllable? => false
         Puppet::Type::Service.defaultprovider.stubs(:new).returns(@provider)
     end
 
@@ -132,16 +132,23 @@ describe Puppet::Type::Service, "when setting default attribute values" do
         svc[:path].should == ["testing"]
     end
 
-    it "should default to the binary for the pattern if one is provided" do
+    it "should default 'pattern' to the binary if one is provided" do
         svc = Puppet::Type::Service.create(:name => "other", :binary => "/some/binary")
         svc[:pattern].should == "/some/binary"
     end
 
-    it "should default to the name for the pattern if no pattern is provided" do
+    it "should default 'pattern' to the name if no pattern is provided" do
         svc = Puppet::Type::Service.create(:name => "other")
         svc[:pattern].should == "other"
     end
 
+    it "should default 'control' to the upcased service name with periods replaced by underscores if the provider supports the 'controllable' feature" do
+        provider = stub 'provider', :controllable? => true, :class => Puppet::Type::Service.defaultprovider, :clear => nil
+        Puppet::Type::Service.defaultprovider.stubs(:new).returns(provider)
+        svc = Puppet::Type::Service.create(:name => "nfs.client")
+        svc[:control].should == "NFS_CLIENT_START"
+    end
+
     after { Puppet::Type::Service.clear }
 end
 
diff --git a/spec/unit/ral/types/user.rb b/spec/unit/ral/type/user.rb
index 4e43a8ceb..4e43a8ceb 100755
--- a/spec/unit/ral/types/user.rb
+++ b/spec/unit/ral/type/user.rb
diff --git a/test/network/client/master.rb b/test/network/client/master.rb
index 67c47fa6d..41796575f 100755
--- a/test/network/client/master.rb
+++ b/test/network/client/master.rb
@@ -216,15 +216,7 @@ end
         name = "environment"
         value = "test_environment"
 
-        Puppet[:filetimeout] = -1
-        Puppet[:factsource] = tempfile()
-        Dir.mkdir(Puppet[:factsource])
-        file = File.join(Puppet[:factsource], "#{name}.rb")
-        File.open(file, "w") do |f|
-            f.puts %{Facter.add("#{name}") do setcode { "#{value}" } end }
-        end
-
-        Puppet::Network::Client.master.getfacts
+        Facter.stubs(:to_hash).returns(name => value)
 
         assert_equal(value, Puppet::Network::Client.master.facts[name])
     end
diff --git a/test/rails/railsparameter.rb b/test/rails/railsparameter.rb
index d83115b1a..7c99ac38d 100755
--- a/test/rails/railsparameter.rb
+++ b/test/rails/railsparameter.rb
@@ -25,6 +25,8 @@ class TestRailsParameter < Test::Unit::TestCase
         source = parser.newclass "myclass"
 
         host = Puppet::Rails::Host.new(:name => "myhost")
+
+        host.save
         
         resource = host.resources.create(
             :title => "/tmp/to_resource", 
diff --git a/test/rails/railsresource.rb b/test/rails/railsresource.rb
index 58058472d..3df5001be 100755
--- a/test/rails/railsresource.rb
+++ b/test/rails/railsresource.rb
@@ -29,6 +29,7 @@ class TestRailsResource < Test::Unit::TestCase
     def mktest_resource
         # We need a host for resources
         host = Puppet::Rails::Host.new(:name => "myhost")
+        host.save
 
         # Now build a resource
         resource = host.resources.create(
diff --git a/test/ral/types/basic.rb b/test/ral/type/basic.rb
index 3c5faeee0..3c5faeee0 100755
--- a/test/ral/types/basic.rb
+++ b/test/ral/type/basic.rb
diff --git a/test/ral/types/cron.rb b/test/ral/type/cron.rb
index 73e941894..73e941894 100755
--- a/test/ral/types/cron.rb
+++ b/test/ral/type/cron.rb
diff --git a/test/ral/types/exec.rb b/test/ral/type/exec.rb
index 4133d8519..e2a3dd9ed 100755
--- a/test/ral/types/exec.rb
+++ b/test/ral/type/exec.rb
@@ -394,8 +394,8 @@ class TestExec < Test::Unit::TestCase
         assert_apply(exec)
 
         assert_nothing_raised {
-            exec[:command] = "echo logoutput is warning"
-            exec[:logoutput] = "warning"
+            exec[:command] = "echo logoutput is on_failure"
+            exec[:logoutput] = "on_failure"
         }
 
         assert_apply(exec)
diff --git a/test/ral/types/file.rb b/test/ral/type/file.rb
index cbbe818ae..cbbe818ae 100755
--- a/test/ral/types/file.rb
+++ b/test/ral/type/file.rb
diff --git a/test/ral/types/file/target.rb b/test/ral/type/file/target.rb
index 035ea905b..035ea905b 100755
--- a/test/ral/types/file/target.rb
+++ b/test/ral/type/file/target.rb
diff --git a/test/ral/types/filebucket.rb b/test/ral/type/filebucket.rb
index f9706663b..f9706663b 100755
--- a/test/ral/types/filebucket.rb
+++ b/test/ral/type/filebucket.rb
diff --git a/test/ral/types/fileignoresource.rb b/test/ral/type/fileignoresource.rb
index ff867c879..ff867c879 100755
--- a/test/ral/types/fileignoresource.rb
+++ b/test/ral/type/fileignoresource.rb
diff --git a/test/ral/types/filesources.rb b/test/ral/type/filesources.rb
index a7bb6fefa..a7bb6fefa 100755
--- a/test/ral/types/filesources.rb
+++ b/test/ral/type/filesources.rb
diff --git a/test/ral/types/group.rb b/test/ral/type/group.rb
index d28c8eea5..d28c8eea5 100755
--- a/test/ral/types/group.rb
+++ b/test/ral/type/group.rb
diff --git a/test/ral/types/host.rb b/test/ral/type/host.rb
index a5d645bd1..a5d645bd1 100755
--- a/test/ral/types/host.rb
+++ b/test/ral/type/host.rb
diff --git a/test/ral/types/mailalias.rb b/test/ral/type/mailalias.rb
index ff1dd562a..ff1dd562a 100755
--- a/test/ral/types/mailalias.rb
+++ b/test/ral/type/mailalias.rb
diff --git a/test/ral/types/parameter.rb b/test/ral/type/parameter.rb
index e1b8e00b3..e1b8e00b3 100755
--- a/test/ral/types/parameter.rb
+++ b/test/ral/type/parameter.rb
diff --git a/test/ral/types/port.rb b/test/ral/type/port.rb
index e28904d55..e28904d55 100755
--- a/test/ral/types/port.rb
+++ b/test/ral/type/port.rb
diff --git a/test/ral/types/property.rb b/test/ral/type/property.rb
index 6a3370caa..6a3370caa 100755
--- a/test/ral/types/property.rb
+++ b/test/ral/type/property.rb
diff --git a/test/ral/types/resources.rb b/test/ral/type/resources.rb
index 0663fe795..0663fe795 100755
--- a/test/ral/types/resources.rb
+++ b/test/ral/type/resources.rb
diff --git a/test/ral/types/service.rb b/test/ral/type/service.rb
index 01533c63e..01533c63e 100755
--- a/test/ral/types/service.rb
+++ b/test/ral/type/service.rb
diff --git a/test/ral/types/sshkey.rb b/test/ral/type/sshkey.rb
index b9aed20e8..b9aed20e8 100755
--- a/test/ral/types/sshkey.rb
+++ b/test/ral/type/sshkey.rb
diff --git a/test/ral/types/tidy.rb b/test/ral/type/tidy.rb
index 60fad6516..60fad6516 100755
--- a/test/ral/types/tidy.rb
+++ b/test/ral/type/tidy.rb
diff --git a/test/ral/types/user.rb b/test/ral/type/user.rb
index b280acfed..b280acfed 100755
--- a/test/ral/types/user.rb
+++ b/test/ral/type/user.rb
diff --git a/test/ral/types/yumrepo.rb b/test/ral/type/yumrepo.rb
index 899a02135..899a02135 100755
--- a/test/ral/types/yumrepo.rb
+++ b/test/ral/type/yumrepo.rb
diff --git a/test/ral/types/zone.rb b/test/ral/type/zone.rb
index eb485b944..eb485b944 100755
--- a/test/ral/types/zone.rb
+++ b/test/ral/type/zone.rb