Fixing #387, hopefully.

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

3 files changed, 36 insertions, 33 deletions

diff --git a/lib/puppet/pgraph.rb b/lib/puppet/pgraph.rb
index 36df0cc96..bd023d576 100644
--- a/lib/puppet/pgraph.rb
+++ b/lib/puppet/pgraph.rb
@@ -94,38 +94,37 @@ class Puppet::PGraph < GRATR::Digraph
     # This creates direct relationships where there were previously
     # indirect relationships through the containers. 
     def splice!(other, type)
-        vertices.each do |vertex|
-            # Go through each vertex and replace the edges with edges
-            # to the leaves instead
-            next unless vertex.is_a?(type)
-            
-            #oleaves = other.leaves(vertex)
-            oleaves = other.adjacent(vertex, :direction => :out)
-            if oleaves.empty?
+        vertices.find_all { |v| v.is_a?(type) }.each do |vertex|
+            # Get the list of children from the other graph.
+            children = other.adjacent(vertex, :direction => :out)
+
+            # Just remove the container if it's empty.
+            if children.empty?
                 remove_vertex!(vertex)
                 next
             end
             
             # First create new edges for each of the :in edges
-            adjacent(vertex, :direction => :in, :type => :edges).each do |edge|
-                oleaves.each do |leaf|
-                    add_edge!(edge.source, leaf, edge.label)
-                    if cyclic?
-                        raise ArgumentError,
-                            "%s => %s results in a loop" %
-                            [edge.source, leaf]
-                    end
-                end
-            end
-            
-            # Then for each of the out edges
-            adjacent(vertex, :direction => :out, :type => :edges).each do |edge|
-                oleaves.each do |leaf|
-                    add_edge!(leaf, edge.target, edge.label)
-                    if cyclic?
-                        raise ArgumentError,
-                            "%s => %s results in a loop" %
-                            [leaf, edge.target]
+            [:in, :out].each do |dir|
+                adjacent(vertex, :direction => dir, :type => :edges).each do |edge|
+                    children.each do |leaf|
+                        if dir == :in
+                            s = edge.source
+                            t = leaf
+                        else
+                            s = leaf
+                            t = edge.target
+                        end
+
+                        # It *appears* that we're having a problem
+                        # with multigraphs.
+                        next if edge?(s, t)
+                        add_edge!(s, t, edge.label)
+                        if cyclic?
+                            raise ArgumentError,
+                                "%s => %s results in a loop" %
+                                [s, t]
+                        end
                     end
                 end
             end
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index d115fbfe7..88c498172 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -243,6 +243,10 @@ class Transaction
 
         begin
             allevents = @sorted_resources.collect { |resource|
+                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)
diff --git a/test/other/pgraph.rb b/test/other/pgraph.rb
index 5ee76bfe9..c83df93e4 100644..100755
--- a/test/other/pgraph.rb
+++ b/test/other/pgraph.rb
@@ -88,8 +88,8 @@ class TestPGraph < Test::Unit::TestCase
             deps.add_vertex(v)
         end
         
-        # We have to specify a relationship to our empty container, else it never makes it
-        # into the dep graph in the first place.
+        # 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|
             deps.add_edge!(source, target, :callback => :refresh)
         end
@@ -98,6 +98,10 @@ class TestPGraph < Test::Unit::TestCase
         
         assert(! deps.cyclic?, "Created a cyclic graph")
         
+        # Make sure there are no container objects remaining
+        c = deps.vertices.find_all { |v| v.is_a?(Container) }
+        assert(c.empty?, "Still have containers %s" % c.inspect)
+        
         # Now make sure the containers got spliced correctly.
         contgraph.leaves(middle).each do |leaf|
             assert(deps.edge?("h", leaf), "no edge for h => %s" % leaf)
@@ -108,10 +112,6 @@ class TestPGraph < Test::Unit::TestCase
             end
         end
         
-        # Make sure there are no container objects remaining
-        c = deps.vertices.find_all { |v| v.is_a?(Container) }
-        assert(c.empty?, "Still have containers %s" % c.inspect)
-        
         nons = deps.vertices.find_all { |v| ! v.is_a?(String) }
         assert(nons.empty?,
             "still contain non-strings %s" % nons.inspect)