[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5

Nick Lewis nick at puppetlabs.com
Tue May 10 08:12:47 UTC 2011


The following commit has been merged in the experimental branch:
commit 739260b28d7c09bacbb34cd4f4d4a32cfee01385
Author: Markus Roberts <Markus at reality.com>
Date:   Sun Oct 24 23:29:38 2010 -0700

    Towards 5027 -- add options hash to lookupvar as with setvar
    
    This patch adds an options hash to lookupvar analogous to the one taken by
    setvar and uses it to pass in source location for error reporting.  It also
    fixes the mechanism used by setvar (file was not being passed correctly), adds
    line and file information to errors in templates, and extends/corrects tests.
    
    As presently written it does not gather userful line numbers from inline
    templates and there are no tests for the template line number generation.

diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb
index b61634d..c8ebc94 100644
--- a/lib/puppet/parser/ast/leaf.rb
+++ b/lib/puppet/parser/ast/leaf.rb
@@ -124,7 +124,7 @@ class Puppet::Parser::AST
     # not include syntactical constructs, like '$' and '{}').
     def evaluate(scope)
       parsewrap do
-        if (var = scope.lookupvar(@value)) == :undefined
+        if (var = scope.lookupvar(@value, :file => file, :line => line)) == :undefined
           var = :undef
         end
         var
@@ -141,7 +141,7 @@ class Puppet::Parser::AST
 
     def evaluate_container(scope)
       container = variable.respond_to?(:evaluate) ? variable.safeevaluate(scope) : variable
-      (container.is_a?(Hash) or container.is_a?(Array)) ? container : scope.lookupvar(container)
+      (container.is_a?(Hash) or container.is_a?(Array)) ? container : scope.lookupvar(container, :file => file, :line => line)
     end
 
     def evaluate_key(scope)
diff --git a/lib/puppet/parser/ast/vardef.rb b/lib/puppet/parser/ast/vardef.rb
index 6de1860..b766311 100644
--- a/lib/puppet/parser/ast/vardef.rb
+++ b/lib/puppet/parser/ast/vardef.rb
@@ -20,7 +20,7 @@ class Puppet::Parser::AST
         name = @name.safeevaluate(scope)
 
         parsewrap do
-          scope.setvar(name,value, :file => @file, :line => @line, :append => @append)
+          scope.setvar(name,value, :file => file, :line => line, :append => @append)
         end
       end
     end
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index 95b765b..a61a857 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -223,21 +223,22 @@ class Puppet::Parser::Scope
   private :qualified_scope
 
   # Look up a variable.  The simplest value search we do.  
-  def lookupvar(name)
+  def lookupvar(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)
+        qualified_scope($1).lookupvar($2,options)
       rescue RuntimeError => e
-        warning "Could not look up qualified variable '#{name}'; #{e.message}"
+        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
       table[name]
     elsif parent
-      parent.lookupvar(name)
+      parent.lookupvar(name,options)
     else
       :undefined
     end
diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb
index 73fcb8a..180a03d 100644
--- a/lib/puppet/parser/templatewrapper.rb
+++ b/lib/puppet/parser/templatewrapper.rb
@@ -18,9 +18,14 @@ class Puppet::Parser::TemplateWrapper
     @__scope__
   end
 
+  def script_line
+    # find which line in the template (if any) we were called from
+    caller.find { |l| l =~ /#{file}:/ }.first[/:(\d+):/,1]
+  end
+
   # Should return true if a variable is defined, false if it is not
   def has_variable?(name)
-    scope.lookupvar(name.to_s) != :undefined
+    scope.lookupvar(name.to_s, :file => file, :line => script_line) != :undefined
   end
 
   # Allow templates to access the defined classes
@@ -51,13 +56,13 @@ class Puppet::Parser::TemplateWrapper
   # the missing_method definition here until we declare the syntax finally
   # dead.
   def method_missing(name, *args)
-    value = scope.lookupvar(name.to_s)
+    value = scope.lookupvar(name.to_s,:file => file,:line => script_line)
     if value != :undefined
       return value
     else
       # Just throw an error immediately, instead of searching for
       # other missingmethod things or whatever.
-      raise Puppet::ParseError, "Could not find value for '#{name}'"
+      raise Puppet::ParseError.new("Could not find value for '#{name}'", at file,script_line)
     end
   end
 
@@ -97,6 +102,7 @@ class Puppet::Parser::TemplateWrapper
     result = nil
     benchmark(:debug, "Interpolated template #{template_source}") do
       template = ERB.new(self.string, 0, "-")
+      template.filename = file
       result = template.result(binding)
     end
 
diff --git a/spec/unit/parser/ast/arithmetic_operator_spec.rb b/spec/unit/parser/ast/arithmetic_operator_spec.rb
index 0aab6f0..9b54f09 100755
--- a/spec/unit/parser/ast/arithmetic_operator_spec.rb
+++ b/spec/unit/parser/ast/arithmetic_operator_spec.rb
@@ -60,14 +60,4 @@ describe Puppet::Parser::AST::ArithmeticOperator do
     operator.evaluate(@scope).should == 4.33
   end
 
-  it "should work for variables too" do
-    @scope.expects(:lookupvar).with("one").returns(1)
-    @scope.expects(:lookupvar).with("two").returns(2)
-    one = ast::Variable.new( :value => "one" )
-    two = ast::Variable.new( :value => "two" )
-
-    operator = ast::ArithmeticOperator.new :lval => one, :operator => "+", :rval => two
-    operator.evaluate(@scope).should == 3
-  end
-
 end
diff --git a/spec/unit/parser/ast/comparison_operator_spec.rb b/spec/unit/parser/ast/comparison_operator_spec.rb
index 3efe28b..0608273 100755
--- a/spec/unit/parser/ast/comparison_operator_spec.rb
+++ b/spec/unit/parser/ast/comparison_operator_spec.rb
@@ -95,8 +95,8 @@ describe Puppet::Parser::AST::ComparisonOperator do
     one = Puppet::Parser::AST::Variable.new( :value => "one" )
     two = Puppet::Parser::AST::Variable.new( :value => "two" )
 
-    @scope.expects(:lookupvar).with("one").returns(1)
-    @scope.expects(:lookupvar).with("two").returns(2)
+    one.expects(:safeevaluate).with(@scope).returns(1)
+    two.expects(:safeevaluate).with(@scope).returns(2)
 
     operator = Puppet::Parser::AST::ComparisonOperator.new :lval => one, :operator => "<", :rval => two
     operator.evaluate(@scope).should == true
diff --git a/spec/unit/parser/ast/leaf_spec.rb b/spec/unit/parser/ast/leaf_spec.rb
index 97c996b..64fdcd6 100755
--- a/spec/unit/parser/ast/leaf_spec.rb
+++ b/spec/unit/parser/ast/leaf_spec.rb
@@ -107,7 +107,7 @@ describe Puppet::Parser::AST::HashOrArrayAccess do
 
   describe "when evaluating" do
     it "should evaluate the variable part if necessary" do
-      @scope.stubs(:lookupvar).with("a").returns(["b"])
+      @scope.stubs(:lookupvar).with { |name,options| name == 'a'}.returns(["b"])
 
       variable = stub 'variable', :evaluate => "a"
       access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => variable, :key => 0 )
@@ -118,7 +118,7 @@ describe Puppet::Parser::AST::HashOrArrayAccess do
     end
 
     it "should evaluate the access key part if necessary" do
-      @scope.stubs(:lookupvar).with("a").returns(["b"])
+      @scope.stubs(:lookupvar).with { |name,options| name == 'a'}.returns(["b"])
 
       index = stub 'index', :evaluate => 0
       access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => index )
@@ -129,7 +129,7 @@ describe Puppet::Parser::AST::HashOrArrayAccess do
     end
 
     it "should be able to return an array member" do
-      @scope.stubs(:lookupvar).with("a").returns(["val1", "val2", "val3"])
+      @scope.stubs(:lookupvar).with { |name,options| name == 'a'}.returns(["val1", "val2", "val3"])
 
       access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => 1 )
 
@@ -153,7 +153,7 @@ describe Puppet::Parser::AST::HashOrArrayAccess do
     end
 
     it "should be able to return an hash value" do
-      @scope.stubs(:lookupvar).with("a").returns({ "key1" => "val1", "key2" => "val2", "key3" => "val3" })
+      @scope.stubs(:lookupvar).with { |name,options| name == 'a'}.returns({ "key1" => "val1", "key2" => "val2", "key3" => "val3" })
 
       access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => "key2" )
 
@@ -177,7 +177,7 @@ describe Puppet::Parser::AST::HashOrArrayAccess do
     end
 
     it "should raise an error if the variable wasn't in the scope" do
-      @scope.stubs(:lookupvar).with("a").returns(nil)
+      @scope.stubs(:lookupvar).with { |name,options| name == 'a'}.returns(nil)
 
       access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => "key2" )
 
@@ -190,7 +190,7 @@ describe Puppet::Parser::AST::HashOrArrayAccess do
     end
 
     it "should work with recursive hash access" do
-      @scope.stubs(:lookupvar).with("a").returns({ "key" => { "subkey" => "b" }})
+      @scope.stubs(:lookupvar).with { |name,options| name == 'a'}.returns({ "key" => { "subkey" => "b" }})
 
       access1 = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => "key")
       access2 = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => access1, :key => "subkey")
@@ -199,7 +199,7 @@ describe Puppet::Parser::AST::HashOrArrayAccess do
     end
 
     it "should work with interleaved array and hash access" do
-      @scope.stubs(:lookupvar).with("a").returns({ "key" => [ "a" , "b" ]})
+      @scope.stubs(:lookupvar).with { |name,options| name == 'a'}.returns({ "key" => [ "a" , "b" ]})
 
       access1 = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => "key")
       access2 = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => access1, :key => 1)
@@ -332,16 +332,21 @@ end
 describe Puppet::Parser::AST::Variable do
   before :each do
     @scope = stub 'scope'
-    @var = Puppet::Parser::AST::Variable.new(:value => "myvar")
+    @var = Puppet::Parser::AST::Variable.new(:value => "myvar", :file => 'my.pp', :line => 222)
   end
 
   it "should lookup the variable in scope" do
-    @scope.expects(:lookupvar).with("myvar").returns(:myvalue)
+    @scope.expects(:lookupvar).with { |name,options| name == "myvar" }.returns(:myvalue)
+    @var.safeevaluate(@scope).should == :myvalue
+  end
+
+  it "should pass the source location to lookupvar" do
+    @scope.expects(:lookupvar).with { |name,options| name == "myvar" and options[:file] == 'my.pp' and options[:line] == 222 }.returns(:myvalue)
     @var.safeevaluate(@scope).should == :myvalue
   end
 
   it "should return undef if the variable wasn't set" do
-    @scope.expects(:lookupvar).with("myvar").returns(:undefined)
+    @scope.expects(:lookupvar).with { |name,options| name == "myvar" }.returns(:undefined)
     @var.safeevaluate(@scope).should == :undef
   end
 
diff --git a/spec/unit/parser/ast/vardef_spec.rb b/spec/unit/parser/ast/vardef_spec.rb
index a462e8e..5a851bb 100755
--- a/spec/unit/parser/ast/vardef_spec.rb
+++ b/spec/unit/parser/ast/vardef_spec.rb
@@ -16,8 +16,7 @@ describe Puppet::Parser::AST::VarDef do
       name.expects(:safeevaluate).with(@scope)
       value.expects(:safeevaluate).with(@scope)
 
-      vardef = Puppet::Parser::AST::VarDef.new :name => name, :value => value, :file => nil,
-        :line => nil
+      vardef = Puppet::Parser::AST::VarDef.new :name => name, :value => value, :file => nil, :line => nil
       vardef.evaluate(@scope)
     end
 
@@ -27,8 +26,7 @@ describe Puppet::Parser::AST::VarDef do
 
       @scope.expects(:setvar).with { |name,value,options| options[:append] == nil }
 
-      vardef = Puppet::Parser::AST::VarDef.new :name => name, :value => value, :file => nil,
-        :line => nil
+      vardef = Puppet::Parser::AST::VarDef.new :name => name, :value => value, :file => nil, :line => nil
       vardef.evaluate(@scope)
     end
 
@@ -38,8 +36,17 @@ describe Puppet::Parser::AST::VarDef do
 
       @scope.expects(:setvar).with { |name,value,options| options[:append] == true }
 
-      vardef = Puppet::Parser::AST::VarDef.new :name => name, :value => value, :file => nil,
-        :line => nil, :append => true
+      vardef = Puppet::Parser::AST::VarDef.new :name => name, :value => value, :file => nil, :line => nil, :append => true
+      vardef.evaluate(@scope)
+    end
+
+    it "should call pass the source location to setvar" do
+      name = stub 'name', :safeevaluate => "var"
+      value = stub 'value', :safeevaluate => "1"
+
+      @scope.expects(:setvar).with { |name,value,options| options[:file] == 'setvar.pp' and options[:line] == 917 }
+
+      vardef = Puppet::Parser::AST::VarDef.new :name => name, :value => value, :file => 'setvar.pp', :line => 917
       vardef.evaluate(@scope)
     end
 
diff --git a/spec/unit/parser/templatewrapper_spec.rb b/spec/unit/parser/templatewrapper_spec.rb
index 4a713b8..0c7199b 100755
--- a/spec/unit/parser/templatewrapper_spec.rb
+++ b/spec/unit/parser/templatewrapper_spec.rb
@@ -17,6 +17,12 @@ describe Puppet::Parser::TemplateWrapper do
     @tw = Puppet::Parser::TemplateWrapper.new(@scope)
   end
 
+  def mock_template(source=nil)
+    template_mock = mock("template", :result => "woot!")
+    ERB.expects(:new).with("template contents", 0, "-").returns(template_mock)
+    template_mock.expects(:filename=).with(source)
+  end
+
   it "should create a new object TemplateWrapper from a scope" do
     tw = Puppet::Parser::TemplateWrapper.new(@scope)
 
@@ -54,41 +60,38 @@ describe Puppet::Parser::TemplateWrapper do
   end
 
   it "should return the processed template contents with a call to result" do
-    template_mock = mock("template", :result => "woot!")
+    mock_template("/tmp/fake_template")
     File.expects(:read).with("/tmp/fake_template").returns("template contents")
-    ERB.expects(:new).with("template contents", 0, "-").returns(template_mock)
 
     @tw.file = @file
     @tw.result.should eql("woot!")
   end
 
   it "should return the processed template contents with a call to result and a string" do
-    template_mock = mock("template", :result => "woot!")
-    ERB.expects(:new).with("template contents", 0, "-").returns(template_mock)
-
+    mock_template
     @tw.result("template contents").should eql("woot!")
   end
 
   it "should return the contents of a variable if called via method_missing" do
-    @scope.expects(:lookupvar).with("chicken").returns("is good")
+    @scope.expects(:lookupvar).with { |name,options| name == "chicken"}.returns("is good")
     tw = Puppet::Parser::TemplateWrapper.new(@scope)
     tw.chicken.should eql("is good")
   end
 
   it "should throw an exception if a variable is called via method_missing and it does not exist" do
-    @scope.expects(:lookupvar).with("chicken").returns(:undefined)
+    @scope.expects(:lookupvar).with { |name,options| name == "chicken"}.returns(:undefined)
     tw = Puppet::Parser::TemplateWrapper.new(@scope)
     lambda { tw.chicken }.should raise_error(Puppet::ParseError)
   end
 
   it "should allow you to check whether a variable is defined with has_variable?" do
-    @scope.expects(:lookupvar).with("chicken").returns("is good")
+    @scope.expects(:lookupvar).with { |name,options| name == "chicken"}.returns("is good")
     tw = Puppet::Parser::TemplateWrapper.new(@scope)
     tw.has_variable?("chicken").should eql(true)
   end
 
   it "should allow you to check whether a variable is not defined with has_variable?" do
-    @scope.expects(:lookupvar).with("chicken").returns(:undefined)
+    @scope.expects(:lookupvar).with { |name,options| name == "chicken"}.returns(:undefined)
     tw = Puppet::Parser::TemplateWrapper.new(@scope)
     tw.has_variable?("chicken").should eql(false)
   end
@@ -114,9 +117,7 @@ describe Puppet::Parser::TemplateWrapper do
   end
 
   it "should set all of the scope's variables as instance variables" do
-    template_mock = mock("template", :result => "woot!")
-    ERB.expects(:new).with("template contents", 0, "-").returns(template_mock)
-
+    mock_template
     @scope.expects(:to_hash).returns("one" => "foo")
     @tw.result("template contents")
 
@@ -124,8 +125,7 @@ describe Puppet::Parser::TemplateWrapper do
   end
 
   it "should not error out if one of the variables is a symbol" do
-    template_mock = mock("template", :result => "woot!")
-    ERB.expects(:new).with("template contents", 0, "-").returns(template_mock)
+    mock_template
 
     @scope.expects(:to_hash).returns(:_timestamp => "1234")
     @tw.result("template contents")
@@ -133,13 +133,11 @@ describe Puppet::Parser::TemplateWrapper do
 
   %w{! . ; :}.each do |badchar|
     it "should translate #{badchar} to _ when setting the instance variables" do
-    template_mock = mock("template", :result => "woot!")
-    ERB.expects(:new).with("template contents", 0, "-").returns(template_mock)
-
-    @scope.expects(:to_hash).returns("one#{badchar}" => "foo")
-    @tw.result("template contents")
+      mock_template
+      @scope.expects(:to_hash).returns("one#{badchar}" => "foo")
+      @tw.result("template contents")
 
-    @tw.instance_variable_get("@one_").should == "foo"
-  end
+      @tw.instance_variable_get("@one_").should == "foo"
+    end
   end
 end

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list