From 03c76deb60927375e9b35c74a903130930ffddf2 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Fri, 18 Jul 2008 15:21:57 +1000 Subject: Expose all puppet variables as instance member variables of the template wrapper. This helps resolve redmine #1427, by providing a safe mechanism to access variables. * Implement Puppet::Parser::Scope#to_hash, which returns a hash containing all the variable bindings in the current and, optionally, parent scope. * Use that to set instance member variables into Puppet::Parser::Templatewrapper * Report the time taken for variable binding at debug level, to help identify any performance regression that is encountered in the real world. * Rename the @scope and @file members of the template wrapper, to avoid clashing with a scope variable exposed within puppet. Signed-off-by: Daniel Pittman (cherry picked from commit ba220b41e4f509f2632e2664d332e49b20a70ea7) --- lib/puppet/parser/scope.rb | 19 +++++++++++++ lib/puppet/parser/templatewrapper.rb | 53 +++++++++++++++++++++++++++--------- 2 files changed, 59 insertions(+), 13 deletions(-) (limited to 'lib/puppet/parser') diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb index a6e43e7b3..32b127a6b 100644 --- a/lib/puppet/parser/scope.rb +++ b/lib/puppet/parser/scope.rb @@ -183,6 +183,25 @@ class Puppet::Parser::Scope end end + # Return a hash containing our variables and their values, optionally (and + # by default) including the values defined in our parent. Local values + # shadow parent values. + def to_hash(recursive = true) + if recursive and parent then + target = parent.to_hash(recursive) + end + target ||= Hash.new + @symtable.keys.each { |name| + value = @symtable[name] + if value == :undef then + target.delete(name) + else + target[name] = value + end + } + return target + end + def namespaces @namespaces.dup end diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb index 4790cea30..298428f63 100644 --- a/lib/puppet/parser/templatewrapper.rb +++ b/lib/puppet/parser/templatewrapper.rb @@ -5,36 +5,63 @@ class Puppet::Parser::TemplateWrapper include Puppet::Util Puppet::Util.logmethods(self) - def initialize(scope, file) - @scope = scope - @file = Puppet::Module::find_template(file, @scope.compiler.environment) + def initialize(scope, filename) + @__scope__ = scope + @__file__ = Puppet::Module::find_template(filename, scope.compiler.environment) - unless FileTest.exists?(@file) + unless FileTest.exists?(file) raise Puppet::ParseError, "Could not find template %s" % file end # We'll only ever not have a parser in testing, but, eh. - if @scope.parser - @scope.parser.watch_file(@file) + if scope.parser + scope.parser.watch_file(file) end + + # Expose all the variables in our scope as instance variables of the + # current object, making it possible to access them without conflict + # to the regular methods. + benchmark(:debug, "Bound template variables for #{file}") do + scope.to_hash.each { |name, value| + instance_variable_set("@#{name}", value) + } + end + end + + def scope + @__scope__ + end + + def file + @__file__ end # 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 + if scope.lookupvar(name.to_s, false) != :undefined true else false end end - # Ruby treats variables like methods, so we can cheat here and - # trap missing vars like they were missing methods. + # Ruby treats variables like methods, so we used to expose variables + # within scope to the ERB code via method_missing. As per RedMine #1427, + # though, this means that conflicts between methods in our inheritance + # tree (Kernel#fork) and variable names (fork => "yes/no") could arise. + # + # Worse, /new/ conflicts could pop up when a new kernel or object method + # was added to Ruby, causing templates to suddenly fail mysteriously when + # Ruby was upgraded. + # + # To ensure that legacy templates using unqualified names work we retain + # 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, false) if value != :undefined return value else @@ -47,8 +74,8 @@ class Puppet::Parser::TemplateWrapper def result result = nil - benchmark(:debug, "Interpolated template #{@file}") do - template = ERB.new(File.read(@file), 0, "-") + benchmark(:debug, "Interpolated template #{file}") do + template = ERB.new(File.read(file), 0, "-") result = template.result(binding) end @@ -56,7 +83,7 @@ class Puppet::Parser::TemplateWrapper end def to_s - "template[%s]" % @file + "template[%s]" % file end end -- cgit From 8a0cb16abd4c6a9cbf27d88593aa2b42a7375b94 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 7 Aug 2008 18:53:02 -0700 Subject: Added tests for TemplateWrapper's use of Scope#to_hash. We should deprecate the method_missing stuff in 0.25. Signed-off-by: Luke Kanies --- lib/puppet/parser/templatewrapper.rb | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) (limited to 'lib/puppet/parser') diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb index 298428f63..3b74e62d4 100644 --- a/lib/puppet/parser/templatewrapper.rb +++ b/lib/puppet/parser/templatewrapper.rb @@ -18,15 +18,6 @@ class Puppet::Parser::TemplateWrapper if scope.parser scope.parser.watch_file(file) end - - # Expose all the variables in our scope as instance variables of the - # current object, making it possible to access them without conflict - # to the regular methods. - benchmark(:debug, "Bound template variables for #{file}") do - scope.to_hash.each { |name, value| - instance_variable_set("@#{name}", value) - } - end end def scope @@ -67,12 +58,20 @@ class Puppet::Parser::TemplateWrapper else # Just throw an error immediately, instead of searching for # other missingmethod things or whatever. - raise Puppet::ParseError, - "Could not find value for '%s'" % name + raise Puppet::ParseError, "Could not find value for '%s'" % name end end def result + # Expose all the variables in our scope as instance variables of the + # current object, making it possible to access them without conflict + # to the regular methods. + benchmark(:debug, "Bound template variables for #{file}") do + scope.to_hash.each { |name, value| + instance_variable_set("@#{name}", value) + } + end + result = nil benchmark(:debug, "Interpolated template #{file}") do template = ERB.new(File.read(file), 0, "-") -- cgit From 6676b6b104e3f60dd14be18cbfb91b4cd96ec06e Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Thu, 14 Aug 2008 00:32:30 +1000 Subject: Fixes #1274 - allow class names to start with numbers --- lib/puppet/parser/lexer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/puppet/parser') diff --git a/lib/puppet/parser/lexer.rb b/lib/puppet/parser/lexer.rb index 51026ea1b..2c5f66e5a 100644 --- a/lib/puppet/parser/lexer.rb +++ b/lib/puppet/parser/lexer.rb @@ -131,7 +131,7 @@ class Puppet::Parser::Lexer TOKENS.add_tokens "Whatever" => :DQTEXT, "Nomatter" => :SQTEXT, "alsonomatter" => :BOOLEAN - TOKENS.add_token :NAME, %r{[a-z][-\w]*} do |lexer, value| + TOKENS.add_token :NAME, %r{[a-z0-9][-\w]*} do |lexer, value| string_token = self # we're looking for keywords here if tmp = KEYWORDS.lookup(value) -- cgit From 2ec4e298c3274abc8eaad4230bca8d39a48d2e35 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Mon, 11 Aug 2008 10:52:50 +0200 Subject: Fix #1502 - abysmal storeconfig performance - part2 Resource parameters whose values are a resource reference (ie require, notify...) where always DELETEd/INSERTed because the code comparing resource reference compared object instances instead of their values (since Puppet::Parser::Resource::Reference doesn't override == ), leading to storeconfig performance issues. The correct fix would have been to define == in Puppet::Parser::Resource::Reference but that might introduce some side effects I don't know. Hence, the fix introduces a local compare() method that knows how to compare Puppet::Parser::Resource::Reference. Signed-off-by: Brice Figureau --- lib/puppet/parser/resource/param.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'lib/puppet/parser') diff --git a/lib/puppet/parser/resource/param.rb b/lib/puppet/parser/resource/param.rb index 9dd3f26d2..c8dd78a26 100644 --- a/lib/puppet/parser/resource/param.rb +++ b/lib/puppet/parser/resource/param.rb @@ -66,6 +66,14 @@ class Puppet::Parser::Resource::Param def to_s "%s => %s" % [self.name, self.value] end + + def compare(v,db_value) + if (v.is_a?(Puppet::Parser::Resource::Reference)) + return v.to_s == db_value.to_s + else + return v == db_value + end + end def values_to_remove(db_values) values = munge_for_rails(value) @@ -73,7 +81,7 @@ class Puppet::Parser::Resource::Param db_values.collect do |db| db unless (db.line == line_number && values.find { |v| - v == db.value + compare(v,db.value) } ) end.compact end @@ -82,7 +90,7 @@ class Puppet::Parser::Resource::Param values = munge_for_rails(value) line_number = line_to_i() values.collect do |v| - v unless db_values.find { |db| (v == db.value && + v unless db_values.find { |db| (compare(v,db.value) && line_number == db.line) } end.compact end -- cgit