[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:33 UTC 2011
The following commit has been merged in the experimental branch:
commit 31f8e660c8f4c0ec01f140322cf7c585144a0888
Author: Markus Roberts <Markus at reality.com>
Date: Thu Oct 21 17:24:09 2010 -0700
Refactor en route to #5027 -- remove usestring parameter from lookupvar
The usestring parameter to lookupvar was objectionable for several reasons;
first, it performed a function orthogonal to the main purpose of the method,
second its default was the least common value, and third it was causing other
code to work for reasons that were not obvious (extlookup).
This refactor breaks the value-transforming function out into a seperate
method which allows the user to specify the value to be used in lieu of :undef
and removes the parameter. The function, Scope#undef_as(default,exp) is
written so that it can be used in user code (templates, functions, etc.) if
needed.
This refactor will introduce a user-visible behaviour change in the case where
users were counting on lookupvar to return "" for undefined variables. The
best solution is to have them use undef_as, replacing:
lookupvar('myvar')
with
undef_as('',lookupvar('myvar'))
(with the option to specify another default value if desired). If this is too
objectionable, we could rename the existing lookupvar as raw_lookupvar and
define
def lookupvar(v)
undef_as('',raw_lookupvar(v))
end
to restore the present behaviour.
diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb
index 77617e9..b61634d 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, false)) == :undefined
+ if (var = scope.lookupvar(@value)) == :undefined
var = :undef
end
var
diff --git a/lib/puppet/parser/functions/extlookup.rb b/lib/puppet/parser/functions/extlookup.rb
index b5688d6..5fbf26c 100644
--- a/lib/puppet/parser/functions/extlookup.rb
+++ b/lib/puppet/parser/functions/extlookup.rb
@@ -91,9 +91,9 @@ This is for back compatibility to interpolate variables with %. % interpolation
raise Puppet::ParseError, ("extlookup(): wrong number of arguments (#{args.length}; must be <= 3)") if args.length > 3
- extlookup_datadir = lookupvar('::extlookup_datadir')
+ extlookup_datadir = undef_as('',lookupvar('::extlookup_datadir'))
- extlookup_precedence = lookupvar('::extlookup_precedence').collect { |var| var.gsub(/%\{(.+?)\}/) { lookupvar("::#{$1}") } }
+ extlookup_precedence = undef_as([],lookupvar('::extlookup_precedence')).collect { |var| var.gsub(/%\{(.+?)\}/) { lookupvar("::#{$1}") } }
datafiles = Array.new
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index c007d4d..ace01bb 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -241,7 +241,7 @@ class Puppet::Parser::Resource < Puppet::Resource
def add_backward_compatible_relationship_param(name)
# Skip metaparams for which we get no value.
- return unless val = scope.lookupvar(name.to_s, false) and val != :undefined
+ return unless val = scope.lookupvar(name.to_s) and val != :undefined
# The default case: just set the value
set_parameter(name, val) and return unless @parameters[name]
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index 0734755..df30791 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -210,45 +210,41 @@ class Puppet::Parser::Scope
find_definition(name) || find_hostclass(name)
end
- def lookup_qualified_var(name, usestring)
+ def undef_as(x,v)
+ (v == :undefined) ? x : (v == :undef) ? x : v
+ end
+
+ def lookup_qualified_var(name)
parts = name.split(/::/)
shortname = parts.pop
klassname = parts.join("::")
klass = find_hostclass(klassname)
unless klass
warning "Could not look up qualified variable '#{name}'; class #{klassname} could not be found"
- return usestring ? "" : :undefined
+ return :undefined
end
unless kscope = class_scope(klass)
warning "Could not look up qualified variable '#{name}'; class #{klassname} has not been evaluated"
- return usestring ? "" : :undefined
+ return :undefined
end
- kscope.lookupvar(shortname, usestring)
+ kscope.lookupvar(shortname)
end
private :lookup_qualified_var
- # Look up a variable. The simplest value search we do. Default to returning
- # an empty string for missing values, but support returning a constant.
- def lookupvar(name, usestring = true)
+ # Look up a variable. The simplest value search we do.
+ def lookupvar(name)
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 =~ /::/
- return lookup_qualified_var(name, usestring)
- end
- # We can't use "if table[name]" here because the value might be false
- if ephemeral_include?(name) or table.include?(name)
- if usestring and table[name] == :undef
- return ""
- else
- return table[name]
- end
- elsif self.parent
- return parent.lookupvar(name, usestring)
- elsif usestring
- return ""
+ lookup_qualified_var(name)
+ 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)
else
- return :undefined
+ :undefined
end
end
@@ -333,7 +329,7 @@ class Puppet::Parser::Scope
table[name] = value
else # append case
# lookup the value in the scope if it exists and insert the var
- table[name] = lookupvar(name)
+ table[name] = undef_as('',lookupvar(name))
# concatenate if string, append if array, nothing for other types
case value
when Array
@@ -363,7 +359,7 @@ class Puppet::Parser::Scope
if var and var =~ /^[0-9]+$/ and not ephemeral_include?(var)
next
end
- out << lookupvar(var).to_s || ""
+ out << undef_as('',lookupvar(var)).to_s
end
elsif ss.scan(/^\\(.)/)
# Puppet.debug("Got escape: pos:%d; m:%s" % [ss.pos, ss.matched])
diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb
index 6864aa1..73fcb8a 100644
--- a/lib/puppet/parser/templatewrapper.rb
+++ b/lib/puppet/parser/templatewrapper.rb
@@ -20,11 +20,7 @@ class Puppet::Parser::TemplateWrapper
# Should return true if a variable is defined, false if it is not
def has_variable?(name)
- if scope.lookupvar(name.to_s, false) != :undefined
- true
- else
- false
- end
+ scope.lookupvar(name.to_s) != :undefined
end
# Allow templates to access the defined classes
@@ -55,9 +51,7 @@ class Puppet::Parser::TemplateWrapper
# the missing_method definition here until we declare the syntax finally
# dead.
def method_missing(name, *args)
- # We have to tell lookupvar to return :undefined to us when
- # appropriate; otherwise it converts to "".
- value = scope.lookupvar(name.to_s, false)
+ value = scope.lookupvar(name.to_s)
if value != :undefined
return value
else
diff --git a/spec/unit/parser/ast/arithmetic_operator_spec.rb b/spec/unit/parser/ast/arithmetic_operator_spec.rb
index 6f57fd9..0aab6f0 100755
--- a/spec/unit/parser/ast/arithmetic_operator_spec.rb
+++ b/spec/unit/parser/ast/arithmetic_operator_spec.rb
@@ -61,8 +61,8 @@ describe Puppet::Parser::AST::ArithmeticOperator do
end
it "should work for variables too" do
- @scope.expects(:lookupvar).with("one", false).returns(1)
- @scope.expects(:lookupvar).with("two", false).returns(2)
+ @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" )
diff --git a/spec/unit/parser/ast/comparison_operator_spec.rb b/spec/unit/parser/ast/comparison_operator_spec.rb
index 169d92d..3efe28b 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", false).returns(1)
- @scope.expects(:lookupvar).with("two", false).returns(2)
+ @scope.expects(:lookupvar).with("one").returns(1)
+ @scope.expects(:lookupvar).with("two").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 3b8c14e..97c996b 100755
--- a/spec/unit/parser/ast/leaf_spec.rb
+++ b/spec/unit/parser/ast/leaf_spec.rb
@@ -336,12 +336,12 @@ describe Puppet::Parser::AST::Variable do
end
it "should lookup the variable in scope" do
- @scope.expects(:lookupvar).with("myvar", false).returns(:myvalue)
+ @scope.expects(:lookupvar).with("myvar").returns(:myvalue)
@var.safeevaluate(@scope).should == :myvalue
end
it "should return undef if the variable wasn't set" do
- @scope.expects(:lookupvar).with("myvar", false).returns(:undefined)
+ @scope.expects(:lookupvar).with("myvar").returns(:undefined)
@var.safeevaluate(@scope).should == :undef
end
diff --git a/spec/unit/parser/scope_spec.rb b/spec/unit/parser/scope_spec.rb
index 44e2559..b37cb4a 100755
--- a/spec/unit/parser/scope_spec.rb
+++ b/spec/unit/parser/scope_spec.rb
@@ -76,16 +76,8 @@ describe Puppet::Parser::Scope do
end
describe "when looking up a variable" do
- it "should default to an empty string" do
- @scope.lookupvar("var").should == ""
- end
-
- it "should return an string when asked for a string" do
- @scope.lookupvar("var", true).should == ""
- end
-
- it "should return ':undefined' for unset variables when asked not to return a string" do
- @scope.lookupvar("var", false).should == :undefined
+ it "should return ':undefined' for unset variables" do
+ @scope.lookupvar("var").should == :undefined
end
it "should be able to look up values" do
@@ -151,32 +143,32 @@ describe Puppet::Parser::Scope do
@scope.lookupvar("other::deep::klass::var").should == "otherval"
end
- it "should return an empty string for qualified variables that cannot be found in other classes" do
+ it "should return ':undefined' for qualified variables that cannot be found in other classes" do
other_scope = create_class_scope("other::deep::klass")
- @scope.lookupvar("other::deep::klass::var").should == ""
+ @scope.lookupvar("other::deep::klass::var").should == :undefined
end
- it "should warn and return an empty string for qualified variables whose classes have not been evaluated" do
+ it "should warn and return ':undefined' for qualified variables whose classes have not been evaluated" do
klass = newclass("other::deep::klass")
@scope.expects(:warning)
- @scope.lookupvar("other::deep::klass::var").should == ""
+ @scope.lookupvar("other::deep::klass::var").should == :undefined
end
- it "should warn and return an empty string for qualified variables whose classes do not exist" do
+ it "should warn and return ':undefined' for qualified variables whose classes do not exist" do
@scope.expects(:warning)
- @scope.lookupvar("other::deep::klass::var").should == ""
+ @scope.lookupvar("other::deep::klass::var").should == :undefined
end
it "should return ':undefined' when asked for a non-string qualified variable from a class that does not exist" do
@scope.stubs(:warning)
- @scope.lookupvar("other::deep::klass::var", false).should == :undefined
+ @scope.lookupvar("other::deep::klass::var").should == :undefined
end
it "should return ':undefined' when asked for a non-string qualified variable from a class that has not been evaluated" do
@scope.stubs(:warning)
klass = newclass("other::deep::klass")
- @scope.lookupvar("other::deep::klass::var", false).should == :undefined
+ @scope.lookupvar("other::deep::klass::var").should == :undefined
end
end
end
@@ -291,7 +283,7 @@ describe Puppet::Parser::Scope do
@scope.unset_ephemeral_var
- @scope.lookupvar("1", false).should == :undefined
+ @scope.lookupvar("1").should == :undefined
end
it "should not remove classic variables when unset_ephemeral_var is called" do
@@ -301,7 +293,7 @@ describe Puppet::Parser::Scope do
@scope.unset_ephemeral_var
- @scope.lookupvar("myvar", false).should == :value1
+ @scope.lookupvar("myvar").should == :value1
end
it "should raise an error when setting it again" do
@@ -322,7 +314,7 @@ describe Puppet::Parser::Scope do
@scope.setvar("0", :earliest, :ephemeral => true)
@scope.new_ephemeral
@scope.setvar("0", :latest, :ephemeral => true)
- @scope.lookupvar("0", false).should == :latest
+ @scope.lookupvar("0").should == :latest
end
it "should be able to report the current level" do
@@ -353,7 +345,7 @@ describe Puppet::Parser::Scope do
@scope.setvar("1", :value1, :ephemeral => true)
@scope.new_ephemeral
@scope.setvar("0", :value2, :ephemeral => true)
- @scope.lookupvar("1", false).should == :value1
+ @scope.lookupvar("1").should == :value1
end
describe "when calling unset_ephemeral_var without a level" do
@@ -364,7 +356,7 @@ describe Puppet::Parser::Scope do
@scope.unset_ephemeral_var
- @scope.lookupvar("1", false).should == :undefined
+ @scope.lookupvar("1").should == :undefined
end
end
@@ -378,7 +370,7 @@ describe Puppet::Parser::Scope do
@scope.unset_ephemeral_var(2)
- @scope.lookupvar("1", false).should == :value2
+ @scope.lookupvar("1").should == :value2
end
end
end
@@ -563,13 +555,13 @@ describe Puppet::Parser::Scope do
it "should be able to unset normal variables" do
@scope.setvar("foo", "bar")
@scope.unsetvar("foo")
- @scope.lookupvar("foo").should == ""
+ @scope.lookupvar("foo").should == :undefined
end
it "should be able to unset ephemeral variables" do
@scope.setvar("0", "bar", :ephemeral => true)
@scope.unsetvar("0")
- @scope.lookupvar("0").should == ""
+ @scope.lookupvar("0").should == :undefined
end
it "should not unset ephemeral variables in previous ephemeral scope" do
diff --git a/spec/unit/parser/templatewrapper_spec.rb b/spec/unit/parser/templatewrapper_spec.rb
index afb0032..4a713b8 100755
--- a/spec/unit/parser/templatewrapper_spec.rb
+++ b/spec/unit/parser/templatewrapper_spec.rb
@@ -70,25 +70,25 @@ describe Puppet::Parser::TemplateWrapper do
end
it "should return the contents of a variable if called via method_missing" do
- @scope.expects(:lookupvar).with("chicken", false).returns("is good")
+ @scope.expects(:lookupvar).with("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", false).returns(:undefined)
+ @scope.expects(:lookupvar).with("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", false).returns("is good")
+ @scope.expects(:lookupvar).with("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", false).returns(:undefined)
+ @scope.expects(:lookupvar).with("chicken").returns(:undefined)
tw = Puppet::Parser::TemplateWrapper.new(@scope)
tw.has_variable?("chicken").should eql(false)
end
diff --git a/test/language/functions.rb b/test/language/functions.rb
index e882b68..84b1b38 100755
--- a/test/language/functions.rb
+++ b/test/language/functions.rb
@@ -382,17 +382,15 @@ class TestLangFunctions < Test::Unit::TestCase
}.each do |string, value|
scope = mkscope
scope.setvar("yayness", string)
- assert_equal(string, scope.lookupvar("yayness", false))
+ assert_equal(string, scope.lookupvar("yayness"))
assert_nothing_raised("An empty string was not a valid variable value") do
ast.evaluate(scope)
end
-
- assert_equal(
- "template #{value}\n", scope.lookupvar("output"),
-
- "#{string.inspect} did not get evaluated correctly")
+ assert_equal(
+ "template #{value}\n", scope.lookupvar("output"),
+ "#{string.inspect} did not get evaluated correctly")
end
end
diff --git a/test/language/scope.rb b/test/language/scope.rb
index c4154dc..ccc3596 100755
--- a/test/language/scope.rb
+++ b/test/language/scope.rb
@@ -42,22 +42,22 @@ class TestScope < Test::Unit::TestCase
# Set a variable in the top and make sure all three can get it
topscope.setvar("first", "topval")
scopes.each do |name, scope|
- assert_equal("topval", scope.lookupvar("first", false), "Could not find var in #{name}")
+ assert_equal("topval", scope.lookupvar("first"), "Could not find var in #{name}")
end
# Now set a var in the midscope and make sure the mid and bottom can see it but not the top
midscope.setvar("second", "midval")
- assert_equal(:undefined, scopes[:top].lookupvar("second", false), "Found child var in top scope")
+ assert_equal(:undefined, scopes[:top].lookupvar("second"), "Found child var in top scope")
[:mid, :bot].each do |name|
- assert_equal("midval", scopes[name].lookupvar("second", false), "Could not find var in #{name}")
+ assert_equal("midval", scopes[name].lookupvar("second"), "Could not find var in #{name}")
end
# And set something in the bottom, and make sure we only find it there.
botscope.setvar("third", "botval")
[:top, :mid].each do |name|
- assert_equal(:undefined, scopes[name].lookupvar("third", false), "Found child var in top scope")
+ assert_equal(:undefined, scopes[name].lookupvar("third"), "Found child var in top scope")
end
- assert_equal("botval", scopes[:bot].lookupvar("third", false), "Could not find var in bottom scope")
+ assert_equal("botval", scopes[:bot].lookupvar("third"), "Could not find var in bottom scope")
# Test that the scopes convert to hash structures correctly.
@@ -260,18 +260,7 @@ Host <<||>>"
scope = mkscope
scope.setvar("testing", :undef)
-
-
- assert_equal(
- :undef, scope.lookupvar("testing", false),
-
- "undef was not returned as :undef when not string")
-
-
- assert_equal(
- "", scope.lookupvar("testing", true),
-
- "undef was not returned as '' when string")
+ assert_equal(:undef, scope.lookupvar("testing"), "undef was not returned as :undef")
end
end
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list