The following manifest:
case $var {
/match/: {
if $var =~ /matchagain/ {
}
}
}
is failing because the "=~" operators when matching sets an ephemeral
variable in the scope. But the case regex also did it, and since they
both belong to the same scope, and Puppet variables are immutables, the
scope raises an error.
This patch fixes this issue by adding to the current scope a stack
of ephemeral symbol tables. Each new match operator or case/selector
with regex adds a new scope. When we get out of the case/if/selector
structure the scope is reset to the ephemeral level we were when
entering it.
This way the following manifest produces the correct output:
case $var {
/match(rematch)/: {
notice("1. \$0 = $0, \$1 = $1")
if $var =~ /matchagain/ {
notice("2. \$0 = $0, \$1 = $1")
}
notice("3. \$0 = $0, \$1 = $1")
}
}
notice("4. \$0 = $0")
And the output is:
1. $0 = match, $1 = rematch
2. $0 = matchagain, $1 = rematch
3. $0 = match, $1 = rematch
4. $0 =
Signed-off-by: Brice Figureau <[email protected]>
---
lib/puppet/parser/ast/casestatement.rb | 4 +-
lib/puppet/parser/ast/ifstatement.rb | 3 +-
lib/puppet/parser/ast/selector.rb | 3 +-
lib/puppet/parser/scope.rb | 70 ++++++++++++++++++++---
spec/unit/parser/ast/casestatement.rb | 6 +-
spec/unit/parser/ast/ifstatement.rb | 3 +-
spec/unit/parser/ast/selector.rb | 6 +-
spec/unit/parser/scope.rb | 97 +++++++++++++++++++++++++++++++-
8 files changed, 172 insertions(+), 20 deletions(-)
diff --git a/lib/puppet/parser/ast/casestatement.rb
b/lib/puppet/parser/ast/casestatement.rb
index 25b0fc6..ba459f9 100644
--- a/lib/puppet/parser/ast/casestatement.rb
+++ b/lib/puppet/parser/ast/casestatement.rb
@@ -11,6 +11,8 @@ class Puppet::Parser::AST
# Short-curcuit evaluation. Return the value of the statements for
# the first option that matches.
def evaluate(scope)
+ level = scope.ephemeral_level
+
value = @test.safeevaluate(scope)
sensitive = Puppet[:casesensitive]
value = value.downcase if ! sensitive and
value.respond_to?(:downcase)
@@ -34,7 +36,7 @@ class Puppet::Parser::AST
Puppet.debug "No true answers and no default"
return nil
ensure
- scope.unset_ephemeral_var
+ scope.unset_ephemeral_var(level)
end
def each
diff --git a/lib/puppet/parser/ast/ifstatement.rb
b/lib/puppet/parser/ast/ifstatement.rb
index 9d52123..d84445b 100644
--- a/lib/puppet/parser/ast/ifstatement.rb
+++ b/lib/puppet/parser/ast/ifstatement.rb
@@ -16,6 +16,7 @@ class Puppet::Parser::AST
# else if there's an 'else' setting, evaluate it.
# the first option that matches.
def evaluate(scope)
+ level = scope.ephemeral_level
value = @test.safeevaluate(scope)
# let's emulate a new scope for each branches
@@ -30,7 +31,7 @@ class Puppet::Parser::AST
end
end
ensure
- scope.unset_ephemeral_var
+ scope.unset_ephemeral_var(level)
end
end
end
diff --git a/lib/puppet/parser/ast/selector.rb
b/lib/puppet/parser/ast/selector.rb
index 84bc2a7..d8c1007 100644
--- a/lib/puppet/parser/ast/selector.rb
+++ b/lib/puppet/parser/ast/selector.rb
@@ -12,6 +12,7 @@ class Puppet::Parser::AST
# Find the value that corresponds with the test.
def evaluate(scope)
+ level = scope.ephemeral_level
# Get our parameter.
paramvalue = @param.safeevaluate(scope)
@@ -39,7 +40,7 @@ class Puppet::Parser::AST
self.fail Puppet::ParseError, "No matching value for selector
param '%s'" % paramvalue
ensure
- scope.unset_ephemeral_var
+ scope.unset_ephemeral_var(level)
end
def to_s
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index d6d6630..3623f40 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -20,6 +20,30 @@ class Puppet::Parser::Scope
attr_accessor :top, :translated, :compiler
attr_writer :parent
+ # thin wrapper around an ephemeral
+ # symbol table.
+ # when a symbol
+ class Ephemeral
+ def initialize(parent=nil)
+ @symbols = {}
+ @parent = parent
+ end
+
+ [:include?, :delete, :[]=].each do |m|
+ define_method(m) do |*args|
+ @symbols.send(m, *args)
+ end
+ end
+
+ def [](name)
+ unless @symbols.include?(name) or @parent.nil?
+ @parent[name]
+ else
+ @symbols[name]
+ end
+ end
+ end
+
# A demeterific shortcut to the catalog.
def catalog
compiler.catalog
@@ -131,7 +155,9 @@ class Puppet::Parser::Scope
# the ephemeral symbol tables
# those should not persist long, and are used for the moment only
# for $0..$xy capture variables of regexes
- @ephemeral = {}
+ # this is actually implemented as a stack, with each ephemeral scope
+ # shadowing the previous one
+ @ephemeral = [ Ephemeral.new ]
# All of the defaults set for types. It's a hash of hashes,
# with the first key being the type, then the second key being
@@ -194,13 +220,13 @@ class Puppet::Parser::Scope
# 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)
- table = ephemeral?(name) ? @ephemeral : @symtable
+ 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 table.include?(name)
+ if ephemeral_include?(name) or table.include?(name)
if usestring and table[name] == :undef
return ""
else
@@ -293,7 +319,7 @@ class Puppet::Parser::Scope
# in scopes above, but will not allow variables in the current scope
# to be reassigned.
def setvar(name,value, options = {})
- table = options[:ephemeral] ? @ephemeral : @symtable
+ table = options[:ephemeral] ? @ephemeral.last : @symtable
#Puppet.debug "Setting %s to '%s' at level %s mode append %s" %
# [name.inspect,value,self.level, append]
if table.include?(name)
@@ -338,7 +364,7 @@ class Puppet::Parser::Scope
else # look the variable up
# make sure $0-$9 are lookupable only if ephemeral
var = ss[1] || ss[3] || ss[4]
- if var and var =~ /^[0-9]+$/ and not ephemeral?(var)
+ if var and var =~ /^[0-9]+$/ and not
ephemeral_include?(var)
next
end
out << lookupvar(var).to_s || ""
@@ -403,23 +429,49 @@ class Puppet::Parser::Scope
# Undefine a variable; only used for testing.
def unsetvar(var)
- table = ephemeral?(var) ? @ephemeral : @symtable
+ table = ephemeral?(var) ? @ephemeral.last : @symtable
if table.include?(var)
table.delete(var)
end
end
- def unset_ephemeral_var
- @ephemeral = {}
+ # remove ephemeral scope up to level
+ def unset_ephemeral_var(level=:all)
+ if level == :all
+ @ephemeral = [ Ephemeral.new ]
+ else
+ (@ephemeral.size - level).times do
+ @ephemeral.pop
+ end
+ end
+ end
+
+ # check if name exists in one of the ephemeral scope.
+ def ephemeral_include?(name)
+ @ephemeral.reverse.each do |eph|
+ return true if eph.include?(name)
+ end
+ false
end
+ # is name an ephemeral variable?
def ephemeral?(name)
- @ephemeral.include?(name)
+ name =~ /^\d+$/
+ end
+
+ def ephemeral_level
+ @ephemeral.size
+ end
+
+ def new_ephemeral
+ @ephemeral.push(Ephemeral.new(@ephemeral.last))
end
def ephemeral_from(match, file = nil, line = nil)
raise(ArgumentError,"Invalid regex match data") unless
match.is_a?(MatchData)
+ new_ephemeral
+
setvar("0", match[0], :file => file, :line => line, :ephemeral => true)
match.captures.each_with_index do |m,i|
setvar("#{i+1}", m, :file => file, :line => line, :ephemeral =>
true)
diff --git a/spec/unit/parser/ast/casestatement.rb
b/spec/unit/parser/ast/casestatement.rb
index 554e295..8db7963 100755
--- a/spec/unit/parser/ast/casestatement.rb
+++ b/spec/unit/parser/ast/casestatement.rb
@@ -121,19 +121,21 @@ describe Puppet::Parser::AST::CaseStatement do
end
it "should unset scope ephemeral variables after option
evaluation" do
+ @scope.stubs(:ephemeral_level).returns(:level)
@opval1.stubs(:evaluate_match).with { |*arg| arg[0] == "value"
and arg[1] == @scope }.returns(true)
@option1.stubs(:safeevaluate).with(@scope).returns(:result)
- @scope.expects(:unset_ephemeral_var)
+ @scope.expects(:unset_ephemeral_var).with(:level)
@casestmt.evaluate(@scope)
end
it "should not leak ephemeral variables even if evaluation fails"
do
+ @scope.stubs(:ephemeral_level).returns(:level)
@opval1.stubs(:evaluate_match).with { |*arg| arg[0] == "value"
and arg[1] == @scope }.returns(true)
@option1.stubs(:safeevaluate).with(@scope).raises
- @scope.expects(:unset_ephemeral_var)
+ @scope.expects(:unset_ephemeral_var).with(:level)
lambda { @casestmt.evaluate(@scope) }.should raise_error
end
diff --git a/spec/unit/parser/ast/ifstatement.rb
b/spec/unit/parser/ast/ifstatement.rb
index 10d877a..83121cd 100755
--- a/spec/unit/parser/ast/ifstatement.rb
+++ b/spec/unit/parser/ast/ifstatement.rb
@@ -64,10 +64,11 @@ describe Puppet::Parser::AST::IfStatement do
end
it "should reset ephemeral statements after evaluation" do
+ @scope.expects(:ephemeral_level).returns(:level)
Puppet::Parser::Scope.stubs(:true?).returns(true)
@stmt.expects(:safeevaluate).with(@scope)
- @scope.expects(:unset_ephemeral_var)
+ @scope.expects(:unset_ephemeral_var).with(:level)
@ifstmt.evaluate(@scope)
end
diff --git a/spec/unit/parser/ast/selector.rb b/spec/unit/parser/ast/selector.rb
index 2ba83ad..56618ff 100755
--- a/spec/unit/parser/ast/selector.rb
+++ b/spec/unit/parser/ast/selector.rb
@@ -128,19 +128,21 @@ describe Puppet::Parser::AST::Selector do
end
it "should unset scope ephemeral variables after option
evaluation" do
+ @scope.stubs(:ephemeral_level).returns(:level)
@param1.stubs(:evaluate_match).with { |*arg| arg[0] == "value"
and arg[1] == @scope }.returns(true)
@value1.stubs(:safeevaluate).with(@scope).returns(:result)
- @scope.expects(:unset_ephemeral_var)
+ @scope.expects(:unset_ephemeral_var).with(:level)
@selector.evaluate(@scope)
end
it "should not leak ephemeral variables even if evaluation fails"
do
+ @scope.stubs(:ephemeral_level).returns(:level)
@param1.stubs(:evaluate_match).with { |*arg| arg[0] == "value"
and arg[1] == @scope }.returns(true)
@value1.stubs(:safeevaluate).with(@scope).raises
- @scope.expects(:unset_ephemeral_var)
+ @scope.expects(:unset_ephemeral_var).with(:level)
lambda { @selector.evaluate(@scope) }.should raise_error
end
diff --git a/spec/unit/parser/scope.rb b/spec/unit/parser/scope.rb
index d7800e4..4413be2 100755
--- a/spec/unit/parser/scope.rb
+++ b/spec/unit/parser/scope.rb
@@ -221,6 +221,85 @@ describe Puppet::Parser::Scope do
@scope.lookupvar("myvar", false).should == :value1
end
+
+ it "should raise an error when setting it again" do
+ @scope.setvar("1", :value2, :ephemeral => true)
+ lambda { @scope.setvar("1", :value3, :ephemeral => true) }.should
raise_error
+ end
+
+ it "should declare ephemeral number only variable names" do
+ @scope.ephemeral?("0").should be_true
+ end
+
+ it "should not declare ephemeral other variable names" do
+ @scope.ephemeral?("abc0").should be_nil
+ end
+
+ describe "with more than one level" do
+ it "should prefer latest ephemeral scopes" do
+ @scope.setvar("0", :earliest, :ephemeral => true)
+ @scope.new_ephemeral
+ @scope.setvar("0", :latest, :ephemeral => true)
+ @scope.lookupvar("0", false).should == :latest
+ end
+
+ it "should be able to report the current level" do
+ @scope.ephemeral_level.should == 1
+ @scope.new_ephemeral
+ @scope.ephemeral_level.should == 2
+ end
+
+ it "should check presence of an ephemeral variable accross
multiple levels" do
+ @scope.new_ephemeral
+ @scope.setvar("1", :value1, :ephemeral => true)
+ @scope.new_ephemeral
+ @scope.setvar("0", :value2, :ephemeral => true)
+ @scope.new_ephemeral
+ @scope.ephemeral_include?("1").should be_true
+ end
+
+ it "should return false when an ephemeral variable doesn't exist
in any ephemeral scope" do
+ @scope.new_ephemeral
+ @scope.setvar("1", :value1, :ephemeral => true)
+ @scope.new_ephemeral
+ @scope.setvar("0", :value2, :ephemeral => true)
+ @scope.new_ephemeral
+ @scope.ephemeral_include?("2").should be_false
+ end
+
+ it "should get ephemeral values from earlier scope when not in
later" do
+ @scope.setvar("1", :value1, :ephemeral => true)
+ @scope.new_ephemeral
+ @scope.setvar("0", :value2, :ephemeral => true)
+ @scope.lookupvar("1", false).should == :value1
+ end
+
+ describe "when calling unset_ephemeral_var without a level" do
+ it "should remove all the variables values" do
+ @scope.setvar("1", :value1, :ephemeral => true)
+ @scope.new_ephemeral
+ @scope.setvar("1", :value2, :ephemeral => true)
+
+ @scope.unset_ephemeral_var
+
+ @scope.lookupvar("1", false).should == :undefined
+ end
+ end
+
+ describe "when calling unset_ephemeral_var with a level" do
+ it "should remove ephemeral scopes up to this level" do
+ @scope.setvar("1", :value1, :ephemeral => true)
+ @scope.new_ephemeral
+ @scope.setvar("1", :value2, :ephemeral => true)
+ @scope.new_ephemeral
+ @scope.setvar("1", :value3, :ephemeral => true)
+
+ @scope.unset_ephemeral_var(2)
+
+ @scope.lookupvar("1", false).should == :value2
+ end
+ end
+ end
end
describe "when interpolating string" do
@@ -278,6 +357,11 @@ describe Puppet::Parser::Scope do
@scope.ephemeral_from(@match)
end
+
+ it "should create a new ephemeral level" do
+ @scope.expects(:new_ephemeral)
+ @scope.ephemeral_from(@match)
+ end
end
describe "when unsetting variables" do
@@ -288,9 +372,16 @@ describe Puppet::Parser::Scope do
end
it "should be able to unset ephemeral variables" do
- @scope.setvar("foo", "bar", :ephemeral => true)
- @scope.unsetvar("foo")
- @scope.lookupvar("foo").should == ""
+ @scope.setvar("0", "bar", :ephemeral => true)
+ @scope.unsetvar("0")
+ @scope.lookupvar("0").should == ""
+ end
+
+ it "should not unset ephemeral variables in previous ephemeral scope"
do
+ @scope.setvar("0", "bar", :ephemeral => true)
+ @scope.new_ephemeral
+ @scope.unsetvar("0")
+ @scope.lookupvar("0").should == "bar"
end
end
end
--
1.6.6.1
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
