[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:30:08 UTC 2010
The following commit has been merged in the upstream branch:
commit 8bafc37a532b7bef541186e7bb719f50c0eda600
Author: Markus Roberts <Markus at reality.com>
Date: Sat Mar 13 14:44:56 2010 -0800
Move scope parenting & class_scope from Compiler to Scope
This refactor fixes about a quarter of the test failures on master and (I
hope) will simplify some of the integration issues on the testing branch.
It is my best guess at The Right Thing To Do (or at least a step in that
direction) but I could be persuaded otherwise.
The basic idea is to take responsibility for maintaining scope hierarchy and
class_name -> class_scope mapping out of the compiler class and put it in the
scope class where it arguably belongs. To maintain the semantics, class
scopes are all tracked by the "top level" scope, though this could be relaxed
if the nesting semantics were ever needed.
If this winds up being the right thing to do, related routines (e.g. newscope)
should be sorted out as well.
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index 6b6cd68..51df86e 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -51,32 +51,11 @@ class Puppet::Parser::Compiler
parser.nodes?
end
- # Store the fact that we've evaluated a class, and store a reference to
- # the scope in which it was evaluated, so that we can look it up later.
- def class_set(name, scope)
- if existing = @class_scopes[name]
- if existing.nodescope? != scope.nodescope?
- raise Puppet::ParseError, "Cannot have classes, nodes, or definitions with the same name"
- else
- raise Puppet::DevError, "Somehow evaluated %s %s twice" % [ existing.nodescope? ? "node" : "class", name]
- end
- end
- @class_scopes[name] = scope
+ # Store the fact that we've evaluated a class
+ def add_class(name)
@catalog.add_class(name) unless name == ""
end
- # Return the scope associated with a class. This is just here so
- # that subclasses can set their parent scopes to be the scope of
- # their parent class, and it's also used when looking up qualified
- # variables.
- def class_scope(klass)
- # They might pass in either the class or class name
- if klass.respond_to?(:name)
- @class_scopes[klass.name]
- else
- @class_scopes[klass]
- end
- end
# Return a list of all of the defined classes.
def classlist
@@ -138,7 +117,7 @@ class Puppet::Parser::Compiler
classes.each do |name|
# If we can find the class, then make a resource that will evaluate it.
if klass = scope.find_hostclass(name)
- found << name and next if class_scope(klass)
+ found << name and next if scope.class_scope(klass)
resource = klass.mk_plain_resource(scope)
@@ -180,27 +159,16 @@ class Puppet::Parser::Compiler
end
# Create a new scope, with either a specified parent scope or
- # using the top scope. Adds an edge between the scope and
- # its parent to the graph.
+ # using the top scope.
def newscope(parent, options = {})
parent ||= topscope
options[:compiler] = self
options[:parser] ||= self.parser
scope = Puppet::Parser::Scope.new(options)
- @scope_graph.add_edge(parent, scope)
+ scope.parent = parent
scope
end
- # Find the parent of a given scope. Assumes scopes only ever have
- # one in edge, which will always be true.
- def parent(scope)
- if ary = @scope_graph.adjacent(scope, :direction => :in) and ary.length > 0
- ary[0]
- else
- nil
- end
- end
-
# Return any overrides for the given resource.
def resource_overrides(resource)
@resource_overrides[resource.ref]
@@ -236,7 +204,7 @@ class Puppet::Parser::Compiler
# Now set the node scope appropriately, so that :topscope can
# behave differently.
- @node_scope = class_scope(astnode)
+ @node_scope = topscope.class_scope(astnode)
end
# Evaluate our collections and return true if anything returned an object.
@@ -384,15 +352,10 @@ class Puppet::Parser::Compiler
def init_main
# Create our initial scope and a resource that will evaluate main.
@topscope = Puppet::Parser::Scope.new(:compiler => self, :parser => self.parser)
- @scope_graph.add_vertex(@topscope)
end
# Set up all of our internal variables.
def initvars
- # The table for storing class singletons. This will only actually
- # be used by top scopes and node scopes.
- @class_scopes = {}
-
# The list of objects that will available for export.
@exported_resources = {}
@@ -406,9 +369,6 @@ class Puppet::Parser::Compiler
# but they each refer back to the scope that created them.
@collections = []
- # A graph for maintaining scope relationships.
- @scope_graph = Puppet::SimpleGraph.new
-
# For maintaining the relationship between scopes and their resources.
@catalog = Puppet::Resource::Catalog.new(@node.name)
@catalog.version = @parser.version
diff --git a/lib/puppet/parser/resource_type.rb b/lib/puppet/parser/resource_type.rb
index a679659..3dbcbcb 100644
--- a/lib/puppet/parser/resource_type.rb
+++ b/lib/puppet/parser/resource_type.rb
@@ -165,6 +165,7 @@ class Puppet::Parser::ResourceType
scope.setvar("title", resource.title) unless set.include? :title
scope.setvar("name", resource.name) unless set.include? :name
+ scope.class_set(self.name,scope)
end
# Create a new subscope in which to evaluate our code.
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index d6d6630..eda53a8 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -18,7 +18,7 @@ class Puppet::Parser::Scope
attr_accessor :level, :parser, :source, :resource
attr_accessor :base, :keyword, :nodescope
attr_accessor :top, :translated, :compiler
- attr_writer :parent
+ attr_accessor :parent
# A demeterific shortcut to the catalog.
def catalog
@@ -139,6 +139,34 @@ class Puppet::Parser::Scope
@defaults = Hash.new { |dhash,type|
dhash[type] = {}
}
+
+ # The table for storing class singletons. This will only actually
+ # be used by top scopes and node scopes.
+ @class_scopes = {}
+ end
+
+ # Store the fact that we've evaluated a class, and store a reference to
+ # the scope in which it was evaluated, so that we can look it up later.
+ def class_set(name, scope)
+ return parent.class_set(name,scope) if parent
+ if existing = @class_scopes[name]
+ if existing.nodescope? != scope.nodescope?
+ raise Puppet::ParseError, "Cannot have classes, nodes, or definitions with the same name"
+ else
+ raise Puppet::DevError, "Somehow evaluated %s %s twice" % [ existing.nodescope? ? "node" : "class", name]
+ end
+ end
+ @class_scopes[name] = scope
+ end
+
+ # Return the scope associated with a class. This is just here so
+ # that subclasses can set their parent scopes to be the scope of
+ # their parent class, and it's also used when looking up qualified
+ # variables.
+ def class_scope(klass)
+ # They might pass in either the class or class name
+ k = klass.respond_to?(:name) ? klass.name : klass
+ @class_scopes[k] || (parent && parent.class_scope(k))
end
# Collect all of the defaults set at any higher scopes.
@@ -182,7 +210,7 @@ class Puppet::Parser::Scope
warning "Could not look up qualified variable '%s'; class %s could not be found" % [name, klassname]
return usestring ? "" : :undefined
end
- unless kscope = compiler.class_scope(klass)
+ unless kscope = class_scope(klass)
warning "Could not look up qualified variable '%s'; class %s has not been evaluated" % [name, klassname]
return usestring ? "" : :undefined
end
@@ -251,15 +279,6 @@ class Puppet::Parser::Scope
self.nodescope
end
- # We probably shouldn't cache this value... But it's a lot faster
- # than doing lots of queries.
- def parent
- unless defined?(@parent)
- @parent = compiler.parent(self)
- end
- @parent
- end
-
# Return the list of scopes up to the top scope, ordered with our own first.
# This is used for looking up variables and defaults.
def scope_path
diff --git a/spec/unit/parser/compiler.rb b/spec/unit/parser/compiler.rb
index 8a41242..d23d225 100755
--- a/spec/unit/parser/compiler.rb
+++ b/spec/unit/parser/compiler.rb
@@ -40,26 +40,10 @@ describe Puppet::Parser::Compiler do
@compiler = Puppet::Parser::Compiler.new(@node, @parser)
end
- it "should be able to store references to class scopes" do
- lambda { @compiler.class_set "myname", "myscope" }.should_not raise_error
- end
-
- it "should be able to retrieve class scopes by name" do
- @compiler.class_set "myname", "myscope"
- @compiler.class_scope("myname").should == "myscope"
- end
-
- it "should be able to retrieve class scopes by object" do
- klass = mock 'ast_class'
- klass.expects(:name).returns("myname")
- @compiler.class_set "myname", "myscope"
- @compiler.class_scope(klass).should == "myscope"
- end
-
- it "should be able to return a class list containing all set classes" do
- @compiler.class_set "", "empty"
- @compiler.class_set "one", "yep"
- @compiler.class_set "two", "nope"
+ it "should be able to return a class list containing all added classes" do
+ @compiler.add_class ""
+ @compiler.add_class "one"
+ @compiler.add_class "two"
@compiler.classlist.sort.should == %w{one two}.sort
end
@@ -116,7 +100,14 @@ describe Puppet::Parser::Compiler do
scope = mock 'scope'
newscope = @compiler.newscope(scope)
- @compiler.parent(newscope).should equal(scope)
+ newscope.parent.should equal(scope)
+ end
+
+ it "should set the parent scope of the new scope to its topscope if the parent passed in is nil" do
+ scope = mock 'scope'
+ newscope = @compiler.newscope(nil)
+
+ newscope.parent.should equal(@compiler.topscope)
end
end
@@ -406,6 +397,7 @@ describe Puppet::Parser::Compiler do
@compiler.catalog.stubs(:tag)
@class.expects(:mk_plain_resource).with(@scope)
+ @scope.stubs(:class_scope).with(@class)
@compiler.evaluate_classes(%w{myclass}, @scope)
end
@@ -416,6 +408,7 @@ describe Puppet::Parser::Compiler do
@resource.expects(:evaluate).never
@class.expects(:mk_plain_resource).returns(@resource)
+ @scope.stubs(:class_scope).with(@class)
@compiler.evaluate_classes(%w{myclass}, @scope)
end
@@ -425,6 +418,7 @@ describe Puppet::Parser::Compiler do
@resource.expects(:evaluate)
@class.expects(:mk_plain_resource).returns(@resource)
+ @scope.stubs(:class_scope).with(@class)
@compiler.evaluate_classes(%w{myclass}, @scope, false)
end
@@ -432,7 +426,7 @@ describe Puppet::Parser::Compiler do
it "should skip classes that have already been evaluated" do
@compiler.catalog.stubs(:tag)
- @compiler.expects(:class_scope).with(@class).returns("something")
+ @scope.stubs(:class_scope).with(@class).returns("something")
@compiler.expects(:add_resource).never
@@ -445,7 +439,7 @@ describe Puppet::Parser::Compiler do
it "should skip classes previously evaluated with different capitalization" do
@compiler.catalog.stubs(:tag)
@scope.stubs(:find_hostclass).with("MyClass").returns(@class)
- @compiler.expects(:class_scope).with(@class).returns("something")
+ @scope.stubs(:class_scope).with(@class).returns("something")
@compiler.expects(:add_resource).never
@resource.expects(:evaluate).never
Puppet::Parser::Resource.expects(:new).never
@@ -457,6 +451,7 @@ describe Puppet::Parser::Compiler do
@compiler.stubs(:add_resource)
@scope.stubs(:find_hostclass).with("notfound").returns(nil)
+ @scope.stubs(:class_scope).with(@class)
Puppet::Parser::Resource.stubs(:new).returns(@resource)
@class.stubs :mk_plain_resource
@@ -534,7 +529,7 @@ describe Puppet::Parser::Compiler do
# The #evaluate method normally does this.
scope = stub 'scope', :source => "mysource"
- @compiler.class_set(node_class.name, scope)
+ @compiler.topscope.expects(:class_scope).with(node_class).returns(scope)
node_resource.stubs(:evaluate)
@compiler.compile
@@ -582,23 +577,4 @@ describe Puppet::Parser::Compiler do
lambda { @compiler.compile }.should raise_error(Puppet::ParseError)
end
end
-
- # #620 - Nodes and classes should conflict, else classes don't get evaluated
- describe "when evaluating nodes and classes with the same name (#620)" do
-
- before do
- @node = stub :nodescope? => true
- @class = stub :nodescope? => false
- end
-
- it "should fail if a node already exists with the same name as the class being evaluated" do
- @compiler.class_set("one", @node)
- lambda { @compiler.class_set("one", @class) }.should raise_error(Puppet::ParseError)
- end
-
- it "should fail if a class already exists with the same name as the node being evaluated" do
- @compiler.class_set("one", @class)
- lambda { @compiler.class_set("one", @node) }.should raise_error(Puppet::ParseError)
- end
- end
end
diff --git a/spec/unit/parser/resource_type.rb b/spec/unit/parser/resource_type.rb
index ddf8ce9..3afab69 100755
--- a/spec/unit/parser/resource_type.rb
+++ b/spec/unit/parser/resource_type.rb
@@ -214,6 +214,7 @@ describe Puppet::Parser::ResourceType do
@scope.expects(:setvar).with("foo", "bar")
@scope.expects(:setvar).with("boo", "baz")
+ @scope.stubs(:class_set).with("foo", at scope)
@type.set_resource_parameters(@resource, @scope)
end
@@ -222,6 +223,7 @@ describe Puppet::Parser::ResourceType do
@type.set_arguments :foo => nil
@resource.expects(:to_hash).returns(:foo => "bar")
@scope.expects(:setvar).with("foo", "bar")
+ @scope.stubs(:class_set).with("foo", at scope)
@type.set_resource_parameters(@resource, @scope)
end
@@ -238,6 +240,7 @@ describe Puppet::Parser::ResourceType do
@resource.expects(:to_hash).returns({})
@scope.expects(:setvar).with("foo", "something")
+ @scope.stubs(:class_set).with("foo", at scope)
@type.set_resource_parameters(@resource, @scope)
end
@@ -254,6 +257,7 @@ describe Puppet::Parser::ResourceType do
@resource.expects(:title).returns 'teetle'
@scope.expects(:setvar).with("title", "teetle")
+ @scope.stubs(:class_set).with("foo", at scope)
@type.set_resource_parameters(@resource, @scope)
end
@@ -263,6 +267,7 @@ describe Puppet::Parser::ResourceType do
@resource.expects(:name).returns 'nombre'
@scope.expects(:setvar).with("name", "nombre")
+ @scope.stubs(:class_set).with("foo", at scope)
@type.set_resource_parameters(@resource, @scope)
end
diff --git a/spec/unit/parser/scope.rb b/spec/unit/parser/scope.rb
index d7800e4..04ae304 100755
--- a/spec/unit/parser/scope.rb
+++ b/spec/unit/parser/scope.rb
@@ -10,6 +10,41 @@ describe Puppet::Parser::Scope do
@scope = Puppet::Parser::Scope.new()
@scope.parent = @topscope
end
+
+ it "should be able to store references to class scopes" do
+ lambda { @scope.class_set "myname", "myscope" }.should_not raise_error
+ end
+
+ it "should be able to retrieve class scopes by name" do
+ @scope.class_set "myname", "myscope"
+ @scope.class_scope("myname").should == "myscope"
+ end
+
+ it "should be able to retrieve class scopes by object" do
+ klass = mock 'ast_class'
+ klass.expects(:name).returns("myname")
+ @scope.class_set "myname", "myscope"
+ @scope.class_scope(klass).should == "myscope"
+ end
+
+ # #620 - Nodes and classes should conflict, else classes don't get evaluated
+ describe "when evaluating nodes and classes with the same name (#620)" do
+
+ before do
+ @node = stub :nodescope? => true
+ @class = stub :nodescope? => false
+ end
+
+ it "should fail if a node already exists with the same name as the class being evaluated" do
+ @scope.class_set("one", @node)
+ lambda { @scope.class_set("one", @class) }.should raise_error(Puppet::ParseError)
+ end
+
+ it "should fail if a class already exists with the same name as the node being evaluated" do
+ @scope.class_set("one", @class)
+ lambda { @scope.class_set("one", @node) }.should raise_error(Puppet::ParseError)
+ end
+ end
describe "when looking up a variable" do
it "should default to an empty string" do
@@ -52,7 +87,7 @@ describe Puppet::Parser::Scope do
klass = @parser.newclass(name)
Puppet::Parser::Resource.new(:type => "class", :title => name, :scope => @scope, :source => mock('source')).evaluate
- return @compiler.class_scope(klass)
+ return @scope.class_scope(klass)
end
it "should be able to look up explicitly fully qualified variables from main" do
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list