Part 2 of fix for #1175 (functions in environments)

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

10 files changed, 49 insertions, 52 deletions

diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index d1a126aaa..4363eea41 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 4c3a5dc05..ba4977935 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 f869383dc..4357a3a34 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 6ba3669c0..e0973c19d 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 991e12302..140c8c1b5 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 3268eae37..bcb7fa322 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 7358618f9..4a687bf34 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 0da8c4a31..03b0ef9dd 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 7093279b6..b14b2d3b6 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 220cb5f68..5862073ea 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"