From 3e7ebbbb08120243f7e982f34b8256ec58af67b0 Mon Sep 17 00:00:00 2001
From: James Turnbull <james@lovedthanlost.net>
Date: Wed, 2 Mar 2011 16:49:13 +1100
Subject: Fixed #6554 - Missing $haveftool if/else conditional in install.rb
 breaking Ruby 1.9

---
 install.rb | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/install.rb b/install.rb
index 7627a8d11..2608e31da 100755
--- a/install.rb
+++ b/install.rb
@@ -92,8 +92,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/')
-- 
cgit 


From 127501e11bf89894ca01681259e54de45821aacd Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Wed, 30 Mar 2011 15:32:59 -0700
Subject: (6911) Use normal methods to implement "depthfirst?" test

There was a class instance variable that was used to determine if a resource
types's children should be processed before or after the parent, to support
the one type (tidy) which did this.  Instead, we define a normal function in
Type to return false and override it in Tidy to return true.

Paired-with: Jesse Wolfe
---
 lib/puppet/type.rb      | 8 +-------
 lib/puppet/type/file.rb | 2 --
 lib/puppet/type/tidy.rb | 4 +++-
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index d24cc8554..5ecc430d4 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -598,14 +598,8 @@ class Type
   ###############################
   # Code related to the container behaviour.
 
-  # this is a retarded hack method to get around the difference between
-  # component children and file children
-  def self.depthfirst?
-    @depthfirst
-  end
-
   def depthfirst?
-    self.class.depthfirst?
+    false
   end
 
   # Remove an object.  The argument determines whether the object's
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 1a6d0c3ac..2c5a95bbc 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -303,8 +303,6 @@ Puppet::Type.newtype(:file) do
     return self.new(:name => base, :recurse => true, :recurselimit => 1, :audit => :all).recurse_local.values
   end
 
-  @depthfirst = false
-
   # Determine the user to write files as.
   def asuser
     if self.should(:owner) and ! self.should(:owner).is_a?(Symbol)
diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb
index 146481fed..d2e9ebe22 100755
--- a/lib/puppet/type/tidy.rb
+++ b/lib/puppet/type/tidy.rb
@@ -209,7 +209,9 @@ Puppet::Type.newtype(:tidy) do
     []
   end
 
-  @depthfirst = true
+  def depthfirst?
+    true
+  end
 
   def initialize(hash)
     super
-- 
cgit 


From 87ca3130c570df65ab1296cc18de697afa6a55f5 Mon Sep 17 00:00:00 2001
From: Nick Lewis <nick@puppetlabs.com>
Date: Wed, 30 Mar 2011 16:08:22 -0700
Subject: (#5670) Don't trigger refresh from a failed resource

Resources were triggering their subscribing/notified resources when they
failed, which is incorrect. Now, events are only queued if the resource was
successful.

Paired-With: Max Martin
---
 lib/puppet/transaction.rb            |  2 +-
 spec/integration/transaction_spec.rb | 61 ++++++++++++++++++++----------------
 2 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index aa650eea1..48154ad6f 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -47,7 +47,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/spec/integration/transaction_spec.rb b/spec/integration/transaction_spec.rb
index 2c12b3d5f..66a049efe 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
-- 
cgit 


From f76db9e8ce98dde154cbee971dcaecd9f8e73510 Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Wed, 30 Mar 2011 17:55:49 -0700
Subject: (maint) Fix for require order issue

The recent AIX work added a dependency on Puppet::Parameter::Keyvalue in
the group type, but didn't add the requisite require, causing failures
under some load orders.
---
 lib/puppet/type/group.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/puppet/type/group.rb b/lib/puppet/type/group.rb
index aa96bd9c3..2573633f9 100755
--- a/lib/puppet/type/group.rb
+++ b/lib/puppet/type/group.rb
@@ -1,6 +1,7 @@
 
 require 'etc'
 require 'facter'
+require 'puppet/property/keyvalue'
 
 module Puppet
   newtype(:group) do
-- 
cgit 


From 647a640fcac281e3a8cda05b92b51c44c93f1d19 Mon Sep 17 00:00:00 2001
From: Ben Hughes <git@mumble.org.uk>
Date: Wed, 30 Mar 2011 16:43:56 +1100
Subject: (#6893) Document the cron type in the case of specials.

Add in a better desc block for "specials" in cron provider, and outline
it's limitations.

The previous text was purely a placeholder.
---
 lib/puppet/type/cron.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

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}
-- 
cgit 


From 7b83cd9ff9f2e2b61ce4c99057ec0697140a5a5e Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Thu, 31 Mar 2011 16:30:17 -0700
Subject: (6911) Refactor to localize eval_generate dependency assumptions

To implement #6911 we will need to be able to make incremental descisions about
order of application based only on the contents of the resource graph and local
"working data."  This commit begins to pull the needed structure into a method
(visit_resources) while, for the moment, maintaining the original semantic.

Paired-with: Jesse Wolfe
---
 lib/puppet/resource/catalog.rb                     |  42 ++++----
 lib/puppet/transaction.rb                          | 107 +++++++++++----------
 lib/puppet/type/file.rb                            |  12 +--
 lib/puppet/type/tidy.rb                            |   4 -
 .../indirector/catalog/compiler_spec.rb            |   7 +-
 spec/integration/type/file_spec.rb                 |  13 ++-
 spec/unit/resource/catalog_spec.rb                 |   6 --
 spec/unit/transaction_spec.rb                      |   4 +-
 8 files changed, 89 insertions(+), 106 deletions(-)

diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index a8668d844..fed0f46da 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -61,36 +61,32 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
     [$1, $2]
   end
 
-  # Add one or more resources to our graph and to our resource table.
+  # Add a resource to our graph and to our resource table.
   # This is actually a relatively complicated method, because it handles multiple
   # aspects of Catalog behaviour:
   # * Add the resource to the resource table
   # * Add the resource to the resource graph
   # * Add the resource to the relationship graph
   # * Add any aliases that make sense for the resource (e.g., name != title)
-  def add_resource(*resources)
-    resources.each do |resource|
-      raise ArgumentError, "Can only add objects that respond to :ref, not instances of #{resource.class}" unless resource.respond_to?(:ref)
-    end.each { |resource| fail_on_duplicate_type_and_title(resource) }.each do |resource|
-      title_key = title_key_for_ref(resource.ref)
-
-      @transient_resources << resource if applying?
-      @resource_table[title_key] = resource
-
-      # If the name and title differ, set up an alias
-
-      if resource.respond_to?(:name) and resource.respond_to?(:title) and resource.respond_to?(:isomorphic?) and resource.name != resource.title
-        self.alias(resource, resource.uniqueness_key) if resource.isomorphic?
-      end
-
-      resource.catalog = self if resource.respond_to?(:catalog=)
-
-      add_vertex(resource)
-
-      @relationship_graph.add_vertex(resource) if @relationship_graph
-
-      yield(resource) if block_given?
+  def add_resource(*resource)
+    add_resource(*resource[0..-2]) if resource.length > 1
+    resource = resource.pop
+    raise ArgumentError, "Can only add objects that respond to :ref, not instances of #{resource.class}" unless resource.respond_to?(:ref)
+    fail_on_duplicate_type_and_title(resource)
+    title_key = title_key_for_ref(resource.ref)
+ 
+    @transient_resources << resource if applying?
+    @resource_table[title_key] = resource
+
+    # If the name and title differ, set up an alias
+
+    if resource.respond_to?(:name) and resource.respond_to?(:title) and resource.respond_to?(:isomorphic?) and resource.name != resource.title
+      self.alias(resource, resource.uniqueness_key) if resource.isomorphic?
     end
+
+    resource.catalog = self if resource.respond_to?(:catalog=)
+    add_vertex(resource)
+    @relationship_graph.add_vertex(resource) if @relationship_graph
   end
 
   # Create an alias for a resource.
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index eba601cfe..aa0eec3ee 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -12,7 +12,7 @@ class Puppet::Transaction
   require 'puppet/resource/status'
 
   attr_accessor :component, :catalog, :ignoreschedules
-  attr_accessor :sorted_resources, :configurator
+  attr_accessor :configurator
 
   # The report, once generated.
   attr_accessor :report
@@ -57,32 +57,23 @@ class Puppet::Transaction
     report.resource_statuses.values.find_all { |status| status.changed }.collect { |status| catalog.resource(status.resource) }
   end
 
+  # Find all of the applied resources (including failed attempts).
+  def applied_resources
+    report.resource_statuses.values.collect { |status| catalog.resource(status.resource) }
+  end
+
   # Copy an important relationships from the parent to the newly-generated
   # child resource.
-  def make_parent_child_relationship(resource, children)
-    depthfirst = resource.depthfirst?
-
-    children.each do |gen_child|
-      if depthfirst
-        edge = [gen_child, resource]
-      else
-        edge = [resource, gen_child]
-      end
-      relationship_graph.add_vertex(gen_child)
-
-      unless relationship_graph.edge?(edge[1], edge[0])
-        relationship_graph.add_edge(*edge)
-      else
-        resource.debug "Skipping automatic relationship to #{gen_child}"
-      end
+  def make_parent_child_relationship(parent, child)
+    relationship_graph.add_vertex(child)
+    edge = parent.depthfirst? ? [child, parent] : [parent, child]
+    if relationship_graph.edge?(*edge.reverse)
+      parent.debug "Skipping automatic relationship to #{child}"
+    else
+      relationship_graph.add_edge(*edge)
     end
   end
 
-  # See if the resource generates new resources at evaluation time.
-  def eval_generate(resource)
-    generate_additional_resources(resource, :eval_generate)
-  end
-
   # Evaluate a single resource.
   def eval_resource(resource, ancestor = nil)
     if skip?(resource)
@@ -97,26 +88,7 @@ class Puppet::Transaction
 
   def eval_children_and_apply_resource(resource, ancestor = nil)
     resource_status(resource).scheduled = true
-
-    # We need to generate first regardless, because the recursive
-    # actions sometimes change how the top resource is applied.
-    children = eval_generate(resource)
-
-    if ! children.empty? and resource.depthfirst?
-      children.each do |child|
-        # The child will never be skipped when the parent isn't
-        eval_resource(child, ancestor || resource)
-      end
-    end
-
-    # Perform the actual changes
     apply(resource, ancestor)
-
-    if ! children.empty? and ! resource.depthfirst?
-      children.each do |child|
-        eval_resource(child, ancestor || resource)
-      end
-    end
   end
 
   # This method does all the actual work of running a transaction.  It
@@ -131,7 +103,7 @@ class Puppet::Transaction
     Puppet.info "Applying configuration version '#{catalog.version}'" if catalog.version
 
     begin
-      @sorted_resources.each do |resource|
+      visit_resources do |resource|
         next if stop_processing?
         if resource.is_a?(Puppet::Type::Component)
           Puppet.warning "Somehow left a component in the relationship graph"
@@ -177,9 +149,31 @@ class Puppet::Transaction
     found_failed
   end
 
+  def eval_generate(resource)
+    return [] unless resource.respond_to?(:eval_generate)
+    begin
+      made = resource.eval_generate
+    rescue => detail
+      puts detail.backtrace if Puppet[:trace]
+      resource.err "Failed to generate additional resources using 'eval_generate: #{detail}"
+    end
+    parents = [resource]
+    [made].flatten.compact.uniq.each do |res|
+      begin
+        res.tag(*resource.tags)
+        @catalog.add_resource(res)
+        res.finish
+        make_parent_child_relationship(parents.reverse.find { |r| r.name == res.name[0,r.name.length]}, res)
+        parents << res
+      rescue Puppet::Resource::Catalog::DuplicateResourceError
+        res.info "Duplicate generated resource; skipping"
+      end
+    end
+  end
+
   # A general method for recursively generating new resources from a
   # resource.
-  def generate_additional_resources(resource, method)
+  def generate_additional_resources(resource, method, presume_prefix_dependencies=false)
     return [] unless resource.respond_to?(method)
     begin
       made = resource.send(method)
@@ -192,13 +186,10 @@ class Puppet::Transaction
     made.uniq.find_all do |res|
       begin
         res.tag(*resource.tags)
-        @catalog.add_resource(res) do |r|
-          r.finish
-          make_parent_child_relationship(resource, [r])
-
-          # Call 'generate' recursively
-          generate_additional_resources(r, method)
-        end
+        @catalog.add_resource(res)
+        res.finish
+        make_parent_child_relationship(resource, res)
+        generate_additional_resources(res, method, presume_prefix_dependencies)
         true
       rescue Puppet::Resource::Catalog::DuplicateResourceError
         res.info "Duplicate generated resource; skipping"
@@ -269,9 +260,21 @@ class Puppet::Transaction
     # Then prefetch.  It's important that we generate and then prefetch,
     # so that any generated resources also get prefetched.
     prefetch
+  end
 
-    # This will throw an error if there are cycles in the graph.
-    @sorted_resources = relationship_graph.topsort
+  def visit_resources(&block)
+    eval_generated = {}
+    while r = relationship_graph.topsort.find { |r| !applied_resources.include?(r) }
+      #p relationship_graph.topsort.collect { |r0| r0.title[/(-0.*)/,1] }
+      if !eval_generated[r]
+        #p [:generate,r.title]
+        eval_generate(r)
+        eval_generated[r] = true
+      else
+        #p [:apply,r.title]
+        yield r
+      end
+    end
   end
 
   def relationship_graph
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 2c5a95bbc..cbc1f77da 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -462,8 +462,7 @@ Puppet::Type.newtype(:file) do
   # be used to copy remote files, manage local files, and/or make links
   # to map to another directory.
   def recurse
-    children = {}
-    children = recurse_local if self[:recurse] != :remote
+    children = (self[:recurse] == :remote) ? {} : recurse_local
 
     if self[:target]
       recurse_link(children)
@@ -500,11 +499,7 @@ Puppet::Type.newtype(:file) do
 
   # A simple method for determining whether we should be recursing.
   def recurse?
-    return false unless @parameters.include?(:recurse)
-
-    val = @parameters[:recurse].value
-
-    !!(val and (val == true or val == :remote))
+    self[:recurse] == true or self[:recurse] == :remote
   end
 
   # Recurse the target of the link.
@@ -576,13 +571,10 @@ Puppet::Type.newtype(:file) do
   end
 
   def perform_recursion(path)
-
     Puppet::FileServing::Metadata.indirection.search(
-
       path,
       :links => self[:links],
       :recurse => (self[:recurse] == :remote ? true : self[:recurse]),
-
       :recurselimit => self[:recurselimit],
       :ignore => self[:ignore],
       :checksum_type => (self[:source] || self[:content]) ? self[:checksum] : :none
diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb
index d2e9ebe22..1a308e17d 100755
--- a/lib/puppet/type/tidy.rb
+++ b/lib/puppet/type/tidy.rb
@@ -240,10 +240,6 @@ Puppet::Type.newtype(:tidy) do
     []
   end
 
-  def eval_generate
-    []
-  end
-
   def generate
     return [] unless stat(self[:path])
 
diff --git a/spec/integration/indirector/catalog/compiler_spec.rb b/spec/integration/indirector/catalog/compiler_spec.rb
index 1146c20b0..dafa1af7c 100755
--- a/spec/integration/indirector/catalog/compiler_spec.rb
+++ b/spec/integration/indirector/catalog/compiler_spec.rb
@@ -10,11 +10,8 @@ describe Puppet::Resource::Catalog::Compiler do
   before do
     Facter.stubs(:value).returns "something"
     @catalog = Puppet::Resource::Catalog.new
-
-    @one = Puppet::Resource.new(:file, "/one")
-
-    @two = Puppet::Resource.new(:file, "/two")
-    @catalog.add_resource(@one, @two)
+    @catalog.add_resource(@one = Puppet::Resource.new(:file, "/one"))
+    @catalog.add_resource(@two = Puppet::Resource.new(:file, "/two"))
   end
 
   after { Puppet.settings.clear }
diff --git a/spec/integration/type/file_spec.rb b/spec/integration/type/file_spec.rb
index 46900a0f1..513b96e41 100755
--- a/spec/integration/type/file_spec.rb
+++ b/spec/integration/type/file_spec.rb
@@ -28,7 +28,8 @@ describe Puppet::Type.type(:file) do
       bucket = Puppet::Type.type(:filebucket).new :path => tmpfile("filebucket"), :name => "mybucket"
       file = Puppet::Type.type(:file).new :path => tmpfile("bucket_backs"), :backup => "mybucket", :content => "foo"
       catalog = Puppet::Resource::Catalog.new
-      catalog.add_resource file, bucket
+      catalog.add_resource file
+      catalog.add_resource bucket
 
       File.open(file[:path], "w") { |f| f.puts "bar" }
 
@@ -80,7 +81,8 @@ describe Puppet::Type.type(:file) do
       bucket = Puppet::Type.type(:filebucket).new :path => tmpfile("filebucket"), :name => "mybucket"
       file = Puppet::Type.type(:file).new :path => link, :target => dest2, :ensure => :link, :backup => "mybucket"
       catalog = Puppet::Resource::Catalog.new
-      catalog.add_resource file, bucket
+      catalog.add_resource file
+      catalog.add_resource bucket
 
       File.open(dest1, "w") { |f| f.puts "whatever" }
       File.symlink(dest1, link)
@@ -113,7 +115,8 @@ describe Puppet::Type.type(:file) do
       bucket = Puppet::Type.type(:filebucket).new :path => tmpfile("filebucket"), :name => "mybucket"
       file = Puppet::Type.type(:file).new :path => tmpfile("bucket_backs"), :backup => "mybucket", :content => "foo", :force => true
       catalog = Puppet::Resource::Catalog.new
-      catalog.add_resource file, bucket
+      catalog.add_resource file
+      catalog.add_resource bucket
 
       Dir.mkdir(file[:path])
       foofile = File.join(file[:path], "foo")
@@ -337,10 +340,10 @@ describe Puppet::Type.type(:file) do
     it "should have an edge to each resource in the relationship graph" do
       @catalog.apply do |trans|
         one = @catalog.resource(:file, File.join(@dest, "one"))
-        @catalog.relationship_graph.should be_edge(@file, one)
+        @catalog.relationship_graph.edge?(@file, one).should be
 
         two = @catalog.resource(:file, File.join(@dest, "two"))
-        @catalog.relationship_graph.should be_edge(@file, two)
+        @catalog.relationship_graph.edge?(@file, two).should be
       end
     end
   end
diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb
index 42850c23b..411d10b32 100755
--- a/spec/unit/resource/catalog_spec.rb
+++ b/spec/unit/resource/catalog_spec.rb
@@ -398,12 +398,6 @@ describe Puppet::Resource::Catalog, "when compiling" do
       relgraph.should be_vertex(@one)
     end
 
-    it "should yield added resources if a block is provided" do
-      yielded = []
-      @catalog.add_resource(@one, @two) { |r| yielded << r }
-      yielded.length.should == 2
-    end
-
     it "should set itself as the resource's catalog if it is not a relationship graph" do
       @one.expects(:catalog=).with(@catalog)
       @catalog.add_resource @one
diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb
index 3677bfd87..5867f2e83 100755
--- a/spec/unit/transaction_spec.rb
+++ b/spec/unit/transaction_spec.rb
@@ -244,6 +244,8 @@ describe Puppet::Transaction do
       @catalog.stubs(:add_resource)
 
       child.expects(:tag).with("one", "two")
+      child.expects(:finish)
+      generator.expects(:depthfirst?)
 
       @transaction.generate_additional_resources(generator, :generate)
     end
@@ -390,7 +392,7 @@ describe Puppet::Transaction do
         @resource = stub 'resource', :ref => 'some_ref'
         @catalog.add_resource @resource
         @transaction.stubs(:prepare)
-        @transaction.sorted_resources = [@resource]
+        @transaction.stubs(:visit_resources).yields(@resource)
       end
 
       it 'should stop processing if :stop_processing? is true' do
-- 
cgit 


From fa5c2b1b1494cc760f95c95e6e50a304c2651c7b Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Thu, 31 Mar 2011 16:41:11 -0700
Subject: (6911) Cleanup of generate_additional_resources

The previous commit left only one meaningful value for the method parameter
of generate_additional_resources, making it a constant not a parameter.  This
commit removes it.

Paired-with: Jesse Wolfe
---
 lib/puppet/transaction.rb     | 12 ++++++------
 spec/unit/transaction_spec.rb |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index aa0eec3ee..ad79f176e 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -173,13 +173,13 @@ class Puppet::Transaction
 
   # A general method for recursively generating new resources from a
   # resource.
-  def generate_additional_resources(resource, method, presume_prefix_dependencies=false)
-    return [] unless resource.respond_to?(method)
+  def generate_additional_resources(resource)
+    return [] unless resource.respond_to?(:generate)
     begin
-      made = resource.send(method)
+      made = resource.generate
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
-      resource.err "Failed to generate additional resources using '#{method}': #{detail}"
+      resource.err "Failed to generate additional resources using 'generate': #{detail}"
     end
     return [] unless made
     made = [made] unless made.is_a?(Array)
@@ -189,7 +189,7 @@ class Puppet::Transaction
         @catalog.add_resource(res)
         res.finish
         make_parent_child_relationship(resource, res)
-        generate_additional_resources(res, method, presume_prefix_dependencies)
+        generate_additional_resources(res)
         true
       rescue Puppet::Resource::Catalog::DuplicateResourceError
         res.info "Duplicate generated resource; skipping"
@@ -205,7 +205,7 @@ class Puppet::Transaction
     newlist = []
     while ! list.empty?
       list.each do |resource|
-        newlist += generate_additional_resources(resource, :generate)
+        newlist += generate_additional_resources(resource)
       end
       list = newlist
       newlist = []
diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb
index 5867f2e83..57bf8005e 100755
--- a/spec/unit/transaction_spec.rb
+++ b/spec/unit/transaction_spec.rb
@@ -197,7 +197,7 @@ describe Puppet::Transaction do
       second.expects(:generate).returns [third]
       third.expects(:generate)
 
-      @transaction.generate_additional_resources(first, :generate)
+      @transaction.generate_additional_resources(first)
     end
 
     it "should finish all resources" do
@@ -213,7 +213,7 @@ describe Puppet::Transaction do
 
       resource.expects(:finish)
 
-      @transaction.generate_additional_resources(generator, :generate)
+      @transaction.generate_additional_resources(generator)
     end
 
     it "should skip generated resources that conflict with existing resources" do
@@ -230,7 +230,7 @@ describe Puppet::Transaction do
       resource.expects(:finish).never
       resource.expects(:info) # log that it's skipped
 
-      @transaction.generate_additional_resources(generator, :generate).should be_empty
+      @transaction.generate_additional_resources(generator).should be_empty
     end
 
     it "should copy all tags to the newly generated resources" do
@@ -247,7 +247,7 @@ describe Puppet::Transaction do
       child.expects(:finish)
       generator.expects(:depthfirst?)
 
-      @transaction.generate_additional_resources(generator, :generate)
+      @transaction.generate_additional_resources(generator)
     end
   end
 
-- 
cgit 


From ee1df783ddd4b72bd959735443b5dbdfea69e323 Mon Sep 17 00:00:00 2001
From: Ben Hughes <ben@puppetlabs.com>
Date: Fri, 1 Apr 2011 12:48:38 +1100
Subject: (#6937) Document the recurse parameter of File type.

Update the desc block with information gleaned from #1469 and the code
about recurse => remote and other types of recursion.

The auto generated documentation was sparse and this is an area that often
comes up on the mailing list/IRC.
---
 lib/puppet/type/file.rb | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 1a6d0c3ac..a73ada57e 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -122,7 +122,17 @@ Puppet::Type.newtype(:file) do
 
   newparam(:recurse) do
     desc "Whether and how deeply to do recursive
-      management."
+      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.
+    "
 
     newvalues(:true, :false, :inf, :remote, /^[0-9]+$/)
 
-- 
cgit 


From 2cdadf9b7fa7a4a1b826150549e5faffc4e494f3 Mon Sep 17 00:00:00 2001
From: Ben Hughes <ben@puppetlabs.com>
Date: Fri, 1 Apr 2011 12:48:38 +1100
Subject: (#6937) Document the recurse parameter of File type.

Update the desc block with information gleaned from #1469 and the code
about recurse => remote and other types of recursion.

The auto generated documentation was sparse and this is an area that often
comes up on the mailing list/IRC.
---
 lib/puppet/type/file.rb | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index e1a4ecbb9..79f4b81ab 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -122,7 +122,17 @@ Puppet::Type.newtype(:file) do
 
   newparam(:recurse) do
     desc "Whether and how deeply to do recursive
-      management."
+      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.
+    "
 
     newvalues(:true, :false, :inf, :remote, /^[0-9]+$/)
 
-- 
cgit 


From 0675c9a7ae90b25e1d723c19a7691b12eac9becd Mon Sep 17 00:00:00 2001
From: nfagerlund <nick.fagerlund@gmail.com>
Date: Fri, 1 Apr 2011 10:35:41 -0700
Subject: (#6937) Adjust formatting of recurse's desc

Prevous formatting would result in broken Markdown after the docs were generated,
as Markdown does not recognize a two-space tab as a syntactical element.
This patch also changes the list of values to a bulleted list instead of a code block.
---
 lib/puppet/type/file.rb | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 79f4b81ab..630ebe5de 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]+$/)
-- 
cgit 


From 1b66c28fb2ac5f5f10ce32b87ec459a480dcf161 Mon Sep 17 00:00:00 2001
From: Jacob Helwig <jacob@puppetlabs.com>
Date: Fri, 1 Apr 2011 11:15:47 -0700
Subject: (#5437) Invalidate cached TypeCollection when there was an error
 parsing

The caching of the TypeCollection in Puppet::Node::Environment would cause
parse errors to occur (and be reported) only once and never again, until
the file had changed on disk.  This would also cause empty catalogs to be
sent down to the agents further hiding the problem.

Now, when a file fails to parse, it will be re-parsed every time on every
following compilation, causing the parse error to be reported every time,
and preventing sending down empty catalogs to agents.

Paired-with: Nick Lewis <nick@puppetlabs.com>
---
 lib/puppet/node/environment.rb             |  2 +-
 lib/puppet/resource/type_collection.rb     |  6 ++++++
 spec/unit/node/environment_spec.rb         | 29 +++++++++++++++--------------
 spec/unit/resource/type_collection_spec.rb |  9 ++++++++-
 4 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index b64fb8a1f..807479bc8 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.perform_initial_import
       end
diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb
index 347e1c0e0..4210475ad 100644
--- a/lib/puppet/resource/type_collection.rb
+++ b/lib/puppet/resource/type_collection.rb
@@ -166,12 +166,18 @@ class Puppet::Resource::TypeCollection
     end
     parser.parse
   rescue => detail
+    @parse_failed = true
+
     msg = "Could not parse for environment #{environment}: #{detail}"
     error = Puppet::Error.new(msg)
     error.set_backtrace(detail.backtrace)
     raise error
   end
 
+  def require_reparse?
+    @parse_failed || stale?
+  end
+
   def stale?
     @watched_files.values.detect { |file| file.changed? }
   end
diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb
index 6edcce56c..6109007b9 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|
diff --git a/spec/unit/resource/type_collection_spec.rb b/spec/unit/resource/type_collection_spec.rb
index cf7039a51..1576c0615 100644
--- a/spec/unit/resource/type_collection_spec.rb
+++ b/spec/unit/resource/type_collection_spec.rb
@@ -387,7 +387,7 @@ describe Puppet::Resource::TypeCollection 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
       @code = Puppet::Resource::TypeCollection.new("env")
     end
@@ -423,6 +423,13 @@ describe Puppet::Resource::TypeCollection do
       @parser.stubs(:file=)
       lambda { @code.perform_initial_import }.should raise_error(Puppet::Error)
     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 ...")
+
+      lambda { @code.perform_initial_import }.should raise_error(Puppet::Error, /Syntax error at .../)
+      @code.require_reparse?.should be_true
+    end
   end
 
   describe "when determining the configuration version" do
-- 
cgit 


From 8b5ffde9c4464d942aa0e2cae35220b933b7e3a6 Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Fri, 1 Apr 2011 15:12:54 -0700
Subject: (6911) Add bookkeeping facade around Transaction#relationship_graph

To implement graph frontiers transactions need to track information about the
catalog's relationship graph.  For various reasons (serialzation, lifetime,
etc.) the data belongs with the transaction rather than the catalog or its
relationship graph.  This commit introduces a facade around the property used
to cheat Demeter which has the apropriate lifetime and can be used to hold the
state information durring a traversal.

Paired-with: Jesse Wolfe
---
 lib/puppet/transaction.rb | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index ad79f176e..928a1b488 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -277,8 +277,39 @@ class Puppet::Transaction
     end
   end
 
+  # We want to monitor changes in the relationship graph of our
+  # catalog but this is complicated by the fact that the catalog
+  # both is_a graph and has_a graph, by the fact that changes to 
+  # the structure of the object can have adverse serialization
+  # effects, by threading issues, by order-of-initialization issues,
+  # etc.  
+  #
+  # Since the proper lifetime/scope of the monitoring is a transaction
+  # and the transaction is already commiting a mild law-of-demeter 
+  # transgression, we cut the Gordian knot here by simply wrapping the
+  # transaction's view of the resource graph to capture and maintain
+  # the information we need.  Nothing outside the transaction needs
+  # this information, and nothing outside the transaction can see it
+  # except via the Transaction#relationship_graph
+
+  class Relationship_graph_wrapper
+    def initialize(real_graph,transaction)
+      @real_graph = real_graph
+      @transaction = transaction
+    end
+    def method_missing(*args,&block)
+      @real_graph.send(*args,&block)
+    end
+    def add_vertex(v)
+      @real_graph.add_vertex(v)
+    end
+    def add_edge(f,t)
+      @real_graph.add_edge(f,t)
+    end
+  end
+
   def relationship_graph
-    catalog.relationship_graph
+    @relationship_graph ||= Relationship_graph_wrapper.new(catalog.relationship_graph,self)
   end
 
   def add_resource_status(status)
-- 
cgit 


From 8af29c8f9a4da0d9e4a1c15f032f9c0bb9348d62 Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Fri, 1 Apr 2011 17:49:37 -0700
Subject: (6911) Core change -- replace topsort with frontier ordered by salted
 SHA1

This is the core change of the ticket; rather than using a topological sort to statically
determine the order in which resources should be applied, we use the graph wrapper
introduced in the prior commit to dynamically determine the order in which to apply
resources based on 1) the status of the resource (ready, done) 2) the explicit &
implied dependencies, 3) the salted SHA1 of the title (for stability).

Further work is needed:

1) Resolving the handling of failed resources
2) Tests of the new behavior, to the extent posible
3) Newly-dead-code removal in simple_graph & transaction
4) Fix the name-prefix ordering hack in eval_generate by either:
    a) Moving the logic into file
    b) Refactoring Type#eval_generate to return a tree
    c) ....?
5) Rough performace testing to look for hotspots
6) Investigation of possible interaction with #3788, #5351, #5414, #5876, #6020, #6810,
   and #6944 which may simplify or complicate their resolution.

Paired-with: Jesse Wolfe
---
 lib/puppet/simple_graph.rb    |  8 ++++++++
 lib/puppet/transaction.rb     | 46 ++++++++++++++++++++++++++-----------------
 spec/unit/transaction_spec.rb |  4 +++-
 3 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/lib/puppet/simple_graph.rb b/lib/puppet/simple_graph.rb
index 27e068e30..31858f136 100644
--- a/lib/puppet/simple_graph.rb
+++ b/lib/puppet/simple_graph.rb
@@ -453,6 +453,10 @@ class Puppet::SimpleGraph
     result
   end
 
+  def direct_dependents_of(v)
+    @out_from[v].keys 
+  end
+
   def upstream_from_vertex(v)
     return @upstream_from[v] if @upstream_from[v]
     result = @upstream_from[v] = {}
@@ -463,6 +467,10 @@ class Puppet::SimpleGraph
     result
   end
 
+  def direct_dependencies_of(v)
+    @in_to[v].keys 
+  end
+
   # LAK:FIXME This is just a paste of the GRATR code with slight modifications.
 
   # Return a DOT::DOTDigraph for directed graphs or a DOT::DOTSubgraph for an
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 928a1b488..79fdb044c 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -4,6 +4,7 @@
 require 'puppet'
 require 'puppet/util/tagging'
 require 'puppet/application'
+require 'sha1'
 
 class Puppet::Transaction
   require 'puppet/transaction/event'
@@ -103,7 +104,7 @@ class Puppet::Transaction
     Puppet.info "Applying configuration version '#{catalog.version}'" if catalog.version
 
     begin
-      visit_resources do |resource|
+      relationship_graph.traverse do |resource|
         next if stop_processing?
         if resource.is_a?(Puppet::Type::Component)
           Puppet.warning "Somehow left a component in the relationship graph"
@@ -262,20 +263,6 @@ class Puppet::Transaction
     prefetch
   end
 
-  def visit_resources(&block)
-    eval_generated = {}
-    while r = relationship_graph.topsort.find { |r| !applied_resources.include?(r) }
-      #p relationship_graph.topsort.collect { |r0| r0.title[/(-0.*)/,1] }
-      if !eval_generated[r]
-        #p [:generate,r.title]
-        eval_generate(r)
-        eval_generated[r] = true
-      else
-        #p [:apply,r.title]
-        yield r
-      end
-    end
-  end
 
   # We want to monitor changes in the relationship graph of our
   # catalog but this is complicated by the fact that the catalog
@@ -293,18 +280,41 @@ class Puppet::Transaction
   # except via the Transaction#relationship_graph
 
   class Relationship_graph_wrapper
+    attr_reader :real_graph,:transaction,:ready,:generated,:done
     def initialize(real_graph,transaction)
       @real_graph = real_graph
       @transaction = transaction
+      @ready = {}
+      @generated = {}
+      @done = {}
+      vertices.each { |v| check_if_now_ready(v) }
     end
     def method_missing(*args,&block)
-      @real_graph.send(*args,&block)
+      real_graph.send(*args,&block)
     end
     def add_vertex(v)
-      @real_graph.add_vertex(v)
+      real_graph.add_vertex(v)
     end
     def add_edge(f,t)
-      @real_graph.add_edge(f,t)
+      ready.delete(t)
+      real_graph.add_edge(f,t)
+    end
+    def check_if_now_ready(r)
+      ready[r] = true if direct_dependencies_of(r).all? { |r2| done[r2] }
+    end  
+    def traverse(&block)
+      unguessable_deterministic_key = Hash.new { |h,k| h[k] = Digest::SHA1.hexdigest("NaCl, MgSO4 (salts) and then #{k.title}") }
+      while r = ready.keys.sort_by { |r0| unguessable_deterministic_key[r0] }.first
+        if !generated[r]
+          transaction.eval_generate(r)
+          generated[r] = true
+        else
+          ready.delete(r)
+          yield r
+          done[r] = true
+          direct_dependents_of(r).each { |v| check_if_now_ready(v) }
+        end
+      end
     end
   end
 
diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb
index 57bf8005e..076b626bb 100755
--- a/spec/unit/transaction_spec.rb
+++ b/spec/unit/transaction_spec.rb
@@ -390,9 +390,11 @@ describe Puppet::Transaction do
     describe 'within an evaluate call' do
       before do
         @resource = stub 'resource', :ref => 'some_ref'
+        @relationship_graph = stub 'relationship_graph'
         @catalog.add_resource @resource
         @transaction.stubs(:prepare)
-        @transaction.stubs(:visit_resources).yields(@resource)
+        @transaction.stubs(:relationship_graph).returns @relationship_graph
+        @relationship_graph.stubs(:traverse).yields @resource
       end
 
       it 'should stop processing if :stop_processing? is true' do
-- 
cgit 


From 5dc994c594680203a4bbbbaa3d6f3b00640c1530 Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Fri, 1 Apr 2011 22:53:44 -0700
Subject: (6911) Cleanup and renaming of transaction internals

The preceeding changes left some rough edges in the Transactions (a short,
badly named method that was only used in one place and would be clearer in-
line, a return value that was carfully retained and never used, etc.) This
commit clears some of that up.
---
 lib/puppet/transaction.rb     | 31 ++++++++++++-------------------
 spec/unit/transaction_spec.rb | 16 +++-------------
 2 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 79fdb044c..c502fc627 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -80,18 +80,14 @@ class Puppet::Transaction
     if skip?(resource)
       resource_status(resource).skipped = true
     else
-      eval_children_and_apply_resource(resource, ancestor)
+      resource_status(resource).scheduled = true
+      apply(resource, ancestor)
     end
 
     # Check to see if there are any events queued for this resource
     event_manager.process_events(resource)
   end
 
-  def eval_children_and_apply_resource(resource, ancestor = nil)
-    resource_status(resource).scheduled = true
-    apply(resource, ancestor)
-  end
-
   # This method does all the actual work of running a transaction.  It
   # collects all of the changes, executes them, and responds to any
   # necessary events.
@@ -105,18 +101,12 @@ class Puppet::Transaction
 
     begin
       relationship_graph.traverse do |resource|
-        next if stop_processing?
         if resource.is_a?(Puppet::Type::Component)
           Puppet.warning "Somehow left a component in the relationship graph"
-          next
-        end
-        ret = nil
-        seconds = thinmark do
-          ret = eval_resource(resource)
+        else
+          seconds = thinmark { eval_resource(resource) }
+          resource.info "Evaluated in %0.2f seconds" % seconds if Puppet[:evaltrace] and @catalog.host_config?
         end
-
-        resource.info "Evaluated in %0.2f seconds" % seconds if Puppet[:evaltrace] and @catalog.host_config?
-        ret
       end
     ensure
       # And then close the transaction log.
@@ -280,13 +270,14 @@ class Puppet::Transaction
   # except via the Transaction#relationship_graph
 
   class Relationship_graph_wrapper
-    attr_reader :real_graph,:transaction,:ready,:generated,:done
+    attr_reader :real_graph,:transaction,:ready,:generated,:done,:unguessable_deterministic_key
     def initialize(real_graph,transaction)
       @real_graph = real_graph
       @transaction = transaction
       @ready = {}
       @generated = {}
       @done = {}
+      @unguessable_deterministic_key = Hash.new { |h,k| h[k] = Digest::SHA1.hexdigest("NaCl, MgSO4 (salts) and then #{k.title}") }
       vertices.each { |v| check_if_now_ready(v) }
     end
     def method_missing(*args,&block)
@@ -301,10 +292,12 @@ class Puppet::Transaction
     end
     def check_if_now_ready(r)
       ready[r] = true if direct_dependencies_of(r).all? { |r2| done[r2] }
-    end  
+    end
+    def next_resource
+      ready.keys.sort_by { |r0| unguessable_deterministic_key[r0] }.first
+    end
     def traverse(&block)
-      unguessable_deterministic_key = Hash.new { |h,k| h[k] = Digest::SHA1.hexdigest("NaCl, MgSO4 (salts) and then #{k.title}") }
-      while r = ready.keys.sort_by { |r0| unguessable_deterministic_key[r0] }.first
+      while (r = next_resource) && !transaction.stop_processing?
         if !generated[r]
           transaction.eval_generate(r)
           generated[r] = true
diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb
index 076b626bb..8d40136fb 100755
--- a/spec/unit/transaction_spec.rb
+++ b/spec/unit/transaction_spec.rb
@@ -114,7 +114,6 @@ describe Puppet::Transaction do
   describe "when evaluating a resource" do
     before do
       @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)
-      @transaction.stubs(:eval_children_and_apply_resource)
       @transaction.stubs(:skip?).returns false
 
       @resource = Puppet::Type.type(:file).new :path => @basepath
@@ -126,12 +125,6 @@ describe Puppet::Transaction do
       @transaction.eval_resource(@resource)
     end
 
-    it "should eval and apply children" do
-      @transaction.expects(:eval_children_and_apply_resource).with(@resource, nil)
-
-      @transaction.eval_resource(@resource)
-    end
-
     it "should process events" do
       @transaction.event_manager.expects(:process_events).with(@resource)
 
@@ -389,22 +382,19 @@ describe Puppet::Transaction do
 
     describe 'within an evaluate call' do
       before do
-        @resource = stub 'resource', :ref => 'some_ref'
-        @relationship_graph = stub 'relationship_graph'
+        @resource = Puppet::Type.type(:notify).new :title => "foobar"
         @catalog.add_resource @resource
         @transaction.stubs(:prepare)
-        @transaction.stubs(:relationship_graph).returns @relationship_graph
-        @relationship_graph.stubs(:traverse).yields @resource
       end
 
       it 'should stop processing if :stop_processing? is true' do
-        @transaction.expects(:stop_processing?).returns(true)
+        @transaction.stubs(:stop_processing?).returns(true)
         @transaction.expects(:eval_resource).never
         @transaction.evaluate
       end
 
       it 'should continue processing if :stop_processing? is false' do
-        @transaction.expects(:stop_processing?).returns(false)
+        @transaction.stubs(:stop_processing?).returns(false)
         @transaction.expects(:eval_resource).returns(nil)
         @transaction.evaluate
       end
-- 
cgit 


From d2bacd308ea214151f5c1f43ec62aad8075da41b Mon Sep 17 00:00:00 2001
From: James Turnbull <james@lovedthanlost.net>
Date: Mon, 4 Apr 2011 04:50:38 +1000
Subject: Fixed #3127 - Fixed gem selection regex

Previously if the gem json_pure was installed and you requested the
gem json then the regex matched too soon and falshly indicated that
the json gem was already installed.

Also updated to add the --no-ri and no-rdoc options and fix tests.
---
 lib/puppet/provider/package/gem.rb     |  5 +++--
 spec/unit/provider/package/gem_spec.rb | 12 +++++++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb
index 19414cec4..64544db4e 100755
--- a/lib/puppet/provider/package/gem.rb
+++ b/lib/puppet/provider/package/gem.rb
@@ -1,5 +1,6 @@
 require 'puppet/provider/package'
 require 'uri'
+require 'pp'
 
 # Ruby gems support.
 Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package do
@@ -22,7 +23,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 +95,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/spec/unit/provider/package/gem_spec.rb b/spec/unit/provider/package/gem_spec.rb
index 063e1474b..32c067a60 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
 
-- 
cgit 


From 9bb301814865db25a43b97869f0261f3bea9faf7 Mon Sep 17 00:00:00 2001
From: James Turnbull <james@lovedthanlost.net>
Date: Tue, 5 Apr 2011 01:48:23 +1000
Subject: Fixed #3127 - removed legacy debug code

---
 lib/puppet/provider/package/gem.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb
index 64544db4e..28731c849 100755
--- a/lib/puppet/provider/package/gem.rb
+++ b/lib/puppet/provider/package/gem.rb
@@ -1,6 +1,5 @@
 require 'puppet/provider/package'
 require 'uri'
-require 'pp'
 
 # Ruby gems support.
 Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package do
-- 
cgit 


From 306aa3032a3770c0d668907ae08042be88ec725e Mon Sep 17 00:00:00 2001
From: Brice Figureau <brice-puppet@daysofwonder.com>
Date: Fri, 12 Nov 2010 13:39:41 +0100
Subject: Fix #4339 - Save a last run report summary to
 $statedir/last_run_summary.yaml

Once a configuration run is done, puppetd will save on the node a
yaml summary report roughly akin to:

---
  time:
    notify: 0.001025
    last_run: 1289561427
    schedule: 0.00071
    config_retrieval: 0.039518
    filebucket: 0.000126
  resources:
    changed: 1
    total: 8
    out_of_sync: 1
  events:
    total: 1
    success: 1
  changes:
    total: 1

This is almost an hash version of the current --summarize output, with
the notable exception that the time section includes the last run unix
timestamp.

The whole idea is to be able to monitor locally if a puppetd does its job.
For instance this could be used in a nagios check or to send an SNMP trap.
The last_run information might help detect staleness, and this summary
can also be used for performance monitoring (ie time section).
The resource section can also show the number of failed resources.

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>
---
 lib/puppet/configurer.rb             | 11 ++++++++-
 lib/puppet/defaults.rb               |  4 +++
 lib/puppet/transaction/report.rb     | 38 ++++++++++++++++++++--------
 lib/puppet/util/metric.rb            |  8 +++---
 spec/integration/configurer_spec.rb  | 43 ++++++++++++++++++++++++++++----
 spec/unit/configurer_spec.rb         | 48 ++++++++++++++++++++++++++++++++++++
 spec/unit/transaction/report_spec.rb | 15 +++++++++--
 7 files changed, 144 insertions(+), 23 deletions(-)

diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index d3c902576..9f68ca499 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -166,19 +166,28 @@ class Puppet::Configurer
     execute_postrun_command
 
     Puppet::Util::Log.close(report)
-
     send_report(report, transaction)
   end
 
   def send_report(report, trans)
     report.finalize_report if trans
     puts report.summary if Puppet[:summarize]
+    save_last_run_summary(report)
     report.save if Puppet[:report]
   rescue => detail
     puts detail.backtrace if Puppet[:trace]
     Puppet.err "Could not send report: #{detail}"
   end
 
+  def save_last_run_summary(report)
+    Puppet::Util::FileLocking.writelock(Puppet[:lastrunfile], 0660) do |file|
+      file.print YAML.dump(report.raw_summary)
+    end
+  rescue => detail
+    puts detail.backtrace if Puppet[:trace]
+    Puppet.err "Could not save last run local report: #{detail}"
+  end
+
   private
 
   def self.timeout
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 36159bcf7..23b3b6652 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -607,6 +607,10 @@ module Puppet
     :report => [false,
       "Whether to send reports after every transaction."
     ],
+    :lastrunfile =>  { :default => "$statedir/last_run_summary.yaml",
+      :mode => 0660,
+      :desc => "Where puppet agent stores the last run report summary in yaml format."
+    },
     :graph => [false, "Whether to create dot graph files for the different
       configuration graphs.  These dot files can be interpreted by tools
       like OmniGraffle or dot (which is part of ImageMagick)."],
diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index 8e04759ad..16fee42ae 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -80,30 +80,49 @@ class Puppet::Transaction::Report
     host
   end
 
-  # Provide a summary of this report.
+  # Provide a human readable textual summary of this report.
   def summary
+    report = raw_summary
+
     ret = ""
+    report.keys.sort { |a,b| a.to_s <=> b.to_s }.each do |key|
+      ret += "#{Puppet::Util::Metric.labelize(key)}:\n"
 
-    @metrics.sort { |a,b| a[1].label <=> b[1].label }.each do |name, metric|
-      ret += "#{metric.label}:\n"
-      metric.values.sort { |a,b|
+      report[key].keys.sort { |a,b|
         # sort by label
-        if a[0] == :total
+        if a == :total
           1
-        elsif b[0] == :total
+        elsif b == :total
           -1
         else
-          a[1] <=> b[1]
+          report[key][a].to_s <=> report[key][b].to_s
         end
-      }.each do |name, label, value|
+      }.each do |label|
+        value = report[key][label]
         next if value == 0
         value = "%0.2f" % value if value.is_a?(Float)
-        ret += "   %15s %s\n" % [label + ":", value]
+        ret += "   %15s %s\n" % [Puppet::Util::Metric.labelize(label) + ":", value]
       end
     end
     ret
   end
 
+  # Provide a raw hash summary of this report.
+  def raw_summary
+    report = {}
+
+    @metrics.each do |name, metric|
+      key = metric.name.to_s
+      report[key] = {}
+      metric.values.each do |name, label, value|
+        report[key][name.to_s] = value
+      end
+      report[key]["total"] = 0 unless key == "time" or report[key].include?("total")
+    end
+    (report["time"] ||= {})["last_run"] = Time.now.tv_sec
+    report
+  end
+
   # Based on the contents of this report's metrics, compute a single number
   # that represents the report. The resulting number is a bitmask where
   # individual bits represent the presence of different metrics.
@@ -142,7 +161,6 @@ class Puppet::Transaction::Report
     metrics["total"] = resource_statuses.length
 
     resource_statuses.each do |name, status|
-
       Puppet::Resource::Status::STATES.each do |state|
         metrics[state.to_s] += 1 if status.send(state)
       end
diff --git a/lib/puppet/util/metric.rb b/lib/puppet/util/metric.rb
index 09bbb6137..835e1d610 100644
--- a/lib/puppet/util/metric.rb
+++ b/lib/puppet/util/metric.rb
@@ -122,7 +122,7 @@ class Puppet::Util::Metric
   def initialize(name,label = nil)
     @name = name.to_s
 
-    @label = label || labelize(name)
+    @label = label || self.class.labelize(name)
 
     @values = []
   end
@@ -133,7 +133,7 @@ class Puppet::Util::Metric
 
   def newvalue(name,value,label = nil)
     raise ArgumentError.new("metric name #{name.inspect} is not a string") unless name.is_a? String
-    label ||= labelize(name)
+    label ||= self.class.labelize(name)
     @values.push [name,label,value]
   end
 
@@ -174,10 +174,8 @@ class Puppet::Util::Metric
     @values.sort { |a, b| a[1] <=> b[1] }
   end
 
-  private
-
   # Convert a name into a label.
-  def labelize(name)
+  def self.labelize(name)
     name.to_s.capitalize.gsub("_", " ")
   end
 end
diff --git a/spec/integration/configurer_spec.rb b/spec/integration/configurer_spec.rb
index 9a8b66fe4..780c9bbd0 100755
--- a/spec/integration/configurer_spec.rb
+++ b/spec/integration/configurer_spec.rb
@@ -5,6 +5,8 @@ require File.dirname(__FILE__) + '/../spec_helper'
 require 'puppet/configurer'
 
 describe Puppet::Configurer do
+  include PuppetSpec::Files
+
   describe "when downloading plugins" do
     it "should use the :pluginsignore setting, split on whitespace, for ignoring remote files" do
       resource = Puppet::Type.type(:notify).new :name => "yay"
@@ -17,19 +19,50 @@ describe Puppet::Configurer do
   end
 
   describe "when running" do
-    it "should send a transaction report with valid data" do
-      catalog = Puppet::Resource::Catalog.new
-      catalog.add_resource(Puppet::Type.type(:notify).new(:title => "testing"))
+    before(:each) do
+      @catalog = Puppet::Resource::Catalog.new
+      @catalog.add_resource(Puppet::Type.type(:notify).new(:title => "testing"))
 
-      configurer = Puppet::Configurer.new
+      # Make sure we don't try to persist the local state after the transaction ran, 
+      # because it will fail during test (the state file is in an not existing directory)
+      # and we need the transaction to be successful to be able to produce a summary report
+      @catalog.host_config = false
+
+      @configurer = Puppet::Configurer.new
+    end
+
+    it "should send a transaction report with valid data" do
 
+      @configurer.stubs(:save_last_run_summary)
       Puppet::Transaction::Report.indirection.expects(:save).with do |x, report|
         report.time.class == Time and report.logs.length > 0
       end
 
       Puppet[:report] = true
 
-      configurer.run :catalog => catalog
+      @configurer.run :catalog => @catalog
+    end
+
+    it "should save a correct last run summary" do
+      report = Puppet::Transaction::Report.new("apply")
+      report.stubs(:save)
+
+      Puppet[:lastrunfile] = tmpfile("lastrunfile")
+      Puppet[:report] = true
+
+      @configurer.run :catalog => @catalog, :report => report
+
+      summary = nil
+      File.open(Puppet[:lastrunfile], "r") do |fd|
+        summary = YAML.load(fd.read)
+      end
+
+      summary.should be_a(Hash)
+      %w{time changes events resources}.each do |key|
+        summary.should be_key(key)
+      end
+      summary["time"].should be_key("notify")
+      summary["time"]["last_run"].should >= Time.now.tv_sec
     end
   end
 end
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
index cf73a3706..a4b627c08 100755
--- a/spec/unit/configurer_spec.rb
+++ b/spec/unit/configurer_spec.rb
@@ -81,6 +81,7 @@ describe Puppet::Configurer, "when executing a catalog run" do
     @catalog = Puppet::Resource::Catalog.new
     @catalog.stubs(:apply)
     @agent.stubs(:retrieve_catalog).returns @catalog
+    @agent.stubs(:save_last_run_summary)
 
     Puppet::Util::Log.stubs(:newdestination)
     Puppet::Util::Log.stubs(:close)
@@ -225,9 +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
+    Puppet[:lastrunfile] = tmpfile('last_run_file')
 
     @report = Puppet::Transaction::Report.new("apply")
     @trans = stub 'transaction'
@@ -268,6 +272,20 @@ describe Puppet::Configurer, "when sending a report" do
     @configurer.send_report(@report, nil)
   end
 
+  it "should save the last run summary if reporting is enabled" do
+    Puppet.settings[:report] = true
+
+    @configurer.expects(:save_last_run_summary).with(@report)
+    @configurer.send_report(@report, nil)
+  end
+
+  it "should save the last run summary if reporting is disabled" do
+    Puppet.settings[:report] = false
+
+    @configurer.expects(:save_last_run_summary).with(@report)
+    @configurer.send_report(@report, nil)
+  end
+
   it "should log but not fail if saving the report fails" do
     Puppet.settings[:report] = true
 
@@ -278,6 +296,36 @@ describe Puppet::Configurer, "when sending a report" do
   end
 end
 
+describe Puppet::Configurer, "when saving the summary report file" do
+  before do
+    Puppet.settings.stubs(:use).returns(true)
+    @configurer = Puppet::Configurer.new
+
+    @report = stub 'report'
+    @trans = stub 'transaction'
+    @lastrunfd = stub 'lastrunfd'
+    Puppet::Util::FileLocking.stubs(:writelock).yields(@lastrunfd)
+  end
+
+  it "should write the raw summary to the lastrunfile setting value" do
+    Puppet::Util::FileLocking.expects(:writelock).with(Puppet[:lastrunfile], 0660)
+    @configurer.save_last_run_summary(@report)
+  end
+
+  it "should write the raw summary as yaml" do
+    @report.expects(:raw_summary).returns("summary")
+    @lastrunfd.expects(:print).with(YAML.dump("summary"))
+    @configurer.save_last_run_summary(@report)
+  end
+
+  it "should log but not fail if saving the last run summary fails" do
+    Puppet::Util::FileLocking.expects(:writelock).raises "exception"
+    Puppet.expects(:err)
+    lambda { @configurer.save_last_run_summary(@report) }.should_not raise_error
+  end
+
+end
+
 describe Puppet::Configurer, "when retrieving a catalog" do
   before do
     Puppet.settings.stubs(:use).returns(true)
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 766d4f14d..81efa340e 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -273,8 +273,19 @@ describe Puppet::Transaction::Report do
       @report.finalize_report
     end
 
-    %w{Changes Total Resources}.each do |main|
-      it "should include information on #{main} in the summary" do
+    %w{changes time resources events}.each do |main|
+      it "should include the key #{main} in the raw summary hash" do
+        @report.raw_summary.should be_key main
+      end
+    end
+
+    it "should include the last run time in the raw summary hash" do
+      Time.stubs(:now).returns(Time.utc(2010,11,10,12,0,24))
+      @report.raw_summary["time"]["last_run"].should == 1289390424
+    end
+
+    %w{Changes Total Resources Time Events}.each do |main|
+      it "should include information on #{main} in the textual summary" do
         @report.summary.should be_include(main)
       end
     end
-- 
cgit 


From 5d1cb02cd1ee509647a6dbe157152bd74d5e24ae Mon Sep 17 00:00:00 2001
From: Brice Figureau <brice-puppet@daysofwonder.com>
Date: Sat, 13 Nov 2010 11:41:18 +0100
Subject: Fix #4339 - Locally save the last report to $lastrunreport

Using the cache terminus system, when --report is on, we are now
caching the last report as a yaml file in the $lastrunreport file
(which by default is $statedir/last_run_report.yaml).

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>
---
 lib/puppet/application/agent.rb          |  2 ++
 lib/puppet/application/apply.rb          |  3 +++
 lib/puppet/defaults.rb                   |  4 ++++
 lib/puppet/indirector/report/yaml.rb     | 11 +++++++++
 spec/unit/application/agent_spec.rb      |  7 ++++++
 spec/unit/application/apply_spec.rb      |  6 +++++
 spec/unit/indirector/report/yaml_spec.rb | 38 ++++++++++++++++++++++++++++++++
 7 files changed, 71 insertions(+)
 create mode 100644 lib/puppet/indirector/report/yaml.rb
 create mode 100644 spec/unit/indirector/report/yaml_spec.rb

diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb
index 3749241f8..9f5921de1 100644
--- a/lib/puppet/application/agent.rb
+++ b/lib/puppet/application/agent.rb
@@ -229,6 +229,8 @@ class Puppet::Application::Agent < Puppet::Application
     Puppet::SSL::Host.ca_location = options[:fingerprint] ? :none : :remote
 
     Puppet::Transaction::Report.terminus_class = :rest
+    # we want the last report to be persisted locally
+    Puppet::Transaction::Report.cache_class = :yaml
 
     # Override the default; puppetd needs this, usually.
     # You can still override this on the command-line with, e.g., :compiler.
diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
index cc733e1f5..7f0b95a89 100644
--- a/lib/puppet/application/apply.rb
+++ b/lib/puppet/application/apply.rb
@@ -148,6 +148,9 @@ class Puppet::Application::Apply < Puppet::Application
       exit(1)
     end
 
+    # we want the last report to be persisted locally
+    Puppet::Transaction::Report.cache_class = :yaml
+
     if options[:debug]
       Puppet::Util::Log.level = :debug
     elsif options[:verbose]
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 23b3b6652..2a1ded592 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -611,6 +611,10 @@ module Puppet
       :mode => 0660,
       :desc => "Where puppet agent stores the last run report summary in yaml format."
     },
+    :lastrunreport =>  { :default => "$statedir/last_run_report.yaml",
+      :mode => 0660,
+      :desc => "Where puppet agent stores the last run report in yaml format."
+    },
     :graph => [false, "Whether to create dot graph files for the different
       configuration graphs.  These dot files can be interpreted by tools
       like OmniGraffle or dot (which is part of ImageMagick)."],
diff --git a/lib/puppet/indirector/report/yaml.rb b/lib/puppet/indirector/report/yaml.rb
new file mode 100644
index 000000000..bf7bf4fe5
--- /dev/null
+++ b/lib/puppet/indirector/report/yaml.rb
@@ -0,0 +1,11 @@
+require 'puppet/transaction/report'
+require 'puppet/indirector/yaml'
+
+class Puppet::Transaction::Report::Yaml < Puppet::Indirector::Yaml
+  desc "Store last report as a flat file, serialized using YAML."
+
+  # Force report to be saved there
+  def path(name,ext='.yaml')
+    Puppet[:lastrunreport]
+  end
+end
diff --git a/spec/unit/application/agent_spec.rb b/spec/unit/application/agent_spec.rb
index 8f498d4ba..c9d9a4509 100755
--- a/spec/unit/application/agent_spec.rb
+++ b/spec/unit/application/agent_spec.rb
@@ -179,6 +179,7 @@ describe Puppet::Application::Agent do
       Puppet.settings.stubs(:print_config)
       Puppet::SSL::Host.stubs(:ca_location=)
       Puppet::Transaction::Report.stubs(:terminus_class=)
+      Puppet::Transaction::Report.stubs(:cache_class=)
       Puppet::Resource::Catalog.stubs(:terminus_class=)
       Puppet::Resource::Catalog.stubs(:cache_class=)
       Puppet::Node::Facts.stubs(:terminus_class=)
@@ -309,6 +310,12 @@ describe Puppet::Application::Agent do
       @puppetd.setup
     end
 
+    it "should tell the report handler to cache locally as yaml" do
+      Puppet::Transaction::Report.expects(:cache_class=).with(:yaml)
+
+      @puppetd.setup
+    end
+
     it "should change the catalog_terminus setting to 'rest'" do
       Puppet.expects(:[]=).with(:catalog_terminus, :rest)
       @puppetd.setup
diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb
index d4f39abe0..67edd4ed7 100755
--- a/spec/unit/application/apply_spec.rb
+++ b/spec/unit/application/apply_spec.rb
@@ -56,6 +56,7 @@ describe Puppet::Application::Apply do
       Puppet.stubs(:parse_config)
       Puppet::FileBucket::Dipper.stubs(:new)
       STDIN.stubs(:read)
+      Puppet::Transaction::Report.stubs(:cache_class=)
 
       @apply.options.stubs(:[]).with(any_parameters)
     end
@@ -113,6 +114,11 @@ describe Puppet::Application::Apply do
       lambda { @apply.setup }.should raise_error(SystemExit)
     end
 
+    it "should tell the report handler to cache locally as yaml" do
+      Puppet::Transaction::Report.expects(:cache_class=).with(:yaml)
+
+      @apply.setup
+    end
   end
 
   describe "when executing" do
diff --git a/spec/unit/indirector/report/yaml_spec.rb b/spec/unit/indirector/report/yaml_spec.rb
new file mode 100644
index 000000000..610c9ae43
--- /dev/null
+++ b/spec/unit/indirector/report/yaml_spec.rb
@@ -0,0 +1,38 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+require 'puppet/transaction/report'
+require 'puppet/indirector/report/yaml'
+
+describe Puppet::Transaction::Report::Yaml do
+  it "should be a subclass of the Yaml terminus" do
+    Puppet::Transaction::Report::Yaml.superclass.should equal(Puppet::Indirector::Yaml)
+  end
+
+  it "should have documentation" do
+    Puppet::Transaction::Report::Yaml.doc.should_not be_nil
+  end
+
+  it "should be registered with the report indirection" do
+    indirection = Puppet::Indirector::Indirection.instance(:report)
+    Puppet::Transaction::Report::Yaml.indirection.should equal(indirection)
+  end
+
+  it "should have its name set to :yaml" do
+    Puppet::Transaction::Report::Yaml.name.should == :yaml
+  end
+
+  it "should inconditionnally save/load from the --lastrunreport setting" do
+    indirection = stub 'indirection', :name => :my_yaml, :register_terminus_type => nil
+    Puppet::Indirector::Indirection.stubs(:instance).with(:my_yaml).returns(indirection)
+    store_class = Class.new(Puppet::Transaction::Report::Yaml) do
+      def self.to_s
+        "MyYaml::MyType"
+      end
+    end
+    store = store_class.new
+
+    store.path(:me).should == Puppet[:lastrunreport]
+  end
+end
-- 
cgit 


From e20e6185f7f26d02c7ea275f8adf43c088169129 Mon Sep 17 00:00:00 2001
From: Max Martin <max@puppetlabs.com>
Date: Tue, 22 Mar 2011 18:36:01 -0700
Subject: (#5528) Add REST API for signing, revoking, retrieving, cleaning
 certs

This commit introduces a new Indirector terminus, certificate_status,
which allows for signing, revoking, listing, and cleaning
SSL certificates over HTTP via REST. Documentation for these new
features can be found in our REST API documentation on the docs site:

http://docs.puppetlabs.com/guides/rest_api.html

This documentation has not been updated as of the writing of this
commit, but will be very soon. Puppet::SSL::Host is now fully integrated
into the Indirector.

Paired-with:Matt Robinson, Jacob Helwig, Jesse Wolfe, Richard Crowley,
Luke Kanies
---
 lib/puppet/indirector/certificate_status.rb        |   4 +
 lib/puppet/indirector/certificate_status/file.rb   |  82 +++++++++
 lib/puppet/indirector/certificate_status/rest.rb   |  10 ++
 lib/puppet/network/http/api/v1.rb                  |   1 +
 lib/puppet/ssl/host.rb                             |  78 +++++++--
 .../indirector/certificate_status/file_spec.rb     | 188 ++++++++++++++++++++
 .../indirector/certificate_status/rest_spec.rb     |  15 ++
 spec/unit/ssl/host_spec.rb                         | 192 +++++++++++++++------
 8 files changed, 502 insertions(+), 68 deletions(-)
 create mode 100644 lib/puppet/indirector/certificate_status.rb
 create mode 100644 lib/puppet/indirector/certificate_status/file.rb
 create mode 100644 lib/puppet/indirector/certificate_status/rest.rb
 create mode 100644 spec/unit/indirector/certificate_status/file_spec.rb
 create mode 100644 spec/unit/indirector/certificate_status/rest_spec.rb

diff --git a/lib/puppet/indirector/certificate_status.rb b/lib/puppet/indirector/certificate_status.rb
new file mode 100644
index 000000000..47c3adcd4
--- /dev/null
+++ b/lib/puppet/indirector/certificate_status.rb
@@ -0,0 +1,4 @@
+require 'puppet/indirector'
+
+class Puppet::Indirector::CertificateStatus
+end
diff --git a/lib/puppet/indirector/certificate_status/file.rb b/lib/puppet/indirector/certificate_status/file.rb
new file mode 100644
index 000000000..9061d9423
--- /dev/null
+++ b/lib/puppet/indirector/certificate_status/file.rb
@@ -0,0 +1,82 @@
+require 'puppet'
+require 'puppet/indirector/certificate_status'
+require 'puppet/ssl/certificate'
+require 'puppet/ssl/certificate_authority'
+require 'puppet/ssl/certificate_request'
+require 'puppet/ssl/host'
+require 'puppet/ssl/key'
+
+class Puppet::Indirector::CertificateStatus::File < Puppet::Indirector::Code
+  def ca
+    raise ArgumentError, "This process is not configured as a certificate authority" unless Puppet::SSL::CertificateAuthority.ca?
+    Puppet::SSL::CertificateAuthority.new
+  end
+
+  def destroy(request)
+    deleted = []
+    [
+      Puppet::SSL::Certificate,
+      Puppet::SSL::CertificateRequest,
+      Puppet::SSL::Key,
+    ].collect do |part|
+      if part.indirection.destroy(request.key)
+        deleted << "#{part}"
+      end
+    end
+
+    return "Nothing was deleted" if deleted.empty?
+    "Deleted for #{request.key}: #{deleted.join(", ")}"
+  end
+
+  def save(request)
+    if request.instance.desired_state == "signed"
+      certificate_request = Puppet::SSL::CertificateRequest.indirection.find(request.key)
+      raise Puppet::Error, "Cannot sign for host #{request.key} without a certificate request" unless certificate_request
+      ca.sign(request.key)
+    elsif request.instance.desired_state == "revoked"
+      certificate = Puppet::SSL::Certificate.indirection.find(request.key)
+      raise Puppet::Error, "Cannot revoke host #{request.key} because has it doesn't have a signed certificate" unless certificate
+      ca.revoke(request.key)
+    else
+      raise Puppet::Error, "State #{request.instance.desired_state} invalid; Must specify desired state of 'signed' or 'revoked' for host #{request.key}"
+    end
+
+  end
+
+  def search(request)
+    # Support historic interface wherein users provide classes to filter
+    # the search.  When used via the REST API, the arguments must be
+    # a Symbol or an Array containing Symbol objects.
+    klasses = case request.options[:for]
+    when Class
+      [request.options[:for]]
+    when nil
+      [
+        Puppet::SSL::Certificate,
+        Puppet::SSL::CertificateRequest,
+        Puppet::SSL::Key,
+      ]
+    else
+      [request.options[:for]].flatten.map do |klassname|
+        indirection.class.model(klassname.to_sym)
+      end
+    end
+
+    klasses.collect do |klass|
+      klass.indirection.search(request.key, request.options)
+    end.flatten.collect do |result|
+      result.name
+    end.uniq.collect &Puppet::SSL::Host.method(:new)
+  end
+
+  def find(request)
+    ssl_host = Puppet::SSL::Host.new(request.key)
+    public_key = Puppet::SSL::Certificate.indirection.find(request.key)
+
+    if ssl_host.certificate_request || public_key
+      ssl_host
+    else
+      nil
+    end
+  end
+end
diff --git a/lib/puppet/indirector/certificate_status/rest.rb b/lib/puppet/indirector/certificate_status/rest.rb
new file mode 100644
index 000000000..c53b663b5
--- /dev/null
+++ b/lib/puppet/indirector/certificate_status/rest.rb
@@ -0,0 +1,10 @@
+require 'puppet/ssl/host'
+require 'puppet/indirector/rest'
+require 'puppet/indirector/certificate_status'
+
+class Puppet::Indirector::CertificateStatus::Rest < Puppet::Indirector::REST
+  desc "Sign, revoke, search for, or clean certificates & certificate requests over HTTP."
+
+  use_server_setting(:ca_server)
+  use_port_setting(:ca_port)
+end
diff --git a/lib/puppet/network/http/api/v1.rb b/lib/puppet/network/http/api/v1.rb
index dcb0e0a22..5fe143979 100644
--- a/lib/puppet/network/http/api/v1.rb
+++ b/lib/puppet/network/http/api/v1.rb
@@ -61,6 +61,7 @@ module Puppet::Network::HTTP::API::V1
     # that leads to the fix being too long.
     return :singular if indirection == "facts"
     return :singular if indirection == "status"
+    return :singular if indirection == "certificate_status"
     return :plural if indirection == "inventory"
 
     result = (indirection =~ /s$|_search$/) ? :plural : :singular
diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb
index 7f71ced99..b9215effd 100644
--- a/lib/puppet/ssl/host.rb
+++ b/lib/puppet/ssl/host.rb
@@ -1,3 +1,4 @@
+require 'puppet/indirector'
 require 'puppet/ssl'
 require 'puppet/ssl/key'
 require 'puppet/ssl/certificate'
@@ -15,11 +16,17 @@ class Puppet::SSL::Host
   CertificateRequest = Puppet::SSL::CertificateRequest
   CertificateRevocationList = Puppet::SSL::CertificateRevocationList
 
+  extend Puppet::Indirector
+  indirects :certificate_status, :terminus_class => :file
+
   attr_reader :name
   attr_accessor :ca
 
   attr_writer :key, :certificate, :certificate_request
 
+  # This accessor is used in instances for indirector requests to hold desired state
+  attr_accessor :desired_state
+
   class << self
     include Puppet::Util::Cacher
 
@@ -47,6 +54,13 @@ class Puppet::SSL::Host
     CertificateRequest.indirection.terminus_class = terminus
     CertificateRevocationList.indirection.terminus_class = terminus
 
+    host_map = {:ca => :file, :file => nil, :rest => :rest}
+    if term = host_map[terminus]
+      self.indirection.terminus_class = term
+    else
+      self.indirection.reset_terminus_class
+    end
+
     if cache
       # This is weird; we don't actually cache our keys, we
       # use what would otherwise be the cache as our normal
@@ -85,30 +99,34 @@ class Puppet::SSL::Host
 
   # Specify how we expect to interact with our certificate authority.
   def self.ca_location=(mode)
-    raise ArgumentError, "CA Mode can only be #{CA_MODES.collect { |m| m.to_s }.join(", ")}" unless CA_MODES.include?(mode)
+    modes = CA_MODES.collect { |m, vals| m.to_s }.join(", ")
+    raise ArgumentError, "CA Mode can only be one of: #{modes}" unless CA_MODES.include?(mode)
 
     @ca_location = mode
 
     configure_indirection(*CA_MODES[@ca_location])
   end
 
-  # Remove all traces of a given host
+  # Puppet::SSL::Host is actually indirected now so the original implementation
+  # has been moved into the certificate_status indirector.  This method is in-use
+  # in `puppet cert -c <certname>`.
   def self.destroy(name)
-    [Key, Certificate, CertificateRequest].collect { |part| part.indirection.destroy(name) }.any? { |x| x }
+    indirection.destroy(name)
   end
 
-  # Search for more than one host, optionally only specifying
-  # an interest in hosts with a given file type.
-  # This just allows our non-indirected class to have one of
-  # indirection methods.
-  def self.search(options = {})
-    classlist = [options[:for] || [Key, CertificateRequest, Certificate]].flatten
-
-    # Collect the results from each class, flatten them, collect all of the names, make the name list unique,
-    # then create a Host instance for each one.
-    classlist.collect { |klass| klass.indirection.search }.flatten.collect { |r| r.name }.uniq.collect do |name|
-      new(name)
+  def self.from_pson(pson)
+    instance = new(pson["name"])
+    if pson["desired_state"]
+      instance.desired_state = pson["desired_state"]
     end
+    instance
+  end
+
+  # Puppet::SSL::Host is actually indirected now so the original implementation
+  # has been moved into the certificate_status indirector.  This method does not
+  # appear to be in use in `puppet cert -l`.
+  def self.search(options = {})
+    indirection.search("*", options)
   end
 
   # Is this a ca host, meaning that all of its files go in the CA location?
@@ -221,6 +239,24 @@ class Puppet::SSL::Host
     @ssl_store
   end
 
+  def to_pson(*args)
+    my_cert = Puppet::SSL::Certificate.indirection.find(name)
+    pson_hash = { :name  => name }
+
+    my_state = state
+
+    pson_hash[:state] = my_state
+    pson_hash[:desired_state] = desired_state if desired_state
+
+    if my_state == 'requested'
+      pson_hash[:fingerprint] = certificate_request.fingerprint
+    else
+      pson_hash[:fingerprint] = my_cert.fingerprint
+    end
+
+    pson_hash.to_pson(*args)
+  end
+
   # Attempt to retrieve a cert, if we don't already have one.
   def wait_for_cert(time)
     begin
@@ -257,6 +293,20 @@ class Puppet::SSL::Host
       end
     end
   end
+
+  def state
+    my_cert = Puppet::SSL::Certificate.indirection.find(name)
+    if certificate_request
+      return 'requested'
+    end
+
+    begin
+      Puppet::SSL::CertificateAuthority.new.verify(my_cert)
+      return 'signed'
+    rescue Puppet::SSL::CertificateAuthority::CertificateVerificationError
+      return 'revoked'
+    end
+  end
 end
 
 require 'puppet/ssl/certificate_authority'
diff --git a/spec/unit/indirector/certificate_status/file_spec.rb b/spec/unit/indirector/certificate_status/file_spec.rb
new file mode 100644
index 000000000..6cc0bb547
--- /dev/null
+++ b/spec/unit/indirector/certificate_status/file_spec.rb
@@ -0,0 +1,188 @@
+#!/usr/bin/env ruby
+
+require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper.rb')
+require 'puppet/ssl/host'
+require 'puppet/indirector/certificate_status'
+require 'tempfile'
+
+describe "Puppet::Indirector::CertificateStatus::File" do
+  include PuppetSpec::Files
+
+  before do
+    Puppet::SSL::CertificateAuthority.stubs(:ca?).returns true
+    @terminus = Puppet::SSL::Host.indirection.terminus(:file)
+
+    @tmpdir = tmpdir("certificate_status_ca_testing")
+    Puppet[:confdir] = @tmpdir
+    Puppet[:vardir] = @tmpdir
+
+    # localcacert is where each client stores the CA certificate
+    # cacert is where the master stores the CA certificate
+    # Since we need to play the role of both for testing we need them to be the same and exist
+    Puppet[:cacert] = Puppet[:localcacert]
+  end
+
+  def generate_csr(host)
+    host.generate_key
+    csr = Puppet::SSL::CertificateRequest.new(host.name)
+    csr.generate(host.key.content)
+    Puppet::SSL::CertificateRequest.indirection.save(csr)
+  end
+
+  def sign_csr(host)
+    host.desired_state = "signed"
+    @terminus.save(Puppet::Indirector::Request.new(:certificate_status, :save, host.name, host))
+  end
+
+  def generate_signed_cert(host)
+    generate_csr(host)
+    sign_csr(host)
+
+    @terminus.find(Puppet::Indirector::Request.new(:certificate_status, :find, host.name, host))
+  end
+
+  def generate_revoked_cert(host)
+    generate_signed_cert(host)
+
+    host.desired_state = "revoked"
+
+    @terminus.save(Puppet::Indirector::Request.new(:certificate_status, :save, host.name, host))
+  end
+
+  it "should be a terminus on SSL::Host" do
+    @terminus.should be_instance_of(Puppet::Indirector::CertificateStatus::File)
+  end
+
+  it "should create a CA instance if none is present" do
+    @terminus.ca.should be_instance_of(Puppet::SSL::CertificateAuthority)
+  end
+
+  describe "when creating the CA" do
+    it "should fail if it is not a valid CA" do
+      Puppet::SSL::CertificateAuthority.expects(:ca?).returns false
+      lambda { @terminus.ca }.should raise_error(ArgumentError, "This process is not configured as a certificate authority")
+    end
+  end
+
+  it "should be indirected with the name 'certificate_status'" do
+    Puppet::SSL::Host.indirection.name.should == :certificate_status
+  end
+
+  describe "when finding" do
+    before do
+      @host = Puppet::SSL::Host.new("foo")
+      Puppet.settings.use(:main)
+    end
+
+    it "should return the Puppet::SSL::Host when a CSR exists for the host" do
+      generate_csr(@host)
+      request = Puppet::Indirector::Request.new(:certificate_status, :find, "foo", @host)
+
+      retrieved_host = @terminus.find(request)
+
+      retrieved_host.name.should == @host.name
+      retrieved_host.certificate_request.content.to_s.chomp.should == @host.certificate_request.content.to_s.chomp
+    end
+
+    it "should return the Puppet::SSL::Host when a public key exist for the host" do
+      generate_signed_cert(@host)
+      request = Puppet::Indirector::Request.new(:certificate_status, :find, "foo", @host)
+
+      retrieved_host = @terminus.find(request)
+
+      retrieved_host.name.should == @host.name
+      retrieved_host.certificate.content.to_s.chomp.should == @host.certificate.content.to_s.chomp
+    end
+
+    it "should return nil when neither a CSR nor public key exist for the host" do
+      request = Puppet::Indirector::Request.new(:certificate_status, :find, "foo", @host)
+      @terminus.find(request).should == nil
+    end
+  end
+
+  describe "when saving" do
+    before do
+      @host = Puppet::SSL::Host.new("foobar")
+      Puppet.settings.use(:main)
+    end
+
+    describe "when signing a cert" do
+      before do
+        @host.desired_state = "signed"
+        @request = Puppet::Indirector::Request.new(:certificate_status, :save, "foobar", @host)
+      end
+
+      it "should fail if no CSR is on disk" do
+        lambda { @terminus.save(@request) }.should raise_error(Puppet::Error, /certificate request/)
+      end
+
+      it "should sign the on-disk CSR when it is present" do
+        signed_host = generate_signed_cert(@host)
+
+        signed_host.state.should == "signed"
+        Puppet::SSL::Certificate.indirection.find("foobar").should be_instance_of(Puppet::SSL::Certificate)
+      end
+    end
+
+    describe "when revoking a cert" do
+      before do
+        @request = Puppet::Indirector::Request.new(:certificate_status, :save, "foobar", @host)
+      end
+
+      it "should fail if no certificate is on disk" do
+        @host.desired_state = "revoked"
+        lambda { @terminus.save(@request) }.should raise_error(Puppet::Error, /Cannot revoke/)
+      end
+
+      it "should revoke the certificate when it is present" do
+        generate_revoked_cert(@host)
+
+        @host.state.should == 'revoked'
+      end
+    end
+  end
+
+  describe "when deleting" do
+    before do
+      Puppet.settings.use(:main)
+    end
+
+    it "should not delete anything if no certificate, request, or key is on disk" do
+      host = Puppet::SSL::Host.new("clean_me")
+      request = Puppet::Indirector::Request.new(:certificate_status, :delete, "clean_me", host)
+      @terminus.destroy(request).should == "Nothing was deleted"
+    end
+
+    it "should clean certs, cert requests, keys" do
+      signed_host = Puppet::SSL::Host.new("clean_signed_cert")
+      generate_signed_cert(signed_host)
+      signed_request = Puppet::Indirector::Request.new(:certificate_status, :delete, "clean_signed_cert", signed_host)
+      @terminus.destroy(signed_request).should == "Deleted for clean_signed_cert: Puppet::SSL::Certificate, Puppet::SSL::Key"
+
+      requested_host = Puppet::SSL::Host.new("clean_csr")
+      generate_csr(requested_host)
+      csr_request = Puppet::Indirector::Request.new(:certificate_status, :delete, "clean_csr", requested_host)
+      @terminus.destroy(csr_request).should == "Deleted for clean_csr: Puppet::SSL::CertificateRequest, Puppet::SSL::Key"
+    end
+  end
+
+  describe "when searching" do
+    it "should return a list of all hosts with certificate requests, signed certs, or revoked certs" do
+      Puppet.settings.use(:main)
+
+      signed_host = Puppet::SSL::Host.new("signed_host")
+      generate_signed_cert(signed_host)
+
+      requested_host = Puppet::SSL::Host.new("requested_host")
+      generate_csr(requested_host)
+
+      revoked_host = Puppet::SSL::Host.new("revoked_host")
+      generate_revoked_cert(revoked_host)
+
+      retrieved_hosts = @terminus.search(Puppet::Indirector::Request.new(:certificate_status, :search, "all", signed_host))
+
+      results = retrieved_hosts.map {|h| [h.name, h.state]}.sort{ |h,i| h[0] <=> i[0] }
+      results.should == [["ca","signed"],["requested_host","requested"],["revoked_host","revoked"],["signed_host","signed"]]
+    end
+  end
+end
diff --git a/spec/unit/indirector/certificate_status/rest_spec.rb b/spec/unit/indirector/certificate_status/rest_spec.rb
new file mode 100644
index 000000000..f44eac671
--- /dev/null
+++ b/spec/unit/indirector/certificate_status/rest_spec.rb
@@ -0,0 +1,15 @@
+#!/usr/bin/env ruby
+
+require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper.rb')
+require 'puppet/ssl/host'
+require 'puppet/indirector/certificate_status'
+
+describe "Puppet::CertificateStatus::Rest" do
+  before do
+    @terminus = Puppet::SSL::Host.indirection.terminus(:rest)
+  end
+
+  it "should be a terminus on Puppet::SSL::Host" do
+    @terminus.should be_instance_of(Puppet::Indirector::CertificateStatus::Rest)
+  end
+end
diff --git a/spec/unit/ssl/host_spec.rb b/spec/unit/ssl/host_spec.rb
index d8f15e738..885bd45e2 100755
--- a/spec/unit/ssl/host_spec.rb
+++ b/spec/unit/ssl/host_spec.rb
@@ -3,16 +3,19 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
 
 require 'puppet/ssl/host'
+require 'puppet/sslcertificates'
+require 'puppet/sslcertificates/ca'
 
 describe Puppet::SSL::Host do
   before do
-    @class = Puppet::SSL::Host
-    @host = @class.new("myname")
+    Puppet::SSL::Host.indirection.terminus_class = :file
+    @host = Puppet::SSL::Host.new("myname")
   end
 
   after do
     # Cleaned out any cached localhost instance.
     Puppet::Util::Cacher.expire
+    Puppet::SSL::Host.ca_location = :none
   end
 
   it "should use any provided name as its name" do
@@ -140,13 +143,6 @@ describe Puppet::SSL::Host do
   end
 
   describe "when specifying the CA location" do
-    before do
-      [Puppet::SSL::Key, Puppet::SSL::Certificate, Puppet::SSL::CertificateRequest, Puppet::SSL::CertificateRevocationList].each do |klass|
-        klass.indirection.stubs(:terminus_class=)
-        klass.indirection.stubs(:cache_class=)
-      end
-    end
-
     it "should support the location ':local'" do
       lambda { Puppet::SSL::Host.ca_location = :local }.should_not raise_error
     end
@@ -168,80 +164,88 @@ describe Puppet::SSL::Host do
     end
 
     describe "as 'local'" do
-      it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest as :file" do
-        Puppet::SSL::Certificate.indirection.expects(:cache_class=).with :file
-        Puppet::SSL::CertificateRequest.indirection.expects(:cache_class=).with :file
-        Puppet::SSL::CertificateRevocationList.indirection.expects(:cache_class=).with :file
-
+      before do
         Puppet::SSL::Host.ca_location = :local
       end
 
-      it "should set the terminus class for Key as :file" do
-        Puppet::SSL::Key.indirection.expects(:terminus_class=).with :file
+      it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest as :file" do
+        Puppet::SSL::Certificate.indirection.cache_class.should == :file
+        Puppet::SSL::CertificateRequest.indirection.cache_class.should == :file
+        Puppet::SSL::CertificateRevocationList.indirection.cache_class.should == :file
+      end
 
-        Puppet::SSL::Host.ca_location = :local
+      it "should set the terminus class for Key and Host as :file" do
+        Puppet::SSL::Key.indirection.terminus_class.should == :file
+        Puppet::SSL::Host.indirection.terminus_class.should == :file
       end
 
       it "should set the terminus class for Certificate, CertificateRevocationList, and CertificateRequest as :ca" do
-        Puppet::SSL::Certificate.indirection.expects(:terminus_class=).with :ca
-        Puppet::SSL::CertificateRequest.indirection.expects(:terminus_class=).with :ca
-        Puppet::SSL::CertificateRevocationList.indirection.expects(:terminus_class=).with :ca
-
-        Puppet::SSL::Host.ca_location = :local
+        Puppet::SSL::Certificate.indirection.terminus_class.should == :ca
+        Puppet::SSL::CertificateRequest.indirection.terminus_class.should == :ca
+        Puppet::SSL::CertificateRevocationList.indirection.terminus_class.should == :ca
       end
     end
 
     describe "as 'remote'" do
-      it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest as :file" do
-        Puppet::SSL::Certificate.indirection.expects(:cache_class=).with :file
-        Puppet::SSL::CertificateRequest.indirection.expects(:cache_class=).with :file
-        Puppet::SSL::CertificateRevocationList.indirection.expects(:cache_class=).with :file
-
+      before do
         Puppet::SSL::Host.ca_location = :remote
       end
 
-      it "should set the terminus class for Key as :file" do
-        Puppet::SSL::Key.indirection.expects(:terminus_class=).with :file
-
-        Puppet::SSL::Host.ca_location = :remote
+      it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest as :file" do
+        Puppet::SSL::Certificate.indirection.cache_class.should == :file
+        Puppet::SSL::CertificateRequest.indirection.cache_class.should == :file
+        Puppet::SSL::CertificateRevocationList.indirection.cache_class.should == :file
       end
 
-      it "should set the terminus class for Certificate, CertificateRevocationList, and CertificateRequest as :rest" do
-        Puppet::SSL::Certificate.indirection.expects(:terminus_class=).with :rest
-        Puppet::SSL::CertificateRequest.indirection.expects(:terminus_class=).with :rest
-        Puppet::SSL::CertificateRevocationList.indirection.expects(:terminus_class=).with :rest
+      it "should set the terminus class for Key as :file" do
+        Puppet::SSL::Key.indirection.terminus_class.should == :file
+      end
 
-        Puppet::SSL::Host.ca_location = :remote
+      it "should set the terminus class for Host, Certificate, CertificateRevocationList, and CertificateRequest as :rest" do
+        Puppet::SSL::Host.indirection.terminus_class.should == :rest
+        Puppet::SSL::Certificate.indirection.terminus_class.should == :rest
+        Puppet::SSL::CertificateRequest.indirection.terminus_class.should == :rest
+        Puppet::SSL::CertificateRevocationList.indirection.terminus_class.should == :rest
       end
     end
 
     describe "as 'only'" do
-      it "should set the terminus class for Key, Certificate, CertificateRevocationList, and CertificateRequest as :ca" do
-        Puppet::SSL::Key.indirection.expects(:terminus_class=).with :ca
-        Puppet::SSL::Certificate.indirection.expects(:terminus_class=).with :ca
-        Puppet::SSL::CertificateRequest.indirection.expects(:terminus_class=).with :ca
-        Puppet::SSL::CertificateRevocationList.indirection.expects(:terminus_class=).with :ca
-
+      before do
         Puppet::SSL::Host.ca_location = :only
       end
 
-      it "should reset the cache class for Certificate, CertificateRevocationList, and CertificateRequest to nil" do
-        Puppet::SSL::Certificate.indirection.expects(:cache_class=).with nil
-        Puppet::SSL::CertificateRequest.indirection.expects(:cache_class=).with nil
-        Puppet::SSL::CertificateRevocationList.indirection.expects(:cache_class=).with nil
+      it "should set the terminus class for Key, Certificate, CertificateRevocationList, and CertificateRequest as :ca" do
+        Puppet::SSL::Key.indirection.terminus_class.should == :ca
+        Puppet::SSL::Certificate.indirection.terminus_class.should == :ca
+        Puppet::SSL::CertificateRequest.indirection.terminus_class.should == :ca
+        Puppet::SSL::CertificateRevocationList.indirection.terminus_class.should == :ca
+      end
 
-        Puppet::SSL::Host.ca_location = :only
+      it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest to nil" do
+        Puppet::SSL::Certificate.indirection.cache_class.should be_nil
+        Puppet::SSL::CertificateRequest.indirection.cache_class.should be_nil
+        Puppet::SSL::CertificateRevocationList.indirection.cache_class.should be_nil
+      end
+
+      it "should set the terminus class for Host to :file" do
+        Puppet::SSL::Host.indirection.terminus_class.should == :file
       end
     end
 
     describe "as 'none'" do
+      before do
+        Puppet::SSL::Host.ca_location = :none
+      end
+
       it "should set the terminus class for Key, Certificate, CertificateRevocationList, and CertificateRequest as :file" do
-        Puppet::SSL::Key.indirection.expects(:terminus_class=).with :file
-        Puppet::SSL::Certificate.indirection.expects(:terminus_class=).with :file
-        Puppet::SSL::CertificateRequest.indirection.expects(:terminus_class=).with :file
-        Puppet::SSL::CertificateRevocationList.indirection.expects(:terminus_class=).with :file
+        Puppet::SSL::Key.indirection.terminus_class.should == :file
+        Puppet::SSL::Certificate.indirection.terminus_class.should == :file
+        Puppet::SSL::CertificateRequest.indirection.terminus_class.should == :file
+        Puppet::SSL::CertificateRevocationList.indirection.terminus_class.should == :file
+      end
 
-        Puppet::SSL::Host.ca_location = :none
+      it "should set the terminus class for Host to 'none'" do
+        lambda { Puppet::SSL::Host.indirection.terminus_class }.should raise_error(Puppet::DevError)
       end
     end
   end
@@ -271,8 +275,8 @@ describe Puppet::SSL::Host do
       Puppet::SSL::Host.destroy("myhost").should be_true
     end
 
-    it "should return false if none of the classes returned true" do
-      Puppet::SSL::Host.destroy("myhost").should be_false
+    it "should report that nothing was deleted if none of the classes returned true" do
+      Puppet::SSL::Host.destroy("myhost").should == "Nothing was deleted"
     end
   end
 
@@ -709,4 +713,84 @@ describe Puppet::SSL::Host do
       @host.wait_for_cert(1)
     end
   end
+
+  describe "when handling PSON" do
+    include PuppetSpec::Files
+
+    before do
+      Puppet[:vardir] = tmpdir("ssl_test_vardir")
+      Puppet[:ssldir] = tmpdir("ssl_test_ssldir")
+      Puppet::SSLCertificates::CA.new.mkrootcert
+      # localcacert is where each client stores the CA certificate
+      # cacert is where the master stores the CA certificate
+      # Since we need to play the role of both for testing we need them to be the same and exist
+      Puppet[:cacert] = Puppet[:localcacert]
+
+      @ca=Puppet::SSL::CertificateAuthority.new
+    end
+
+    describe "when converting to PSON" do
+      it "should be able to identify a host with an unsigned certificate request" do
+        host = Puppet::SSL::Host.new("bazinga")
+        host.generate_certificate_request
+        pson_hash = {
+          "fingerprint"          => host.certificate_request.fingerprint,
+          "desired_state"        => 'requested',
+          "name"                 => host.name
+        }
+
+        result = PSON.parse(Puppet::SSL::Host.new(host.name).to_pson)
+        result["fingerprint"].should == pson_hash["fingerprint"]
+        result["name"].should        == pson_hash["name"]
+        result["state"].should       == pson_hash["desired_state"]
+      end
+
+      it "should be able to identify a host with a signed certificate" do
+        host = Puppet::SSL::Host.new("bazinga")
+        host.generate_certificate_request
+        @ca.sign(host.name)
+        pson_hash = {
+          "fingerprint"          => Puppet::SSL::Certificate.indirection.find(host.name).fingerprint,
+          "desired_state"        => 'signed',
+          "name"                 => host.name,
+        }
+
+        result = PSON.parse(Puppet::SSL::Host.new(host.name).to_pson)
+        result["fingerprint"].should == pson_hash["fingerprint"]
+        result["name"].should        == pson_hash["name"]
+        result["state"].should       == pson_hash["desired_state"]
+      end
+
+      it "should be able to identify a host with a revoked certificate" do
+        host = Puppet::SSL::Host.new("bazinga")
+        host.generate_certificate_request
+        @ca.sign(host.name)
+        @ca.revoke(host.name)
+        pson_hash = {
+          "fingerprint"          => Puppet::SSL::Certificate.indirection.find(host.name).fingerprint,
+          "desired_state"        => 'revoked',
+          "name"                 => host.name,
+        }
+
+        result = PSON.parse(Puppet::SSL::Host.new(host.name).to_pson)
+        result["fingerprint"].should == pson_hash["fingerprint"]
+        result["name"].should        == pson_hash["name"]
+        result["state"].should       == pson_hash["desired_state"]
+      end
+    end
+
+    describe "when converting from PSON" do
+      it "should return a Puppet::SSL::Host object with the specified desired state" do
+        host = Puppet::SSL::Host.new("bazinga")
+        host.desired_state="signed"
+        pson_hash = {
+          "name"  => host.name,
+          "desired_state" => host.desired_state,
+        }
+        generated_host = Puppet::SSL::Host.from_pson(pson_hash)
+        generated_host.desired_state.should == host.desired_state
+        generated_host.name.should == host.name
+      end
+    end
+  end
 end
-- 
cgit 


From 2a6c6cb8fabf82d2f2127c90db670c1a856427c5 Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Mon, 4 Apr 2011 12:13:53 -0700
Subject: (5200) -- replace containers with sentinals

This commit removes the last remaining use of topsort (in SimpleGraph#splice!) by
fixing #5200 in a way that is compatible with graph fontiers.  Instead of replacing
containers with many-to-many relationships, we now replace them with a pair of
sentinals (whits) that bracket them.

Thus a graph consisting of two containers, each containing ten resources, and a
dependency between the containers, which would have gone from 21 edges to 100
edges will instead have only 43, and a graph consisting of two containers (e.g.
stages) each containing a similar graph, which would have gone from 45 edges to
400 will only go to 95.

This change had minor consequences on many parts of the system and required lots
of small changes for consistancy, but the core of it is in Catelog#splice! (which
replaces SimpleGraph#splice!) and Transaction#eval_generate.  Everything else is
just adjustments to the fact that some one-step edges are now two-step edges and
tests, event propagation, etc. need to reflect that.

Paired-with: Jesse Wolfe
---
 lib/puppet/resource/catalog.rb                     |  64 +++++++-
 lib/puppet/simple_graph.rb                         | 109 ++++----------
 lib/puppet/transaction.rb                          |  57 +++----
 lib/puppet/transaction/event_manager.rb            |   5 +-
 lib/puppet/type/whit.rb                            |   8 +-
 .../unit/parser/functions/create_resources_spec.rb |  36 ++---
 spec/unit/resource/catalog_spec.rb                 |   2 +-
 spec/unit/simple_graph_spec.rb                     | 165 +++++++++++++--------
 spec/unit/transaction_spec.rb                      |   2 +-
 spec/unit/type/whit_spec.rb                        |   4 +-
 test/lib/puppettest/support/assertions.rb          |   7 +-
 test/lib/puppettest/support/utils.rb               |  19 ---
 test/other/relationships.rb                        |  27 ++--
 test/other/transactions.rb                         |  35 +----
 test/ral/type/file/target.rb                       |  23 +--
 15 files changed, 280 insertions(+), 283 deletions(-)

diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index fed0f46da..98c29657e 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -331,13 +331,75 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
       @relationship_graph.write_graph(:relationships) if host_config?
 
       # Then splice in the container information
-      @relationship_graph.splice!(self, Puppet::Type::Component)
+      splice!(@relationship_graph)
 
       @relationship_graph.write_graph(:expanded_relationships) if host_config?
     end
     @relationship_graph
   end
 
+  # Impose our container information on another graph by using it 
+  # to replace any container vertices X with a pair of verticies 
+  # { admissible_X and completed_X } such that that
+  #
+  #    0) completed_X depends on admissible_X
+  #    1) contents of X each depend on admissible_X
+  #    2) completed_X depends on each on the contents of X
+  #    3) everything which depended on X depens on completed_X
+  #    4) admissible_X depends on everything X depended on
+  #    5) the containers and their edges must be removed
+  #
+  # Note that this requires attention to the possible case of containers
+  # which contain or depend on other containers, but has the advantage
+  # that the number of new edges created scales linearly with the number
+  # of contained verticies regardless of how containers are related; 
+  # alternatives such as replacing container-edges with content-edges 
+  # scale as the product of the number of external dependences, which is
+  # to say geometrically in the case of nested / chained containers.
+  #
+  Default_label = { :callback => :refresh, :event => :ALL_EVENTS }
+  def splice!(other)
+    stage_class      = Puppet::Type.type(:stage)
+    whit_class       = Puppet::Type.type(:whit)
+    component_class  = Puppet::Type.type(:component)
+    containers = vertices.find_all { |v| (v.is_a?(component_class) or v.is_a?(stage_class)) and vertex?(v) }
+    #
+    # These two hashes comprise the aforementioned attention to the possible
+    #   case of containers that contain / depend on other containers; they map
+    #   containers to their sentinals but pass other verticies through.  Thus we
+    #   can "do the right thing" for references to other verticies that may or
+    #   may not be containers.
+    #
+    admissible = Hash.new { |h,k| k }
+    completed  = Hash.new { |h,k| k }
+    containers.each { |x|
+      admissible[x] = whit_class.new(:name => "admissible_#{x.name}", :catalog => self)
+      completed[x]  = whit_class.new(:name => "completed_#{x.name}",  :catalog => self)
+    }
+    #
+    # Implement the six requierments listed above
+    #
+    containers.each { |x|
+      contents = adjacent(x, :direction => :out)
+      other.add_edge(admissible[x],completed[x]) if contents.empty? # (0)
+      contents.each { |v|
+        other.add_edge(admissible[x],admissible[v],Default_label) # (1)
+        other.add_edge(completed[v], completed[x], Default_label) # (2)
+      }
+      # (3) & (5)
+      other.adjacent(x,:direction => :in,:type => :edges).each { |e|
+        other.add_edge(completed[e.source],admissible[x],e.label)
+        other.remove_edge! e
+      }
+      # (4) & (5)
+      other.adjacent(x,:direction => :out,:type => :edges).each { |e|
+        other.add_edge(completed[x],admissible[e.target],e.label)
+        other.remove_edge! e
+      }
+    }
+    containers.each { |x| other.remove_vertex! x } # (5)
+  end
+
   # Remove the resource from our catalog.  Notice that we also call
   # 'remove' on the resource, at least until resource classes no longer maintain
   # references to the resource instances.
diff --git a/lib/puppet/simple_graph.rb b/lib/puppet/simple_graph.rb
index 31858f136..671eef150 100644
--- a/lib/puppet/simple_graph.rb
+++ b/lib/puppet/simple_graph.rb
@@ -19,7 +19,7 @@ class Puppet::SimpleGraph
   #
   # This class is intended to be used with DAGs.  However, if the
   # graph has a cycle, it will not cause non-termination of any of the
-  # algorithms.  The topsort method detects and reports cycles.
+  # algorithms.  
   #
   def initialize
     @in_to = {}
@@ -221,6 +221,7 @@ class Puppet::SimpleGraph
   def report_cycles_in_graph
     cycles = find_cycles_in_graph
     n = cycles.length           # where is "pluralize"? --daniel 2011-01-22
+    return if n == 0
     s = n == 1 ? '' : 's'
 
     message = "Found #{n} dependency cycle#{s}:\n"
@@ -262,36 +263,6 @@ class Puppet::SimpleGraph
     return filename
   end
 
-  # Provide a topological sort.
-  def topsort
-    degree = {}
-    zeros = []
-    result = []
-
-    # Collect each of our vertices, with the number of in-edges each has.
-    vertices.each do |v|
-      edges = @in_to[v]
-      zeros << v if edges.empty?
-      degree[v] = edges.length
-    end
-
-    # Iterate over each 0-degree vertex, decrementing the degree of
-    # each of its out-edges.
-    while v = zeros.pop
-      result << v
-      @out_from[v].each { |v2,es|
-        zeros << v2 if (degree[v2] -= 1) == 0
-      }
-    end
-
-    # If we have any vertices left with non-zero in-degrees, then we've found a cycle.
-    if cycles = degree.values.reject { |ns| ns == 0  } and cycles.length > 0
-      report_cycles_in_graph
-    end
-
-    result
-  end
-
   # Add a new vertex to the graph.
   def add_vertex(vertex)
     @in_to[vertex]    ||= {}
@@ -368,52 +339,6 @@ class Puppet::SimpleGraph
     (options[:type] == :edges) ? ns.values.flatten : ns.keys
   end
 
-  # Take container information from another graph and use it
-  # to replace any container vertices with their respective leaves.
-  # This creates direct relationships where there were previously
-  # indirect relationships through the containers.
-  def splice!(other, type)
-    # We have to get the container list via a topological sort on the
-    # configuration graph, because otherwise containers that contain
-    # other containers will add those containers back into the
-    # graph.  We could get a similar affect by only setting relationships
-    # to container leaves, but that would result in many more
-    # relationships.
-    stage_class = Puppet::Type.type(:stage)
-    whit_class  = Puppet::Type.type(:whit)
-    containers = other.topsort.find_all { |v| (v.is_a?(type) or v.is_a?(stage_class)) and vertex?(v) }
-    containers.each do |container|
-      # Get the list of children from the other graph.
-      children = other.adjacent(container, :direction => :out)
-
-      # MQR TODO: Luke suggests that it should be possible to refactor the system so that
-      #           container nodes are retained, thus obviating the need for the whit.
-      children = [whit_class.new(:name => container.name, :catalog => other)] if children.empty?
-
-      # First create new edges for each of the :in edges
-      [:in, :out].each do |dir|
-        edges = adjacent(container, :direction => dir, :type => :edges)
-        edges.each do |edge|
-          children.each do |child|
-            if dir == :in
-              s = edge.source
-              t = child
-            else
-              s = child
-              t = edge.target
-            end
-
-            add_edge(s, t, edge.label)
-          end
-
-          # Now get rid of the edge, so remove_vertex! works correctly.
-          remove_edge!(edge)
-        end
-      end
-      remove_vertex!(container)
-    end
-  end
-
   # Just walk the tree and pass each edge.
   def walk(source, direction)
     # Use an iterative, breadth-first traversal of the graph. One could do
@@ -454,7 +379,7 @@ class Puppet::SimpleGraph
   end
 
   def direct_dependents_of(v)
-    @out_from[v].keys 
+    (@out_from[v] || {}).keys 
   end
 
   def upstream_from_vertex(v)
@@ -468,7 +393,33 @@ class Puppet::SimpleGraph
   end
 
   def direct_dependencies_of(v)
-    @in_to[v].keys 
+    (@in_to[v] || {}).keys 
+  end
+
+  # Return an array of the edge-sets between a series of n+1 vertices (f=v0,v1,v2...t=vn)
+  #   connecting the two given verticies.  The ith edge set is an array containing all the
+  #   edges between v(i) and v(i+1); these are (by definition) never empty.
+  #
+  #     * if f == t, the list is empty
+  #     * if they are adjacent the result is an array consisting of
+  #       a single array (the edges from f to t)
+  #     * and so on by induction on a vertex m between them
+  #     * if there is no path from f to t, the result is nil
+  #
+  # This implementation is not particularly efficient; it's used in testing where clarity
+  #   is more important than last-mile efficiency. 
+  #
+  def path_between(f,t)
+    if f==t
+      []
+    elsif direct_dependents_of(f).include?(t)
+      [edges_between(f,t)]
+    elsif dependents(f).include?(t)
+      m = (dependents(f) & direct_dependencies_of(t)).first
+      path_between(f,m) + path_between(m,t)
+    else
+      nil
+    end
   end
 
   # LAK:FIXME This is just a paste of the GRATR code with slight modifications.
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index c502fc627..8118178c3 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -65,13 +65,13 @@ class Puppet::Transaction
 
   # Copy an important relationships from the parent to the newly-generated
   # child resource.
-  def make_parent_child_relationship(parent, child)
+  def add_conditional_directed_dependency(parent, child, label=nil)
     relationship_graph.add_vertex(child)
     edge = parent.depthfirst? ? [child, parent] : [parent, child]
     if relationship_graph.edge?(*edge.reverse)
       parent.debug "Skipping automatic relationship to #{child}"
     else
-      relationship_graph.add_edge(*edge)
+      relationship_graph.add_edge(edge[0],edge[1],label)
     end
   end
 
@@ -141,66 +141,65 @@ class Puppet::Transaction
   end
 
   def eval_generate(resource)
-    return [] unless resource.respond_to?(:eval_generate)
+    raise Puppet::DevError,"Depthfirst resources are not supported by eval_generate" if resource.depthfirst?
     begin
-      made = resource.eval_generate
+      made = resource.eval_generate.uniq.reverse
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       resource.err "Failed to generate additional resources using 'eval_generate: #{detail}"
+      return
     end
-    parents = [resource]
-    [made].flatten.compact.uniq.each do |res|
+    made.each do |res|
       begin
         res.tag(*resource.tags)
         @catalog.add_resource(res)
         res.finish
-        make_parent_child_relationship(parents.reverse.find { |r| r.name == res.name[0,r.name.length]}, res)
-        parents << res
       rescue Puppet::Resource::Catalog::DuplicateResourceError
         res.info "Duplicate generated resource; skipping"
       end
     end
+    sentinal = Puppet::Type::Whit.new(:name => "completed_#{resource.title}", :catalog => resource.catalog)
+    relationship_graph.adjacent(resource,:direction => :out,:type => :edges).each { |e|
+      add_conditional_directed_dependency(sentinal, e.target, e.label)
+      relationship_graph.remove_edge! e
+    }
+    default_label = Puppet::Resource::Catalog::Default_label
+    made.each do |res|
+      add_conditional_directed_dependency(made.find { |r| r != res && r.name == res.name[0,r.name.length]} || resource, res)
+      add_conditional_directed_dependency(res, sentinal, default_label)
+    end
+    add_conditional_directed_dependency(resource, sentinal, default_label)
   end
 
   # A general method for recursively generating new resources from a
   # resource.
   def generate_additional_resources(resource)
-    return [] unless resource.respond_to?(:generate)
+    return unless resource.respond_to?(:generate)
     begin
       made = resource.generate
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       resource.err "Failed to generate additional resources using 'generate': #{detail}"
     end
-    return [] unless made
+    return unless made
     made = [made] unless made.is_a?(Array)
-    made.uniq.find_all do |res|
+    made.uniq.each do |res|
       begin
         res.tag(*resource.tags)
         @catalog.add_resource(res)
         res.finish
-        make_parent_child_relationship(resource, res)
+        add_conditional_directed_dependency(resource, res)
         generate_additional_resources(res)
-        true
       rescue Puppet::Resource::Catalog::DuplicateResourceError
         res.info "Duplicate generated resource; skipping"
-        false
       end
     end
   end
 
   # Collect any dynamically generated resources.  This method is called
   # before the transaction starts.
-  def generate
-    list = @catalog.vertices
-    newlist = []
-    while ! list.empty?
-      list.each do |resource|
-        newlist += generate_additional_resources(resource)
-      end
-      list = newlist
-      newlist = []
-    end
+  def xgenerate
+    @catalog.vertices.each { |resource| generate_additional_resources(resource) }
   end
 
   # Should we ignore tags?
@@ -246,7 +245,7 @@ class Puppet::Transaction
   # Prepare to evaluate the resources in a transaction.
   def prepare
     # Now add any dynamically generated resources
-    generate
+    xgenerate
 
     # Then prefetch.  It's important that we generate and then prefetch,
     # so that any generated resources also get prefetched.
@@ -285,10 +284,11 @@ class Puppet::Transaction
     end
     def add_vertex(v)
       real_graph.add_vertex(v)
+      check_if_now_ready(v) # ?????????????????????????????????????????
     end
-    def add_edge(f,t)
+    def add_edge(f,t,label=nil)
       ready.delete(t)
-      real_graph.add_edge(f,t)
+      real_graph.add_edge(f,t,label)
     end
     def check_if_now_ready(r)
       ready[r] = true if direct_dependencies_of(r).all? { |r2| done[r2] }
@@ -297,8 +297,9 @@ class Puppet::Transaction
       ready.keys.sort_by { |r0| unguessable_deterministic_key[r0] }.first
     end
     def traverse(&block)
+      real_graph.report_cycles_in_graph
       while (r = next_resource) && !transaction.stop_processing?
-        if !generated[r]
+        if !generated[r] && r.respond_to?(:eval_generate)
           transaction.eval_generate(r)
           generated[r] = true
         else
diff --git a/lib/puppet/transaction/event_manager.rb b/lib/puppet/transaction/event_manager.rb
index 3ebb0a9d3..a21bbf892 100644
--- a/lib/puppet/transaction/event_manager.rb
+++ b/lib/puppet/transaction/event_manager.rb
@@ -31,7 +31,7 @@ class Puppet::Transaction::EventManager
   # Queue events for other resources to respond to.  All of these events have
   # to be from the same resource.
   def queue_events(resource, events)
-    @events += events
+    #@events += events
 
     # Do some basic normalization so we're not doing so many
     # graph queries for large sets of events.
@@ -47,12 +47,15 @@ class Puppet::Transaction::EventManager
       # Collect the targets of any subscriptions to those events.  We pass
       # the parent resource in so it will override the source in the events,
       # since eval_generated children can't have direct relationships.
+      received = (event.name != :restarted)
       relationship_graph.matching_edges(event, resource).each do |edge|
+        received ||= true unless edge.target.is_a?(Puppet::Type::Whit)
         next unless method = edge.callback
         next unless edge.target.respond_to?(method)
 
         queue_events_for_resource(resource, edge.target, method, list)
       end
+      @events << event if received
 
       queue_events_for_resource(resource, resource, :refresh, [event]) if resource.self_refresh? and ! resource.deleting?
     end
diff --git a/lib/puppet/type/whit.rb b/lib/puppet/type/whit.rb
index 55bfcfb46..55ed0386e 100644
--- a/lib/puppet/type/whit.rb
+++ b/lib/puppet/type/whit.rb
@@ -6,6 +6,12 @@ Puppet::Type.newtype(:whit) do
   end
 
   def to_s
-    "Class[#{name}]"
+    "(#{name})"
+  end
+
+  def refresh
+    # We don't do anything with them, but we need this to
+    #   show that we are "refresh aware" and not break the
+    #   chain of propogation.
   end
 end
diff --git a/spec/unit/parser/functions/create_resources_spec.rb b/spec/unit/parser/functions/create_resources_spec.rb
index d4095b777..366fb536c 100755
--- a/spec/unit/parser/functions/create_resources_spec.rb
+++ b/spec/unit/parser/functions/create_resources_spec.rb
@@ -51,11 +51,12 @@ describe 'function for dynamically creating resources' do
     it 'should be able to add edges' do
       @scope.function_create_resources(['notify', {'foo'=>{'require' => 'Notify[test]'}}])
       @scope.compiler.compile
-      edge = @scope.compiler.catalog.to_ral.relationship_graph.edges.detect do |edge|
-        edge.source.title == 'test'
-      end
-      edge.source.title.should == 'test'
-      edge.target.title.should == 'foo'
+      rg = @scope.compiler.catalog.to_ral.relationship_graph
+      test  = rg.vertices.find { |v| v.title == 'test' }
+      foo   = rg.vertices.find { |v| v.title == 'foo' }
+      test.should be
+      foo.should be
+      rg.path_between(test,foo).should be
     end
   end
   describe 'when dynamically creating resource types' do
@@ -93,11 +94,13 @@ notify{test:}
     it 'should be able to add edges' do
       @scope.function_create_resources(['foo', {'blah'=>{'one'=>'two', 'require' => 'Notify[test]'}}])
       @scope.compiler.compile
-      edge = @scope.compiler.catalog.to_ral.relationship_graph.edges.detect do |edge|
-        edge.source.title == 'test'
-      end
-      edge.source.title.should == 'test'
-      edge.target.title.should == 'blah'
+      rg = @scope.compiler.catalog.to_ral.relationship_graph
+      test = rg.vertices.find { |v| v.title == 'test' }
+      blah = rg.vertices.find { |v| v.title == 'blah' }
+      test.should be
+      blah.should be
+      # (Yoda speak like we do)
+      rg.path_between(test,blah).should be
       @compiler.catalog.resource(:notify, "blah")['message'].should == 'two'
     end
   end
@@ -123,13 +126,12 @@ notify{tester:}
     it 'should be able to add edges' do
       @scope.function_create_resources(['class', {'bar'=>{'one'=>'two', 'require' => 'Notify[tester]'}}])
       @scope.compiler.compile
-      edge = @scope.compiler.catalog.to_ral.relationship_graph.edges.detect do |e|
-        e.source.title == 'tester'
-      end
-      edge.source.title.should == 'tester'
-      edge.target.title.should == 'test'
-      #@compiler.catalog.resource(:notify, "blah")['message'].should == 'two'
+      rg = @scope.compiler.catalog.to_ral.relationship_graph
+      test   = rg.vertices.find { |v| v.title == 'test' }
+      tester = rg.vertices.find { |v| v.title == 'tester' }
+      test.should be
+      tester.should be
+      rg.path_between(tester,test).should be
     end
-
   end
 end
diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb
index 411d10b32..78d1b3223 100755
--- a/spec/unit/resource/catalog_spec.rb
+++ b/spec/unit/resource/catalog_spec.rb
@@ -734,7 +734,7 @@ describe Puppet::Resource::Catalog, "when compiling" do
     end
 
     it "should copy component relationships to all contained resources" do
-      @relationships.edge?(@one, @two).should be_true
+      @relationships.path_between(@one, @two).should be
     end
 
     it "should add automatic relationships to the relationship graph" do
diff --git a/spec/unit/simple_graph_spec.rb b/spec/unit/simple_graph_spec.rb
index c106f550b..99db2a55c 100755
--- a/spec/unit/simple_graph_spec.rb
+++ b/spec/unit/simple_graph_spec.rb
@@ -267,7 +267,7 @@ describe Puppet::SimpleGraph do
     end
   end
 
-  describe "when sorting the graph" do
+  describe "when reporting cycles in the graph" do
     before do
       @graph = Puppet::SimpleGraph.new
     end
@@ -278,29 +278,24 @@ describe Puppet::SimpleGraph do
       end
     end
 
-    it "should sort the graph topologically" do
-      add_edges :a => :b, :b => :c
-      @graph.topsort.should == [:a, :b, :c]
-    end
-
     it "should fail on two-vertex loops" do
       add_edges :a => :b, :b => :a
-      proc { @graph.topsort }.should raise_error(Puppet::Error)
+      proc { @graph.report_cycles_in_graph }.should raise_error(Puppet::Error)
     end
 
     it "should fail on multi-vertex loops" do
       add_edges :a => :b, :b => :c, :c => :a
-      proc { @graph.topsort }.should raise_error(Puppet::Error)
+      proc { @graph.report_cycles_in_graph }.should raise_error(Puppet::Error)
     end
 
     it "should fail when a larger tree contains a small cycle" do
       add_edges :a => :b, :b => :a, :c => :a, :d => :c
-      proc { @graph.topsort }.should raise_error(Puppet::Error)
+      proc { @graph.report_cycles_in_graph }.should raise_error(Puppet::Error)
     end
 
     it "should succeed on trees with no cycles" do
       add_edges :a => :b, :b => :e, :c => :a, :d => :c
-      proc { @graph.topsort }.should_not raise_error
+      proc { @graph.report_cycles_in_graph }.should_not raise_error
     end
 
     it "should produce the correct relationship text" do
@@ -308,7 +303,7 @@ describe Puppet::SimpleGraph do
       # cycle detection starts from a or b randomly
       # so we need to check for either ordering in the error message
       want = %r{Found 1 dependency cycle:\n\((a => b => a|b => a => b)\)\nTry}
-      expect { @graph.topsort }.to raise_error(Puppet::Error, want)
+      expect { @graph.report_cycles_in_graph }.to raise_error(Puppet::Error, want)
     end
 
     it "cycle discovery should be the minimum cycle for a simple graph" do
@@ -405,17 +400,6 @@ describe Puppet::SimpleGraph do
       paths = @graph.paths_in_cycle(cycles.first, 21)
       paths.length.should be == 20
     end
-
-    # Our graph's add_edge method is smart enough not to add
-    # duplicate edges, so we use the objects, which it doesn't
-    # check.
-    it "should be able to sort graphs with duplicate edges" do
-      one = Puppet::Relationship.new(:a, :b)
-      @graph.add_edge(one)
-      two = Puppet::Relationship.new(:a, :b)
-      @graph.add_edge(two)
-      proc { @graph.topsort }.should_not raise_error
-    end
   end
 
   describe "when writing dot files" do
@@ -521,7 +505,7 @@ describe Puppet::SimpleGraph do
 
   require 'puppet/util/graph'
 
-  class Container
+  class Container < Puppet::Type::Component
     include Puppet::Util::Graph
     include Enumerable
     attr_accessor :name
@@ -543,6 +527,7 @@ describe Puppet::SimpleGraph do
     end
   end
 
+  require "puppet/resource/catalog"
   describe "when splicing the graph" do
     def container_graph
       @one = Container.new("one", %w{a b})
@@ -555,13 +540,21 @@ describe Puppet::SimpleGraph do
       @whit  = Puppet::Type.type(:whit)
       @stage = Puppet::Type.type(:stage).new(:name => "foo")
 
-      @contgraph = @top.to_graph
+      @contgraph = @top.to_graph(Puppet::Resource::Catalog.new)
 
       # We have to add the container to the main graph, else it won't
       # be spliced in the dependency graph.
       @contgraph.add_vertex(@empty)
     end
 
+    def containers
+      @contgraph.vertices.select { |x| !x.is_a? String }
+    end
+
+    def contents_of(x)
+      @contgraph.direct_dependents_of(x)
+    end
+
     def dependency_graph
       @depgraph = Puppet::SimpleGraph.new
       @contgraph.vertices.each do |v|
@@ -570,13 +563,34 @@ describe Puppet::SimpleGraph do
 
       # We have to specify a relationship to our empty container, else it
       # never makes it into the dep graph in the first place.
-      {@one => @two, "f" => "c", "h" => @middle, "c" => @empty}.each do |source, target|
+      @explicit_dependencies = {@one => @two, "f" => "c", "h" => @middle, "c" => @empty}
+      @explicit_dependencies.each do |source, target|
         @depgraph.add_edge(source, target, :callback => :refresh)
       end
     end
 
     def splice
-      @depgraph.splice!(@contgraph, Container)
+      @contgraph.splice!(@depgraph)
+    end
+
+    def whit_called(name)
+      x = @depgraph.vertices.find { |v| v.is_a?(@whit) && v.name =~ /#{name}/ }
+      x.should_not be_nil
+      def x.to_s
+        "Whit[#{name}]"
+      end
+      def x.inspect
+        to_s
+      end
+      x
+    end
+
+    def admissible_sentinal_of(x)
+      @depgraph.vertex?(x) ? x : whit_called("admissible_#{x.name}")
+    end
+
+    def completed_sentinal_of(x)
+      @depgraph.vertex?(x) ? x : whit_called("completed_#{x.name}")
     end
 
     before do
@@ -585,63 +599,87 @@ describe Puppet::SimpleGraph do
       splice
     end
 
-    # This is the real heart of splicing -- replacing all containers in
-    # our relationship and exploding their relationships so that each
-    # relationship to a container gets copied to all of its children.
+    # This is the real heart of splicing -- replacing all containers X in our
+    # relationship graph with a pair of whits { admissible_X and completed_X }
+    # such that that
+    #
+    #    0) completed_X depends on admissible_X
+    #    1) contents of X each depend on admissible_X
+    #    2) completed_X depends on each on the contents of X
+    #    3) everything which depended on X depends on completed_X
+    #    4) admissible_X depends on everything X depended on
+    #    5) the containers and their edges must be removed
+    #
+    # Note that this requires attention to the possible case of containers
+    # which contain or depend on other containers.
+    #
+    # Point by point:
+
+    #    0) completed_X depends on admissible_X
+    #
+    it "every container's completed sentinal should depend on its admissible sentinal" do
+      containers.each { |container| 
+        @depgraph.path_between(admissible_sentinal_of(container),completed_sentinal_of(container)).should be
+      }
+    end
+
+    #    1) contents of X each depend on admissible_X
+    #
+    it "all contained objects should depend on their container's admissible sentinal" do
+      containers.each { |container| 
+        contents_of(container).each { |leaf|
+          @depgraph.should be_edge(admissible_sentinal_of(container),admissible_sentinal_of(leaf))
+        }
+      }
+    end
+
+    #    2) completed_X depends on each on the contents of X
+    #
+    it "completed sentinals should depend on their container's contents" do
+      containers.each { |container| 
+        contents_of(container).each { |leaf|
+          @depgraph.should be_edge(completed_sentinal_of(leaf),completed_sentinal_of(container))
+        }
+      }
+    end
+
+    #
+    #    3) everything which depended on X depends on completed_X
+
+    #
+    #    4) admissible_X depends on everything X depended on
+
+    #    5) the containers and their edges must be removed
+    #
     it "should remove all Container objects from the dependency graph" do
       @depgraph.vertices.find_all { |v| v.is_a?(Container) }.should be_empty
     end
 
-    # This is a bit hideous, but required to make stages work with relationships - they're
-    # the top of the graph.
     it "should remove all Stage resources from the dependency graph" do
       @depgraph.vertices.find_all { |v| v.is_a?(Puppet::Type.type(:stage)) }.should be_empty
     end
 
-    it "should add container relationships to contained objects" do
-      @contgraph.leaves(@middle).each do |leaf|
-        @depgraph.should be_edge("h", leaf)
-      end
-    end
-
-    it "should explode container-to-container relationships, making edges between all respective contained objects" do
-      @one.each do |oobj|
-        @two.each do |tobj|
-          @depgraph.should be_edge(oobj, tobj)
-        end
-      end
-    end
-
-    it "should contain a whit-resource to mark the place held by the empty container" do
-      @depgraph.vertices.find_all { |v| v.is_a?(@whit) }.length.should == 1
-    end
-
-    it "should replace edges to empty containers with edges to their residual whit" do
-      emptys_whit = @depgraph.vertices.find_all { |v| v.is_a?(@whit) }.first
-      @depgraph.should be_edge("c", emptys_whit)
-    end
-
     it "should no longer contain anything but the non-container objects" do
       @depgraph.vertices.find_all { |v| ! v.is_a?(String) and ! v.is_a?(@whit)}.should be_empty
     end
 
-    it "should copy labels" do
-      @depgraph.edges.each do |edge|
-        edge.label.should == {:callback => :refresh}
-      end
+    it "should retain labels on non-containment edges" do
+      @explicit_dependencies.each { |f,t|
+        @depgraph.edges_between(completed_sentinal_of(f),admissible_sentinal_of(t))[0].label.should == {:callback => :refresh}
+      }
     end
 
     it "should not add labels to edges that have none" do
       @depgraph.add_edge(@two, @three)
       splice
-      @depgraph.edges_between("c", "i")[0].label.should == {}
+      @depgraph.path_between("c", "i").any? {|segment| segment.all? {|e| e.label == {} }}.should be
     end
 
     it "should copy labels over edges that have none" do
       @depgraph.add_edge("c", @three, {:callback => :refresh})
       splice
       # And make sure the label got copied.
-      @depgraph.edges_between("c", "i")[0].label.should == {:callback => :refresh}
+      @depgraph.path_between("c", "i").flatten.select {|e| e.label == {:callback => :refresh} }.should_not be_empty
     end
 
     it "should not replace a label with a nil label" do
@@ -649,7 +687,7 @@ describe Puppet::SimpleGraph do
       @depgraph.add_edge(@middle, @three)
       @depgraph.add_edge("c", @three, {:callback => :refresh})
       splice
-      @depgraph.edges_between("c", "i")[0].label.should == {:callback => :refresh}
+      @depgraph.path_between("c","i").flatten.select {|e| e.label == {:callback => :refresh} }.should_not be_empty
     end
 
     it "should copy labels to all created edges" do
@@ -658,8 +696,9 @@ describe Puppet::SimpleGraph do
       splice
       @three.each do |child|
         edge = Puppet::Relationship.new("c", child)
-        @depgraph.should be_edge(edge.source, edge.target)
-        @depgraph.edges_between(edge.source, edge.target)[0].label.should == {:callback => :refresh}
+        (path = @depgraph.path_between(edge.source, edge.target)).should be
+        path.should_not be_empty
+        path.flatten.select {|e| e.label == {:callback => :refresh} }.should_not be_empty
       end
     end
   end
diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb
index 8d40136fb..ab76130e3 100755
--- a/spec/unit/transaction_spec.rb
+++ b/spec/unit/transaction_spec.rb
@@ -223,7 +223,7 @@ describe Puppet::Transaction do
       resource.expects(:finish).never
       resource.expects(:info) # log that it's skipped
 
-      @transaction.generate_additional_resources(generator).should be_empty
+      @transaction.generate_additional_resources(generator)
     end
 
     it "should copy all tags to the newly generated resources" do
diff --git a/spec/unit/type/whit_spec.rb b/spec/unit/type/whit_spec.rb
index 46eb0ab6e..0a3324afa 100644
--- a/spec/unit/type/whit_spec.rb
+++ b/spec/unit/type/whit_spec.rb
@@ -5,7 +5,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
 whit = Puppet::Type.type(:whit).new(:name => "Foo::Bar")
 
 describe whit do
-  it "should stringify as though it were the class it represents" do
-    whit.to_s.should == "Class[Foo::Bar]"
+  it "should stringify in a way that users will regognise" do
+    whit.to_s.should == "(Foo::Bar)"
   end
 end
diff --git a/test/lib/puppettest/support/assertions.rb b/test/lib/puppettest/support/assertions.rb
index 31fa3f1da..758c126ce 100644
--- a/test/lib/puppettest/support/assertions.rb
+++ b/test/lib/puppettest/support/assertions.rb
@@ -46,7 +46,12 @@ module PuppetTest
     config = resources2catalog(*resources)
     transaction = Puppet::Transaction.new(config)
 
-    run_events(:evaluate, transaction, events, msg)
+    transaction.evaluate
+    newevents = transaction.events.
+      reject { |e| ['failure', 'audit'].include? e.status }.
+      collect { |e| e.name }
+
+    assert_equal(events, newevents, "Incorrect evaluate #{msg} events")
 
     transaction
   end
diff --git a/test/lib/puppettest/support/utils.rb b/test/lib/puppettest/support/utils.rb
index bca5d9634..4ecc3819e 100644
--- a/test/lib/puppettest/support/utils.rb
+++ b/test/lib/puppettest/support/utils.rb
@@ -82,25 +82,6 @@ module PuppetTest::Support::Utils
     @mygroup = group
   end
 
-  def run_events(type, trans, events, msg)
-    case type
-    when :evaluate, :rollback # things are hunky-dory
-    else
-      raise Puppet::DevError, "Incorrect run_events type"
-    end
-
-    method = type
-
-    trans.send(method)
-    newevents = trans.events.reject { |e| ['failure', 'audit'].include? e.status }.collect { |e|
-      e.name
-    }
-
-    assert_equal(events, newevents, "Incorrect #{type} #{msg} events")
-
-    trans
-  end
-
   def fakefile(name)
     ary = [basedir, "test"]
     ary += name.split("/")
diff --git a/test/other/relationships.rb b/test/other/relationships.rb
index 717353c02..e36dcda71 100755
--- a/test/other/relationships.rb
+++ b/test/other/relationships.rb
@@ -14,11 +14,8 @@ class TestRelationships < Test::Unit::TestCase
 
   def newfile
     assert_nothing_raised {
-
-            return Puppet::Type.type(:file).new(
-                
+      return Puppet::Type.type(:file).new(
         :path => tempfile,
-        
         :check => [:mode, :owner, :group]
       )
     }
@@ -58,18 +55,14 @@ class TestRelationships < Test::Unit::TestCase
   def test_autorequire
     # We know that execs autorequire their cwd, so we'll use that
     path = tempfile
-
-
-          file = Puppet::Type.type(:file).new(
-        :title => "myfile", :path => path,
-        
-      :ensure => :directory)
-
-          exec = Puppet::Type.newexec(
-        :title => "myexec", :cwd => path,
-        
-      :command => "/bin/echo")
-
+    file = Puppet::Type.type(:file).new(
+      :title => "myfile", :path => path,
+      :ensure => :directory
+    )
+    exec = Puppet::Type.newexec(
+      :title => "myexec", :cwd => path,
+      :command => "/bin/echo"
+    )
     catalog = mk_catalog(file, exec)
     reqs = nil
     assert_nothing_raised do
@@ -82,7 +75,7 @@ class TestRelationships < Test::Unit::TestCase
     # Now make sure that these relationships are added to the
     # relationship graph
     catalog.apply do |trans|
-      assert(catalog.relationship_graph.edge?(file, exec), "autorequire edge was not created")
+      assert(catalog.relationship_graph.path_between(file, exec), "autorequire edge was not created")
     end
   end
 
diff --git a/test/other/transactions.rb b/test/other/transactions.rb
index be8cef483..812e519ab 100755
--- a/test/other/transactions.rb
+++ b/test/other/transactions.rb
@@ -114,39 +114,6 @@ class TestTransactions < Test::Unit::TestCase
     assert_equal({inst.title => inst}, $prefetched, "evaluate did not call prefetch")
   end
 
-  # We need to generate resources before we prefetch them, else generated
-  # resources that require prefetching don't work.
-  def test_generate_before_prefetch
-    config = mk_catalog
-    trans = Puppet::Transaction.new(config)
-
-    generate = nil
-    prefetch = nil
-    trans.expects(:generate).with { |*args| generate = Time.now; true }
-    trans.expects(:prefetch).with { |*args| ! generate.nil? }
-    trans.prepare
-    return
-
-    resource = Puppet::Type.type(:file).new :ensure => :present, :path => tempfile
-    other_resource = mock 'generated'
-    def resource.generate
-      [other_resource]
-    end
-
-
-    config = mk_catalog(yay, rah)
-    trans = Puppet::Transaction.new(config)
-
-    assert_nothing_raised do
-      trans.generate
-    end
-
-    %w{ya ra y r}.each do |name|
-      assert(trans.catalog.vertex?(Puppet::Type.type(:generator)[name]), "Generated #{name} was not a vertex")
-      assert($finished.include?(name), "#{name} was not finished")
-    end
-  end
-
   def test_ignore_tags?
     config = Puppet::Resource::Catalog.new
     config.host_config = true
@@ -235,7 +202,7 @@ class TestTransactions < Test::Unit::TestCase
     config = mk_catalog(one, two)
     trans = Puppet::Transaction.new(config)
     assert_raise(Puppet::Error) do
-      trans.prepare
+      trans.evaluate
     end
   end
 
diff --git a/test/ral/type/file/target.rb b/test/ral/type/file/target.rb
index 272128586..d778f2891 100755
--- a/test/ral/type/file/target.rb
+++ b/test/ral/type/file/target.rb
@@ -24,12 +24,9 @@ class TestFileTarget < Test::Unit::TestCase
 
     file = nil
     assert_nothing_raised {
-
-            file = Puppet::Type.type(:file).new(
-                
+      file = Puppet::Type.type(:file).new(
         :title => "somethingelse",
         :ensure => path,
-        
         :path => link
       )
     }
@@ -102,12 +99,9 @@ class TestFileTarget < Test::Unit::TestCase
 
     link = nil
     assert_nothing_raised {
-
-            link = Puppet::Type.type(:file).new(
-                
+      link = Puppet::Type.type(:file).new(
         :ensure => source,
         :path => dest,
-        
         :recurse => true
       )
     }
@@ -140,11 +134,8 @@ class TestFileTarget < Test::Unit::TestCase
 
     link = nil
     assert_nothing_raised {
-
-            link = Puppet::Type.type(:file).new(
-                
+      link = Puppet::Type.type(:file).new(
         :path => dest,
-        
         :ensure => "source"
       )
     }
@@ -161,20 +152,16 @@ class TestFileTarget < Test::Unit::TestCase
 
     resources = []
 
-          resources << Puppet::Type.type(:exec).new(
-                
+    resources << Puppet::Type.type(:exec).new(
       :command => "mkdir #{source}; touch #{source}/file",
       :title => "yay",
-        
       :path => ENV["PATH"]
     )
 
-          resources << Puppet::Type.type(:file).new(
-                
+    resources << Puppet::Type.type(:file).new(
       :ensure => source,
       :path => dest,
       :recurse => true,
-        
       :require => resources[0]
     )
 
-- 
cgit 


From cab567274691e0f39e88d49eb503efbe58d5042c Mon Sep 17 00:00:00 2001
From: Pieter van de Bruggen <pieter@puppetlabs.com>
Date: Wed, 6 Apr 2011 16:37:34 -0700
Subject: (Maint) Fix uninitialized constant.

err: Could not apply complete catalog: Could not autoload group: uninitialized constant Puppet::Property::KeyValue

Encountered this while generating certificate requests via
Puppet Strings/Faces, which doesn't load the full Puppet stack
by default.

Paired-With: Matt Robinson
---
 lib/puppet/type/group.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/puppet/type/group.rb b/lib/puppet/type/group.rb
index aa96bd9c3..2573633f9 100755
--- a/lib/puppet/type/group.rb
+++ b/lib/puppet/type/group.rb
@@ -1,6 +1,7 @@
 
 require 'etc'
 require 'facter'
+require 'puppet/property/keyvalue'
 
 module Puppet
   newtype(:group) do
-- 
cgit 


From 99d78f2c15b6ccf543466d3fa2cdeda7eb3f9987 Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Fri, 25 Mar 2011 12:00:39 -0700
Subject: (#6490) Add plugin initialization callback system to core

The recurring pattern "drop-in code needs to have something done at startup"
is presently being solved with a variety of ad-hock mechanism.  This commit
adds a general, extensible, centralized system for adding such hooks and
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.  Applications can add more places
to look for plugins without risk of adding duplicates or changing the order of
ones that have already been found with:

    Puppet::Plugins.look_in(*paths)

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!  Hi everyone!!!"
        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.

Presently supported hooks:

    before_application_preinit( :application_object => ... )
    after_application_preinit( :application_object => ... )

    before_application_parse_options( :application_object => ... )
    after_application_parse_options( :application_object => ... )

    before_application_setup( :application_object => ... )
    after_application_setup( :application_object => ... )

    before_application_run_command( :application_object => ... )
    after_application_run_command( :application_object => ... )

    on_commandline_initialization(:command_line_object => ... )
    on_application_initialization(:appliation_object => ... )

Paired-with: Daniel Pitman
---
 lib/puppet/application.rb       | 16 ++++++--
 lib/puppet/util/command_line.rb |  7 +++-
 lib/puppet/util/plugins.rb      | 82 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+), 5 deletions(-)
 create mode 100644 lib/puppet/util/plugins.rb

diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb
index c3d7355f6..5e69baef4 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
@@ -297,11 +298,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
@@ -413,5 +414,12 @@ class Application
       $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/util/command_line.rb b/lib/puppet/util/command_line.rb
index 7f74d266a..fb56b2898 100644
--- a/lib/puppet/util/command_line.rb
+++ b/lib/puppet/util/command_line.rb
@@ -1,3 +1,5 @@
+require "puppet/util/plugins"
+
 module Puppet
   module Util
     class CommandLine
@@ -23,6 +25,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,7 +59,9 @@ 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
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
+
-- 
cgit 


From 1e4968e82a65b21ad5d075015830ef3f54efee05 Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Wed, 6 Apr 2011 20:25:45 -0700
Subject: (maint) Indentation fixes

---
 lib/puppet/application.rb       |  8 ++++----
 lib/puppet/util/command_line.rb | 20 +++++++++-----------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb
index 5e69baef4..a028a158f 100644
--- a/lib/puppet/application.rb
+++ b/lib/puppet/application.rb
@@ -408,11 +408,11 @@ 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)
diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb
index fb56b2898..52b5f81ef 100644
--- a/lib/puppet/util/command_line.rb
+++ b/lib/puppet/util/command_line.rb
@@ -4,8 +4,7 @@ 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',
@@ -15,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
@@ -68,14 +66,14 @@ module Puppet
       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
-- 
cgit