diff options
| -rw-r--r-- | lib/puppet/metatype/metaparams.rb | 9 | ||||
| -rw-r--r-- | lib/puppet/metatype/relationships.rb | 1 | ||||
| -rw-r--r-- | lib/puppet/pgraph.rb | 2 | ||||
| -rw-r--r-- | lib/puppet/transaction.rb | 36 | ||||
| -rwxr-xr-x | test/other/transactions.rb | 33 |
5 files changed, 69 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 diff --git a/test/other/transactions.rb b/test/other/transactions.rb index c9d0cec1c..866c5551d 100755 --- a/test/other/transactions.rb +++ b/test/other/transactions.rb @@ -1027,6 +1027,39 @@ class TestTransactions < Test::Unit::TestCase assert_apply(obj) assert(obj.refreshed, "object was not refreshed during transaction") end + + # Testing #433 + def test_explicit_dependencies_beat_automatic + # Create a couple of different resource sets that have automatic relationships and make sure the manual relationships win + rels = {} + # First users and groups + group = Puppet::Type.type(:group).create(:name => nonrootgroup.name, :ensure => :present) + user = Puppet::Type.type(:user).create(:name => nonrootuser.name, :ensure => :present, :gid => group.title) + + # Now add the explicit relationship + group[:require] = user + rels[group] = user + # Now files + d = tempfile() + f = File.join(d, "file") + file = Puppet::Type.newfile(:path => f, :content => "yay") + dir = Puppet::Type.newfile(:path => d, :ensure => :directory, :require => file) + + rels[dir] = file + rels.each do |after, before| + comp = newcomp(before, after) + trans = comp.evaluate + str = "from %s to %s" % [before, after] + + assert_nothing_raised("Failed to create graph %s" % str) do + trans.prepare + end + + graph = trans.relgraph + assert(graph.edge?(before, after), "did not create manual relationship %s" % str) + assert(! graph.edge?(after, before), "created automatic relationship %s" % str) + end + end end # $Id$ |