Modules now can find their own paths

Previously, when you created a module you had to specify
the path.  Now Module instances can use the module path
to look up their paths, and there are methods for determining
whether the module is present (if the path is present).

Also cleaned up the methods for figuring out what's in
the module (plugins, etc.).

Signed-off-by: Luke Kanies <luke@madstop.com>

5 files changed, 80 insertions, 40 deletions

diff --git a/lib/puppet/file_serving/mount/plugins.rb b/lib/puppet/file_serving/mount/plugins.rb
index 487bd043b..c7a3ee6ff 100644
--- a/lib/puppet/file_serving/mount/plugins.rb
+++ b/lib/puppet/file_serving/mount/plugins.rb
@@ -16,7 +16,7 @@ class Puppet::FileServing::Mount::Plugins < Puppet::FileServing::Mount
     def search(relative_path, options = {})
         # We currently only support one kind of search on plugins - return
         # them all.
-        paths = environment(options[:node]).modules.find_all { |mod|  mod.plugins? }.collect { |mod| mod.plugins }
+        paths = environment(options[:node]).modules.find_all { |mod|  mod.plugins? }.collect { |mod| mod.plugin_directory }
         return nil if paths.empty?
         return paths
     end
diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb
index e4bdd16e6..30c2512f3 100644
--- a/lib/puppet/module.rb
+++ b/lib/puppet/module.rb
@@ -25,7 +25,7 @@ class Puppet::Module
 
                 yielded[name] = true
 
-                yield Puppet::Module.new(name, module_path)
+                yield Puppet::Module.new(name)
             end
         end
     end
@@ -41,49 +41,91 @@ class Puppet::Module
     # absolute, or if there is no module whose name is the first component
     # of +path+, return +nil+
     def self.find(modname, environment = nil)
+        return nil unless modname
         Puppet::Node::Environment.new(environment).module(modname)
     end
 
-    attr_reader :name, :path
-    def initialize(name, path)
+    attr_reader :name, :environment
+    def initialize(name, environment = nil)
         @name = name
-        @path = path
+        if environment.is_a?(Puppet::Node::Environment)
+            @environment = environment
+        else
+            @environment = Puppet::Node::Environment.new(environment)
+        end
     end
 
     FILETYPES.each do |type|
-        # Create a method for returning the full path to a given
-        # file type's directory.
-        define_method(type.to_s) do
-            File.join(path, type.to_s)
-        end
-
-        # Create a boolean method for testing whether our module has
-        # files of a given type.
-        define_method(type.to_s + "?") do
-            FileTest.exist?(send(type))
+        # A boolean method to let external callers determine if
+        # we have files of a given type.
+        define_method(type +'?') do
+            return false unless path
+            return false unless FileTest.exist?(subpath(type))
+            return true
         end
 
-        # Finally, a method for returning an individual file
+        # A method for returning a given file of a given type.
+        # e.g., file = mod.manifest("my/manifest.pp")
+        #
+        # If the file name is nil, then the base directory for the
+        # file type is passed; this is used for fileserving.
         define_method(type.to_s.sub(/s$/, '')) do |file|
+            return nil unless path
+
+            # If 'file' is nil then they're asking for the base path.
+            # This is used for things like fileserving.
             if file
-                path = File.join(send(type), file)
+                full_path = File.join(subpath(type), file)
             else
-                path = send(type)
+                full_path = subpath(type)
             end
-            return nil unless FileTest.exist?(path)
-            return path
+
+            return nil unless FileTest.exist?(full_path)
+            return full_path
         end
     end
 
+    def exist?
+        ! path.nil?
+    end
+
+    # Find the first 'files' directory.  This is used by the XMLRPC fileserver.
+    def file_directory
+        subpath("files")
+    end
+
     # Return the list of manifests matching the given glob pattern,
     # defaulting to 'init.pp' for empty modules.
     def match_manifests(rest)
+        return find_init_manifest unless rest # Use init.pp
+
         rest ||= "init.pp"
         p = File::join(path, MANIFESTS, rest)
-        files = Dir.glob(p).reject { |f| FileTest.directory?(f) }
-        if files.size == 0
-            files = Dir.glob(p + ".pp")
+        result = Dir.glob(p).reject { |f| FileTest.directory?(f) }
+        if result.size == 0 and rest !~ /\.pp$/
+            result = Dir.glob(p + ".pp")
         end
-        return files
+        result.flatten.compact
+    end
+
+    # Find this module in the modulepath.
+    def path
+        self.class.modulepath.collect { |path| File.join(path, name) }.find { |d| FileTest.exist?(d) }
+    end
+
+    # Find all plugin directories.  This is used by the Plugins fileserving mount.
+    def plugin_directory
+        subpath("plugins")
+    end
+
+    private
+
+    def find_init_manifest
+        return [] unless file = manifest("init.pp")
+        return [file]
+    end
+
+    def subpath(type)
+        File.join(path, type)
     end
 end
diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb
index aad3b98be..50e2614aa 100755
--- a/lib/puppet/network/handler/fileserver.rb
+++ b/lib/puppet/network/handler/fileserver.rb
@@ -254,8 +254,8 @@ class Puppet::Network::Handler
             end
 
             # And use the environment to look up the module.
-            if mod = Puppet::Node::Environment.new(env).module(module_name)
-                return @mounts[MODULES].copy(mod.name, mod.files)
+            if mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files?
+                return @mounts[MODULES].copy(mod.name, mod.file_directory)
             else
                 return nil
             end
@@ -744,7 +744,7 @@ class Puppet::Network::Handler
 
             private
             def valid_modules(client)
-                Puppet::Node::Environment.new.modules.find_all { |mod| mod.plugins? }
+                Puppet::Node::Environment.new.modules.find_all { |mod| mod.exist? }
             end
             
             def add_to_filetree(f, filetree)
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 445439aa3..3d13af1f8 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -30,11 +30,9 @@ class Puppet::Node::Environment
     end
 
     def module(name)
-        Puppet::Module.each_module(modulepath) do |mod|
-            return mod if mod.name == name
-        end
-
-        return nil
+        mod = Puppet::Module.new(name, self)
+        return nil unless mod.exist?
+        return mod
     end
 
     def modulepath
diff --git a/lib/puppet/parser/files.rb b/lib/puppet/parser/files.rb
index 2cd163f97..ca4fb4f10 100644
--- a/lib/puppet/parser/files.rb
+++ b/lib/puppet/parser/files.rb
@@ -16,16 +16,16 @@ module Puppet::Parser::Files
     def find_manifests(start, options = {})
         cwd = options[:cwd] || Dir.getwd
         module_name, pattern = split_file_path(start)
-        if module_name and mod = Puppet::Module.find(module_name, options[:environment])
+        if mod = Puppet::Module.find(module_name, options[:environment])
             return mod.match_manifests(pattern)
-        else
-            abspat = File::expand_path(start, cwd)
-            files = Dir.glob(abspat).reject { |f| FileTest.directory?(f) }
-            if files.size == 0
-                files = Dir.glob(abspat + ".pp").reject { |f| FileTest.directory?(f) }
-            end
-            return files
         end
+
+        abspat = File::expand_path(start, cwd)
+        files = Dir.glob(abspat).reject { |f| FileTest.directory?(f) }
+        if files.size == 0
+            files = Dir.glob(abspat + ".pp").reject { |f| FileTest.directory?(f) }
+        end
+        return files
     end
 
     # Find the concrete file denoted by +file+. If +file+ is absolute,