PATCH/puppet 1/3] conf/redhat: Rebase rundir-perms patch

---
 conf/redhat/client.init |    2 +-
 conf/redhat/puppet.conf |    2 +-
 conf/redhat/server.init |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/conf/redhat/client.init b/conf/redhat/client.init
index 0c98383..04e0d27 100644
--- a/conf/redhat/client.init
+++ b/conf/redhat/client.init
@@ -15,7 +15,7 @@ export PATH
 
 [ -f /etc/sysconfig/puppet ] && . /etc/sysconfig/puppet
 lockfile=${LOCKFILE-/var/lock/subsys/puppet}
-pidfile=${PIDFILE-/var/run/puppet/puppetd.pid}
+pidfile=${PIDFILE-/var/run/puppet/agent.pid}
 puppetd=${PUPPETD-/usr/sbin/puppetd}
 RETVAL=0
 
diff --git a/conf/redhat/puppet.conf b/conf/redhat/puppet.conf
index d35593f..47501a3 100644
--- a/conf/redhat/puppet.conf
+++ b/conf/redhat/puppet.conf
@@ -11,7 +11,7 @@
     # The default value is '$confdir/ssl'.
     ssldir = $vardir/ssl
 
-[puppetd]
+[agent]
     # The file in which puppetd stores a list of the classes
     # associated with the retrieved configuratiion.  Can be loaded in
     # the separate ``puppet`` executable using the ``--loadclasses``
diff --git a/conf/redhat/server.init b/conf/redhat/server.init
index 4f44206..3d2897d 100644
--- a/conf/redhat/server.init
+++ b/conf/redhat/server.init
@@ -13,7 +13,7 @@ PATH=/usr/bin:/sbin:/bin:/usr/sbin
 export PATH
 
 lockfile=/var/lock/subsys/puppetmaster
-pidfile=/var/run/puppet/puppetmasterd.pid
+pidfile=/var/run/puppet/master.pid
 
 # Source function library.
 . /etc/rc.d/init.d/functions

In Red Hat bug #531116 Ruben Kerkhof noted that we weren't using the
--pidfile $pidfile option to daemon.  This caused 'service puppet start'
to fail if puppet had already been started.  To be consistent, we now
pass the pidfile option to daemon, killproc, and status when those
functions provide such an option.  And we only test for the availability
of the -p/--pidfile option in one place.
---
 conf/redhat/client.init |   17 ++++++++++-------
 conf/redhat/server.init |   14 ++++++++++----
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/conf/redhat/client.init b/conf/redhat/client.init
index 04e0d27..f624d81 100644
--- a/conf/redhat/client.init
+++ b/conf/redhat/client.init
@@ -27,8 +27,12 @@ PUPPET_OPTS=""
 [ -n "$PUPPET_LOG" ] && PUPPET_OPTS="${PUPPET_OPTS}
--logdest=${PUPPET_LOG}"
 [ -n "$PUPPET_PORT" ] && PUPPET_OPTS="${PUPPET_OPTS}
--masterport=${PUPPET_PORT}"
 
-# Determine if we can use the -p option to killproc.  RHEL < 5 can't.
-killproc | grep -q -- '-p' 2>/dev/null && killopts="-p
$pidfile"
+# Determine if we can use the -p option to daemon, killproc, and status.
+# RHEL < 5 can't.
+if status | grep -q -- '-p' 2>/dev/null; then
+    daemonopts="--pidfile $pidfile"
+    pidopts="-p $pidfile"
+fi
 
 # Figure out if the system just booted. Let's assume
 # boot doesn't take longer than 5 minutes
@@ -37,7 +41,7 @@ killproc | grep -q -- '-p' 2>/dev/null &&
killopts="-p $pidfile"
 
 start() {
     echo -n $"Starting puppet: "
-    daemon $puppetd ${PUPPET_OPTS} ${PUPPET_EXTRA_OPTS}
+    daemon $daemonopts $puppetd ${PUPPET_OPTS} ${PUPPET_EXTRA_OPTS}
     RETVAL=$?
     echo
         [ $RETVAL = 0 ] && touch ${lockfile}
@@ -46,7 +50,7 @@ start() {
 
 stop() {
     echo -n $"Stopping puppet: "
-    killproc $killopts $puppetd
+    killproc $pidopts $puppetd
     RETVAL=$?
     echo
     [ $RETVAL = 0 ] && rm -f ${lockfile} ${pidfile}
@@ -54,7 +58,7 @@ stop() {
 
 reload() {
     echo -n $"Restarting puppet: "
-    killproc $killopts $puppetd -HUP
+    killproc $pidopts $puppetd -HUP
     RETVAL=$?
     echo
     return $RETVAL
@@ -66,8 +70,7 @@ restart() {
 }
 
 rh_status() {
-    status | grep -q -- '-p' 2>/dev/null && statusopts="-p
$pidfile"
-    status $statusopts $puppetd
+    status $pidopts $puppetd
     RETVAL=$?
     return $RETVAL
 }
diff --git a/conf/redhat/server.init b/conf/redhat/server.init
index 3d2897d..4d3a72c 100644
--- a/conf/redhat/server.init
+++ b/conf/redhat/server.init
@@ -33,6 +33,13 @@ fi
 PUPPETMASTER_OPTS="${PUPPETMASTER_OPTS} \
     ${PUPPETMASTER_EXTRA_OPTS}"
 
+# Determine if we can use the -p option to daemon, killproc, and status.
+# RHEL < 5 can't.
+if status | grep -q -- '-p' 2>/dev/null; then
+    daemonopts="--pidfile $pidfile"
+    pidopts="-p $pidfile"
+fi
+
 RETVAL=0
 
 prog=puppetmasterd
@@ -50,7 +57,7 @@ start() {
                 ret=$?; [ $ret != 0 ] && RETVAL=$ret
             done
         else
-            daemon $PUPPETMASTER $PUPPETMASTER_OPTS
+            daemon $daemonopts $PUPPETMASTER $PUPPETMASTER_OPTS
             RETVAL=$?
         fi
     else
@@ -72,8 +79,7 @@ stop() {
             ret=$?; [ $ret != 0 ] && RETVAL=$ret
         done
     else
-        killproc | grep -q -- '-p' 2>/dev/null && killopts="-p
$pidfile"
-        killproc $killopts $PUPPETMASTER
+        killproc $pidopts $PUPPETMASTER
         RETVAL=$?
     fi
     echo
@@ -99,7 +105,7 @@ rh_status() {
             ret=$?; [ $ret != 0 ] && RETVAL=$ret
         done
     else
-        status $PUPPETMASTER
+        status $pidopts $PUPPETMASTER
         RETVAL=$?
     fi
     return $RETVAL

This came up because if you ran puppetd with a specific vardir, then
when rundir got set to a hardcoded value its parent directory might not
exist and the whole thing would fail.

This change came about with the concept of run_mode, and this fix is
restoring the behaviour that was in 0.25.x

Reviewed-by: Jesse Wolfe
Signed-off-by: Matt Robinson <ma### @puppetlabs.com>
---
 lib/puppet/util/run_mode.rb     |   18 +++++++-------
 spec/unit/util/run_mode_spec.rb |   51
+++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 9 deletions(-)
 create mode 100644 spec/unit/util/run_mode_spec.rb

diff --git a/lib/puppet/util/run_mode.rb b/lib/puppet/util/run_mode.rb
index eb9c511..450cbf1 100644
--- a/lib/puppet/util/run_mode.rb
+++ b/lib/puppet/util/run_mode.rb
@@ -40,17 +40,17 @@ module Puppet
       end
 
       def run_dir
-        which_dir("/var/run/puppet", "~/.puppet/var")
+        "$vardir/run"
       end
 
       def logopts
-        if name == :master
+        if master?
           {
             :default => "$vardir/log",
-            :mode => 0750,
-            :owner => "service",
-            :group => "service",
-            :desc => "The Puppet log directory."
+            :mode    => 0750,
+            :owner   => "service",
+            :group   => "service",
+            :desc    => "The Puppet log directory."
           }
         else
           ["$vardir/log", "The Puppet log directory."]
@@ -64,9 +64,9 @@ module Puppet
         #       there's a comment that suggests that we do that
         #       and we currently don't.
         expand_path case
-        when name == :master; global
-        when Puppet.features.root?; global
-        else user
+          when name == :master; global
+          when Puppet.features.root?; global
+          else user
         end
       end
 
diff --git a/spec/unit/util/run_mode_spec.rb
b/spec/unit/util/run_mode_spec.rb
new file mode 100644
index 0000000..d6ab08a
--- /dev/null
+++ b/spec/unit/util/run_mode_spec.rb
@@ -0,0 +1,51 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+describe Puppet::Util::RunMode do
+  before do
+    @run_mode = Puppet::Util::RunMode.new('fake')
+  end
+
+  it "should have confdir /etc/puppet when run as root" do
+    Puppet.features.stubs(:root?).returns(true)
+    @run_mode.conf_dir.should == '/etc/puppet'
+  end
+
+  it "should have confdir ~/.puppet when run as non-root" do
+    Puppet.features.stubs(:root?).returns(false)
+   
@run_mode.expects(:expand_path).with("~/.puppet").returns("~/.puppet")
+    @run_mode.conf_dir.should == "~/.puppet"
+  end
+
+  it "should have vardir /var/lib/puppet when run as root" do
+    Puppet.features.stubs(:root?).returns(true)
+    @run_mode.var_dir.should == '/var/lib/puppet'
+  end
+
+  it "should have vardir ~/.puppet/var when run as non-root" do
+    Puppet.features.stubs(:root?).returns(false)
+   
@run_mode.expects(:expand_path).with("~/.puppet/var").returns("~/.puppet/var")
+    @run_mode.var_dir.should == "~/.puppet/var"
+  end
+
+  it "should have rundir depend on vardir" do
+    @run_mode.run_dir.should == '$vardir/run'
+  end
+
+  it "should have logopts return an array with $vardir/log if runmode is
not master" do
+    @run_mode.expects(:master?).returns false
+    @run_mode.logopts.should == ["$vardir/log", "The Puppet log
directory."]
+  end
+
+  it "should have logopts return a hash with $vardir/log and other
metadata if runmode is master" do
+    @run_mode.expects(:master?).returns true
+    @run_mode.logopts.should == {
+      :default => "$vardir/log",
+      :mode    => 0750,
+      :owner   => "service",
+      :group   => "service",
+      :desc    => "The Puppet log directory.",
+    }
+  end
+end

These specs 'use' some settings which create directories belonging
to the 'service' user/group. If the default service group doesn't
exist, these fail. This patch explicitly sets the service group to
the gid of the process, which is known to be accessible by the user.

Signed-off-by: Nick Lewis <ni### @puppetlabs.com>
---
 .../indirector/bucket_file/rest_spec.rb            |    1 +
 .../indirector/certificate/rest_spec.rb            |    1 +
 .../indirector/certificate_request/rest_spec.rb    |    1 +
 .../certificate_revocation_list/rest_spec.rb       |    1 +
 spec/integration/indirector/report/rest_spec.rb    |    1 +
 spec/integration/indirector/rest_spec.rb           |    1 +
 spec/integration/network/server/webrick_spec.rb    |    1 +
 spec/integration/ssl/certificate_authority_spec.rb |    1 +
 spec/integration/ssl/certificate_request_spec.rb   |    1 +
 .../ssl/certificate_revocation_list_spec.rb        |    1 +
 spec/integration/ssl/host_spec.rb                  |    1 +
 11 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/spec/integration/indirector/bucket_file/rest_spec.rb
b/spec/integration/indirector/bucket_file/rest_spec.rb
index 4d90a8c..dc10faa 100644
--- a/spec/integration/indirector/bucket_file/rest_spec.rb
+++ b/spec/integration/indirector/bucket_file/rest_spec.rb
@@ -17,6 +17,7 @@ describe "Filebucket REST Terminus" do
 
     Puppet.settings[:confdir] = @dir
     Puppet.settings[:vardir] = @dir
+    Puppet.settings[:group] = Process.gid
     Puppet.settings[:server] = "127.0.0.1"
     Puppet.settings[:masterport] = "34343"
 
diff --git a/spec/integration/indirector/certificate/rest_spec.rb
b/spec/integration/indirector/certificate/rest_spec.rb
index 356a7d3..58aa96c 100755
--- a/spec/integration/indirector/certificate/rest_spec.rb
+++ b/spec/integration/indirector/certificate/rest_spec.rb
@@ -17,6 +17,7 @@ describe "Certificate REST Terminus" do
 
     Puppet.settings[:confdir] = @dir
     Puppet.settings[:vardir] = @dir
+    Puppet.settings[:group] = Process.gid
     Puppet.settings[:server] = "127.0.0.1"
     Puppet.settings[:masterport] = "34343"
 
diff --git a/spec/integration/indirector/certificate_request/rest_spec.rb
b/spec/integration/indirector/certificate_request/rest_spec.rb
index 2c98ef6..c718b78 100755
--- a/spec/integration/indirector/certificate_request/rest_spec.rb
+++ b/spec/integration/indirector/certificate_request/rest_spec.rb
@@ -19,6 +19,7 @@ describe "Certificate Request REST Terminus" do
 
     Puppet.settings[:confdir] = @dir
     Puppet.settings[:vardir] = @dir
+    Puppet.settings[:group] = Process.gid
     Puppet.settings[:server] = "127.0.0.1"
     Puppet.settings[:masterport] = "34343"
 
diff --git
a/spec/integration/indirector/certificate_revocation_list/rest_spec.rb
b/spec/integration/indirector/certificate_revocation_list/rest_spec.rb
index 62a2f80..86f2b01 100755
--- a/spec/integration/indirector/certificate_revocation_list/rest_spec.rb
+++ b/spec/integration/indirector/certificate_revocation_list/rest_spec.rb
@@ -17,6 +17,7 @@ describe "Certificate REST Terminus" do
 
     Puppet.settings[:confdir] = @dir
     Puppet.settings[:vardir] = @dir
+    Puppet.settings[:group] = Process.gid
     Puppet.settings[:server] = "127.0.0.1"
     Puppet.settings[:masterport] = "34343"
 
diff --git a/spec/integration/indirector/report/rest_spec.rb
b/spec/integration/indirector/report/rest_spec.rb
index 089f8fd..fdc2189 100644
--- a/spec/integration/indirector/report/rest_spec.rb
+++ b/spec/integration/indirector/report/rest_spec.rb
@@ -17,6 +17,7 @@ describe "Report REST Terminus" do
 
     Puppet.settings[:confdir] = @dir
     Puppet.settings[:vardir] = @dir
+    Puppet.settings[:group] = Process.gid
     Puppet.settings[:server] = "127.0.0.1"
     Puppet.settings[:masterport] = "34343"
 
diff --git a/spec/integration/indirector/rest_spec.rb
b/spec/integration/indirector/rest_spec.rb
index e904839..14e9e95 100755
--- a/spec/integration/indirector/rest_spec.rb
+++ b/spec/integration/indirector/rest_spec.rb
@@ -39,6 +39,7 @@ describe Puppet::Indirector::REST do
 
     Puppet.settings[:confdir] = @dir
     Puppet.settings[:vardir] = @dir
+    Puppet.settings[:group] = Process.gid
     Puppet.settings[:server] = "127.0.0.1"
     Puppet.settings[:masterport] = "34343"
 
diff --git a/spec/integration/network/server/webrick_spec.rb
b/spec/integration/network/server/webrick_spec.rb
index 2809df7..2b14dfb 100755
--- a/spec/integration/network/server/webrick_spec.rb
+++ b/spec/integration/network/server/webrick_spec.rb
@@ -18,6 +18,7 @@ describe Puppet::Network::Server do
 
       Puppet.settings[:confdir] = @dir
       Puppet.settings[:vardir] = @dir
+      Puppet.settings[:group] = Process.gid
 
       Puppet::SSL::Host.ca_location = :local
 
diff --git a/spec/integration/ssl/certificate_authority_spec.rb
b/spec/integration/ssl/certificate_authority_spec.rb
index be82b5f..fca17b4 100755
--- a/spec/integration/ssl/certificate_authority_spec.rb
+++ b/spec/integration/ssl/certificate_authority_spec.rb
@@ -17,6 +17,7 @@ describe Puppet::SSL::CertificateAuthority do
 
     Puppet.settings[:confdir] = @dir
     Puppet.settings[:vardir] = @dir
+    Puppet.settings[:group] = Process.gid
 
     Puppet::SSL::Host.ca_location = :local
     @ca = Puppet::SSL::CertificateAuthority.new
diff --git a/spec/integration/ssl/certificate_request_spec.rb
b/spec/integration/ssl/certificate_request_spec.rb
index 365ecce..8426b9d 100755
--- a/spec/integration/ssl/certificate_request_spec.rb
+++ b/spec/integration/ssl/certificate_request_spec.rb
@@ -21,6 +21,7 @@ describe Puppet::SSL::CertificateRequest do
 
     Puppet.settings[:confdir] = @dir
     Puppet.settings[:vardir] = @dir
+    Puppet.settings[:group] = Process.gid
 
     Puppet::SSL::Host.ca_location = :none
 
diff --git a/spec/integration/ssl/certificate_revocation_list_spec.rb
b/spec/integration/ssl/certificate_revocation_list_spec.rb
index 127654c..44eee36 100755
--- a/spec/integration/ssl/certificate_revocation_list_spec.rb
+++ b/spec/integration/ssl/certificate_revocation_list_spec.rb
@@ -17,6 +17,7 @@ describe Puppet::SSL::CertificateRevocationList do
 
     Puppet.settings[:confdir] = @dir
     Puppet.settings[:vardir] = @dir
+    Puppet.settings[:group] = Process.gid
 
     Puppet::SSL::Host.ca_location = :local
   end
diff --git a/spec/integration/ssl/host_spec.rb
b/spec/integration/ssl/host_spec.rb
index 9b4152e..05862df 100755
--- a/spec/integration/ssl/host_spec.rb
+++ b/spec/integration/ssl/host_spec.rb
@@ -17,6 +17,7 @@ describe Puppet::SSL::Host do
 
     Puppet.settings[:confdir] = @dir
     Puppet.settings[:vardir] = @dir
+    Puppet.settings[:group] = Process.gid
 
     Puppet::SSL::Host.ca_location = :local
 

---
 install.rb |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/install.rb b/install.rb
index d35aaa0..b06ec09 100755
--- a/install.rb
+++ b/install.rb
@@ -84,7 +84,7 @@ bins  = glob(%w{bin/*})
 rdoc  = glob(%w{bin/* sbin/* lib/**/*.rb README README-library CHANGELOG
TODO Install}).reject { |e| e=~ /\.(bat|cmd)$/ }
 ri    = glob(%w{bin/*.rb sbin/* lib/**/*.rb}).reject { |e| e=~
/\.(bat|cmd)$/ }
 man   = glob(%w{man/man[0-9]/*})
-libs  = glob(%w{lib/**/*.rb lib/**/*.py})
+libs  = glob(%w{lib/**/*.rb lib/**/*.py lib/puppet/util/command_line/*})
 tests = glob(%w{test/**/*.rb})
 
 def do_bins(bins, target, strip = 's?bin/')

When the interface to Puppet::Resource changed, its subclass
Puppet::Parser::Resource was also affected. One case of initializing
those objects did not get updated when the code changed, causing
storeconfigs to break.

Also, this patch adds a error message that would have made it easier to
catch this problem (as puppet could consume all memory and die trying to
print the old error message)

Signed-off-by: Jesse Wolfe <jes5### @gmail.com>
---
 lib/puppet/rails/resource.rb     |    6 +++---
 lib/puppet/resource.rb           |    3 +++
 spec/unit/rails/resource_spec.rb |   16 ++++++++++++++++
 spec/unit/resource_spec.rb       |    6 ++++++
 4 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/rails/resource.rb b/lib/puppet/rails/resource.rb
index a5cdd0c..cac9de2 100644
--- a/lib/puppet/rails/resource.rb
+++ b/lib/puppet/rails/resource.rb
@@ -212,16 +212,16 @@ class Puppet::Rails::Resource <
ActiveRecord::Base
     end
     hash[:scope] = scope
     hash[:source] = scope.source
-    hash[:params] = []
+    hash[:parameters] = []
     names = []
     self.param_names.each do |pname|
       # We can get the same name multiple times because of how the
       # db layout works.
       next if names.include?(pname.name)
       names << pname.name
-      hash[:params] << pname.to_resourceparam(self, scope.source)
+      hash[:parameters] << pname.to_resourceparam(self,
scope.source)
     end
-    obj = Puppet::Parser::Resource.new(hash)
+    obj = Puppet::Parser::Resource.new(hash["type"], hash["title"], hash)
 
     # Store the ID, so we can check if we're re-collecting the same
resource.
     obj.rails_id = self.id
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index 31237e3..d68e0ee 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -409,6 +409,9 @@ class Puppet::Resource
     if    (argtitle || argtype) =~ /^([^\[\]]+)\[(.+)\]$/m then [ $1,    
            $2            ]
     elsif argtitle                                         then [
argtype,            argtitle      ]
     elsif argtype.is_a?(Puppet::Type)                      then [
argtype.class.name, argtype.title ]
+    elsif argtype.is_a?(Hash)                              then
+      raise ArgumentError, "Puppet::Resource.new does not take a hash as
the first argument. "+
+        "Did you mean (#{(argtype[:type] || argtype["type"]).inspect},
#{(argtype[:title] || argtype["title"]).inspect }) ?"
     else raise ArgumentError, "No title provided and #{argtype.inspect}
is not a valid resource reference"
     end
   end
diff --git a/spec/unit/rails/resource_spec.rb
b/spec/unit/rails/resource_spec.rb
index ac74693..08deda6 100755
--- a/spec/unit/rails/resource_spec.rb
+++ b/spec/unit/rails/resource_spec.rb
@@ -104,4 +104,20 @@ describe "Puppet::Rails::Resource" do
       @resource.merge_parameters(merge_resource)
     end
   end
+
+  describe "#to_resource" do
+    it "should instantiate a Puppet::Parser::Resource" do
+      scope = stub "scope", :source => nil
+
+      @resource = Puppet::Rails::Resource.new
+      @resource.stubs(:attributes).returns({
+        "restype" => 'notify',
+        "title"   => 'hello'
+      })
+      @resource.stubs(:param_names).returns([])
+
+      @resource.to_resource(scope).should be_a(Puppet::Parser::Resource)
+
+    end
+  end
 end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 95f0dd0..204a2b0 100755
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -98,6 +98,12 @@ describe Puppet::Resource do
     lambda { Puppet::Resource.new("foo") }.should
raise_error(ArgumentError)
   end
 
+  it "should fail if the title is a hash and the type is not a valid
resource reference string" do
+    lambda { Puppet::Resource.new({:type => "foo", :title =>
"bar"}) }.should raise_error(ArgumentError,
+      'Puppet::Resource.new does not take a hash as the first argument.
Did you mean ("foo", "bar") ?'
+    )
+  end
+
   it "should be able to produce a backward-compatible reference array" do
     Puppet::Resource.new("foobar", "/f").to_trans_ref.should == %w{Foobar
/f}
   end

The refactoring of using environment instances instead of strings
for initializing the parser, rdoc wasn't updated, thus was unable
to initialize the parser.

Signed-off-by: Brice Figureau <brice-pupp### @daysofwonder.com>
---
 lib/puppet/util/rdoc.rb     |    2 +-
 spec/unit/util/rdoc_spec.rb |   13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/lib/puppet/util/rdoc.rb b/lib/puppet/util/rdoc.rb
index 4a80b06..085d8ec 100644
--- a/lib/puppet/util/rdoc.rb
+++ b/lib/puppet/util/rdoc.rb
@@ -41,7 +41,7 @@ module Puppet::Util::RDoc
   def manifestdoc(files)
     Puppet[:ignoreimport] = true
     files.select { |f| FileTest.file?(f) }.each do |f|
-      parser = Puppet::Parser::Parser.new(:environment =>
Puppet[:environment])
+      parser =
Puppet::Parser::Parser.new(Puppet::Node::Environment.new(Puppet[:environment]))
       parser.file = f
       ast = parser.parse
       output(f, ast)
diff --git a/spec/unit/util/rdoc_spec.rb b/spec/unit/util/rdoc_spec.rb
index 65df261..58c2034 100755
--- a/spec/unit/util/rdoc_spec.rb
+++ b/spec/unit/util/rdoc_spec.rb
@@ -75,6 +75,19 @@ describe Puppet::Util::RDoc do
       Puppet::Util::RDoc.manifestdoc([])
     end
 
+    it "should use a parser with the correct environment" do
+      FileTest.stubs(:file?).returns(true)
+      Puppet::Util::RDoc.stubs(:output)
+
+      parser = stub_everything
+      Puppet::Parser::Parser.stubs(:new).with{ |env|
env.is_a?(Puppet::Node::Environment) }.returns(parser)
+
+      parser.expects(:file=).with("file")
+      parser.expects(:parse)
+
+      Puppet::Util::RDoc.manifestdoc(["file"])
+    end
+
     it "should puppet parse all given files" do
       FileTest.stubs(:file?).returns(true)
       Puppet::Util::RDoc.stubs(:output)

Hi,

Here is the first stab of JRuby Puppet compatibility.
There looks to be more thread issue (the last ones I found are
parser functions initializations), which will be addressed in
subsequent patches.

No patch in this serie have tests, because they most deal with
threading issues that can be reproduced only under JRuby.

Please review,
Brice

Brice Figureau (4):
  JRuby doesn't implement Process.maxgroups
  Fix #4244 - Cached Attributes is not thread safe
  Fix race condition in rack autoloading of request/response
  Fix #4245 - default insertion of ACL is not thread safe

 lib/puppet/network/http/rack.rb       |    3 +++
 lib/puppet/network/rest_authconfig.rb |    9 ++++++---
 lib/puppet/util/cacher.rb             |   31 +++++++++++++++++++++

This fix is based on refactoring Jesse's empirically derived solution
guided by
the assumption that #4297 was introduced / exposed by the fix for #4270; I
do
not have a global understanding of the code here, but am just making an
adjustment that 1) slightly reduces the code complexity, 2) doesn't appear
to
break anything, 3) empirically fixes the observed problem, 4) makes since
in
that resource classes shouldn't have to consult a passed-in scope to find
their
own namespaces.

Basically, I'm fixing what I believe was a thinko in a patch that I don't
fully
understand.

Signed-off-by: Markus Roberts <Mark### @reality.com>
---
 lib/puppet/resource/type.rb |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb
index 85c0979..c27a89a 100644
--- a/lib/puppet/resource/type.rb
+++ b/lib/puppet/resource/type.rb
@@ -179,7 +179,7 @@ class Puppet::Resource::Type
 
     unless @parent_type
       raise "Must pass scope to parent_type when called first time"
unless scope
-      unless @parent_type =
scope.environment.known_resource_types.send("find_#{type}",
scope.namespaces, parent)
+      unless @parent_type =
scope.environment.known_resource_types.send("find_#{type}", [names],
parent)
         fail Puppet::ParseError, "Could not find parent resource type
'#{parent}' of type #{type} in #{scope.environment}"
       end
     end

The run_mode value was incorrectly getting stored to Puppet[:mode], which
was confusing the optparser for applications that declare a --mode
parameter.

Signed-off-by: Jesse Wolfe <jes### @gmail.com>
---
 lib/puppet/application.rb |    2 +-
 lib/puppet/defaults.rb    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb
index 0a8fbc1..2fec38b 100644
--- a/lib/puppet/application.rb
+++ b/lib/puppet/application.rb
@@ -285,7 +285,7 @@ class Application
       Puppet.settings.set_value(:name, Puppet.application_name.to_s,
:mutable_defaults)
       Puppet.settings.set_value(:logdir, Puppet.run_mode.logopts,
:mutable_defaults)
       Puppet.settings.set_value(:rundir, Puppet.run_mode.run_dir,
:mutable_defaults)
-      Puppet.settings.set_value(:mode, Puppet.run_mode.name.to_s,
:mutable_defaults)
+      Puppet.settings.set_value(:run_mode, Puppet.run_mode.name.to_s,
:mutable_defaults)
     end
 
     require 'puppet'
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 84e2d93..0de5f20 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -7,7 +7,7 @@ module Puppet
     :vardir => [Puppet.run_mode.var_dir, "Where Puppet stores dynamic
and growing data.  The default for this parameter is calculated specially,
like `confdir`_."],
     :name => [Puppet.application_name.to_s, "The name of the
application, if we are running as one.  The
       default is essentially $0 without the path or ``.rb``."],
-    :mode => [Puppet.run_mode.name.to_s, "The effective 'run mode' of
the application: master, agent, or user."]
+    :run_mode => [Puppet.run_mode.name.to_s, "The effective 'run mode'
of the application: master, agent, or user."]
   )
 
   setdefaults(:main, :logdir => Puppet.run_mode.logopts)

hey,

There was some reports from joe-mac on irc that it's behaving
inconsistently on 2.6, giving different results between different runs.

I hacked at the code a bit to add a 4th option that will enable copious
amount of debug messages to be printed to hopefully help sort this out.

We should attempt to merge these I gues and ask someone who has 2.6 going
to test, maybe Joe has some time?

----- "Jesse A Wolfe" <jes5### @gmail.com> wrote:

 > Don't we have to move the extlookup code into
 > lib/puppet/parser/functions or something too? (I don't know the
 > answer, but it seems likely).
 
 I did!
 
 > rename from ext/extlookup.rb
 > rename to lib/puppet/parser/functions/
 extlookup.rb
 
 
 On Fri, Jul 23, 2010 at 2:35 PM, Markus Roberts <
 mar### @puppetlabs.com > wrote:
 
 
 Don't we have to move the extlookup code into
 lib/puppet/parser/functions or something too? (I don't know the
 answer, but it seems likely).
 
 -- Markus
 

This patch promotes extlookup() to being a builtin function.
It also adds test and makes some minor tweaks to the code.
The behavior of extlookup has been left unchanged.

Signed-off-by: Jesse Wolfe <jes### @gmail.com>
---
 {ext => lib/puppet/parser/functions}/extlookup.rb |   24 ++----
 spec/unit/parser/functions/extlookup_spec.rb      |   85
+++++++++++++++++++++
 2 files changed, 93 insertions(+), 16 deletions(-)
 rename {ext => lib/puppet/parser/functions}/extlookup.rb (92%)
 create mode 100755 spec/unit/parser/functions/extlookup_spec.rb

diff --git a/ext/extlookup.rb b/lib/puppet/parser/functions/extlookup.rb
similarity index 92%
rename from ext/extlookup.rb
rename to lib/puppet/parser/functions/extlookup.rb
index d87583b..ee230e7 100644
--- a/ext/extlookup.rb
+++ b/lib/puppet/parser/functions/extlookup.rb
@@ -82,11 +82,11 @@ require 'csv'
 module Puppet::Parser::Functions
   newfunction(:extlookup, :type => :rvalue) do |args|
     key = args[0]
-    default = "_ExtUNSET_"
-    datafile = "_ExtUNSET_"
 
-    default = args[1] if args[1]
-    datafile = args[2] if args[2]
+    default  = args[1]
+    datafile = args[2]
+
+    raise Puppet::ParseError, ("extlookup(): wrong number of arguments
(#{args.length}; must be <= 3)") if args.length > 3
 
     extlookup_datadir = lookupvar('extlookup_datadir')
     extlookup_precedence = Array.new
@@ -123,12 +123,13 @@ module Puppet::Parser::Functions
       datafiles << extlookup_datadir + "/#{d}.csv"
     end
 
-    desired = "_ExtUNSET_"
+    desired = nil
 
     datafiles.each do |file|
+      parser = Puppet::Parser::Parser.new(environment)
       parser.watch_file(file) if File.exists?(file)
 
-      if desired == "_ExtUNSET_"
+      if desired.nil?
         if File.exists?(file)
           result = CSV.read(file).find_all do |r|
             r[0] == key
@@ -167,15 +168,6 @@ module Puppet::Parser::Functions
       end
     end
 
-    # don't accidently return nil's and such rather throw a parse error
-    if desired == "_ExtUNSET_" && default == "_ExtUNSET_"
-      raise Puppet::ParseError, "No match found for '#{key}' in any data
file during extlookup()"
-    else
-      desired = default if desired == "_ExtUNSET_"
-    end
-
-    desired
+    desired || default or raise Puppet::ParseError, "No match found for
'#{key}' in any data file during extlookup()"
   end
 end
-
-# vi:tabstop=4:expandtab:ai
diff --git a/spec/unit/parser/functions/extlookup_spec.rb
b/spec/unit/parser/functions/extlookup_spec.rb
new file mode 100755
index 0000000..bf28803
--- /dev/null
+++ b/spec/unit/parser/functions/extlookup_spec.rb
@@ -0,0 +1,85 @@
+#! /usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+require 'tempfile'
+
+describe "the extlookup function" do
+
+  before :each do
+    @scope = Puppet::Parser::Scope.new
+    
+   
@scope.stubs(:environment).returns(Puppet::Node::Environment.new('production'))
+  end
+
+  it "should exist" do
+    Puppet::Parser::Functions.function("extlookup").should ==
"function_extlookup"
+  end
+
+  it "should raise a ParseError if there is less than 1 arguments" do
+    lambda { @scope.function_extlookup([]) }.should(
raise_error(Puppet::ParseError))
+  end
+
+  it "should raise a ParseError if there is more than 3 arguments" do
+    lambda { @scope.function_extlookup(["foo", "bar", "baz", "gazonk"])
}.should( raise_error(Puppet::ParseError))
+  end
+
+  it "should return the default" do
+    result = @scope.function_extlookup([ "key", "default"])
+    result.should == "default"
+  end
+
+  it "should lookup the key in a supplied datafile" do
+    t = Tempfile.new('extlookup.csv') do
+      t.puts 'key,value'
+      t.puts 'nonkey,nonvalue'
+      t.close
+
+      result = @scope.function_extlookup([ "key", "default", t.path])
+      result.should == "value"
+    end
+  end
+
+  it "should return an array if the datafile contains more than two
columns" do
+    t = Tempfile.new('extlookup.csv') do
+      t.puts 'key,value1,value2'
+      t.puts 'nonkey,nonvalue,nonvalue'
+      t.close
+
+      result = @scope.function_extlookup([ "key", "default", t.path])
+      result.should == ["value1", "value2"]
+    end
+  end
+
+  it "should raise an error if there's no matching key and no default" do
+    t = Tempfile.new('extlookup.csv') do
+      t.puts 'key,value'
+      t.puts 'nonkey,nonvalue'
+      t.close
+
+      result = @scope.function_extlookup([ "key", nil, t.path])
+      result.should == "value"
+    end
+  end
+
+  describe "should look in $extlookup_datadir for data files listed by
$extlookup_precedence" do
+    before do 
+      @scope.stubs(:lookupvar).with('extlookup_datadir').returns("/tmp")
+     
@scope.stubs(:lookupvar).with('extlookup_precedence').returns(["one","two"])
+      File.open("/tmp/one.csv","w"){|one| one.puts "key,value1" }
+      File.open("/tmp/two.csv","w") do |two|
+        two.puts "key,value2"
+        two.puts "key2,value_two"
+      end
+    end
+
+    it "when the key is in the first file" do
+      result = @scope.function_extlookup([ "key" ])
+      result.should == "value1"
+    end
+
+    it "when the key is in the second file" do
+      result = @scope.function_extlookup([ "key2" ])
+      result.should == "value_two"
+    end
+  end
+end

Signed-off-by: Marc Fournier <marc.fo### @camptocamp.com>
---
 ext/vim/syntax/puppet.vim |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/ext/vim/syntax/puppet.vim b/ext/vim/syntax/puppet.vim
index 80cd91c..81efa92 100644
--- a/ext/vim/syntax/puppet.vim
+++ b/ext/vim/syntax/puppet.vim
@@ -68,7 +68,7 @@ syn match   puppetNotVariable   "\\$\w\+" contained
 syn match   puppetNotVariable   "\\${\w\+}" contained
 
 syn keyword puppetKeyword       import inherits include
-syn keyword puppetControl       case default if else
+syn keyword puppetControl       case default if else elsif
 syn keyword puppetSpecial       true false undef
 
 " comments last overriding everything else

created init provider method self.get_services
  - accepts an array of filenames to exclude when retrieving services.

added unit tests to verify that I can specify files to exclude.

Signed-off-by: Dan Bode <d### @reductivelabs.com>
---
 lib/puppet/provider/service/init.rb       |    9 +++++--
 lib/puppet/provider/service/redhat.rb     |    5 ++++
 spec/unit/provider/service/init_spec.rb   |   35
+++++++++++++++++++++++++++++
 spec/unit/provider/service/redhat_spec.rb |   19 +++++++++++++++
 4 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/provider/service/init.rb
b/lib/puppet/provider/service/init.rb
index c05a326..58b7f91 100755
--- a/lib/puppet/provider/service/init.rb
+++ b/lib/puppet/provider/service/init.rb
@@ -28,11 +28,13 @@ Puppet::Type.type(:service).provide :init, :parent
=> :base do
 
     # List all services of this type.
     def self.instances
-        self.defpath = [self.defpath] unless self.defpath.is_a? Array
+        get_services(self.defpath)
+    end
 
+    def self.get_services(defpath, exclude=[])
+        defpath = [defpath] unless defpath.is_a? Array
         instances = []
-
-        self.defpath.each do |path|
+        defpath.each do |path|
             unless FileTest.directory?(path)
                 Puppet.debug "Service path %s does not exist" % path
                 next
@@ -47,6 +49,7 @@ Puppet::Type.type(:service).provide :init, :parent =>
:base do
             Dir.entries(path).each do |name|
                 fullpath = File.join(path, name)
                 next if name =~ /^\./
+                next if exclude.include? name 
                 next if not FileTest.executable?(fullpath)
                 instances << new(:name => name, :path =>
path)
             end
diff --git a/lib/puppet/provider/service/redhat.rb
b/lib/puppet/provider/service/redhat.rb
index 45a9074..065efe6 100755
--- a/lib/puppet/provider/service/redhat.rb
+++ b/lib/puppet/provider/service/redhat.rb
@@ -11,6 +11,11 @@ Puppet::Type.type(:service).provide :redhat, :parent
=> :init, :source => :init
 
     defaultfor :operatingsystem => [:redhat, :fedora, :suse, :centos,
:sles, :oel, :ovm]
 
+    def self.instances
+        # this exclude list is all from /sbin/service (5.x), but I did
not exclude kudzu
+        self.get_services(['/etc/init.d/'], ['functions', 'halt',
'killall', 'single', 'linuxconf'])
+    end
+
     def self.defpath
         superclass.defpath
     end
diff --git a/spec/unit/provider/service/init_spec.rb
b/spec/unit/provider/service/init_spec.rb
index 32bfaa2..cabd090 100755
--- a/spec/unit/provider/service/init_spec.rb
+++ b/spec/unit/provider/service/init_spec.rb
@@ -10,6 +10,7 @@ provider_class =
Puppet::Type.type(:service).provider(:init)
 describe provider_class do
 
     before :each do
+        @class = Puppet::Type.type(:service).provider(:init)
         @resource = stub 'resource'
         @resource.stubs(:[]).returns(nil)
         @resource.stubs(:[]).with(:name).returns "myservice"
@@ -22,6 +23,40 @@ describe provider_class do
         @provider.resource = @resource
     end
 
+    describe "when getting all service instances" do
+        before :each do 
+            @services = ['one', 'two', 'three', 'four']
+            Dir.stubs(:entries).returns @services
+            FileTest.stubs(:directory?).returns(true)
+            FileTest.stubs(:executable?).returns(true)
+            @class.stubs(:defpath).returns('tmp')
+        end
+        it "should return instances for all services" do
+            @services.each do |inst|
+                @class.expects(:new).with({:name => inst, :path =>
@class.defpath}).returns("#{inst}_instance")
+            end
+            results = @services.collect {|x| "#{x}_instance"}
+            @class.instances.should == results
+        end
+        it "should omit an array of services from exclude list" do
+            exclude = ['two', 'four']
+            (@services-exclude).each do |inst|
+                @class.expects(:new).with({:name => inst, :path =>
@class.defpath}).returns("#{inst}_instance")
+            end
+            results = (@services-exclude).collect {|x| "#{x}_instance"}
+            @class.get_services(@class.defpath, exclude).should ==
results
+        end
+        it "should omit a single service from the exclude list" do
+            exclude = 'two'
+            (@services-exclude.to_a).each do |inst|
+                @class.expects(:new).with({:name => inst, :path =>
@class.defpath}).returns("#{inst}_instance")
+            end
+            results = @services.reject{|x| x==exclude }.collect {|x|
"#{x}_instance"}
+            @class.get_services(@class.defpath, exclude).should ==
results
+        end
+        it "should use defpath" do
+        end
+    end
 
     describe "when searching for the init script" do
         it "should discard paths that do not exist" do
diff --git a/spec/unit/provider/service/redhat_spec.rb
b/spec/unit/provider/service/redhat_spec.rb
index 47997dc..840b192 100755
--- a/spec/unit/provider/service/redhat_spec.rb
+++ b/spec/unit/provider/service/redhat_spec.rb
@@ -10,6 +10,7 @@ provider_class =
Puppet::Type.type(:service).provider(:redhat)
 describe provider_class do
 
     before :each do
+        @class = Puppet::Type.type(:service).provider(:redhat)
         @resource = stub 'resource'
         @resource.stubs(:[]).returns(nil)
         @resource.stubs(:[]).with(:name).returns "myservice"
@@ -19,6 +20,24 @@ describe provider_class do
         FileTest.stubs(:file?).with('/sbin/service').returns true
         FileTest.stubs(:executable?).with('/sbin/service').returns true
     end
+    
+    # test self.instances
+    describe "when getting all service instances" do
+        before :each do 
+            @services = ['one', 'two', 'three', 'four', 'kudzu',
'functions', 'halt', 'killall', 'single', 'linuxconf']
+            @not_services = ['functions', 'halt', 'killall', 'single',
'linuxconf']
+            Dir.stubs(:entries).returns @services
+            FileTest.stubs(:directory?).returns(true)
+            FileTest.stubs(:executable?).returns(true)
+        end
+        it "should return instances for all services" do
+            (@services-@not_services).each do |inst|
+                @class.expects(:new).with({:name => inst, :path =>
'/etc/init.d/'}).returns("#{inst}_instance")
+            end
+            results = (@services-@not_services).collect {|x|
"#{x}_instance"}
+            @class.instances.should == results
+        end
+    end
 
     it "should have an enabled? method" do
         @provider.should respond_to(:enabled?)

These are a few minor updates to the conf/redhat files for 2.6.0.  The
last patch is something I've submitted previously but hasn't been
included yet.

Todd Zullinger (3):
  conf/redhat: Rebase rundir-perms patch
  conf/redhat: Update conf/init files for single binary
  conf/redhat: Consistently pass pidfile option to daemon, killproc,
    and status

 conf/redhat/client.init        |   19 +++++++++++--------
 conf/redhat/puppet.conf        |    2 +-
 conf/redhat/rundir-perms.patch |   26 +++++++++++++

Signed-off-by: Marc Fournier <marc.fo### @camptocamp.com>
---
 ext/vim/syntax/puppet.vim |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/ext/vim/syntax/puppet.vim b/ext/vim/syntax/puppet.vim
index 81efa92..b24cbf6 100644
--- a/ext/vim/syntax/puppet.vim
+++ b/ext/vim/syntax/puppet.vim
@@ -33,6 +33,7 @@ syn match   puppetNodeRe        "/.*/" contained
 "FIXME: "Foo-bar" doesn't get highlighted as expected, although "foo-bar"
does.
 syn match   puppetInstance     
"[A-Za-z0-9_-]\+\(::[A-Za-z0-9_-]\+\)*\s*{"
contains=puppetTypeName,puppetTypeDefault
 syn match   puppetInstance     
"[A-Z][a-z_-]\+\(::[A-Z][a-z_-]\+\)*\s*[[{]"
contains=puppetTypeName,puppetTypeDefault
+syn match   puppetInstance     
"[A-Z][a-z_-]\+\(::[A-Z][a-z_-]\+\)*\s*<\?<|"
contains=puppetTypeName,puppetTypeDefault
 syn match   puppetTypeName      "[a-z]\w*" contained
 syn match   puppetTypeDefault   "[A-Z]\w*" contained
 

Todd Zullinger wrote:
 These are a few minor updates to the conf/redhat files for 2.6.0. 
The
 last patch is something I've submitted previously but hasn't been
 included yet.
 
 Todd Zullinger (3):
   conf/redhat: Rebase rundir-perms patch
   conf/redhat: Update conf/init files for single binary
   conf/redhat: Consistently pass pidfile option to daemon, killproc,
     and status
 
  conf/redhat/client.init        |   19 +++++++++++--------
  conf/redhat/puppet.conf        |    2 +-
  conf/redhat/rundir-perms.patch |   26 +++++++++++++

This is a modification of the Nick/Jesse/Matt patch, retaining their tests
and
the analysis of the problem but reversing the implementation direction of
the
solution.

Rather than trying to make the already somewhat brittle slurpstring
smarter,
which requires telling it what following strings will be accepted by the
caller
with a zero-width-lookahead negation of the regular expression used to
extract
a variable name, this patch keeps that responsibility in the caller where
it
belongs.

The caller (tokenize_interpolated_string) now checks to see if it got a
variable name _before_ emitting a variable token; if it got one, it
proceeds
normally, but if it didn't it simply tries again from that point in the
string
(accumulating the false match as a prefix).  This change actually
simplifies
the logic of tokenize_interpolated_string somewhat.

Signed-off-by: Markus Roberts <Mar### @reality.com>
---
 lib/puppet/parser/lexer.rb     |   16 +++++++++-------
 spec/unit/parser/lexer_spec.rb |    8 +++++++-
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/lib/puppet/parser/lexer.rb b/lib/puppet/parser/lexer.rb
index 6a9f1cf..1e10ff9 100644
--- a/lib/puppet/parser/lexer.rb
+++ b/lib/puppet/parser/lexer.rb
@@ -540,15 +540,17 @@ class Puppet::Parser::Lexer
     [ str[0..-2],str[-1,1] ]
   end
 
-  def tokenize_interpolated_string(token_type)
+  def tokenize_interpolated_string(token_type,preamble='')
     value,terminator = slurpstring('"$')
-    token_queue << [TOKENS[token_type[terminator]],value]
-    while terminator == '$' and not @scanner.scan(/\{/)
-      token_queue <<
[TOKENS[:VARIABLE],@scanner.scan(%r{(\w*::)*\w+|[0-9]})]
-      value,terminator = slurpstring('"$')
-      token_queue <<
[TOKENS[DQ_continuation_token_types[terminator]],value]
+    token_queue << [TOKENS[token_type[terminator]],preamble+value]
+    if terminator != '$' or @scanner.scan(/\{/)
+      token_queue.shift 
+    elsif var_name = @scanner.scan(%r{(\w*::)*\w+|[0-9]})
+      token_queue << [TOKENS[:VARIABLE],var_name]
+      tokenize_interpolated_string(DQ_continuation_token_types)
+    else
+      tokenize_interpolated_string(token_type,token_queue.pop.last +
terminator)
     end
-    token_queue.shift
   end
 
   # just parse a string, not a whole file
diff --git a/spec/unit/parser/lexer_spec.rb
b/spec/unit/parser/lexer_spec.rb
index 81e76a3..d3d2a0a 100755
--- a/spec/unit/parser/lexer_spec.rb
+++ b/spec/unit/parser/lexer_spec.rb
@@ -426,7 +426,13 @@ describe Puppet::Parser::Lexer,"when lexing strings"
do
     %q{a hardest "scanner \"test\""}                                =>
[[:NAME,"a"],[:NAME,"hardest"],[:STRING,'scanner "test"']],
     %Q{a hardestest "scanner \\"test\\"\n"}                         =>
[[:NAME,"a"],[:NAME,"hardestest"],[:STRING,%Q{scanner "test"\n}]],
     %q{function("call")}                                            =>
[[:NAME,"function"],[:LPAREN,"("],[:STRING,'call'],[:RPAREN,")"]],
-    %q["string with ${(3+5)/4} nested math."]                       =>
[[:DQPRE,"string with
"],:LPAREN,[:NAME,"3"],:PLUS,[:NAME,"5"],:RPAREN,:DIV,[:NAME,"4"],[:DQPOST,"
nested math."]]
+    %q["string with ${(3+5)/4} nested math."]                       =>
[[:DQPRE,"string with
"],:LPAREN,[:NAME,"3"],:PLUS,[:NAME,"5"],:RPAREN,:DIV,[:NAME,"4"],[:DQPOST,"
nested math."]],
+    %q["$$$$"]                                                      =>
[[:STRING,"$$$$"]],
+    %q["$variable"]                                                 =>
[[:DQPRE,""],[:VARIABLE,"variable"],[:DQPOST,""]],
+    %q["$var$other"]                                                =>
[[:DQPRE,""],[:VARIABLE,"var"],[:DQMID,""],[:VARIABLE,"other"],[:DQPOST,""]],
+    %q["foo$bar$"]                                                  =>
[[:DQPRE,"foo"],[:VARIABLE,"bar"],[:DQPOST,"$"]],
+    %q["foo$$bar"]                                                  =>
[[:DQPRE,"foo$"],[:VARIABLE,"bar"],[:DQPOST,""]],
+    %q[""]                                                          =>
[[:STRING,""]],
   }.each { |src,expected_result|
     it "should handle #{src} correctly" do
       tokens_scanned_from(src).should be_like(*expected_result)

The comparisons operator (and more particularly == and !=) were not
treating
the undef value as '', like case and selector did since #2818.

This patch makes sure comparison operator uses AST leaf matching.

Unfortunately, doing this introduces a behavior change compared to
the previous versions:
Numbers embedded in strings will now be matched as numbers in case and
selector statements instead of string matching.

Signed-off-by: Brice Figureau <brice-pup### @daysofwonder.com>
---
 lib/puppet/parser/ast/comparison_operator.rb     |   13 ++--
 lib/puppet/parser/ast/leaf.rb                    |    3 +
 spec/unit/parser/ast/comparison_operator_spec.rb |   80
+++++++++++++++-------
 spec/unit/parser/ast/leaf_spec.rb                |   21 ++++++
 4 files changed, 87 insertions(+), 30 deletions(-)

diff --git a/lib/puppet/parser/ast/comparison_operator.rb
b/lib/puppet/parser/ast/comparison_operator.rb
index c8694bb..bcb4671 100644
--- a/lib/puppet/parser/ast/comparison_operator.rb
+++ b/lib/puppet/parser/ast/comparison_operator.rb
@@ -16,17 +16,18 @@ class Puppet::Parser::AST
     def evaluate(scope)
       # evaluate the operands, should return a boolean value
       lval = @lval.safeevaluate(scope)
-      rval = @rval.safeevaluate(scope)
 
       # convert to number if operands are number
       lval = Puppet::Parser::Scope.number?(lval) || lval
-      rval = Puppet::Parser::Scope.number?(rval) || rval
 
-      # return result
-      unless @operator == '!='
-        lval.send(@operator,rval)
+      case @operator
+      when "==","!="
+        @rval.evaluate_match(lval, scope) ? @operator == '==' : @operator
== '!='
       else
-        lval != rval
+        rval = @rval.safeevaluate(scope)
+        rval = Puppet::Parser::Scope.number?(rval) || rval
+
+        lval.send(@operator,rval)
       end
     end
 
diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb
index 3b9163d..49f4302 100644
--- a/lib/puppet/parser/ast/leaf.rb
+++ b/lib/puppet/parser/ast/leaf.rb
@@ -17,6 +17,9 @@ class Puppet::Parser::AST
       obj   = obj.downcase   if obj.respond_to?(:downcase)
       value = value.downcase if value.respond_to?(:downcase)
 
+      obj   = Puppet::Parser::Scope.number?(obj)   || obj
+      value = Puppet::Parser::Scope.number?(value) || value
+
       # "" == undef for case/selector/if
       obj == value or (obj == "" and value == :undef)
     end
diff --git a/spec/unit/parser/ast/comparison_operator_spec.rb
b/spec/unit/parser/ast/comparison_operator_spec.rb
index 724b6c6..6d6c377 100755
--- a/spec/unit/parser/ast/comparison_operator_spec.rb
+++ b/spec/unit/parser/ast/comparison_operator_spec.rb
@@ -5,21 +5,22 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 describe Puppet::Parser::AST::ComparisonOperator do
   before :each do
     @scope = Puppet::Parser::Scope.new
-    @one = stub 'one', :safeevaluate => "1"
-    @two = stub 'two', :safeevaluate => "2"
+    @one = Puppet::Parser::AST::Leaf.new(:value => "1")
+    @two = Puppet::Parser::AST::Leaf.new(:value => "2")
+
+    @lval = Puppet::Parser::AST::Leaf.new(:value => "one")
+    @rval = Puppet::Parser::AST::Leaf.new(:value => "two")
   end
 
-  it "should evaluate both branches" do
-    lval = stub "lval"
-    lval.expects(:safeevaluate).with(@scope)
-    rval = stub "rval"
-    rval.expects(:safeevaluate).with(@scope)
+  it "should evaluate both values" do
+    @lval.expects(:safeevaluate).with(@scope)
+    @rval.expects(:safeevaluate).with(@scope)
 
-    operator = Puppet::Parser::AST::ComparisonOperator.new :lval =>
lval, :operator => "==", :rval => rval
+    operator = Puppet::Parser::AST::ComparisonOperator.new :lval =>
@lval, :operator => "==", :rval => @rval
     operator.evaluate(@scope)
   end
 
-  it "should convert arguments strings to numbers if they are" do
+  it "should convert the arguments to numbers if they are numbers in
string" do
     Puppet::Parser::Scope.expects(:number?).with("1").returns(1)
     Puppet::Parser::Scope.expects(:number?).with("2").returns(2)
 
@@ -27,25 +28,56 @@ describe Puppet::Parser::AST::ComparisonOperator do
     operator.evaluate(@scope)
   end
 
-  %w{< > <= >= ==}.each do |oper|
+  %w{< > <= >=}.each do |oper|
     it "should use string comparison #{oper} if operands are strings" do
-      lval = stub 'one', :safeevaluate => "one"
-      rval = stub 'two', :safeevaluate => "two"
-      Puppet::Parser::Scope.stubs(:number?).with("one").returns(nil)
-      Puppet::Parser::Scope.stubs(:number?).with("two").returns(nil)
-
-      operator = Puppet::Parser::AST::ComparisonOperator.new :lval =>
lval, :operator => oper, :rval => rval
+      operator = Puppet::Parser::AST::ComparisonOperator.new :lval =>
@lval, :operator => oper, :rval => @rval
       operator.evaluate(@scope).should == "one".send(oper,"two")
     end
   end
 
-  it "should fail with arguments of different types" do
-    lval = stub 'one', :safeevaluate => "one"
-    rval = stub 'two', :safeevaluate => "2"
-    Puppet::Parser::Scope.stubs(:number?).with("one").returns(nil)
-    Puppet::Parser::Scope.stubs(:number?).with("2").returns(2)
+  describe "with string comparison" do
+    it "should use matching" do
+      @rval.expects(:evaluate_match).with("one", @scope)
+
+      operator = Puppet::Parser::AST::ComparisonOperator.new :lval =>
@lval, :operator => "==", :rval => @rval
+      operator.evaluate(@scope)
+    end
+
+    it "should return true for :undef to '' equality" do
+      astundef = Puppet::Parser::AST::Leaf.new(:value => :undef)
+      empty = Puppet::Parser::AST::Leaf.new(:value => '')
+
+      operator = Puppet::Parser::AST::ComparisonOperator.new :lval =>
astundef, :operator => "==", :rval => empty
+      operator.evaluate(@scope)
+    end
+
+    [true, false].each do |result|
+      it "should return #{(result).inspect} with '==' when matching
return #{result.inspect}" do
+        @rval.expects(:evaluate_match).with("one", @scope).returns result
 
-    operator = Puppet::Parser::AST::ComparisonOperator.new :lval =>
lval, :operator => ">", :rval => rval
+        operator = Puppet::Parser::AST::ComparisonOperator.new :lval
=> @lval, :operator => "==", :rval => @rval
+        operator.evaluate(@scope).should == result
+      end
+
+      it "should return #{(!result).inspect} with '!=' when matching
return #{result.inspect}" do
+        @rval.expects(:evaluate_match).with("one", @scope).returns result
+
+        operator = Puppet::Parser::AST::ComparisonOperator.new :lval
=> @lval, :operator => "!=", :rval => @rval
+        operator.evaluate(@scope).should == !result
+      end
+    end
+  end
+
+
+  it "should return matching result" do
+    @rval.expects(:evaluate_match).with("one", @scope)
+
+    operator = Puppet::Parser::AST::ComparisonOperator.new :lval =>
@lval, :operator => "==", :rval => @rval
+    operator.evaluate(@scope)
+  end
+
+  it "should fail with arguments of different types" do
+    operator = Puppet::Parser::AST::ComparisonOperator.new :lval =>
@one, :operator => ">", :rval => @rval
     lambda { operator.evaluate(@scope) }.should
raise_error(ArgumentError)
   end
 
@@ -81,8 +113,8 @@ describe Puppet::Parser::AST::ComparisonOperator do
   # see ticket #1759
   %w{< > <= >=}.each do |oper|
     it "should return the correct result of using '#{oper}' to compare 10
and 9" do
-      ten = stub 'one', :safeevaluate => "10"
-      nine = stub 'two', :safeevaluate => "9"
+      ten = Puppet::Parser::AST::Leaf.new(:value => "10")
+      nine = Puppet::Parser::AST::Leaf.new(:value => "9")
       operator = Puppet::Parser::AST::ComparisonOperator.new :lval =>
ten, :operator => oper, :rval => nine
 
       operator.evaluate(@scope).should == 10.send(oper,9)
diff --git a/spec/unit/parser/ast/leaf_spec.rb
b/spec/unit/parser/ast/leaf_spec.rb
index 379cbfd..d21cbf5 100755
--- a/spec/unit/parser/ast/leaf_spec.rb
+++ b/spec/unit/parser/ast/leaf_spec.rb
@@ -35,6 +35,27 @@ describe Puppet::Parser::AST::Leaf do
       @leaf.evaluate_match("value", @scope)
     end
 
+    it "should convert values to number" do
+      @leaf.stubs(:safeevaluate).with(@scope).returns(@value)
+      Puppet::Parser::Scope.expects(:number?).with(@value).returns(2)
+      Puppet::Parser::Scope.expects(:number?).with("23").returns(23)
+
+      @leaf.evaluate_match("23", @scope)
+    end
+
+    it "should compare 'numberized' values" do
+      @leaf.stubs(:safeevaluate).with(@scope).returns(@value)
+      two = stub_everything 'two'
+      one = stub_everything 'one'
+
+      Puppet::Parser::Scope.stubs(:number?).with(@value).returns(one)
+      Puppet::Parser::Scope.stubs(:number?).with("2").returns(two)
+
+      one.expects(:==).with(two)
+
+      @leaf.evaluate_match("2", @scope)
+    end
+
     it "should match undef if value is an empty string" do
       @leaf.stubs(:safeevaluate).with(@scope).returns("")
 

The log will now queue any log messages created when there is no
destination, and will flush the queue when a destination is added.

Signed-off-by: Nick Lewis <nic### @puppetlabs.com>
---
 lib/puppet/util/log.rb         |   17 +++++++++++++++++
 spec/unit/util/log_spec.rb     |    6 ++++++
 spec/unit/util/logging_spec.rb |    6 ++++++
 3 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb
index ba23e12..64a8d14 100644
--- a/lib/puppet/util/log.rb
+++ b/lib/puppet/util/log.rb
@@ -31,6 +31,8 @@ class Puppet::Util::Log
 
     @destinations = {}
 
+    @queued = []
+
     class << self
         include Puppet::Util
         include Puppet::Util::ClassGen
@@ -145,6 +147,7 @@ class Puppet::Util::Log
             else
                 @destinations[dest] = type.new()
             end
+            flushqueue
         rescue => detail
             if Puppet[:debug]
                 puts detail.backtrace
@@ -167,6 +170,8 @@ class Puppet::Util::Log
             return
         end
 
+        queuemessage(msg) if @destinations.count == 0
+
         @destinations.each do |name, dest|
             threadlock(dest) do
                 dest.handle(msg)
@@ -174,6 +179,18 @@ class Puppet::Util::Log
         end
     end
 
+    def Log.queuemessage(msg)
+        @queued.push(msg)
+    end
+
+    def Log.flushqueue
+        return unless @destinations.size >= 1
+        @queued.each do |msg|
+            Log.newmessage(msg)
+        end
+        @queued.clear
+    end
+
     def Log.sendlevel?(level)
         @levels.index(level) >= @loglevel
     end
diff --git a/spec/unit/util/log_spec.rb b/spec/unit/util/log_spec.rb
index 7aaa580..c72078f 100755
--- a/spec/unit/util/log_spec.rb
+++ b/spec/unit/util/log_spec.rb
@@ -84,6 +84,12 @@ describe Puppet::Util::Log do
             Puppet::Util::Log.new(:level => :notice, :message =>
:foo).message.should == "foo"
         end
 
+        it "should write queued logs when the first destination is
specified" do
+            Hash.any_instance.stubs(:[]=)
+            Puppet::Util::Log.expects(:flushqueue)
+            Puppet::Util::Log.newdestination(:console)
+        end
+
         it "should convert the level to a symbol if it's passed in as a
string" do
             Puppet::Util::Log.new(:level => "notice", :message =>
:foo).level.should == :notice
         end
diff --git a/spec/unit/util/logging_spec.rb
b/spec/unit/util/logging_spec.rb
index aee308e..41b07d4 100755
--- a/spec/unit/util/logging_spec.rb
+++ b/spec/unit/util/logging_spec.rb
@@ -40,6 +40,12 @@ describe Puppet::Util::Logging do
             @logger.notice "foo"
         end
 
+        it "should queue logs sent without a specified destination" do
+            Puppet::Util::Log.expects(:queuemessage)
+
+            @logger.notice "foo"
+        end
+
         it "should use the path of any provided resource type" do
             resource = Puppet::Type.type(:mount).new :name => "foo"