+1
On Sun, 2011-06-05 at 09:12 +0100, R.I.Pienaar wrote:
> This allows rubygems to be used to extend or override puppet behavior.
>
> Unfortunately the features system uses the autoloader so checking for
> the rubygems feature causes a call loop. It's possible that a more
> elegant way to avoid this exist rather than just checking the @path
> but this did what I needed to do.
>
> Without caching the directories the tests would take forever to run
> and eventually crash my server. Caching seems to avoid the biggest
> of the performance hit.
>
> Signed-off-by: R.I.Pienaar <r...@devco.net>
> ---
> Local-branch: feature/master/7788
> lib/puppet/configurer.rb | 5 +++--
> lib/puppet/parser/compiler.rb | 1 +
> lib/puppet/util/autoload.rb | 19 +++++++++++++++++--
> spec/unit/util/autoload_spec.rb | 10 +++++++++-
> 4 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
> index cfeb73a..00c3b18 100644
> --- a/lib/puppet/configurer.rb
> +++ b/lib/puppet/configurer.rb
> @@ -156,9 +156,10 @@ class Puppet::Configurer
> return
> end
> ensure
> - # Make sure we forget the retained module_directories of any autoload
> - # we might have used.
> + # Make sure we forget the retained module_directories and gem search
> + # path of any autoload we might have used.
> Thread.current[:env_module_directories] = nil
> + Thread.current[:gemsearch_directories] = nil
>
> # Now close all of our existing http connections, since there's no
> # reason to leave them lying open.
> diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
> index c1daade..68498ad 100644
> --- a/lib/puppet/parser/compiler.rb
> +++ b/lib/puppet/parser/compiler.rb
> @@ -22,6 +22,7 @@ class Puppet::Parser::Compiler
> # stick until the next entry to this function.
> Thread.current[:known_resource_types] = nil
> Thread.current[:env_module_directories] = nil
> + Thread.current[:gemsearch_directories] = nil
>
> # ...and we actually do the compile now we have caching ready.
> new(node).compile.to_resource
> diff --git a/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb
> index 6537a4a..e61f4cd 100644
> --- a/lib/puppet/util/autoload.rb
> +++ b/lib/puppet/util/autoload.rb
> @@ -124,9 +124,24 @@ class Puppet::Util::Autoload
> searchpath.map { |dir| Dir.glob("#{dir}/*.rb") }.flatten
> end
>
> - # The list of directories to search through for loadable plugins.
> + # features uses the autoloader which causes a loop so avoid trying to
> + # ask Gem for its path during feature loading.
> + #
> + # We're using a per-thread cache based on the code in #module_directories.
> + def gemsearchpaths
> + unless @path == "puppet/feature"
> + if Puppet.features.rubygems?
> + return Thread.current[:gemsearch_directories] ||= Gem.all_load_paths
> + end
> + end
> +
> + return []
> + end
> +
> + # The list of directories to search through for loadable plugins. We
> search gem paths first
> + # so that people can use the gem packages to override core puppet features
> if they wish.
> def searchpath(env=nil)
> - search_directories(env).uniq.collect { |d| File.join(d, @path)
> }.find_all { |d| FileTest.directory?(d) }
> + [gemsearchpaths, search_directories(env)].flatten.uniq.collect { |d|
> File.join(d, @path) }.find_all { |d| FileTest.directory?(d) }
> end
>
> def module_directories(env=nil)
> diff --git a/spec/unit/util/autoload_spec.rb b/spec/unit/util/autoload_spec.rb
> index d61b768..e962b1e 100755
> --- a/spec/unit/util/autoload_spec.rb
> +++ b/spec/unit/util/autoload_spec.rb
> @@ -51,13 +51,21 @@ describe Puppet::Util::Autoload do
> @autoload.search_directories.should == %w{/one /two /libdir1
> /lib/dir/two /third/lib/dir} + $LOAD_PATH
> end
>
> + it "should not try to merge rubygems dirs when called from the feature
> system" do
> + @autoload = Puppet::Util::Autoload.new("foo", "puppet/feature")
> + Puppet.features.expects(:rubygems?).never
> + @autoload.gemsearchpaths
> + end
> +
> it "should include in its search path all of the unique search
> directories that have a subdirectory matching the autoload path" do
> @autoload = Puppet::Util::Autoload.new("foo", "loaddir")
> @autoload.expects(:search_directories).returns %w{/one /two /three
> /three}
> + @autoload.expects(:gemsearchpaths).returns %w{/four /four}
> FileTest.expects(:directory?).with("/one/loaddir").returns true
> FileTest.expects(:directory?).with("/two/loaddir").returns false
> FileTest.expects(:directory?).with("/three/loaddir").returns true
> - @autoload.searchpath.should == ["/one/loaddir", "/three/loaddir"]
> + FileTest.expects(:directory?).with("/four/loaddir").returns true
> + @autoload.searchpath.should == ["/four/loaddir", "/one/loaddir",
> "/three/loaddir"]
> end
> end
>
> --
> 1.7.1
>
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
