Merge branch 'next'

* next:
  (#7101) Fix template error messages in Ruby 1.8.5
  (#3420) Nagios "name" attribute does not output correctly
  (#4487) When setting environment on a host, ensure it is a string.
  add test for ticket 7101
  add test for ticket 7101
  (#7220) Add the ability to "inherit" options.
  (#6487) Add some testing for OS X version support in DirectoryService provider
  (#6487) Directoryservice provider will fail in future OS releases
  (#6368) Make the File type autorequire its nearest ancestor directory

11 files changed, 285 insertions, 11 deletions

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 08bc0a345..f8eef69b1 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -217,6 +217,11 @@ WRAPPER
     option
   end
 
+  def inherit_options_from(action)
+    options = action.options.map { |opt| action.get_option(opt, false) }
+    options.reject!(&:nil?).uniq.each { |option| add_option(option) }
+  end
+
   def option?(name)
     @options.include? name.to_sym
   end
diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb
index 2ffa38709..fd8b0856f 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -38,6 +38,16 @@ class Puppet::Interface::ActionBuilder
     @action.add_option(option)
   end
 
+  def inherit_options_from(action)
+    if action.is_a? Symbol
+      name = action
+      action = @face.get_action(name) or
+      raise ArgumentError, "This Face has no action named #{name}!"
+    end
+
+    @action.inherit_options_from(action)
+  end
+
   def default(value = true)
     @action.default = !!value
   end
diff --git a/lib/puppet/provider/naginator.rb b/lib/puppet/provider/naginator.rb
index 5c610fb31..17cc24086 100644
--- a/lib/puppet/provider/naginator.rb
+++ b/lib/puppet/provider/naginator.rb
@@ -30,7 +30,15 @@ class Puppet::Provider::Naginator < Puppet::Provider::ParsedFile
   end
 
   def self.to_file(records)
-    header + records.collect { |record| record.to_s }.join("\n").gsub("_naginator_name", NAME_STRING)
+    header + records.collect { |record|
+        # Remap the TYPE_name or _naginator_name params to the
+        # name if the record is a template (register == 0)
+        if record.to_s =~ /register\s+0/
+            record.to_s.sub("_naginator_name", "name").sub(record.type.to_s + "_name", "name")
+        else
+            record.to_s.sub("_naginator_name", NAME_STRING)
+        end
+    }.join("\n")
   end
 
   def self.skip_record?(record)
diff --git a/lib/puppet/provider/nameservice/directoryservice.rb b/lib/puppet/provider/nameservice/directoryservice.rb
index c1139a679..35ac8d76a 100644
--- a/lib/puppet/provider/nameservice/directoryservice.rb
+++ b/lib/puppet/provider/nameservice/directoryservice.rb
@@ -221,11 +221,12 @@ class DirectoryService < Puppet::Provider::NameService
     # have a lot of choice. Ultimately this should all be done using Ruby
     # to access the DirectoryService APIs directly, but that's simply not
     # feasible for a while yet.
-    case self.get_macosx_version_major
-    when "10.4"
-      dscl_plist = self.parse_dscl_url_data(dscl_output)
-    when "10.5", "10.6"
+    if self.get_macosx_version_major > "10.4"
       dscl_plist = self.parse_dscl_plist_data(dscl_output)
+    elsif self.get_macosx_version_major == "10.4"
+      dscl_plist = self.parse_dscl_url_data(dscl_output)
+    else
+      fail("Puppet does not support OS X versions < 10.4")
     end
 
     self.generate_attribute_hash(dscl_plist, *type_properties)
@@ -243,12 +244,14 @@ class DirectoryService < Puppet::Provider::NameService
     # different format for the -url output with objects with spaces in
     # their values. *sigh*. Use -url for 10.4 in the hope this can be
     # deprecated one day, and use -plist for 10.5 and higher.
-    case self.get_macosx_version_major
-    when "10.4"
-      command_vector = [ command(:dscl), "-url", "." ]
-    when "10.5", "10.6"
+    if self.get_macosx_version_major > "10.4"
       command_vector = [ command(:dscl), "-plist", "." ]
+    elsif self.get_macosx_version_major == "10.4"
+      command_vector = [ command(:dscl), "-url", "." ]
+    else
+      fail("Puppet does not support OS X versions < 10.4")
     end
+
     # JJM: The actual action to perform.  See "man dscl"
     #      Common actiosn: -create, -delete, -merge, -append, -passwd
     command_vector << ds_action
diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb
index b9dea2a3d..e5360217c 100644
--- a/lib/puppet/rails/host.rb
+++ b/lib/puppet/rails/host.rb
@@ -1,3 +1,4 @@
+require 'puppet/node/environment'
 require 'puppet/rails'
 require 'puppet/rails/resource'
 require 'puppet/rails/fact_name'
@@ -28,6 +29,12 @@ class Puppet::Rails::Host < ActiveRecord::Base
     host
   end
 
+  # Override the setter for environment to force it to be a string, lest it
+  # be YAML encoded.  See #4487.
+  def environment=(value)
+    super value.to_s
+  end
+
   # returns a hash of fact_names.name => [ fact_values ] for this host.
   # Note that 'fact_values' is actually a list of the value instances, not
   # just actual values.
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 715836b22..1790c5e92 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -257,11 +257,17 @@ Puppet::Type.newtype(:file) do
     newvalues(:first, :all)
   end
 
-  # Autorequire any parent directories.
+  # Autorequire the nearest ancestor directory found in the catalog.
   autorequire(:file) do
     basedir = ::File.dirname(self[:path])
     if basedir != self[:path]
-      basedir
+      parents = []
+      until basedir == parents.last
+        parents << basedir
+        basedir = File.dirname(basedir)
+      end
+      # The filename of the first ancestor found, or nil
+      parents.find { |dir| catalog.resource(:file, dir) }
     else
       nil
     end
diff --git a/spec/unit/interface/action_builder_spec.rb b/spec/unit/interface/action_builder_spec.rb
index bf7afa74e..6f36d2b44 100755
--- a/spec/unit/interface/action_builder_spec.rb
+++ b/spec/unit/interface/action_builder_spec.rb
@@ -52,6 +52,61 @@ describe Puppet::Interface::ActionBuilder do
     end
   end
 
+  describe "#inherit_options_from" do
+    let :face do
+      Puppet::Interface.new(:face_with_some_options, '0.0.1') do
+        option '-w'
+
+        action(:foo) do
+          option '-x', '--ex'
+          option '-y', '--why'
+        end
+
+        action(:bar) do
+          option '-z', '--zee'
+        end
+
+        action(:baz) do
+          option '-z', '--zed'
+        end
+      end
+    end
+
+    it 'should add the options from the specified action' do
+      foo = face.get_action(:foo)
+      action = Puppet::Interface::ActionBuilder.build(face, :inherit_options) do
+        inherit_options_from foo
+      end
+      action.options.should == foo.options
+    end
+
+    it 'should add the options from multiple actions' do
+      foo = face.get_action(:foo)
+      bar = face.get_action(:bar)
+      action = Puppet::Interface::ActionBuilder.build(face, :inherit_options) do
+        inherit_options_from foo
+        inherit_options_from bar
+      end
+      action.options.should == (foo.options + bar.options).uniq.sort
+    end
+
+    it 'should permit symbolic names for actions in the same face' do
+      foo = face.get_action(:foo)
+      action = Puppet::Interface::ActionBuilder.build(face, :inherit_options) do
+        inherit_options_from :foo
+      end
+      action.options.should == foo.options
+    end
+
+    it 'should raise a useful error if you supply a bad action name' do
+      expect do
+        Puppet::Interface::ActionBuilder.build(face, :inherit_options) do
+          inherit_options_from :nowhere
+        end
+      end.to raise_error /nowhere/
+    end
+  end
+
   context "inline documentation" do
     it "should set the summary" do
       action = Puppet::Interface::ActionBuilder.build(face, :foo) do
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index 24826a6ef..735fbcb72 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -68,6 +68,61 @@ describe Puppet::Interface::Action do
     end
   end
 
+  describe "#inherit_options_from" do
+    let :face do
+      Puppet::Interface.new(:face_with_some_options, '0.0.1') do
+        option '-w'
+
+        action(:foo) do
+          option '-x', '--ex'
+          option '-y', '--why'
+        end
+
+        action(:bar) do
+          option '-z', '--zee'
+        end
+
+        action(:baz) do
+          option '-z', '--zed'
+        end
+
+        action(:noopts) do
+          # no options declared
+        end
+      end
+    end
+
+    subject { action = face.action(:new_action) { } }
+
+    it 'should add the options from the specified action' do
+      subject.inherit_options_from(foo = face.get_action(:foo))
+      subject.options.should == foo.options
+    end
+
+    it 'should not die when the specified action has no options' do
+      original_options = subject.options
+      subject.inherit_options_from(face.get_action(:noopts))
+      subject.options.should == original_options
+    end
+
+    it 'should add the options from multiple actions' do
+      subject.inherit_options_from(foo = face.get_action(:foo))
+      subject.inherit_options_from(bar = face.get_action(:bar))
+      subject.options.should == (foo.options + bar.options).uniq.sort
+    end
+
+    it 'should not inherit face options' do
+      subject.expects(:add_option)
+      subject.expects(:add_option).with(face.get_option(:w)).never
+      subject.inherit_options_from(face.get_action(:bar))
+    end
+
+    it 'should raise an error if inheritance would duplicate options' do
+      subject.inherit_options_from(face.get_action(:bar))
+      expect { subject.inherit_options_from(face.get_action(:baz)) }.to raise_error
+    end
+  end
+
   describe "when invoking" do
     it "should be able to call other actions on the same object" do
       face = Puppet::Interface.new(:my_face, '0.0.1') do
diff --git a/spec/unit/provider/nameservice/directoryservice_spec.rb b/spec/unit/provider/nameservice/directoryservice_spec.rb
index 47f2ad0cd..7a83d7f20 100755
--- a/spec/unit/provider/nameservice/directoryservice_spec.rb
+++ b/spec/unit/provider/nameservice/directoryservice_spec.rb
@@ -35,3 +35,63 @@ require 'spec_helper'
     end
   end
 end
+
+describe 'DirectoryService.single_report' do
+  it 'should fail on OS X < 10.4' do
+    Puppet::Provider::NameService::DirectoryService.stubs(:get_macosx_version_major).returns("10.3")
+
+    lambda {
+      Puppet::Provider::NameService::DirectoryService.single_report('resource_name')
+    }.should raise_error(RuntimeError, "Puppet does not support OS X versions < 10.4")
+  end
+
+  it 'should use url data on 10.4' do
+    Puppet::Provider::NameService::DirectoryService.stubs(:get_macosx_version_major).returns("10.4")
+    Puppet::Provider::NameService::DirectoryService.stubs(:get_ds_path).returns('Users')
+    Puppet::Provider::NameService::DirectoryService.stubs(:list_all_present).returns(
+      ['root', 'user1', 'user2', 'resource_name']
+    )
+    Puppet::Provider::NameService::DirectoryService.stubs(:generate_attribute_hash)
+    Puppet::Provider::NameService::DirectoryService.stubs(:execute)
+    Puppet::Provider::NameService::DirectoryService.expects(:parse_dscl_url_data)
+
+    Puppet::Provider::NameService::DirectoryService.single_report('resource_name')
+  end
+
+  it 'should use plist data on > 10.4' do
+    Puppet::Provider::NameService::DirectoryService.stubs(:get_macosx_version_major).returns("10.5")
+    Puppet::Provider::NameService::DirectoryService.stubs(:get_ds_path).returns('Users')
+    Puppet::Provider::NameService::DirectoryService.stubs(:list_all_present).returns(
+      ['root', 'user1', 'user2', 'resource_name']
+    )
+    Puppet::Provider::NameService::DirectoryService.stubs(:generate_attribute_hash)
+    Puppet::Provider::NameService::DirectoryService.stubs(:execute)
+    Puppet::Provider::NameService::DirectoryService.expects(:parse_dscl_plist_data)
+
+    Puppet::Provider::NameService::DirectoryService.single_report('resource_name')
+  end
+end
+
+describe 'DirectoryService.get_exec_preamble' do
+  it 'should fail on OS X < 10.4' do
+    Puppet::Provider::NameService::DirectoryService.stubs(:get_macosx_version_major).returns("10.3")
+
+    lambda {
+      Puppet::Provider::NameService::DirectoryService.get_exec_preamble('-list')
+    }.should raise_error(RuntimeError, "Puppet does not support OS X versions < 10.4")
+  end
+
+  it 'should use url data on 10.4' do
+    Puppet::Provider::NameService::DirectoryService.stubs(:get_macosx_version_major).returns("10.4")
+    Puppet::Provider::NameService::DirectoryService.stubs(:get_ds_path).returns('Users')
+
+    Puppet::Provider::NameService::DirectoryService.get_exec_preamble('-list').should include("-url")
+  end
+
+  it 'should use plist data on > 10.4' do
+    Puppet::Provider::NameService::DirectoryService.stubs(:get_macosx_version_major).returns("10.5")
+    Puppet::Provider::NameService::DirectoryService.stubs(:get_ds_path).returns('Users')
+
+    Puppet::Provider::NameService::DirectoryService.get_exec_preamble('-list').should include("-plist")
+  end
+end
diff --git a/spec/unit/rails/host_spec.rb b/spec/unit/rails/host_spec.rb
index df0b2fa1d..98541c0a8 100755
--- a/spec/unit/rails/host_spec.rb
+++ b/spec/unit/rails/host_spec.rb
@@ -1,6 +1,8 @@
 #!/usr/bin/env rspec
 require 'spec_helper'
 
+require 'puppet/node/environment'
+
 describe "Puppet::Rails::Host", :if => Puppet.features.rails? do
   def column(name, type)
     ActiveRecord::ConnectionAdapters::Column.new(name, nil, type, false)
@@ -42,6 +44,12 @@ describe "Puppet::Rails::Host", :if => Puppet.features.rails? do
       Puppet::Rails::Host.from_puppet(@node)
     end
 
+    it "should stringify the environment" do
+      host = Puppet::Rails::Host.new
+      host.environment = Puppet::Node::Environment.new("production")
+      host.environment.class.should == String
+    end
+
     it "should copy the ipaddress from the Puppet instance" do
       Puppet::Rails::Host.expects(:find_by_name).with("foo").returns @host
 
diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb
index 683c3654b..6cd3ab701 100755
--- a/spec/unit/type/file_spec.rb
+++ b/spec/unit/type/file_spec.rb
@@ -168,6 +168,25 @@ describe Puppet::Type.type(:file) do
         reqs[0].target.must == file
       end
 
+      it "should autorequire its nearest ancestor directory" do
+        file = Puppet::Type::File.new(:path => "/foo/bar/baz")
+        dir = Puppet::Type::File.new(:path => "/foo")
+        root = Puppet::Type::File.new(:path => "/")
+        @catalog.add_resource file
+        @catalog.add_resource dir
+        @catalog.add_resource root
+        reqs = file.autorequire
+        reqs.length.must == 1
+        reqs[0].source.must == dir
+        reqs[0].target.must == file
+      end
+
+      it "should not autorequire anything when there is no nearest ancestor directory" do
+        file = Puppet::Type::File.new(:path => "/foo/bar/baz")
+        @catalog.add_resource file
+        file.autorequire.should be_empty
+      end
+
       it "should not autorequire its parent dir if its parent dir is itself" do
         file = Puppet::Type::File.new(:path => "/")
         @catalog.add_resource file
@@ -241,6 +260,25 @@ describe Puppet::Type.type(:file) do
         reqs[0].target.must == file
       end
 
+      it "should autorequire its nearest ancestor directory" do
+        file = Puppet::Type::File.new(:path => "X:/foo/bar/baz")
+        dir = Puppet::Type::File.new(:path => "X:/foo")
+        root = Puppet::Type::File.new(:path => "X:/")
+        @catalog.add_resource file
+        @catalog.add_resource dir
+        @catalog.add_resource root
+        reqs = file.autorequire
+        reqs.length.must == 1
+        reqs[0].source.must == dir
+        reqs[0].target.must == file
+      end
+
+      it "should not autorequire anything when there is no nearest ancestor directory" do
+        file = Puppet::Type::File.new(:path => "X:/foo/bar/baz")
+        @catalog.add_resource file
+        file.autorequire.should be_empty
+      end
+
       it "should not autorequire its parent dir if its parent dir is itself" do
         file = Puppet::Type::File.new(:path => "X:/")
         @catalog.add_resource file
@@ -302,6 +340,25 @@ describe Puppet::Type.type(:file) do
         reqs[0].target.must == file
       end
 
+      it "should autorequire its nearest ancestor directory" do
+        file = Puppet::Type::File.new(:path => "//server/foo/bar/baz/qux")
+        dir = Puppet::Type::File.new(:path => "//server/foo/bar")
+        root = Puppet::Type::File.new(:path => "//server/foo")
+        @catalog.add_resource file
+        @catalog.add_resource dir
+        @catalog.add_resource root
+        reqs = file.autorequire
+        reqs.length.must == 1
+        reqs[0].source.must == dir
+        reqs[0].target.must == file
+      end
+
+      it "should not autorequire anything when there is no nearest ancestor directory" do
+        file = Puppet::Type::File.new(:path => "//server/foo/bar/baz/qux")
+        @catalog.add_resource file
+        file.autorequire.should be_empty
+      end
+
       it "should not autorequire its parent dir if its parent dir is itself" do
         file = Puppet::Type::File.new(:path => "//server/foo")
         @catalog.add_resource file