[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.5-639-g8f94f35

test branch puppet-dev at googlegroups.com
Wed Jul 14 10:33:35 UTC 2010


The following commit has been merged in the upstream branch:
commit 0d4fd60c7c143cc1f4e4b0f99f359c09cbfbf21e
Author: Luke Kanies <luke at puppetlabs.com>
Date:   Tue Apr 13 12:16:05 2010 -0700

    Fixing #1903 - metaparam inheritance is much faster
    
    This doesn't actually fix the specific request in #1903,
    which said there should be no inheritance at all, but
    I've changed my mind on that.  Static inheritance is good,
    it should just be faster.
    
    This change could result in up to 70% speed improvements
    in compiling.
    
    Signed-off-by: Luke Kanies <luke at puppetlabs.com>

diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index fd7d968..ae4af76 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -361,6 +361,50 @@ class Puppet::Parser::Compiler
 
             resource.finish if resource.respond_to?(:finish)
         end
+
+        add_resource_metaparams
+    end
+
+    def add_resource_metaparams
+        unless main = catalog.resource(:class, :main)
+            raise "Couldn't find main"
+        end
+
+        names = []
+        Puppet::Type.eachmetaparam do |name|
+            next if Puppet::Parser::Resource.relationship_parameter?(name)
+            names << name
+        end
+
+        data = {}
+        catalog.walk(main, :out) do |source, target|
+            if source_data = data[source] || metaparams_as_data(source, names)
+                # only store anything in the data hash if we've actually got
+                # data
+                data[source] ||= source_data
+                source_data.each do |param, value|
+                    target[param] = value if target[param].nil?
+                end
+                data[target] = source_data.merge(metaparams_as_data(target, names))
+            end
+
+            target.tag(*(source.tags))
+        end
+    end
+
+    def metaparams_as_data(resource, params)
+        data = nil
+        params.each do |param|
+            unless resource[param].nil?
+                # Because we could be creating a hash for every resource,
+                # and we actually probably don't often have any data here at all,
+                # we're optimizing a bit by only creating a hash if there's
+                # any data to put in it.
+                data ||= {}
+                data[param] = resource[param]
+            end
+        end
+        data
     end
 
     # Initialize the top-level scope, class, and resource.
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 3e01224..a458772 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -100,7 +100,6 @@ class Puppet::Parser::Resource < Puppet::Resource
         @finished = true
         add_defaults()
         add_metaparams()
-        add_scope_tags()
         validate()
     end
 
@@ -279,23 +278,8 @@ class Puppet::Parser::Resource < Puppet::Resource
         compat_mode = metaparam_compatibility_mode?
 
         Puppet::Type.eachmetaparam do |name|
-            if self.class.relationship_parameter?(name)
-                add_backward_compatible_relationship_param(name) if compat_mode
-                next
-            end
-
-            next if @parameters[name]
-
-            # Skip metaparams for which we get no value.
-            next unless val = scope.lookupvar(name.to_s, false) and val != :undefined
-
-            set_parameter(name, val)
-        end
-    end
-
-    def add_scope_tags
-        if scope_resource = scope.resource
-            tag(*scope_resource.tags)
+            next unless self.class.relationship_parameter?(name)
+            add_backward_compatible_relationship_param(name) if compat_mode
         end
     end
 
diff --git a/spec/unit/parser/compiler.rb b/spec/unit/parser/compiler.rb
index c36113f..d5d46b1 100755
--- a/spec/unit/parser/compiler.rb
+++ b/spec/unit/parser/compiler.rb
@@ -35,9 +35,10 @@ describe Puppet::Parser::Compiler do
         @node = Puppet::Node.new "testnode"
         @known_resource_types = Puppet::Resource::TypeCollection.new "development"
 
-        @scope_resource = stub 'scope_resource', :builtin? => true, :finish => nil, :ref => 'Class[main]', :type => "class"
-        @scope = stub 'scope', :resource => @scope_resource, :source => mock("source")
         @compiler = Puppet::Parser::Compiler.new(@node)
+        @scope = Puppet::Parser::Scope.new(:compiler => @compiler, :source => "fake_source")
+        @scope_resource = Puppet::Parser::Resource.new(:file, "/my/file", :scope => @scope)
+        @scope.resource = @scope_resource
         @compiler.environment.stubs(:known_resource_types).returns @known_resource_types
     end
 
@@ -249,35 +250,103 @@ describe Puppet::Parser::Compiler do
             @compiler.compile
         end
 
-        it "should call finish() on all resources" do
-            # Add a resource that does respond to :finish
-            resource = Puppet::Parser::Resource.new "file", "finish", :scope => @scope
-            resource.expects(:finish)
+        describe "when finishing" do
+            before do
+                @compiler.send(:evaluate_main)
+                @catalog = @compiler.catalog
+            end
 
-            @compiler.add_resource(@scope, resource)
+            def add_resource(name, parent = nil)
+                resource = Puppet::Parser::Resource.new "file", name, :scope => @scope
+                @compiler.add_resource(@scope, resource)
+                @catalog.add_edge(parent, resource) if parent
+                resource
+            end
 
-            # And one that does not
-            dnf = stub "dnf", :ref => "File[dnf]", :type => "file"
+            it "should call finish() on all resources" do
+                # Add a resource that does respond to :finish
+                resource = Puppet::Parser::Resource.new "file", "finish", :scope => @scope
+                resource.expects(:finish)
 
-            @compiler.add_resource(@scope, dnf)
+                @compiler.add_resource(@scope, resource)
 
-            @compiler.send(:finish)
-        end
+                # And one that does not
+                dnf = stub "dnf", :ref => "File[dnf]", :type => "file"
 
-        it "should call finish() in add_resource order" do
-            resources = sequence('resources')
+                @compiler.add_resource(@scope, dnf)
 
-            resource1 = Puppet::Parser::Resource.new "file", "finish1", :scope => @scope
-            resource1.expects(:finish).in_sequence(resources)
+                @compiler.send(:finish)
+            end
 
-            @compiler.add_resource(@scope, resource1)
+            it "should call finish() in add_resource order" do
+                resources = sequence('resources')
 
-            resource2 = Puppet::Parser::Resource.new "file", "finish2", :scope => @scope
-            resource2.expects(:finish).in_sequence(resources)
+                resource1 = add_resource("finish1")
+                resource1.expects(:finish).in_sequence(resources)
 
-            @compiler.add_resource(@scope, resource2)
+                resource2 = add_resource("finish2")
+                resource2.expects(:finish).in_sequence(resources)
+
+                @compiler.send(:finish)
+            end
+
+            it "should add each container's metaparams to its contained resources" do
+                main = @catalog.resource(:class, :main)
+                main[:noop] = true
+
+                resource1 = add_resource("meh", main)
+
+                @compiler.send(:finish)
+                resource1[:noop].should be_true
+            end
+
+            it "should add metaparams recursively" do
+                main = @catalog.resource(:class, :main)
+                main[:noop] = true
+
+                resource1 = add_resource("meh", main)
+                resource2 = add_resource("foo", resource1)
+
+                @compiler.send(:finish)
+                resource2[:noop].should be_true
+            end
+
+            it "should prefer metaparams from immediate parents" do
+                main = @catalog.resource(:class, :main)
+                main[:noop] = true
+
+                resource1 = add_resource("meh", main)
+                resource2 = add_resource("foo", resource1)
+
+                resource1[:noop] = false
+
+                @compiler.send(:finish)
+                resource2[:noop].should be_false
+            end
+
+            it "should merge tags downward" do
+                main = @catalog.resource(:class, :main)
+                main.tag("one")
+
+                resource1 = add_resource("meh", main)
+                resource1.tag "two"
+                resource2 = add_resource("foo", resource1)
+
+                @compiler.send(:finish)
+                resource2.tags.should be_include("one")
+                resource2.tags.should be_include("two")
+            end
+
+            it "should work if only middle resources have metaparams set" do
+                main = @catalog.resource(:class, :main)
+
+                resource1 = add_resource("meh", main)
+                resource1[:noop] = true
+                resource2 = add_resource("foo", resource1)
 
-            @compiler.send(:finish)
+                @compiler.send(:finish)
+                resource2[:noop].should be_true
+            end
         end
 
         it "should return added resources in add order" do
diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb
index 5b2a16a..9d407c0 100755
--- a/spec/unit/parser/resource.rb
+++ b/spec/unit/parser/resource.rb
@@ -219,23 +219,6 @@ describe Puppet::Parser::Resource do
             @resource.should_not be_metaparam_compatibility_mode
         end
 
-        it "should copy metaparams from its scope" do
-            @scope.setvar("noop", "true")
-
-            @resource.class.publicize_methods(:add_metaparams)  { @resource.add_metaparams }
-
-            @resource["noop"].should == "true"
-        end
-
-        it "should not copy metaparams that it already has" do
-            @resource.set_parameter("noop", "false")
-            @scope.setvar("noop", "true")
-
-            @resource.class.publicize_methods(:add_metaparams)  { @resource.add_metaparams }
-
-            @resource["noop"].should == "false"
-        end
-
         it "should not copy relationship metaparams when not in metaparam compatibility mode" do
             @scope.setvar("require", "bar")
 
@@ -263,26 +246,6 @@ describe Puppet::Parser::Resource do
 
             @resource["require"].should == ["foo", "bar"]
         end
-
-        it "should copy all metaparams that it finds" do
-            @scope.setvar("noop", "foo")
-            @scope.setvar("schedule", "bar")
-
-            @resource.class.publicize_methods(:add_metaparams)  { @resource.add_metaparams }
-
-            @resource["noop"].should == "foo"
-            @resource["schedule"].should == "bar"
-        end
-
-        it "should add any tags from the scope resource" do
-            scope_resource = stub 'scope_resource', :tags => %w{one two}
-            @scope.stubs(:resource).returns(scope_resource)
-
-            @resource.class.publicize_methods(:add_scope_tags)  { @resource.add_scope_tags }
-
-            @resource.tags.should be_include("one")
-            @resource.tags.should be_include("two")
-        end
     end
 
     describe "when being tagged" do

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list