Almost all tests now pass. I have basically reached the point where I was before I integrated graphing, except that all of the relationship handling is now inside the transaction, and any kind of recursion (including file) is *tons* easier to model and manage.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1905 980ebf18-57e1-0310-9a29-db15c13687c0

5 files changed, 45 insertions, 16 deletions

diff --git a/lib/puppet/client/master.rb b/lib/puppet/client/master.rb
index 97d224065..fb7922937 100644
--- a/lib/puppet/client/master.rb
+++ b/lib/puppet/client/master.rb
@@ -125,20 +125,16 @@ class Puppet::Client::MasterClient < Puppet::Client
         ensure
             Puppet::Storage.store
         end
-
+        
         if Puppet[:report]
-            begin
-                report = transaction.report()
-                if Puppet[:rrdgraph] == true
-                    report.graph()
-                end
-                reportclient().report(report)
-            rescue => detail
-                Puppet.err "Reporting failed: %s" % detail
-            end
+            report(transaction)
         end
 
         return transaction
+    ensure
+        if defined? transaction and transaction
+            transaction.cleanup
+        end
     end
 
     # Cache the config
@@ -496,6 +492,8 @@ class Puppet::Client::MasterClient < Puppet::Client
 
     private
 
+    # Download files from the remote server, returning a list of all
+    # changed files.
     def self.download(args)
         objects = Puppet::Type.type(:component).create(
             :name => "#{args[:name]}_collector"
@@ -529,12 +527,16 @@ class Puppet::Client::MasterClient < Puppet::Client
 
         # Now source all of the changed objects, but only source those
         # that are top-level.
+        files = []
         trans.changed?.find_all do |object|
-            yield object
+            yield object if block_given?
+            files << object[:path]
         end
+        trans.cleanup
 
         # Now clean up after ourselves
         objects.remove
+        files
     end
 
     # Retrieve facts from the central server.
@@ -610,6 +612,19 @@ class Puppet::Client::MasterClient < Puppet::Client
             loaddir(dir, "fact")
         end
     end
+    
+    # Send off the transaction report.
+    def report(transaction)
+        begin
+            report = transaction.report()
+            if Puppet[:rrdgraph] == true
+                report.graph()
+            end
+            reportclient().report(report)
+        rescue => detail
+            Puppet.err "Reporting failed: %s" % detail
+        end
+    end
 
     def reportclient
         unless defined? @reportclient
diff --git a/lib/puppet/pgraph.rb b/lib/puppet/pgraph.rb
index bf156ed2d..d988ff36a 100644
--- a/lib/puppet/pgraph.rb
+++ b/lib/puppet/pgraph.rb
@@ -14,6 +14,13 @@ class Puppet::PGraph < GRATR::Digraph
     # This is the type used for splicing.
     attr_accessor :container_type
     
+    def clear
+        @vertex_dict.clear
+        if defined? @edge_number
+            @edge_number.clear
+        end
+    end
+    
     # The dependencies for a given resource.
     def dependencies(resource)
         tree_from_vertex(resource, :dfs).keys
diff --git a/lib/puppet/server/pelement.rb b/lib/puppet/server/pelement.rb
index 3001cd9a1..0c9537957 100755
--- a/lib/puppet/server/pelement.rb
+++ b/lib/puppet/server/pelement.rb
@@ -40,6 +40,9 @@ class Server::PElement < Server::Handler
         # And then apply the configuration.  This way we're reusing all
         # the code in there.  It should probably just be separated out, though.
         transaction = client.apply
+        
+        # And then clean up
+        component.remove
 
         # It'd be nice to return some kind of report, but... at this point
         # we have no such facility.
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index d6d1669a1..99b03b435 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -105,12 +105,16 @@ class Transaction
         }.uniq
     end
     
-    # Do any necessary cleanup.  Basically just removes any generated
-    # resources.
+    # Do any necessary cleanup.  If we don't get rid of the graphs, the
+    # contained resources might never get cleaned up.
     def cleanup
         @generated.each do |resource|
             resource.remove
         end
+        if defined? @relgraph
+            @relgraph.clear
+        end
+        @resources.clear
     end
     
     # See if the resource generates new resources at evaluation time.
@@ -210,8 +214,6 @@ class Transaction
             # And then close the transaction log.
             Puppet::Log.close(@report)
         end
-        
-        cleanup()
 
         Puppet.debug "Finishing transaction %s with %s changes" %
             [self.object_id, @count]
@@ -235,7 +237,6 @@ class Transaction
         # enough to check the immediate dependencies, which is why we use
         # a tree from the reversed graph.
         skip = false
-        resource.info "checking for failed deps"
         @relgraph.reversal.tree_from_vertex(resource, :dfs).keys.each do |dep|
             if fails = failed?(dep)
                 resource.notice "Dependency %s[%s] has %s failures" %
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 24f961a62..0d69ffdda 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -174,6 +174,9 @@ module Puppet
 
         # Autorequire any parent directories.
         autorequire(:file) do
+            unless self[:path]
+                raise "no path for %s" % self.ref
+            end
             File.dirname(self[:path])
         end