Cleaning up plugin handling a bit -- they can now be colon-separated paths, and I added a separate "plugindest" setting for determining where plugins are synchronized to. The major feature I added, though, is that Puppet::Type now knows how to load plugins, and treats :pluginpath as a search path for plugins.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1330 980ebf18-57e1-0310-9a29-db15c13687c0

7 files changed, 116 insertions, 36 deletions

diff --git a/lib/puppet/client/master.rb b/lib/puppet/client/master.rb
index 1abb6c0c6..3f54fa27f 100644
--- a/lib/puppet/client/master.rb
+++ b/lib/puppet/client/master.rb
@@ -22,7 +22,10 @@ class Puppet::Client::MasterClient < Puppet::Client
     Puppet.setdefaults("puppet",
         :pluginpath => ["$vardir/plugins",
             "Where Puppet should look for plugins.  Multiple directories should
-            be comma-separated."],
+            be colon-separated, like normal PATH variables."],
+        :plugindest => ["$vardir/plugins",
+            "Where Puppet should store plugins that it pulls down from the central
+            server."],
         :pluginsource => ["puppet://puppet/plugins",
             "From where to retrieve plugins.  The standard Puppet ``file`` type
              is used for retrieval, so anything that is a valid file source can
@@ -439,7 +442,7 @@ class Puppet::Client::MasterClient < Puppet::Client
             :name => "plugin_collector"
         )
         plugins.push Puppet::Type.type(:file).create(
-            :path => Puppet[:pluginpath],
+            :path => Puppet[:plugindest],
             :recurse => true,
             :source => Puppet[:pluginsource],
             :ignore => Puppet[:pluginsignore].split(/\s+/),
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 2578323bc..20b65abed 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -179,6 +179,11 @@ class Transaction
         allevents
     end
 
+    # Determine whether a given object has failed.
+    def failed?(obj)
+        @failures[obj] > 0
+    end
+
     # this should only be called by a Puppet::Container object now
     # and it should only receive an array
     def initialize(objects)
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 850cacd46..fbd8b9347 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -77,20 +77,6 @@ class Type < Puppet::Element
         end
     end
 
-    def disabled_inspect
-        str = "Type(%s)" % self.name
-        if defined? @states
-            str += " States(" + @states.inspect + ")"
-        end
-        if defined? @parameters
-            str += " Parameters(" + @parameters.inspect + ")"
-        end
-        if defined? @metaparams
-            str += " Metaparams(" + @metaparams.inspect + ")"
-        end
-        str
-    end
-
     # iterate across all of the subclasses of Type
     def self.eachtype
         @types.each do |name, type|
@@ -101,6 +87,18 @@ class Type < Puppet::Element
         end
     end
 
+    # Create the 'ensure' class.  This is a separate method so other types
+    # can easily call it and create their own 'ensure' values.
+    def self.ensurable(&block)
+        if block_given?
+            self.newstate(:ensure, Puppet::State::Ensure, &block)
+        else
+            self.newstate(:ensure, Puppet::State::Ensure) do
+                self.defaultvalues
+            end
+        end
+    end
+
     # Should we add the 'ensure' state to this class?
     def self.ensurable?
         # If the class has all three of these methods defined, then it's
@@ -148,11 +146,33 @@ class Type < Puppet::Element
 
     end
 
+    # Do an on-demand plugin load
+    def self.loadplugin(name)
+        unless Puppet[:pluginpath].split(":").include?(Puppet[:plugindest])
+            Puppet.notice "Adding plugin destination %s to plugin search path" %
+                Puppet[:plugindest]
+            Puppet[:pluginpath] += ":" + Puppet[:plugindest]
+        end
+        Puppet[:pluginpath].split(":").each do |dir|
+            file = ::File.join(dir, name.to_s + ".rb")
+            if FileTest.exists?(file)
+                begin
+                    load file
+                    Puppet.info "loaded %s" % file
+                    return true
+                rescue LoadError => detail
+                    Puppet.info "Could not load %s: %s" %
+                        [file, detail]
+                    return false
+                end
+            end
+        end
+    end
+
     # Define a new type.
     def self.newtype(name, parent = nil, &block)
         parent ||= Puppet::Type
-        Puppet::Util.symbolize(name)
-
+        name = Puppet::Util.symbolize(name)
 
         # Create the class, with the correct name.
         t = Class.new(parent) do
@@ -185,18 +205,6 @@ class Type < Puppet::Element
         t
     end
 
-    # Create the 'ensure' class.  This is a separate method so other types
-    # can easily call it and create their own 'ensure' values.
-    def self.ensurable(&block)
-        if block_given?
-            self.newstate(:ensure, Puppet::State::Ensure, &block)
-        else
-            self.newstate(:ensure, Puppet::State::Ensure) do
-                self.defaultvalues
-            end
-        end
-    end
-
     # Return a Type instance by name.
     def self.type(name)
         @types ||= {}
@@ -212,7 +220,12 @@ class Type < Puppet::Element
                     Puppet.warning "Loaded puppet/type/#{name} but no class was created"
                 end
             rescue LoadError => detail
-                # nothing
+                # If we can't load it from there, try loading it as a plugin.
+                if loadplugin(name)
+                    unless @types.include?(name)
+                        Puppet.warning "Loaded plugin %s but no type was defined" % name
+                    end
+                end
             end
         end
 
@@ -409,7 +422,7 @@ class Type < Puppet::Element
     # Create a new metaparam.  Requires a block and a name, stores it in the
     # @parameters array, and does some basic checking on it.
     def self.newmetaparam(name, &block)
-        Puppet::Util.symbolize(name)
+        name = Puppet::Util.symbolize(name)
         param = Class.new(Puppet::Parameter) do
             @name = name
         end
@@ -435,7 +448,7 @@ class Type < Puppet::Element
     # Create a new parameter.  Requires a block and a name, stores it in the
     # @parameters array, and does some basic checking on it.
     def self.newparam(name, &block)
-        Puppet::Util.symbolize(name)
+        name = Puppet::Util.symbolize(name)
         param = Class.new(Puppet::Parameter) do
             @name = name
         end
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 0e7a198ce..a157ecf79 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -304,7 +304,7 @@ module Util
     def self.symbolize(value)
         case value
         when String: value = value.intern
-        when Symbol: # nothing
+        when Symbol: value
         else
             raise ArgumentError, "'%s' must be a string or symbol" % value
         end
diff --git a/test/client/master.rb b/test/client/master.rb
index a2dbad22d..0a35a78c3 100644
--- a/test/client/master.rb
+++ b/test/client/master.rb
@@ -154,7 +154,7 @@ end
             client.send(:getplugins)
         }
 
-        destfile = File.join(Puppet[:pluginpath], "myplugin.rb")
+        destfile = File.join(Puppet[:plugindest], "myplugin.rb")
 
         assert(File.exists?(destfile), "Did not get plugin")
 
@@ -196,5 +196,12 @@ end
 
         assert(! obj.validattr?(:argument),
             "Old namevar is still valid")
+
+        # Now make sure it works with multiple paths specified.
+        newdir = tempfile()
+        Dir.mkdir(newdir)
+        Puppet[:pluginpath] = [Puppet[:pluginpath], newdir].join(":")
+
+        client.send(:getplugins)
     end
 end
diff --git a/test/types/filesources.rb b/test/types/filesources.rb
index b9f3a7fdb..b69e83008 100755
--- a/test/types/filesources.rb
+++ b/test/types/filesources.rb
@@ -541,12 +541,16 @@ class TestFileSources < Test::Unit::TestCase
         assert(FileTest.file?(dest), "Destination is not a file")
 
         # Now copy the links
-        assert_raise(Puppet::FileServerError) {
+        #assert_raise(Puppet::FileServerError) {
+        trans = nil
+        assert_nothing_raised {
             file[:links] = :manage
             comp = newcomp(file)
             trans = comp.evaluate
             trans.evaluate
         }
+
+        assert(trans.failed?(file), "Object did not fail to copy links")
     end
 
     def test_changes
diff --git a/test/types/type.rb b/test/types/type.rb
index d9e5d7577..287d02be6 100644
--- a/test/types/type.rb
+++ b/test/types/type.rb
@@ -320,6 +320,54 @@ class TestType < Test::Unit::TestCase
             comp.parent = file
         end
     end
+
+    def test_loadplugins
+        names = %w{loadedplugin1 loadplugin2 loadplugin3}
+        dirs = []
+        3.times { dirs << tempfile() }
+        # Set plugindest to something random
+        Puppet[:plugindest] = tempfile()
+
+        Puppet[:pluginpath] = dirs.join(":")
+
+        names.each do |name|
+            dir = dirs.shift
+            Dir.mkdir(dir)
+
+            # Create an extra file for later
+            [name, name + "2ness"].each do |n|
+                file = File.join(dir, n + ".rb")
+                File.open(file, "w") do |f|
+                    f.puts %{Puppet::Type.newtype('#{n}') do
+    newparam(:argument) do
+        isnamevar
+    end
+end
+}
+                end
+            end
+
+            assert(Puppet::Type.type(name),
+                "Did not get loaded plugin")
+
+            assert_nothing_raised {
+                Puppet::Type.type(name).create(
+                    :name => "myname"
+                )
+            }
+        end
+
+        # Now make sure the plugindest got added to our pluginpath
+        assert(Puppet[:pluginpath].split(":").include?(Puppet[:plugindest]),
+            "Plugin dest did not get added to plugin path")
+
+        # Now make sure it works with just a single path, using the extra files
+        # created above.
+        Puppet[:pluginpath] = Puppet[:pluginpath].split(":")[0]
+        assert(Puppet::Type.type("loadedplugin12ness"),
+            "Did not get loaded plugin")
+
+    end
 end
 
 # $Id$