+1 ! On Thu, Sep 30, 2010 at 7:52 AM, Paul Berry <[email protected]> wrote:
> The evaluate_definitions method was first figuring out which resources > needed to be evaluated (using unevaluated_resources), and then > evaluating them one by one. As a result, if evaluating one resource > triggered another resource to be evaluated, the latter resource could > get evaluated twice. This bug could occur, for example, if both > resources were classes that were included into the node by an external > node classifier, and if the first of the two classes included the > second. > > Modified evaluate_definitions so that it checks each resource to see > whether it needs to be evaluated right before evaluating it, rather > than creating a list of resources to evaluate and then evaluating them > each in turn. > > Also added an integration test to verify the fix. > > Note: the evaluate_definitions method (and unevaluated_resources, > which it used to call) were subject to an optimization effort in July, > which I believe was unrelated to this bug (see commit > dd03ac9fa29fce36eb64a5f831be8757f2f96f5c). This fix merely changes > when we check whether a resource has been evaluated, not how we do the > check or how many checks we perform, so it should not introduce any > significant performance bottleneck. > > Signed-off-by: Paul Berry <[email protected]> > --- > lib/puppet/parser/compiler.rb | 15 +++++++-------- > spec/integration/parser/compiler_spec.rb | 21 +++++++++++++++++++++ > 2 files changed, 28 insertions(+), 8 deletions(-) > > diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb > index e1227e7..feeaa36 100644 > --- a/lib/puppet/parser/compiler.rb > +++ b/lib/puppet/parser/compiler.rb > @@ -251,9 +251,15 @@ class Puppet::Parser::Compiler > # We return true if any resources have, so that we know to continue the > # evaluate_generators loop. > def evaluate_definitions > + something_evaluated = false > exceptwrap do > - !unevaluated_resources.each { |resource| resource.evaluate }.empty? > + resources.each do |resource| > + next if resource.evaluated? or resource.virtual? or > resource.builtin_type? > + something_evaluated = true > + resource.evaluate > + end > end > + something_evaluated > end > > # Iterate over collections and resources until we're sure that the whole > @@ -464,11 +470,4 @@ class Puppet::Parser::Compiler > scope.setvar name.to_s, environment[name] > end > end > - > - # Return an array of all of the unevaluated resources. These will be > definitions, > - # which need to get evaluated into native resources. > - def unevaluated_resources > - # The order of these is significant for speed due to short-circuting > - resources.reject { |resource| resource.evaluated? or resource.virtual? > or resource.builtin_type? } > - end > end > diff --git a/spec/integration/parser/compiler_spec.rb > b/spec/integration/parser/compiler_spec.rb > index 9158ad1..f80221e 100755 > --- a/spec/integration/parser/compiler_spec.rb > +++ b/spec/integration/parser/compiler_spec.rb > @@ -27,6 +27,27 @@ describe Puppet::Parser::Compiler do > @compiler.catalog.version.should == version > end > > + it "should not create duplicate resources when a class is referenced > both directly and indirectly by the node classifier (4792)" do > + Puppet[:code] = <<-PP > + class foo > + { > + notify { foo_notify: } > + include bar > + } > + class bar > + { > + notify { bar_notify: } > + } > + PP > + > + @node.stubs(:classes).returns(['foo', 'bar']) > + > + catalog = Puppet::Parser::Compiler.compile(@node) > + > + catalog.resource("Notify[foo_notify]").should_not be_nil > + catalog.resource("Notify[bar_notify]").should_not be_nil > + end > + > describe "when resolving class references" do > it "should favor local scope, even if there's an included class in > topscope" do > Puppet[:code] = <<-PP > -- > 1.7.2 > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]<puppet-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
