The configurations should now be functional again --

file recursion was previously not working, because
the relationship graph was setting itself as a resource's
primary configuration, which caused it to try creating its
own relationship graph.

I've now found that the current code is about 5x slower than
the released code, so now I hope to resolve that.

4 files changed, 43 insertions, 13 deletions

diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb
index f2bbfca00..1365767b6 100644
--- a/lib/puppet/node/configuration.rb
+++ b/lib/puppet/node/configuration.rb
@@ -33,6 +33,11 @@ class Puppet::Node::Configuration < Puppet::PGraph
     # that the host configuration needs.
     attr_accessor :host_config
 
+    # Whether this graph is another configuration's relationship graph.
+    # We don't want to accidentally create a relationship graph for another
+    # relationship graph.
+    attr_accessor :is_relationship_graph
+
     # Add classes to our class list.
     def add_class(*classes)
         classes.each do |klass|
@@ -56,7 +61,7 @@ class Puppet::Node::Configuration < Puppet::PGraph
             else
                 @resource_table[ref] = resource
             end
-            resource.configuration = self
+            resource.configuration = self unless is_relationship_graph
             add_vertex!(resource)
         end
     end
@@ -167,6 +172,9 @@ class Puppet::Node::Configuration < Puppet::PGraph
 
         @transient_resources << resource if applying?
         add_resource(resource)
+        if @relationship_graph
+            @relationship_graph.add_resource(resource) unless @relationship_graph.resource(resource.ref)
+        end
         resource
     end
 
@@ -273,6 +281,8 @@ class Puppet::Node::Configuration < Puppet::PGraph
     
     # Create a graph of all of the relationships in our configuration.
     def relationship_graph
+        raise(Puppet::DevError, "Tried get a relationship graph for a relationship graph") if self.is_relationship_graph
+
         unless defined? @relationship_graph and @relationship_graph
             # It's important that we assign the graph immediately, because
             # the debug messages below use the relationships in the
@@ -281,6 +291,7 @@ class Puppet::Node::Configuration < Puppet::PGraph
             # then we get into an infinite loop.
             @relationship_graph = Puppet::Node::Configuration.new
             @relationship_graph.host_config = host_config?
+            @relationship_graph.is_relationship_graph = true
             
             # First create the dependency graph
             self.vertices.each do |vertex|
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 3c72f6e97..e30159c24 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -202,12 +202,7 @@ class Transaction
             end
             
             if children
-                children.each do |child|
-                    child.finish
-                    # Make sure that the vertex is in the relationship graph.
-                    relationship_graph.add_resource(child) unless relationship_graph.resource(child.ref)
-                    child.configuration = relationship_graph
-                end
+                children.each { |child| child.finish }
                 @generated += children
                 return children
             end
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index f0a805525..73c60bd14 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -574,9 +574,8 @@ module Puppet
         # Create a new file or directory object as a child to the current
         # object.
         def newchild(path, local, hash = {})
-            unless configuration
-                raise Puppet::DevError, "File recursion cannot happen without a configuration"
-            end
+            raise(Puppet::DevError, "File recursion cannot happen without a configuration") unless configuration
+
             # make local copy of arguments
             args = symbolize_options(@arghash)
 
@@ -655,9 +654,7 @@ module Puppet
             # LAK:FIXME This shouldn't be necessary, but as long as we're
             # modeling the relationship graph specifically, it is.
             configuration.relationship_graph.add_edge! self, child
-            unless child.parent
-                raise "Did not set parent of %s" % child.ref
-            end
+
             return child
         end
 
diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index 5f807b9b6..e9dc6b85d 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -312,6 +312,17 @@ describe Puppet::Node::Configuration, " when functioning as a resource container
         @config.resource(@two.ref).should equal(@two)
     end
 
+    it "should set itself as the resource's configuration if it is not a relationship graph" do
+        @one.expects(:configuration=).with(@config)
+        @config.add_resource @one
+    end
+
+    it "should not set itself as the resource's configuration if it is a relationship graph" do
+        @one.expects(:configuration=).never
+        @config.is_relationship_graph = true
+        @config.add_resource @one
+    end
+
     it "should make all vertices available by resource reference" do
         @config.add_resource(@one)
         @config.resource(@one.ref).should equal(@one)
@@ -516,6 +527,10 @@ describe Puppet::Node::Configuration, " when creating a relationship graph" do
         @relationships = @config.relationship_graph
     end
 
+    it "should fail when trying to create a relationship graph for a relationship graph" do
+        proc { @relationships.relationship_graph }.should raise_error(Puppet::DevError)
+    end
+
     it "should be able to create a relationship graph" do
         @relationships.should be_instance_of(Puppet::Node::Configuration)
     end
@@ -578,6 +593,18 @@ describe Puppet::Node::Configuration, " when creating a relationship graph" do
         @config.resource("File[/yay]").should equal(resource)
     end
 
+    it "should add implicit resources to the relationship graph if there is one" do
+        args = {:name => "/yay", :ensure => :file}
+        resource = stub 'file', :ref => "File[/yay]", :configuration= => @config
+        resource.expects(:implicit=).with(true)
+        Puppet::Type.type(:file).expects(:create).with(args).returns(resource)
+        # build the graph
+        relgraph = @config.relationship_graph
+
+        @config.create_implicit_resource :file, args
+        relgraph.resource("File[/yay]").should equal(resource)
+    end
+
     it "should remove resources created mid-transaction" do
         args = {:name => "/yay", :ensure => :file}
         resource = stub 'file', :ref => "File[/yay]", :configuration= => @config