Permalink
Browse files

Implement newlookupvar() to replace dynamic scope

lookupvar() is shifted to oldlookupvar() and newlookupvar() is added. An
intermediary lookupvar() function will query both and if the answer
varies then it will throw a deprecation warning for dynamic scoping. The
intermediary and old lookup functions may be removed at a later date,
thus completing the transition.

A test case has been introduced to detect dynamic scoping and the
deprecation warning. Slight modifications to the spec test scoping
objects were made to bring them more in line with the real world.

All scope tests pass. When oldlookupvar is replaced, the deprecated
dynamic scoping test case will fail and all other scope test cases will
pass.
  • Loading branch information...
1 parent 6ed85d7 commit 47da3e5bac12df35942c8877c1eafb9b3d9d7d79 @hunner committed Mar 16, 2012
Showing with 43 additions and 12 deletions.
  1. +39 −8 lib/puppet/parser/scope.rb
  2. +3 −3 spec/unit/parser/scope_spec.rb
  3. +1 −1 spec/unit/resource/type_spec.rb
View
@@ -222,27 +222,58 @@ def qualified_scope(classname)
private :qualified_scope
- # Look up a variable. The simplest value search we do.
+ # Look up a variable with traditional scoping and then with new scoping. If
+ # the answers differ then print a deprecation warning.
def lookupvar(name, options = {})
+ oldval = oldlookupvar(name,options)
+ newval = newlookupvar(name,options)
+ if oldval and newval and newval != oldval
+ location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
+ Puppet.deprecation_warning "Dynamic lookup of $#{name}#{location} is deprecated. Support will be removed in a later version of Puppet. Use a fully-qualified variable name (e.g., $classname::variable) or parameterized classes."
+ end
+ oldval
+ end
+
+ # Look up a variable. The simplest value search we do.
+ def newlookupvar(name, options = {})
+ # Save the originating scope for the request
+ options[:origin] = self unless options[:origin]
@nicklewis

nicklewis Mar 23, 2012

This can just be options[:origin] ||= self.

+ table = ephemeral?(name) ? @ephemeral.last : @symtable
+ if name =~ /^(.*)::(.+)$/
+ begin
+ qualified_scope($1).newlookupvar($2,options.merge({:origin => nil}))
+ rescue RuntimeError => e
+ location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
+ warning "Could not look up qualified variable '#{name}'; #{e.message}#{location}"
+ :undefined
+ end
+ # If the value is present and either we are top/node scope or originating scope...
+ elsif (ephemeral_include?(name) or table.include?(name)) and (compiler and self == compiler.topscope or (self.resource and self.resource.type == "Node") or self == options[:origin])
@nicklewis

nicklewis Mar 23, 2012

Extracting some of these tests into methods would be helpful for clarity.

elsif (ephemeral_include?(name) or table.include?(name)) and (options[:origin] == self or topscope? or node_scope?)

+ table[name]
+ elsif parent
+ parent.newlookupvar(name,options)
+ else
+ :undefined
+ end
+ end
+
+ # Look up a variable. The simplest value search we do.
+ def oldlookupvar(name, options = {})
table = ephemeral?(name) ? @ephemeral.last : @symtable
# If the variable is qualified, then find the specified scope and look the variable up there instead.
if name =~ /^(.*)::(.+)$/
begin
- qualified_scope($1).lookupvar($2,options)
+ qualified_scope($1).oldlookupvar($2,options)
rescue RuntimeError => e
location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
warning "Could not look up qualified variable '#{name}'; #{e.message}#{location}"
:undefined
end
elsif ephemeral_include?(name) or table.include?(name)
# We can't use "if table[name]" here because the value might be false
- if options[:dynamic] and self != compiler.topscope
- location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
- Puppet.deprecation_warning "Dynamic lookup of $#{name}#{location} is deprecated. Support will be removed in Puppet 2.8. Use a fully-qualified variable name (e.g., $classname::variable) or parameterized classes."
- end
table[name]
elsif parent
- parent.lookupvar(name,options.merge(:dynamic => (dynamic || options[:dynamic])))
+ parent.oldlookupvar(name,options.merge(:dynamic => (dynamic || options[:dynamic])))
else
:undefined
end
@@ -374,7 +405,7 @@ def unset_ephemeral_var(level=:all)
# check if name exists in one of the ephemeral scope.
def ephemeral_include?(name)
- @ephemeral.reverse.each do |eph|
+ @ephemeral.reverse_each do |eph|
return true if eph.include?(name)
end
false
@@ -3,13 +3,12 @@
describe Puppet::Parser::Scope do
before :each do
- @topscope = Puppet::Parser::Scope.new
# This is necessary so we don't try to use the compiler to discover our parent.
- @topscope.parent = nil
@scope = Puppet::Parser::Scope.new
@scope.compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("foo"))
+ @scope.source = Puppet::Resource::Type.new(:node, :foo)
+ @topscope = @scope.compiler.topscope
@scope.parent = @topscope
- @topscope.compiler = @scope.compiler
end
it "should be able to store references to class scopes" do
@@ -134,6 +133,7 @@
thirdscope = Puppet::Parser::Scope.new
thirdscope.parent = @scope
thirdscope.source = Puppet::Resource::Type.new(:hostclass, :foo, :module_name => "foo")
+ @scope.source = Puppet::Resource::Type.new(:hostclass, :bar, :module_name => "bar")
@topscope.setvar("var2","parentval")
@scope.setvar("var2","childval")
@@ -238,7 +238,7 @@ def double_convert
describe "when setting its parameters in the scope" do
before do
- @scope = Puppet::Parser::Scope.new(:compiler => stub("compiler", :environment => Puppet::Node::Environment.new), :source => stub("source"))
+ @scope = Puppet::Parser::Scope.new(:compiler => Puppet::Parser::Compiler.new(Puppet::Node.new("foo")), :source => stub("source"))
@resource = Puppet::Parser::Resource.new(:foo, "bar", :scope => @scope)
@type = Puppet::Resource::Type.new(:hostclass, "foo")
end

0 comments on commit 47da3e5

Please sign in to comment.