Fixing #433. I basically just added checks to all the places where I add edges, to make sure automatic relationships lose out to explicit ones.

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

4 files changed, 36 insertions, 12 deletions

diff --git a/lib/puppet/metatype/metaparams.rb b/lib/puppet/metatype/metaparams.rb
index 355ee3ddc..b5f6a0e3b 100644
--- a/lib/puppet/metatype/metaparams.rb
+++ b/lib/puppet/metatype/metaparams.rb
@@ -277,11 +277,9 @@ class Puppet::Type
                     self.fail "Could not retrieve dependency '%s[%s]'" %
                         [tname.to_s.capitalize, name]
                 end
-                
-                self.debug("subscribes to %s" % [object.ref])
 
-                # Are we requiring them, or vice versa?  See the builddepends
-                # method for further docs on this.
+                # Are we requiring them, or vice versa?  See the method docs
+                # for futher info on this.
                 if self.class.direction == :in
                     source = object
                     target = @parent
@@ -295,11 +293,14 @@ class Puppet::Type
                         :event => self.class.events,
                         :callback => method
                     }
+                    self.debug("subscribes to %s" % [object.ref])
                 else
                     # If there's no callback, there's no point in even adding
                     # a label.
                     subargs = nil
+                    self.debug("requires %s" % [object.ref])
                 end
+                
                 rel = Puppet::Relationship.new(source, target, subargs)
             end
         end
diff --git a/lib/puppet/metatype/relationships.rb b/lib/puppet/metatype/relationships.rb
index 467b6187b..cd6b87bed 100644
--- a/lib/puppet/metatype/relationships.rb
+++ b/lib/puppet/metatype/relationships.rb
@@ -40,7 +40,6 @@ class Puppet::Type
                     end
                 end
                 
-                debug "Autorequiring %s" % [dep.ref]
                 reqs << Puppet::Relationship[dep, self]
             }
         }
diff --git a/lib/puppet/pgraph.rb b/lib/puppet/pgraph.rb
index 894a8fe03..f247f4391 100644
--- a/lib/puppet/pgraph.rb
+++ b/lib/puppet/pgraph.rb
@@ -60,7 +60,7 @@ class Puppet::PGraph < GRATR::Digraph
             bad << v unless sorted.include?(v)
         end
 
-        raise Puppet::Error, "Found cycle involving %s" % bad.collect do |v|
+        raise Puppet::Error, "Found dependency cycle involving %s" % bad.collect do |v|
             v.to_s
         end.join(", ")
     end
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 2240ca19f..42935a377 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -161,9 +161,15 @@ class Transaction
         
         children.each do |gen_child|
             if depthfirst
-                @relgraph.add_edge!(gen_child, resource)
+                edge = [gen_child, resource]
             else
-                @relgraph.add_edge!(resource, gen_child)
+                edge = [resource, gen_child]
+            end
+            unless @relgraph.edge?(edge[1], edge[0])
+                @relgraph.add_edge!(*edge)
+            else
+                @relgraph.add_vertex!(gen_child)
+                resource.debug "Skipping automatic relationship to %s" % gen_child
             end
         end
     end
@@ -185,11 +191,14 @@ class Transaction
                     detail
                 return nil
             end
-
+            
             if children
-                copy_relationships(resource, children)
+                children.each do |child|
+                    child.finish
+                    # Make sure that the vertex is in the relationship graph.
+                    @relgraph.add_vertex!(child)
+                end
                 @generated += children
-                children.each { |child| child.finish }
                 return children
             end
         end
@@ -231,6 +240,13 @@ class Transaction
                     events += eval_resource(child, false)
                 end
             end
+
+            # Create a child/parent relationship.  We do this after everything else because
+            # we want explicit relationships to be able to override automatic relationships,
+            # including this one.
+            if children
+                copy_relationships(resource, children)
+            end
             
             # A bit of hackery here -- if skipcheck is true, then we're the
             # top-level resource.  If that's the case, then make sure all of
@@ -254,6 +270,9 @@ class Transaction
         # the parent resource in so it will override the source in the events,
         # since eval_generated children can't have direct relationships.
         @relgraph.matching_edges(events, resource).each do |edge|
+            unless edge
+                raise "wtf?"
+            end
             set_trigger(edge)
         end
 
@@ -470,7 +489,12 @@ class Transaction
         graph.vertices.each do |vertex|
             vertex.autorequire.each do |edge|
                 unless graph.edge?(edge)
-                    graph.add_edge!(edge)
+                    unless graph.edge?(edge.target, edge.source)
+                        vertex.debug "Autorequiring %s" % [edge.source]
+                        graph.add_edge!(edge)
+                    else
+                        vertex.debug "Skipping automatic relationship with %s" % (edge.source == vertex ? edge.target : edge.source)
+                    end
                 end
             end
         end