Merge branch '2.6.x' into next

* 2.6.x:
  (maint) Indentation fixes
  (#6490) Add plugin initialization callback system to core
  Fix #4339 - Locally save the last report to $lastrunreport
  Fix #4339 - Save a last run report summary to $statedir/last_run_summary.yaml
  Fixed #3127 - removed legacy debug code
  Fixed #3127 - Fixed gem selection regex
  (#5437) Invalidate cached TypeCollection when there was an error parsing
  (#6937) Adjust formatting of recurse's desc
  (#6937) Document the recurse parameter of File type.
  (#6893) Document the cron type in the case of specials.
  (#5670) Don't trigger refresh from a failed resource
  Fixed #6554 - Missing $haveftool if/else conditional in install.rb breaking Ruby 1.9

Conflicts (Manually resolved):
	lib/puppet/application/agent.rb
	lib/puppet/application/apply.rb
	lib/puppet/configurer.rb
	lib/puppet/resource/type_collection.rb
	lib/puppet/type/file.rb
	spec/integration/configurer_spec.rb
	spec/unit/application/agent_spec.rb
	spec/unit/application/apply_spec.rb
	spec/unit/configurer_spec.rb
	spec/unit/indirector/report/yaml_spec.rb
	spec/unit/resource/type_collection_spec.rb

Paired-with: Nick Lewis

15 files changed, 219 insertions, 85 deletions

diff --git a/install.rb b/install.rb
index 6854363ca..6bf4f5587 100755
--- a/install.rb
+++ b/install.rb
@@ -80,8 +80,12 @@ def do_configs(configs, target, strip = 'conf/')
   Dir.mkdir(target) unless File.directory? target
   configs.each do |cf|
     ocf = File.join(InstallOptions.config_dir, cf.gsub(/#{strip}/, ''))
-    File.install(cf, ocf, 0644, true)
-  end
+    if $haveftools
+      File.install(cf, ocf, 0644, true)
+    else
+      FileUtils.install(cf, ocf, {:mode => 0644, :verbose => true})
+    end
+   end
 end
 
 def do_bins(bins, target, strip = 's?bin/')
diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb
index 7ef71bc81..57bd88877 100644
--- a/lib/puppet/application.rb
+++ b/lib/puppet/application.rb
@@ -1,4 +1,5 @@
 require 'optparse'
+require 'puppet/util/plugins'
 
 # This class handles all the aspects of a Puppet application/executable
 # * setting up options
@@ -298,11 +299,11 @@ class Application
 
   # This is the main application entry point
   def run
-    exit_on_fail("initialize") { preinit }
-    exit_on_fail("parse options") { parse_options }
+    exit_on_fail("initialize")               { hook('preinit')       { preinit } }
+    exit_on_fail("parse options")            { hook('parse_options') { parse_options } }
     exit_on_fail("parse configuration file") { Puppet.settings.parse } if should_parse_config?
-    exit_on_fail("prepare for execution") { setup }
-    exit_on_fail("run") { run_command }
+    exit_on_fail("prepare for execution")    { hook('setup')         { setup } }
+    exit_on_fail("run")                      { hook('run_command')   { run_command } }
   end
 
   def main
@@ -392,11 +393,18 @@ class Application
   private
 
   def exit_on_fail(message, code = 1)
-      yield
+    yield
   rescue RuntimeError, NotImplementedError => detail
-      puts detail.backtrace if Puppet[:trace]
-      $stderr.puts "Could not #{message}: #{detail}"
-      exit(code)
+    puts detail.backtrace if Puppet[:trace]
+    $stderr.puts "Could not #{message}: #{detail}"
+    exit(code)
+  end
+
+  def hook(step,&block)
+    Puppet::Plugins.send("before_application_#{step}",:application_object => self)
+    x = yield
+    Puppet::Plugins.send("after_application_#{step}",:application_object => self, :return_value => x)
+    x
   end
 end
 end
diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index 72e387c64..cfeb73a1f 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -173,9 +173,7 @@ class Puppet::Configurer
     report.finalize_report if trans
     puts report.summary if Puppet[:summarize]
     save_last_run_summary(report)
-    if Puppet[:report]
-      Puppet::Transaction::Report.indirection.save(report)
-    end
+    Puppet::Transaction::Report.indirection.save(report) if Puppet[:report]
   rescue => detail
     puts detail.backtrace if Puppet[:trace]
     Puppet.err "Could not send report: #{detail}"
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index d50530618..dc631979e 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -79,7 +79,7 @@ class Puppet::Node::Environment
     # environment has changed do we delve deeper.
     Thread.current[:known_resource_types] = nil if (krt = Thread.current[:known_resource_types]) && krt.environment != self
     Thread.current[:known_resource_types] ||= synchronize {
-      if @known_resource_types.nil? or @known_resource_types.stale?
+      if @known_resource_types.nil? or @known_resource_types.require_reparse?
         @known_resource_types = Puppet::Resource::TypeCollection.new(self)
         @known_resource_types.import_ast(perform_initial_import, '')
       end
@@ -160,6 +160,8 @@ class Puppet::Node::Environment
     end
     parser.parse
   rescue => detail
+    known_resource_types.parse_failed = true
+
     msg = "Could not parse for environment #{self}: #{detail}"
     error = Puppet::Error.new(msg)
     error.set_backtrace(detail.backtrace)
diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb
index 19414cec4..28731c849 100755
--- a/lib/puppet/provider/package/gem.rb
+++ b/lib/puppet/provider/package/gem.rb
@@ -22,7 +22,7 @@ Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package d
     end
 
     if name = hash[:justme]
-      command << name
+      command << name + "$"
     end
 
     begin
@@ -94,7 +94,7 @@ Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package d
         command << "--source" << "#{source}" << resource[:name]
       end
     else
-      command << resource[:name]
+      command << "--no-rdoc" << "--no-ri" << resource[:name]
     end
 
     output = execute(command)
diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb
index 6ab978f7c..9fe7cdd06 100644
--- a/lib/puppet/resource/type_collection.rb
+++ b/lib/puppet/resource/type_collection.rb
@@ -1,5 +1,6 @@
 class Puppet::Resource::TypeCollection
   attr_reader :environment
+  attr_accessor :parse_failed
 
   def clear
     @hostclasses.clear
@@ -120,6 +121,10 @@ class Puppet::Resource::TypeCollection
     end
   end
 
+  def require_reparse?
+    @parse_failed || stale?
+  end
+
   def stale?
     @watched_files.values.detect { |file| file.changed? }
   end
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 8118178c3..0533273d9 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -48,7 +48,7 @@ class Puppet::Transaction
   def apply(resource, ancestor = nil)
     status = resource_harness.evaluate(resource)
     add_resource_status(status)
-    event_manager.queue_events(ancestor || resource, status.events)
+    event_manager.queue_events(ancestor || resource, status.events) unless status.failed?
   rescue => detail
     resource.err "Could not evaluate: #{detail}"
   end
diff --git a/lib/puppet/type/cron.rb b/lib/puppet/type/cron.rb
index 4b18e71f9..4f6ea733c 100755
--- a/lib/puppet/type/cron.rb
+++ b/lib/puppet/type/cron.rb
@@ -226,7 +226,9 @@ Puppet::Type.newtype(:cron) do
   end
 
   newproperty(:special) do
-    desc "Special schedules"
+    desc "A special value such as 'reboot' or 'annually'.
+       Only available on supported systems such as Vixie Cron.
+       Overrides more specific time of day/week settings."
 
     def specials
       %w{reboot yearly annually monthly weekly daily midnight hourly}
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 5632d41f1..715836b22 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -123,15 +123,16 @@ Puppet::Type.newtype(:file) do
   newparam(:recurse) do
     desc "Whether and how deeply to do recursive
       management. Options are:
-        inf,true  => Regular style recursion on both remote and local
-                     directory structure.
-        remote    => Descends recursively into the remote directory
-                     but not the local directory. Allows copying of
-                     a few files into a directory containing many
-                     unmanaged files without scanning all the local files.
-        false     => Default of no-recursion.
-        [0-9]+    => Both, but limit recursion. Warning: this syntax
-                     is deprecated and has moved to recurselimit.
+
+      * `inf,true` --- Regular style recursion on both remote and local
+        directory structure.
+      * `remote` --- Descends recursively into the remote directory
+        but not the local directory. Allows copying of
+        a few files into a directory containing many
+        unmanaged files without scanning all the local files.
+      * `false` --- Default of no recursion.
+      * `[0-9]+` --- Same as true, but limit recursion. Warning: this syntax
+        has been deprecated in favor of the `recurselimit` attribute.
     "
 
     newvalues(:true, :false, :inf, :remote, /^[0-9]+$/)
diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb
index 7f74d266a..52b5f81ef 100644
--- a/lib/puppet/util/command_line.rb
+++ b/lib/puppet/util/command_line.rb
@@ -1,9 +1,10 @@
+require "puppet/util/plugins"
+
 module Puppet
   module Util
     class CommandLine
 
-            LegacyName = Hash.new{|h,k| k}.update(
-        {
+      LegacyName = Hash.new{|h,k| k}.update(
         'agent'      => 'puppetd',
         'cert'       => 'puppetca',
         'doc'        => 'puppetdoc',
@@ -13,9 +14,8 @@ module Puppet
         'queue'      => 'puppetqd',
         'resource'   => 'ralsh',
         'kick'       => 'puppetrun',
-        'master'     => 'puppetmasterd',
-        
-      })
+        'master'     => 'puppetmasterd'
+      )
 
       def initialize( zero = $0, argv = ARGV, stdin = STDIN )
         @zero  = zero
@@ -23,6 +23,7 @@ module Puppet
         @stdin = stdin
 
         @subcommand_name, @args = subcommand_and_args( @zero, @argv, @stdin )
+        Puppet::Plugins.on_commandline_initialization(:command_line_object => self)
       end
 
       attr :subcommand_name
@@ -56,21 +57,23 @@ module Puppet
           puts usage_message
         elsif available_subcommands.include?(subcommand_name) #subcommand
           require_application subcommand_name
-          Puppet::Application.find(subcommand_name).new(self).run
+          app = Puppet::Application.find(subcommand_name).new(self)
+          Puppet::Plugins.on_application_initialization(:appliation_object => self)
+          app.run
         else
           abort "Error: Unknown command #{subcommand_name}.\n#{usage_message}" unless execute_external_subcommand
         end
       end
 
       def execute_external_subcommand
-          external_command = "puppet-#{subcommand_name}"
+        external_command = "puppet-#{subcommand_name}"
 
-          require 'puppet/util'
-          path_to_subcommand = Puppet::Util.which( external_command )
-          return false unless path_to_subcommand
+        require 'puppet/util'
+        path_to_subcommand = Puppet::Util.which( external_command )
+        return false unless path_to_subcommand
 
-          system( path_to_subcommand, *args )
-          true
+        system( path_to_subcommand, *args )
+        true
       end
 
       def legacy_executable_name
diff --git a/lib/puppet/util/plugins.rb b/lib/puppet/util/plugins.rb
new file mode 100644
index 000000000..105fdcd75
--- /dev/null
+++ b/lib/puppet/util/plugins.rb
@@ -0,0 +1,82 @@
+#
+# This system manages an extensible set of metadata about plugins which it
+#   collects by searching for files named "plugin_init.rb" in a series of
+#   directories.  Initially, these are simply the $LOAD_PATH.
+#
+# The contents of each file found is executed in the context of a Puppet::Plugins
+#    object (and thus scoped).  An example file might contain:
+#
+# -------------------------------------------------------
+#      @name = "Greet the CA"
+#
+#      @description = %q{
+#        This plugin causes a friendly greeting to print out on a master
+#        that is operating as the CA, after it has been set up but before
+#        it does anything.
+#      }
+#
+#      def after_application_setup(options)
+#        if options[:application_object].is_a?(Puppet::Application::Master) && Puppet::SSL::CertificateAuthority.ca?
+#          puts "Hey, this is the CA!"
+#        end
+#      end
+# -------------------------------------------------------
+#
+# Note that the instance variables are local to this Puppet::Plugin (and so may be used
+#   for maintaining state, etc.) but the plugin system does not provide any thread safety
+#   assurances, so they may not be adequate for some complex use cases.
+#
+#
+module Puppet
+  class Plugins
+    Paths  = [] # Where we might find plugin initialization code
+    Loaded = [] # Code we have found (one-to-one with paths once searched)
+    #
+    # Return all the Puppet::Plugins we know about, searching any new paths
+    #
+    def self.known
+      Paths[Loaded.length...Paths.length].each { |path|
+        file = File.join(path,'plugin_init.rb')
+        Loaded << (File.exist?(file) && new(file))
+      }
+      Loaded.compact
+    end
+    #
+    # Add more places to look for plugins without adding duplicates or changing the
+    #   order of ones we've already found.
+    #   
+    def self.look_in(*paths)
+      Paths.replace Paths | paths.flatten.collect { |path| File.expand_path(path) }
+    end
+    #
+    # Initially just look in $LOAD_PATH
+    #
+    look_in $LOAD_PATH
+    #
+    # Calling methods (hooks) on the class calls the method of the same name on 
+    #   all plugins that use that hook, passing in the same arguments to each
+    #   and returning an array containing the results returned by each plugin as
+    #   an array of [plugin_name,result] pairs.
+    # 
+    def self.method_missing(hook,*args,&block)
+      known.
+        select  { |p| p.respond_to? hook }.
+        collect { |p| [p.name,p.send(hook,*args,&block)] }
+    end
+    #
+    # 
+    #
+    attr_reader :path,:name
+    def initialize(path)
+      @name = @path = path
+      class << self
+        private
+        def define_hooks
+          eval File.read(path),nil,path,1
+        end
+      end
+      define_hooks
+    end
+  end
+end
+
diff --git a/spec/integration/transaction_spec.rb b/spec/integration/transaction_spec.rb
index ff15e597d..e608faa27 100755
--- a/spec/integration/transaction_spec.rb
+++ b/spec/integration/transaction_spec.rb
@@ -135,33 +135,26 @@ describe Puppet::Transaction do
   it "should not let one failed refresh result in other refreshes failing" do
     path = tmpfile("path")
     newfile = tmpfile("file")
-
-          file = Puppet::Type.type(:file).new(
-                
+      file = Puppet::Type.type(:file).new(
       :path => path,
-        
       :ensure => "file"
     )
 
-          exec1 = Puppet::Type.type(:exec).new(
-                
+    exec1 = Puppet::Type.type(:exec).new(
       :path => ENV["PATH"],
       :command => "touch /this/cannot/possibly/exist",
       :logoutput => true,
       :refreshonly => true,
       :subscribe => file,
-        
       :title => "one"
     )
 
-          exec2 = Puppet::Type.type(:exec).new(
-                
+    exec2 = Puppet::Type.type(:exec).new(
       :path => ENV["PATH"],
       :command => "touch #{newfile}",
       :logoutput => true,
       :refreshonly => true,
       :subscribe => [file, exec1],
-        
       :title => "two"
     )
 
@@ -178,22 +171,18 @@ describe Puppet::Transaction do
 
     Puppet[:ignoreschedules] = false
 
-          file = Puppet::Type.type(:file).new(
-                
+    file = Puppet::Type.type(:file).new(
       :name => tmpfile("file"),
-        
       :ensure => "file",
       :backup => false
     )
 
     fname = tmpfile("exec")
 
-          exec = Puppet::Type.type(:exec).new(
-                
+    exec = Puppet::Type.type(:exec).new(
       :name => "touch #{fname}",
       :path => "/usr/bin:/bin",
       :schedule => "monthly",
-        
       :subscribe => Puppet::Resource.new("file", file.name)
     )
 
@@ -230,29 +219,21 @@ describe Puppet::Transaction do
 
   it "should not attempt to evaluate resources with failed dependencies" do
 
-          exec = Puppet::Type.type(:exec).new(
-                
+    exec = Puppet::Type.type(:exec).new(
       :command => "/bin/mkdir /this/path/cannot/possibly/exit",
-        
       :title => "mkdir"
     )
 
-
-          file1 = Puppet::Type.type(:file).new(
-                
+    file1 = Puppet::Type.type(:file).new(
       :title => "file1",
       :path => tmpfile("file1"),
-        
       :require => exec,
       :ensure => :file
     )
 
-
-          file2 = Puppet::Type.type(:file).new(
-                
+    file2 = Puppet::Type.type(:file).new(
       :title => "file2",
       :path => tmpfile("file2"),
-        
       :require => file1,
       :ensure => :file
     )
@@ -264,6 +245,32 @@ describe Puppet::Transaction do
     FileTest.should_not be_exists(file2[:path])
   end
 
+  it "should not trigger subscribing resources on failure" do
+    file1 = tmpfile("file1")
+    file2 = tmpfile("file2")
+
+    create_file1 = Puppet::Type.type(:exec).new(
+      :command => "/usr/bin/touch #{file1}"
+    )
+
+    exec = Puppet::Type.type(:exec).new(
+      :command => "/bin/mkdir /this/path/cannot/possibly/exit",
+      :title => "mkdir",
+      :notify => create_file1
+    )
+
+    create_file2 = Puppet::Type.type(:exec).new(
+      :command => "/usr/bin/touch #{file2}",
+      :subscribe => exec
+    )
+
+    catalog = mk_catalog(exec, create_file1, create_file2)
+    catalog.apply
+
+    FileTest.should_not be_exists(file1)
+    FileTest.should_not be_exists(file2)
+  end
+
   # #801 -- resources only checked in noop should be rescheduled immediately.
   it "should immediately reschedule noop resources" do
     Puppet::Type.type(:schedule).mkdefaultschedules
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
index f8acdd002..d21d86ecf 100755
--- a/spec/unit/configurer_spec.rb
+++ b/spec/unit/configurer_spec.rb
@@ -226,10 +226,12 @@ describe Puppet::Configurer, "when executing a catalog run" do
 end
 
 describe Puppet::Configurer, "when sending a report" do
+  include PuppetSpec::Files
+
   before do
     Puppet.settings.stubs(:use).returns(true)
     @configurer = Puppet::Configurer.new
-    @configurer.stubs(:save_last_run_summary)
+    Puppet[:lastrunfile] = tmpfile('last_run_file')
 
     @report = Puppet::Transaction::Report.new("apply")
     @trans = stub 'transaction'
@@ -277,10 +279,10 @@ describe Puppet::Configurer, "when sending a report" do
     @configurer.send_report(@report, nil)
   end
 
-  it "should not save the last run summary if reporting is disabled" do
+  it "should save the last run summary if reporting is disabled" do
     Puppet.settings[:report] = false
 
-    @configurer.expects(:save_last_run_summary).never
+    @configurer.expects(:save_last_run_summary).with(@report)
     @configurer.send_report(@report, nil)
   end
 
diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb
index 05527e70f..d34bdb000 100755
--- a/spec/unit/node/environment_spec.rb
+++ b/spec/unit/node/environment_spec.rb
@@ -85,28 +85,29 @@ describe Puppet::Node::Environment do
       @env.known_resource_types.should equal(@collection)
     end
 
-    it "should give to all threads the same collection if it didn't change" do
-      Puppet::Resource::TypeCollection.expects(:new).with(@env).returns @collection
-      @env.known_resource_types
+    it "should give to all threads using the same environment the same collection if the collection isn't stale" do
+      original_thread_type_collection = Puppet::Resource::TypeCollection.new(@env)
+      Puppet::Resource::TypeCollection.expects(:new).with(@env).returns original_thread_type_collection
+      @env.known_resource_types.should equal(original_thread_type_collection)
+
+      original_thread_type_collection.expects(:require_reparse?).returns(false)
+      Puppet::Resource::TypeCollection.stubs(:new).with(@env).returns @collection
 
       t = Thread.new {
-        @env.known_resource_types.should equal(@collection)
+        @env.known_resource_types.should equal(original_thread_type_collection)
       }
       t.join
     end
 
-    it "should give to new threads a new collection if it isn't stale" do
-      Puppet::Resource::TypeCollection.expects(:new).with(@env).returns @collection
-      @env.known_resource_types.expects(:stale?).returns(true)
-
-      Puppet::Resource::TypeCollection.expects(:new).returns @collection
+    it "should generate a new TypeCollection if the current one requires reparsing" do
+      old_type_collection = @env.known_resource_types
+      old_type_collection.stubs(:require_reparse?).returns true
+      Thread.current[:known_resource_types] = nil
+      new_type_collection = @env.known_resource_types
 
-      t = Thread.new {
-        @env.known_resource_types.should equal(@collection)
-      }
-      t.join
+      new_type_collection.should be_a Puppet::Resource::TypeCollection
+      new_type_collection.should_not equal(old_type_collection)
     end
-
   end
 
   [:modulepath, :manifestdir].each do |setting|
@@ -277,7 +278,7 @@ describe Puppet::Node::Environment do
 
   describe "when performing initial import" do
     before do
-      @parser = stub 'parser'
+      @parser = Puppet::Parser::Parser.new("test")
       Puppet::Parser::Parser.stubs(:new).returns @parser
       @env = Puppet::Node::Environment.new("env")
     end
@@ -309,6 +310,7 @@ describe Puppet::Node::Environment do
 
     it "should fail helpfully if there is an error importing" do
       File.stubs(:exist?).returns true
+      @env.stubs(:known_resource_types).returns Puppet::Resource::TypeCollection.new(@env)
       @parser.expects(:file=).once
       @parser.expects(:parse).raises ArgumentError
       lambda { @env.instance_eval { perform_initial_import } }.should raise_error(Puppet::Error)
@@ -321,5 +323,13 @@ describe Puppet::Node::Environment do
       @parser.expects(:parse).never
       @env.instance_eval { perform_initial_import }
     end
+
+    it "should mark the type collection as needing a reparse when there is an error parsing" do
+      @parser.expects(:parse).raises Puppet::ParseError.new("Syntax error at ...")
+      @env.stubs(:known_resource_types).returns Puppet::Resource::TypeCollection.new(@env)
+
+      lambda { @env.instance_eval { perform_initial_import } }.should raise_error(Puppet::Error, /Syntax error at .../)
+      @env.known_resource_types.require_reparse?.should be_true
+    end
   end
 end
diff --git a/spec/unit/provider/package/gem_spec.rb b/spec/unit/provider/package/gem_spec.rb
index 7c0d34d00..c2bd3cc90 100644
--- a/spec/unit/provider/package/gem_spec.rb
+++ b/spec/unit/provider/package/gem_spec.rb
@@ -42,8 +42,18 @@ describe provider_class do
       @provider.install
     end
 
+    it "should specify that documentation should not be included" do
+      @provider.expects(:execute).with { |args| args[3] == "--no-rdoc" }.returns ""
+      @provider.install
+    end
+
+    it "should specify that RI should not be included" do
+      @provider.expects(:execute).with { |args| args[4] == "--no-ri" }.returns ""
+      @provider.install
+    end
+
     it "should specify the package name" do
-      @provider.expects(:execute).with { |args| args[3] == "myresource" }.returns ""
+      @provider.expects(:execute).with { |args| args[5] == "myresource" }.returns ""
       @provider.install
     end