From 47da3e5bac12df35942c8877c1eafb9b3d9d7d79 Mon Sep 17 00:00:00 2001 From: Hunter Haugen Date: Thu, 15 Mar 2012 17:19:47 -0700 Subject: [PATCH] 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. --- lib/puppet/parser/scope.rb | 47 +++++++++++++++++++++++++++------ spec/unit/parser/scope_spec.rb | 6 ++--- spec/unit/resource/type_spec.rb | 2 +- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb index 746cfbd4448..2a51a072eb8 100644 --- a/lib/puppet/parser/scope.rb +++ b/lib/puppet/parser/scope.rb @@ -222,13 +222,48 @@ 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] + 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]) + 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}" @@ -236,13 +271,9 @@ def lookupvar(name, options = {}) 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 diff --git a/spec/unit/parser/scope_spec.rb b/spec/unit/parser/scope_spec.rb index c3a4ca1a3cd..82a30503bc9 100755 --- a/spec/unit/parser/scope_spec.rb +++ b/spec/unit/parser/scope_spec.rb @@ -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") diff --git a/spec/unit/resource/type_spec.rb b/spec/unit/resource/type_spec.rb index 352f767e45e..027ad7a050a 100755 --- a/spec/unit/resource/type_spec.rb +++ b/spec/unit/resource/type_spec.rb @@ -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