[PATCH/puppet 1/1] Fix for #3094 (libdir should take ":" delimited path)

[Markus Roberts]

Since libdir is also the default for the plugin handler, it needs to
be ":" savvy as well.

Signed-off-by: Markus Roberts <Mar...@reality.com>
---
lib/puppet/configurer/plugin_handler.rb | 6 ++++--
lib/puppet/util/autoload.rb | 4 ++--
spec/unit/configurer/plugin_handler.rb | 14 +++++++-------
spec/unit/util/autoload.rb | 3 ++-
4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/lib/puppet/configurer/plugin_handler.rb b/lib/puppet/configurer/plugin_handler.rb
index e934f58..3d9e087 100644
--- a/lib/puppet/configurer/plugin_handler.rb
+++ b/lib/puppet/configurer/plugin_handler.rb
@@ -7,9 +7,11 @@ module Puppet::Configurer::PluginHandler
end

# Retrieve facts from the central server.
- def download_plugins
+ def download_plugins(dummy_argument=:work_arround_for_ruby_GC_bug)
return nil unless download_plugins?
- Puppet::Configurer::Downloader.new("plugin", Puppet[:plugindest], Puppet[:pluginsource], Puppet[:pluginsignore]).evaluate.each { |file| load_plugin(file) }
+ Puppet[:plugindest].split(':').each { |path|
+ Puppet::Configurer::Downloader.new("plugin", path, Puppet[:pluginsource], Puppet[:pluginsignore]).evaluate.each { |file| load_plugin(file) }
+ }
end

def load_plugin(file)
diff --git a/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb
index ec2f48c..a641d22 100644
--- a/lib/puppet/util/autoload.rb
+++ b/lib/puppet/util/autoload.rb
@@ -152,7 +152,7 @@ class Puppet::Util::Autoload
end
end

- def search_directories
- [module_directories, Puppet[:libdir], $:].flatten
+ def search_directories(dummy_argument=:work_arround_for_ruby_GC_bug)
+ [module_directories, Puppet[:libdir].split(':'), $:].flatten
end
end
diff --git a/spec/unit/configurer/plugin_handler.rb b/spec/unit/configurer/plugin_handler.rb
index 7f59d5b..6bc75f1 100755
--- a/spec/unit/configurer/plugin_handler.rb
+++ b/spec/unit/configurer/plugin_handler.rb
@@ -38,15 +38,15 @@ describe Puppet::Configurer::PluginHandler do
end

it "should use an Agent Downloader, with the name, source, destination, and ignore set correctly, to download plugins when downloading is enabled" do
- downloader = mock 'downloader'
-
- Puppet.settings.expects(:value).with(:pluginsource).returns "psource"
- Puppet.settings.expects(:value).with(:plugindest).returns "pdest"
- Puppet.settings.expects(:value).with(:pluginsignore).returns "pignore"
+ downloader1 = mock('downloader',:evaluate => [])
+ downloader2 = mock('downloader',:evaluate => [])

- Puppet::Configurer::Downloader.expects(:new).with("plugin", "pdest", "psource", "pignore").returns downloader
+ Puppet.settings.stubs(:value).with(:pluginsource).returns "psource"
+ Puppet.settings.stubs(:value).with(:plugindest).returns "pdest1:pdest2"
+ Puppet.settings.stubs(:value).with(:pluginsignore).returns "pignore"

- downloader.expects(:evaluate).returns []
+ Puppet::Configurer::Downloader.expects(:new).with("plugin", "pdest1", "psource", "pignore").returns downloader1
+ Puppet::Configurer::Downloader.expects(:new).with("plugin", "pdest2", "psource", "pignore").returns downloader2

@pluginhandler.expects(:download_plugins?).returns true
@pluginhandler.download_plugins
diff --git a/spec/unit/util/autoload.rb b/spec/unit/util/autoload.rb
index 4e13842..076c4d2 100755
--- a/spec/unit/util/autoload.rb
+++ b/spec/unit/util/autoload.rb
@@ -51,8 +51,9 @@ describe Puppet::Util::Autoload do
end

it "should include the module directories, the Puppet libdir, and all of the Ruby load directories" do
+ Puppet.stubs(:[]).with(:libdir).returns("/libdir1:/lib/dir/two:/third/lib/dir")
@autoload.expects(:module_directories).returns %w{/one /two}
- @autoload.search_directories.should == ["/one", "/two", Puppet[:libdir], $:].flatten
+ @autoload.search_directories.should == %w{/one /two /libdir1 /lib/dir/two /third/lib/dir} + $:
end

it "should include in its search path all of the search directories that have a subdirectory matching the autoload path" do
--
1.6.4

[Luke Kanies]

This isn't really necessary, is it? I think this will download all of
the plugins into multiple directories. It's just the search bit,
below, that seems to be necessary.

> --
> You received this message because you are subscribed to the Google
> Groups "Puppet Developers" group.
> To post to this group, send email to puppe...@googlegroups.com.
> To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com
> .
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en
> .
>

--
I respect faith, but doubt is what gets you an education.
-- Wilson Mizner
---------------------------------------------------------------------
Luke Kanies -|- http://reductivelabs.com -|- +1(615)594-8199

[Markus Roberts]

>>    # Retrieve facts from the central server.
>> -    def download_plugins
>> +    def download_plugins(dummy_argument=:work_arround_for_ruby_GC_bug)
>>        return nil unless download_plugins?
>> -        Puppet::Configurer::Downloader.new("plugin", Puppet[:plugindest],
>> Puppet[:pluginsource], Puppet[:pluginsignore]).evaluate.each { |file|
>> load_plugin(file) }
>> +        Puppet[:plugindest].split(':').each { |path|
>> +            Puppet::Configurer::Downloader.new("plugin", path,
>> Puppet[:pluginsource], Puppet[:pluginsignore]).evaluate.each { |file|
>> load_plugin(file) }
>> +        }
>>    end
>
> This isn't really necessary, is it?  I think this will download all of the
> plugins into multiple directories.  It's just the search bit, below, that
> seems to be necessary.

I see now that I was reading this incorrectly. But something is
needed here. If left as is, it would use a directory with a ':' in
its name which is not in the search path. Perhaps only taking the
first ':' delimited segment (i.e., if there are multiple libdirs the
first receives the downloaded plugins)?

-- Markus

[Luke Kanies]

Normally $plugindest is set to $libdir, and $libdir is one directory,
right? We could just say that if you set $libdir to multiple
directories then you also have to set $plugindest to only one
directory, but that's a bit silly.

It should probably default to the first directory in $libdir, but I
don't think settings can actually default to code, only plain values.
I think the above is how other multiple-path defaults are done, fwiw.

--
There is nothing worse than aggressive stupidity.
-- Johann Wolfgang von Goethe

[Paul Nasrat]

2010/1/22 Markus Roberts <Mar...@reality.com>:

> Since libdir is also the default for the plugin handler, it needs to
> be ":" savvy as well.

This really should be File::PATH_SEPARATOR and not ':'

Paul