[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5
Jesse Wolfe
jes5199 at gmail.com
Tue May 10 07:58:49 UTC 2011
The following commit has been merged in the experimental branch:
commit 6dbd4771265173a9d4c3e7756c35c9ca371ca394
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