[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 2.6.1rc1-141-gcdb2b90

markus (none) markus at AVA-351181.
Mon Aug 16 12:48:47 UTC 2010


The following commit has been merged in the upstream branch:
commit 449315a2c705df2396852462a1d1e14774b9f117
Author: Jesse Wolfe <jes5199 at gmail.com>
Date:   Wed Jul 28 14:54:23 2010 -0700

    [#4397]+[#4344] Move type-name resolution out of Puppet::Resource into the AST resources.
    
    Move type-name resolution out of Puppet::Resource into the AST resources.
    Move find_resource_type out of Puppet::Resource into Scope
    Thus, never pass unqualified type names to Puppet::Resource objects.
    Thus, Puppet::Resource objects don't need the namespace property,
    and Puppet::Resource objects never consult the harddrive to look for
    .pp files that might contain their type definitions,
    Thus, performance is improved.
    
    Also removes the temporary fix for #4257 that caused #4397
    (The code was too eager to look for a class in the topscope)
    
    Paired-With: Paul Berry <paul at puppetlabs.com>
    Signed-off-by: Jesse Wolfe <jes5199 at gmail.com>

diff --git a/lib/puppet/parser/ast/resource.rb b/lib/puppet/parser/ast/resource.rb
index 1b063c9..9149b06 100644
--- a/lib/puppet/parser/ast/resource.rb
+++ b/lib/puppet/parser/ast/resource.rb
@@ -33,11 +33,11 @@ class Resource < AST::ResourceReference
     # This is where our implicit iteration takes place; if someone
     # passed an array as the name, then we act just like the called us
     # many times.
+    resource_type = scope.find_resource_type(type)
     resource_titles.flatten.collect { |resource_title|
       exceptwrap :type => Puppet::ParseError do
-
-              resource = Puppet::Parser::Resource.new(
-        type, resource_title,
+        resource = Puppet::Parser::Resource.new(
+          resource_type.name, resource_title,
           :parameters => paramobjects,
           :file => self.file,
           :line => self.line,
diff --git a/lib/puppet/parser/ast/resource_reference.rb b/lib/puppet/parser/ast/resource_reference.rb
index 5d83343..5b1b0aa 100644
--- a/lib/puppet/parser/ast/resource_reference.rb
+++ b/lib/puppet/parser/ast/resource_reference.rb
@@ -7,8 +7,29 @@ class Puppet::Parser::AST::ResourceReference < Puppet::Parser::AST::Branch
   # Evaluate our object, but just return a simple array of the type
   # and name.
   def evaluate(scope)
-    titles = Array(title.safeevaluate(scope)).collect { |t| Puppet::Resource.new(type, t, :namespaces => scope.namespaces) }
-    return(titles.length == 1 ? titles.pop : titles)
+    a_type = type
+    titles = Array(title.safeevaluate(scope))
+
+    case type.downcase
+    when "class"
+      # resolve the titles
+      titles = titles.collect do |a_title|
+        hostclass = scope.find_hostclass(a_title)
+        hostclass ?  hostclass.name : a_title
+      end
+    when "node"
+      # no-op
+    else
+      # resolve the type
+      resource_type = scope.find_resource_type(type)
+      a_type = resource_type.name if resource_type
+    end
+
+    resources = titles.collect{ |a_title|
+      Puppet::Resource.new(a_type, a_title)
+    }
+
+    return(resources.length == 1 ? resources.pop : resources)
   end
 
   def to_s
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 3c45192..8a5ae88 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -102,9 +102,9 @@ class Puppet::Parser::Resource < Puppet::Resource
   end
 
   def initialize(*args)
+    raise ArgumentError, "Resources require a scope" unless args.last[:scope]
     super
 
-    raise ArgumentError, "Resources require a scope" unless scope
     @source ||= scope.source
   end
 
@@ -140,10 +140,6 @@ class Puppet::Parser::Resource < Puppet::Resource
     self[:name] || self.title
   end
 
-  def namespaces
-    scope.namespaces
-  end
-
   # A temporary occasion, until I get paths in the scopes figured out.
   def path
     to_s
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index ae0f9ea..2ca28d8 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -474,6 +474,20 @@ class Puppet::Parser::Scope
     end
   end
 
+  def find_resource_type(type)
+    # It still works fine without the type == 'class' short-cut, but it is a lot slower.
+    return nil if ["class", "node"].include? type.to_s.downcase
+    find_builtin_resource_type(type) || find_defined_resource_type(type)
+  end
+
+  def find_builtin_resource_type(type)
+    Puppet::Type.type(type.to_s.downcase.to_sym)
+  end
+
+  def find_defined_resource_type(type)
+    environment.known_resource_types.find_definition(namespaces, type.to_s.downcase)
+  end
+
   private
 
   def extend_with_functions_module
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index 96d22e4..c4e52af 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -13,7 +13,7 @@ class Puppet::Resource
   extend Puppet::Util::Pson
   include Enumerable
   attr_accessor :file, :line, :catalog, :exported, :virtual, :validate_parameters, :strict
-  attr_reader :namespaces
+  attr_reader :type, :title
 
   require 'puppet/indirector'
   extend Puppet::Indirector
@@ -157,27 +157,26 @@ class Puppet::Resource
   # Create our resource.
   def initialize(type, title = nil, attributes = {})
     @parameters = {}
-    @namespaces = [""]
 
-    # Set things like namespaces and strictness first.
+    # Set things like strictness first.
     attributes.each do |attr, value|
       next if attr == :parameters
       send(attr.to_s + "=", value)
     end
 
-    # We do namespaces first, and use tmp variables, so our title
-    # canonicalization works (i.e., namespaces are set and resource
-    # types can be looked up)
-    tmp_type, tmp_title = extract_type_and_title(type, title)
-    self.type = tmp_type
-    self.title = tmp_title
+    @type, @title = extract_type_and_title(type, title)
+
+    @type = munge_type_name(@type)
+
+    if @type == "Class"
+      @title = :main if @title == ""
+      @title = munge_type_name(@title)
+    end
 
     if params = attributes[:parameters]
       extract_parameters(params)
     end
 
-    resolve_type_and_title
-
     tag(self.type)
     tag(self.title) if valid_tag?(self.title)
 
@@ -193,17 +192,12 @@ class Puppet::Resource
     return(catalog ? catalog.resource(to_s) : nil)
   end
 
-  def title=(value)
-    @unresolved_title = value
-    @title = nil
-  end
-
   def resource_type
     @resource_type ||= case type
-    when "Class"; find_hostclass(title)
-    when "Node"; find_node(title)
+    when "Class"; known_resource_types.hostclass(title == :main ? "" : title)
+    when "Node"; known_resource_types.node(title)
     else
-      find_resource_type(type)
+      Puppet::Type.type(type.to_s.downcase.to_sym) || known_resource_types.definition(type)
     end
   end
 
@@ -314,28 +308,6 @@ class Puppet::Resource
     self
   end
 
-  # We have to lazy-evaluate this.
-  def title=(value)
-    @title = nil
-    @unresolved_title = value
-  end
-
-  # We have to lazy-evaluate this.
-  def type=(value)
-    @type = nil
-    @unresolved_type = value || "Class"
-  end
-
-  def title
-    resolve_type_and_title unless @title
-    @title
-  end
-
-  def type
-    resolve_type_and_title unless @type
-    @type
-  end
-
   def valid_parameter?(name)
     resource_type.valid_parameter?(name)
   end
@@ -346,29 +318,6 @@ class Puppet::Resource
 
   private
 
-  def find_node(name)
-    known_resource_types.node(name)
-  end
-
-  def find_hostclass(title)
-    name = title == :main ? "" : title
-    known_resource_types.find_hostclass(namespaces, name)
-  end
-
-  def find_resource_type(type)
-    # It still works fine without the type == 'class' short-cut, but it is a lot slower.
-    return nil if ["class", "node"].include? type.to_s.downcase
-    find_builtin_resource_type(type) || find_defined_resource_type(type)
-  end
-
-  def find_builtin_resource_type(type)
-    Puppet::Type.type(type.to_s.downcase.to_sym)
-  end
-
-  def find_defined_resource_type(type)
-    known_resource_types.find_definition(namespaces, type.to_s.downcase)
-  end
-
   # Produce a canonical method name.
   def parameter_name(param)
     param = param.to_s.downcase.to_sym
@@ -378,10 +327,6 @@ class Puppet::Resource
     param
   end
 
-  def namespaces=(ns)
-    @namespaces = Array(ns)
-  end
-
   # The namevar for our resource type. If the type doesn't exist,
   # always use :name.
   def namevar
@@ -428,54 +373,9 @@ class Puppet::Resource
     value.to_s.split("::").collect { |s| s.capitalize }.join("::")
   end
 
-  # This is an annoyingly complicated method for resolving qualified
-  # types as necessary, and putting them in type or title attributes.
-  def resolve_type_and_title
-    if @unresolved_type
-      @type = resolve_type
-      @unresolved_type = nil
-    end
-    if @unresolved_title
-      @title = resolve_title
-      @unresolved_title = nil
-    end
-  end
-
-  def resolve_type
-    case type = munge_type_name(@unresolved_type)
-    when "Class", "Node";
-      type
-    else
-      # Otherwise, some kind of builtin or defined resource type
-      munge_type_name( (r = find_resource_type(type)) ? r.name : type)
-    end
-  end
-
-  # This method only works if resolve_type was called first
-  def resolve_title
-    case @type
-    when "Node"; return @unresolved_title
-    when "Class";
-      resolve_title_for_class(@unresolved_title)
-    else
-      @unresolved_title
-    end
-  end
-
-  def resolve_title_for_class(title)
-    if title == "" or title == :main
-      return :main
-    end
-
-    if klass = find_hostclass(title)
-      result = klass.name
-    end
-    munge_type_name(result || title)
-  end
-
   def parse_title
     h = {}
-    type = find_resource_type(@type)
+    type = resource_type
     if type.respond_to? :title_patterns
       type.title_patterns.each { |regexp, symbols_and_lambdas|
         if captures = regexp.match(title.to_s)
diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb
index 90b6df9..63d1103 100644
--- a/lib/puppet/resource/type_collection.rb
+++ b/lib/puppet/resource/type_collection.rb
@@ -96,8 +96,8 @@ class Puppet::Resource::TypeCollection
     #Array("") == [] for some reason
     namespaces = [namespaces] unless namespaces.is_a?(Array)
 
-    if r = find_fully_qualified(name, type)
-      return r
+    if name =~ /^::/
+      return send(type, name.sub(/^::/, ''))
     end
 
     namespaces.each do |namespace|
@@ -198,10 +198,6 @@ class Puppet::Resource::TypeCollection
 
   private
 
-  def find_fully_qualified(name, type)
-    send(type, name.sub(/^::/, ''))
-  end
-
   def find_partially_qualified(namespace, name, type)
     send(type, [namespace, name].join("::"))
   end
diff --git a/spec/integration/parser/compiler_spec.rb b/spec/integration/parser/compiler_spec.rb
index 83bbf95..9158ad1 100755
--- a/spec/integration/parser/compiler_spec.rb
+++ b/spec/integration/parser/compiler_spec.rb
@@ -26,4 +26,47 @@ describe Puppet::Parser::Compiler do
 
     @compiler.catalog.version.should == version
   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
+        class experiment {
+          class baz {
+          }
+          notify {"x" : require => Class[Baz] }
+        }
+        class baz {
+        }
+        include baz
+        include experiment
+        include experiment::baz
+      PP
+
+      catalog = Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode"))
+
+      notify_resource = catalog.resource( "Notify[x]" )
+
+      notify_resource[:require].title.should == "Experiment::Baz"
+    end
+
+    it "should favor local scope, even if there's an unincluded class in topscope" do
+      Puppet[:code] = <<-PP
+        class experiment {
+          class baz {
+          }
+          notify {"x" : require => Class[Baz] }
+        }
+        class baz {
+        }
+        include experiment
+        include experiment::baz
+      PP
+
+      catalog = Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode"))
+
+      notify_resource = catalog.resource( "Notify[x]" )
+
+      notify_resource[:require].title.should == "Experiment::Baz"
+    end
+  end
 end
diff --git a/spec/unit/parser/ast/resource_reference_spec.rb b/spec/unit/parser/ast/resource_reference_spec.rb
index 7b48119..93419d9 100755
--- a/spec/unit/parser/ast/resource_reference_spec.rb
+++ b/spec/unit/parser/ast/resource_reference_spec.rb
@@ -32,11 +32,6 @@ describe Puppet::Parser::AST::ResourceReference do
     ]
   end
 
-  it "should pass its scope's namespaces to all created resource references" do
-    @scope.add_namespace "foo"
-    newref("File", "/tmp/yay").evaluate(@scope).namespaces.should == ["foo"]
-  end
-
   it "should return a correct representation when converting to string" do
     type = stub 'type', :is_a? => true, :to_s => "file"
     title = stub 'title', :is_a? => true, :to_s => "[/tmp/a, /tmp/b]"
diff --git a/spec/unit/parser/functions/require_spec.rb b/spec/unit/parser/functions/require_spec.rb
index bd42fa5..49c4bbf 100755
--- a/spec/unit/parser/functions/require_spec.rb
+++ b/spec/unit/parser/functions/require_spec.rb
@@ -6,7 +6,7 @@ describe "the require function" do
 
   before :each do
     @catalog = stub 'catalog'
-    @compiler = stub 'compiler', :catalog => @catalog
+    @compiler = stub 'compiler', :catalog => @catalog, :environment => nil
 
     @scope = Puppet::Parser::Scope.new
     @scope.stubs(:findresource)
diff --git a/spec/unit/parser/functions/tag_spec.rb b/spec/unit/parser/functions/tag_spec.rb
index ff37bad..dac1341 100755
--- a/spec/unit/parser/functions/tag_spec.rb
+++ b/spec/unit/parser/functions/tag_spec.rb
@@ -6,6 +6,7 @@ describe "the 'tag' function" do
 
   before :each do
     @scope = Puppet::Parser::Scope.new
+    @scope.stubs(:environment).returns(nil)
   end
 
   it "should exist" do
diff --git a/spec/unit/parser/resource_spec.rb b/spec/unit/parser/resource_spec.rb
index da49940..dae22fc 100755
--- a/spec/unit/parser/resource_spec.rb
+++ b/spec/unit/parser/resource_spec.rb
@@ -58,23 +58,17 @@ describe Puppet::Parser::Resource do
   end
 
   it "should get its environment from its scope" do
-    scope = stub 'scope', :source => stub("source")
-    scope.expects(:environment).returns "foo"
+    scope = stub 'scope', :source => stub("source"), :namespaces => nil
+    scope.expects(:environment).returns("foo").at_least_once
     Puppet::Parser::Resource.new("file", "whatever", :scope => scope).environment.should == "foo"
   end
 
-  it "should get its namespaces from its scope" do
-    scope = stub 'scope', :source => stub("source")
-    scope.expects(:namespaces).returns %w{one two}
-    Puppet::Parser::Resource.new("file", "whatever", :scope => scope).namespaces.should == %w{one two}
-  end
-
   it "should use the resource type collection helper module" do
     Puppet::Parser::Resource.ancestors.should be_include(Puppet::Resource::TypeCollectionHelper)
   end
 
   it "should use the scope's environment as its environment" do
-    @scope.expects(:environment).returns "myenv"
+    @scope.expects(:environment).returns("myenv").at_least_once
     Puppet::Parser::Resource.new("file", "whatever", :scope => @scope).environment.should == "myenv"
   end
 
diff --git a/spec/unit/rails/resource_spec.rb b/spec/unit/rails/resource_spec.rb
index 08deda6..73c7f7a 100755
--- a/spec/unit/rails/resource_spec.rb
+++ b/spec/unit/rails/resource_spec.rb
@@ -107,7 +107,7 @@ describe "Puppet::Rails::Resource" do
 
   describe "#to_resource" do
     it "should instantiate a Puppet::Parser::Resource" do
-      scope = stub "scope", :source => nil
+      scope = stub "scope", :source => nil, :environment => nil, :namespaces => nil
 
       @resource = Puppet::Rails::Resource.new
       @resource.stubs(:attributes).returns({
diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb
index 10cff91..2b6beb5 100755
--- a/spec/unit/resource/catalog_spec.rb
+++ b/spec/unit/resource/catalog_spec.rb
@@ -224,7 +224,7 @@ describe Puppet::Resource::Catalog, "when compiling" do
     end
 
     it "should convert parser resources to plain resources" do
-      resource = Puppet::Parser::Resource.new(:file, "foo", :scope => stub("scope"), :source => stub("source"))
+      resource = Puppet::Parser::Resource.new(:file, "foo", :scope => stub("scope", :environment => nil, :namespaces => nil), :source => stub("source"))
       catalog = Puppet::Resource::Catalog.new("whev")
       catalog.add_resource(resource)
       new = catalog.to_resource
diff --git a/spec/unit/resource/type_collection_spec.rb b/spec/unit/resource/type_collection_spec.rb
index 45fc05d..577aea4 100644
--- a/spec/unit/resource/type_collection_spec.rb
+++ b/spec/unit/resource/type_collection_spec.rb
@@ -258,6 +258,32 @@ describe Puppet::Resource::TypeCollection do
       loader.add instance
       loader.find("foo::bar", "eh", :hostclass).should be_nil
     end
+
+    describe "when topscope has a class that has the same name as a local class" do
+      before do
+        @loader = Puppet::Resource::TypeCollection.new("env")
+        [ "foo::bar", "bar" ].each do |name|
+          @loader.add Puppet::Resource::Type.new(:hostclass, name)
+        end
+      end
+
+      it "should favor the local class, if the name is unqualified" do
+        @loader.find("foo", "bar",   :hostclass).name.should == 'foo::bar'
+      end
+
+      it "should only look in the topclass, if the name is qualified" do
+        @loader.find("foo", "::bar", :hostclass).name.should == 'bar'
+      end
+
+    end
+    
+    it "should not look in the local scope for classes when the name is qualified" do
+        @loader = Puppet::Resource::TypeCollection.new("env")
+        @loader.add Puppet::Resource::Type.new(:hostclass, "foo::bar")
+
+        @loader.find("foo", "::bar", :hostclass).should == nil
+    end
+
   end
 
   it "should use the generic 'find' method with an empty namespace to find nodes" do
@@ -437,4 +463,5 @@ describe Puppet::Resource::TypeCollection do
     end
 
   end
+
 end
diff --git a/spec/unit/resource/type_spec.rb b/spec/unit/resource/type_spec.rb
index 0ef4a51..4d3942c 100755
--- a/spec/unit/resource/type_spec.rb
+++ b/spec/unit/resource/type_spec.rb
@@ -369,7 +369,8 @@ describe Puppet::Resource::Type do
     end
 
     it "should cache a reference to the parent type" do
-      @code.expects(:hostclass).once.with("bar").returns @parent
+      @code.stubs(:hostclass).with("foo::bar").returns nil
+      @code.expects(:hostclass).with("bar").once.returns @parent
       @child.parent_type(@scope)
       @child.parent_type
     end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 204a2b0..e65e8a1 100755
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -123,18 +123,6 @@ describe Puppet::Resource do
     Puppet::Resource.new("file", "/my/file", :environment => :foo).environment.name.should == :foo
   end
 
-  it "should support specifying namespaces" do
-    Puppet::Resource.new("file", "/my/file", :namespaces => ["foo"]).namespaces.should == ["foo"]
-  end
-
-  it "should convert namespaces to an array if not specified as one" do
-    Puppet::Resource.new("file", "/my/file", :namespaces => "foo").namespaces.should == ["foo"]
-  end
-
-  it "should default to a single amespace of an empty string" do
-    Puppet::Resource.new("file", "/my/file").namespaces.should == [""]
-  end
-
   describe "and munging its type and title" do
     describe "when modeling a builtin resource" do
       it "should be able to find the resource type" do
@@ -164,16 +152,6 @@ describe Puppet::Resource do
         it "should set its title to the provided title" do
           Puppet::Resource.new("foo::bar", "/my/file").title.should == "/my/file"
         end
-
-        describe "and the resource is unqualified and models a qualified resource type" do
-          it "should set its type to the fully qualified resource type" do
-            Puppet::Resource.new("bar", "/my/file", :namespaces => %w{foo}).type.should == "Foo::Bar"
-          end
-
-          it "should be able to find the resource type" do
-            Puppet::Resource.new("bar", "/my/file", :namespaces => %w{foo}).resource_type.should equal(@type)
-          end
-        end
       end
 
       describe "that does not exist" do
@@ -210,20 +188,6 @@ describe Puppet::Resource do
         it "should be able to find the resource type" do
           Puppet::Resource.new("class", "foo::bar").resource_type.should equal(@type)
         end
-
-        describe "and the resource is unqualified and models a qualified class" do
-          it "should set its title to the fully qualified resource type" do
-            Puppet::Resource.new("class", "bar", :namespaces => %w{foo}).title.should == "Foo::Bar"
-          end
-
-          it "should be able to find the resource type" do
-            Puppet::Resource.new("class", "bar", :namespaces => %w{foo}).resource_type.should equal(@type)
-          end
-
-          it "should set its type to 'Class'" do
-            Puppet::Resource.new("class", "bar", :namespaces => %w{foo}).type.should == "Class"
-          end
-        end
       end
 
       describe "that does not exist" do
diff --git a/spec/unit/type/schedule_spec.rb b/spec/unit/type/schedule_spec.rb
index 6975529..420cffd 100755
--- a/spec/unit/type/schedule_spec.rb
+++ b/spec/unit/type/schedule_spec.rb
@@ -41,7 +41,7 @@ end
 
 describe Puppet::Type.type(:schedule) do
   before :each do
-    Puppet.settings.stubs(:value).with(:ignoreschedules).returns(false)
+    Puppet[:ignoreschedules] = false
 
     @schedule = Puppet::Type.type(:schedule).new(:name => "testing")
   end

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list