Jesse and I are shooting for the minimal viable fix here, with the idea that a great deal of refactoring is needed but isn't appropriate at this time. The changes in this commit are:
* Index the function-holding modules by environment * We need to know the "current environment" when we're defining a function so we can attach it to the proper module, and this information isn't dynamically available when user-defined functions are being created (we're being called by user written code that doesn't "know" about environments) so we cheat and stash the value in Puppet::Node::Environment * since we must do this anyway, it turns out to be cleaner & safer to do the same when we are evaluating a functon. This is the main change from the prior version of this patch. * Add a special *root* environment for the built in functions, and extend all scopes with it. * Index the function characteristics (name, type, docstring, etc.) by environment * Make the autoloader environment aware, so that it uses the modulepath for the specified environment rather than the default * Turn off caching of the modulepath since it potentially changes for each node * Tweak tests that weren't environment aware Signed-off-by: Markus Roberts <[email protected]> --- lib/puppet/node/environment.rb | 12 +++++++++ lib/puppet/parser/ast/function.rb | 3 +- lib/puppet/parser/compiler.rb | 1 + lib/puppet/parser/functions.rb | 45 ++++++++++++-------------------- lib/puppet/parser/scope.rb | 3 +- lib/puppet/parser/type_loader.rb | 2 +- lib/puppet/util/autoload.rb | 19 ++++++-------- spec/unit/parser/functions/defined.rb | 4 +- spec/unit/parser/scope.rb | 8 +++-- spec/unit/util/autoload.rb | 4 --- 10 files changed, 49 insertions(+), 52 deletions(-) diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb index d1a126a..4363eea 100644 --- a/lib/puppet/node/environment.rb +++ b/lib/puppet/node/environment.rb @@ -41,6 +41,18 @@ class Puppet::Node::Environment @seen[symbol] = obj end + def self.current + @current || root + end + + def self.current=(env) + @current = new(env) + end + + def self.root + @root ||= new(:'*root*') + end + # This is only used for testing. def self.clear @seen.clear diff --git a/lib/puppet/parser/ast/function.rb b/lib/puppet/parser/ast/function.rb index 4c3a5dc..ba49779 100644 --- a/lib/puppet/parser/ast/function.rb +++ b/lib/puppet/parser/ast/function.rb @@ -13,7 +13,7 @@ class Puppet::Parser::AST def evaluate(scope) # Make sure it's a defined function - unless @fname + unless @fname = Puppet::Parser::Functions.function(@name) raise Puppet::ParseError, "Unknown function %s" % @name end @@ -48,7 +48,6 @@ class Puppet::Parser::AST super(hash) - @fname = Puppet::Parser::Functions.function(@name) # Lastly, check the parity end diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index beba438..22e2f07 100644 --- a/lib/puppet/parser/compiler.rb +++ b/lib/puppet/parser/compiler.rb @@ -127,6 +127,7 @@ class Puppet::Parser::Compiler @environment = nil end end + Puppet::Node::Environment.current = @environment @environment end diff --git a/lib/puppet/parser/functions.rb b/lib/puppet/parser/functions.rb index 6ba3669..e0973c1 100644 --- a/lib/puppet/parser/functions.rb +++ b/lib/puppet/parser/functions.rb @@ -6,7 +6,7 @@ require 'puppet/parser/scope' # class. module Puppet::Parser::Functions - @functions = {} + @functions = Hash.new { |h,k| h[k] = {} } @modules = {} class << self @@ -24,15 +24,17 @@ module Puppet::Parser::Functions @autoloader end + Environment = Puppet::Node::Environment + def self.environment_module(env = nil) - @module ||= Module.new + @modules[ env || Environment.current || Environment.root ] ||= Module.new end # Create a new function type. def self.newfunction(name, options = {}, &block) name = symbolize(name) - if @functions.include? name + if functions.include?(name) raise Puppet::DevError, "Function %s already defined" % name end @@ -46,10 +48,10 @@ module Puppet::Parser::Functions environment_module.send(:define_method, fname, &block) # Someday we'll support specifying an arity, but for now, nope - #...@functions[name] = {:arity => arity, :type => ftype} - @functions[name] = {:type => ftype, :name => fname} + #functions[name] = {:arity => arity, :type => ftype} + functions[name] = {:type => ftype, :name => fname} if options[:doc] - @functions[name][:doc] = options[:doc] + functions[name][:doc] = options[:doc] end end @@ -57,11 +59,11 @@ module Puppet::Parser::Functions def self.rmfunction(name) name = symbolize(name) - unless @functions.include? name + unless functions.include? name raise Puppet::DevError, "Function %s is not defined" % name end - @functions.delete(name) + functions.delete name fname = "function_" + name.to_s environment_module.send(:remove_method, fname) @@ -71,15 +73,11 @@ module Puppet::Parser::Functions def self.function(name) name = symbolize(name) - unless @functions.include? name - autoloader.load(name) + unless functions.include?(name) or functions(Puppet::Node::Environment.root).include?(name) + autoloader.load(name,Environment.current || Environment.root) end - if @functions.include? name - return @functions[name][:name] - else - return false - end + ( functions(Environment.root)[name] || functions[name] || {:name => false} )[:name] end def self.functiondocs @@ -87,7 +85,7 @@ module Puppet::Parser::Functions ret = "" - @functions.sort { |a,b| a[0].to_s <=> b[0].to_s }.each do |name, hash| + functions.sort { |a,b| a[0].to_s <=> b[0].to_s }.each do |name, hash| #ret += "%s\n%s\n" % [name, hash[:type]] ret += "%s\n%s\n" % [name, "-" * name.to_s.length] if hash[:doc] @@ -102,22 +100,13 @@ module Puppet::Parser::Functions return ret end - def self.functions - @functions.keys + def self.functions(env = nil) + @functions[ env || Environment.current || Environment.root ] end # Determine if a given function returns a value or not. def self.rvalue?(name) - name = symbolize(name) - - if @functions.include? name - case @functions[name][:type] - when :statement; return false - when :rvalue; return true - end - else - return false - end + (functions[symbolize(name)] || {})[:type] == :rvalue end # Runs a newfunction to create a function for each of the log levels diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb index 991e123..140c8c1 100644 --- a/lib/puppet/parser/scope.rb +++ b/lib/puppet/parser/scope.rb @@ -505,6 +505,7 @@ class Puppet::Parser::Scope private def extend_with_functions_module - extend Puppet::Parser::Functions.environment_module(compiler ? environment : nil) + extend Puppet::Parser::Functions.environment_module(Puppet::Node::Environment.root) + extend Puppet::Parser::Functions.environment_module(compiler ? environment : nil ) end end diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb index 3268eae..bcb7fa3 100644 --- a/lib/puppet/parser/type_loader.rb +++ b/lib/puppet/parser/type_loader.rb @@ -114,7 +114,7 @@ class Puppet::Parser::TypeLoader end def parse_file(file) - Puppet.debug("importing '#{file}'") + Puppet.debug("importing '#{file}' in environment #{environment}") parser = Puppet::Parser::Parser.new(environment) parser.file = file parser.parse diff --git a/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb index 7358618..4a687bf 100644 --- a/lib/puppet/util/autoload.rb +++ b/lib/puppet/util/autoload.rb @@ -73,12 +73,12 @@ class Puppet::Util::Autoload # Load a single plugin by name. We use 'load' here so we can reload a # given plugin. - def load(name) + def load(name,env=nil) return false if named_file_missing?(name) path = name.to_s + ".rb" - searchpath.each do |dir| + searchpath(env).each do |dir| file = File.join(dir, path) next unless file_exist?(file) begin @@ -130,25 +130,22 @@ class Puppet::Util::Autoload end # The list of directories to search through for loadable plugins. - # We have to hard-code the ttl because this library is used by - # so many other classes it's hard to get the load-order such that - # the defaults load before this. - cached_attr(:searchpath, :ttl => 15) do - search_directories.collect { |d| File.join(d, @path) }.find_all { |d| FileTest.directory?(d) } + def searchpath(env=nil) + search_directories(env).collect { |d| File.join(d, @path) }.find_all { |d| FileTest.directory?(d) } end - def module_directories + def module_directories(env=nil) # We have to require this late in the process because otherwise we might have # load order issues. require 'puppet/node/environment' - Puppet::Node::Environment.new.modulepath.collect do |dir| + Puppet::Node::Environment.new(env).modulepath.collect do |dir| Dir.entries(dir).reject { |f| f =~ /^\./ }.collect { |f| File.join(dir, f) } end.flatten.collect { |d| [File.join(d, "plugins"), File.join(d, "lib")] }.flatten.find_all do |d| FileTest.directory?(d) end end - def search_directories(dummy_argument=:work_arround_for_ruby_GC_bug) - [module_directories, Puppet[:libdir].split(File::PATH_SEPARATOR), $:].flatten + def search_directories(env=nil) + [module_directories(env), Puppet[:libdir].split(File::PATH_SEPARATOR), $:].flatten end end diff --git a/spec/unit/parser/functions/defined.rb b/spec/unit/parser/functions/defined.rb index 0da8c4a..03b0ef9 100755 --- a/spec/unit/parser/functions/defined.rb +++ b/spec/unit/parser/functions/defined.rb @@ -5,9 +5,9 @@ require File.dirname(__FILE__) + '/../../../spec_helper' describe "the 'defined' function" do before :each do - @scope = Puppet::Parser::Scope.new() + Puppet::Node::Environment.stubs(:current).returns(nil) @compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("foo")) - @scope.compiler = @compiler + @scope = Puppet::Parser::Scope.new(:compiler => @compiler) end it "should exist" do diff --git a/spec/unit/parser/scope.rb b/spec/unit/parser/scope.rb index 7093279..b14b2d3 100755 --- a/spec/unit/parser/scope.rb +++ b/spec/unit/parser/scope.rb @@ -59,10 +59,12 @@ describe Puppet::Parser::Scope do end describe "when initializing" do - it "should extend itself with its environment's Functions module" do + it "should extend itself with its environment's Functions module as well as the default" do env = Puppet::Node::Environment.new("myenv") compiler = stub 'compiler', :environment => env - mod = Module.new + mod = Module.new + root_mod = Module.new + Puppet::Parser::Functions.expects(:environment_module).with(Puppet::Node::Environment.root).returns root_mod Puppet::Parser::Functions.expects(:environment_module).with(env).returns mod Puppet::Parser::Scope.new(:compiler => compiler).metaclass.ancestors.should be_include(mod) @@ -70,7 +72,7 @@ describe Puppet::Parser::Scope do it "should extend itself with the default Functions module if it has no environment" do mod = Module.new - Puppet::Parser::Functions.expects(:environment_module).with(nil).returns mod + Puppet::Parser::Functions.expects(:environment_module).with(Puppet::Node::Environment.root).returns(mod) Puppet::Parser::Scope.new().metaclass.ancestors.should be_include(mod) end diff --git a/spec/unit/util/autoload.rb b/spec/unit/util/autoload.rb index 220cb5f..5862073 100755 --- a/spec/unit/util/autoload.rb +++ b/spec/unit/util/autoload.rb @@ -15,10 +15,6 @@ describe Puppet::Util::Autoload do Puppet::Util::Autoload.ancestors.should be_include(Puppet::Util::Cacher) end - it "should use a ttl of 15 for the search path" do - Puppet::Util::Autoload.attr_ttl(:searchpath).should == 15 - end - describe "when building the search path" do it "should collect all of the plugins and lib directories that exist in the current environment's module path" do Puppet.settings.expects(:value).with(:environment).returns "foo" -- 1.6.4 -- 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.
