Some tweaks to graph splicing, although I do not think it will be enough to handle some of the edge cases.

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

2 files changed, 53 insertions, 25 deletions

diff --git a/lib/puppet/pgraph.rb b/lib/puppet/pgraph.rb
index bd023d576..de89e0a71 100644
--- a/lib/puppet/pgraph.rb
+++ b/lib/puppet/pgraph.rb
@@ -96,7 +96,8 @@ class Puppet::PGraph < GRATR::Digraph
     def splice!(other, type)
         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)
+            #children = other.adjacent(vertex, :direction => :out)
+            children = other.leaves(vertex)
 
             # Just remove the container if it's empty.
             if children.empty?
@@ -107,29 +108,46 @@ class Puppet::PGraph < GRATR::Digraph
             # First create new edges for each of the :in edges
             [: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
+                    if dir == :in
+                        nvertex = edge.source
+                    else
+                        nvertex = edge.target
+                    end
+                    if nvertex.is_a?(type)
+                        neighbors = other.leaves(nvertex)
+                    else
+                        neighbors = [nvertex]
+                    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]
+                    children.each do |child|
+                        neighbors.each do |neighbor|
+                            if dir == :in
+                                s = neighbor
+                                t = child
+                            else
+                                s = child
+                                t = neighbor
+                            end
+                            if s.is_a?(type)
+                                raise "Source %s is still a container" % s
+                            end
+                            if t.is_a?(type)
+                                raise "Target %s is still a container" % t
+                            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
             end
-            
-            # And finally, remove the vertex entirely.
             remove_vertex!(vertex)
         end
     end
diff --git a/test/other/pgraph.rb b/test/other/pgraph.rb
index c83df93e4..c8a04cb8d 100755
--- a/test/other/pgraph.rb
+++ b/test/other/pgraph.rb
@@ -71,15 +71,15 @@ class TestPGraph < Test::Unit::TestCase
     
     # Test that we can take a containment graph and rearrange it by dependencies
     def test_splice
-        one, two, middle, top = build_tree
+        one, two, three, middle, top = build_tree
         empty = Container.new("empty", [])
         # Also, add an empty container to top
         top.push empty
         
         contgraph = top.to_graph
         
-        # Now add a couple of child files, so that we can test whether all containers
-        # get spliced, rather than just components.
+        # Now add a couple of child files, so that we can test whether all
+        # containers get spliced, rather than just components.
         
         # Now make a dependency graph
         deps = Puppet::PGraph.new
@@ -90,14 +90,24 @@ class TestPGraph < Test::Unit::TestCase
         
         # 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)
+        #{one => two, three => [middle, two, "c"], "f" => "c", "h" => middle, "c" => empty}.each do |source, targets|
+        {one => two, two => three, middle => three, "f" => "c", "h" => middle, "c" => [three, empty]}.each do |source, targets|
+            targets = [targets] unless targets.is_a?(Array)
+            targets.each do |target|
+                deps.add_edge!(source, target, :callback => :refresh)
+            end
         end
+
+        #num = 6
+        #contgraph.to_jpg(File.expand_path("~/tmp/graphs"), "containers#{num}")
+        #contgraph.reversal.to_jpg(File.expand_path("~/tmp/graphs"), "reversal#{num}")
+        #deps.to_jpg(File.expand_path("~/tmp/graphs"), "relationships#{num}")
         
         deps.splice!(contgraph, Container)
+        #deps.to_jpg(File.expand_path("~/tmp/graphs"), "after_relationships#{num}")
         
         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)