From aa741354a75c0d3b4f4b7f37a736a0154d45234c Mon Sep 17 00:00:00 2001 From: luke Date: Fri, 6 Jul 2007 22:22:10 +0000 Subject: Fixing #596 -- classes in modules now autoload git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2655 980ebf18-57e1-0310-9a29-db15c13687c0 --- CHANGELOG | 3 + lib/puppet.rb | 2 +- lib/puppet/module.rb | 115 +++++++++++++++++++++++++++++++++++++++ lib/puppet/modules.rb | 113 -------------------------------------- lib/puppet/parser/interpreter.rb | 64 ++++++++++++++++++++-- test/language/interpreter.rb | 110 +++++++++++++++++++++++++++++++++++++ test/puppet/modules.rb | 3 +- 7 files changed, 289 insertions(+), 121 deletions(-) create mode 100644 lib/puppet/module.rb delete mode 100644 lib/puppet/modules.rb diff --git a/CHANGELOG b/CHANGELOG index 76de82a67..5025ed003 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,6 @@ + Adding module autoloading (#596) -- you can now 'include' classes + from modules without ever needing to specifically load them. + Class names and node names now conflict (#620). 0.23.0 diff --git a/lib/puppet.rb b/lib/puppet.rb index 52e7d49d1..eccf4dbcb 100644 --- a/lib/puppet.rb +++ b/lib/puppet.rb @@ -421,7 +421,7 @@ module Puppet end require 'puppet/type' -require 'puppet/modules' +require 'puppet/module' require 'puppet/util/storage' require 'puppet/parser/interpreter' if Puppet[:storeconfigs] diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb new file mode 100644 index 000000000..87f849d17 --- /dev/null +++ b/lib/puppet/module.rb @@ -0,0 +1,115 @@ +# Support for modules +class Puppet::Module + + TEMPLATES = "templates" + FILES = "files" + MANIFESTS = "manifests" + + # Return an array of paths by splitting the +modulepath+ config + # parameter. Only consider paths that are absolute and existing + # directories + def self.modulepath + dirs = Puppet[:modulepath].split(":") + if ENV["PUPPETLIB"] + dirs = ENV["PUPPETLIB"].split(":") + dirs + end + dirs.select do |p| + p =~ /^#{File::SEPARATOR}/ && File::directory?(p) + end + end + + # Find and return the +module+ that +path+ belongs to. If +path+ is + # absolute, or if there is no module whose name is the first component + # of +path+, return +nil+ + def self.find(path) + if path =~ %r/^#{File::SEPARATOR}/ + return nil + end + + modname, rest = path.split(File::SEPARATOR, 2) + return nil if modname.nil? || modname.empty? + + modpath = modulepath.collect { |p| + File::join(p, modname) + }.find { |f| File::directory?(f) } + return nil unless modpath + + return self.new(modname, modpath) + end + + # Instance methods + + # Find the concrete file denoted by +file+. If +file+ is absolute, + # return it directly. Otherwise try to find it as a template in a + # module. If that fails, return it relative to the +templatedir+ config + # param. + # In all cases, an absolute path is returned, which does not + # necessarily refer to an existing file + def self.find_template(file) + if file =~ /^#{File::SEPARATOR}/ + return file + end + + mod = find(file) + if mod + return mod.template(file) + else + return File.join(Puppet[:templatedir], file) + end + end + + # Return a list of manifests (as absolute filenames) that match +pat+ + # with the current directory set to +cwd+. If the first component of + # +pat+ does not contain any wildcards and is an existing module, return + # a list of manifests in that module matching the rest of +pat+ + # Otherwise, try to find manifests matching +pat+ relative to +cwd+ + def self.find_manifests(pat, cwd = nil) + cwd ||= Dir.getwd + mod = find(pat) + if mod + return mod.manifests(pat) + else + abspat = File::expand_path(pat, 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 + end + + attr_reader :name, :path + def initialize(name, path) + @name = name + @path = path + end + + def strip(file) + n, rest = file.split(File::SEPARATOR, 2) + rest = nil if rest && rest.empty? + return rest + end + + def template(file) + return File::join(path, TEMPLATES, strip(file)) + end + + def files + return File::join(path, FILES) + end + + def manifests(pat) + rest = strip(pat) + rest ||= "init.pp" + p = File::join(path, MANIFESTS, rest) + files = Dir.glob(p) + if files.size == 0 + files = Dir.glob(p + ".pp") + end + return files + end + + private :initialize +end + +# $Id$ diff --git a/lib/puppet/modules.rb b/lib/puppet/modules.rb deleted file mode 100644 index aa2f75d03..000000000 --- a/lib/puppet/modules.rb +++ /dev/null @@ -1,113 +0,0 @@ -# Support for modules -class Puppet::Module - - TEMPLATES = "templates" - FILES = "files" - MANIFESTS = "manifests" - - # Return an array of paths by splitting the +modulepath+ config - # parameter. Only consider paths that are absolute and existing - # directories - def self.modulepath - dirs = Puppet[:modulepath].split(":") - if ENV["PUPPETLIB"] - dirs = ENV["PUPPETLIB"].split(":") + dirs - end - dirs.select do |p| - p =~ /^#{File::SEPARATOR}/ && File::directory?(p) - end - end - - # Find and return the +module+ that +path+ belongs to. If +path+ is - # absolute, or if there is no module whose name is the first component - # of +path+, return +nil+ - def self.find(path) - if path =~ %r/^#{File::SEPARATOR}/ - return nil - end - - modname, rest = path.split(File::SEPARATOR, 2) - return nil if modname.nil? || modname.empty? - - modpath = modulepath.collect { |p| - File::join(p, modname) - }.find { |f| File::directory?(f) } - return nil unless modpath - - return self.new(modname, modpath) - end - - # Instance methods - - # Find the concrete file denoted by +file+. If +file+ is absolute, - # return it directly. Otherwise try to find it as a template in a - # module. If that fails, return it relative to the +templatedir+ config - # param. - # In all cases, an absolute path is returned, which does not - # necessarily refer to an existing file - def self.find_template(file) - if file =~ /^#{File::SEPARATOR}/ - return file - end - - mod = find(file) - if mod - return mod.template(file) - else - return File.join(Puppet[:templatedir], file) - end - end - - # Return a list of manifests (as absolute filenames) that match +pat+ - # with the current directory set to +cwd+. If the first component of - # +pat+ does not contain any wildcards and is an existing module, return - # a list of manifests in that module matching the rest of +pat+ - # Otherwise, try to find manifests matching +pat+ relative to +cwd+ - def self.find_manifests(pat, cwd = nil) - cwd ||= Dir.getwd - mod = find(pat) - if mod - return mod.manifests(pat) - else - abspat = File::expand_path(pat, 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 - end - - attr_reader :name, :path - def initialize(name, path) - @name = name - @path = path - end - - def strip(file) - n, rest = file.split(File::SEPARATOR, 2) - rest = nil if rest && rest.empty? - return rest - end - - def template(file) - return File::join(path, TEMPLATES, strip(file)) - end - - def files - return File::join(path, FILES) - end - - def manifests(pat) - rest = strip(pat) - rest ||= "init.pp" - p = File::join(path, MANIFESTS, rest) - files = Dir.glob(p) - if files.size == 0 - files = Dir.glob(p + ".pp") - end - return files - end - - private :initialize -end diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb index 3a719e03c..11f5aa15d 100644 --- a/lib/puppet/parser/interpreter.rb +++ b/lib/puppet/parser/interpreter.rb @@ -1,7 +1,3 @@ -# The interepreter's job is to convert from a parsed file to the configuration -# for a given client. It really doesn't do any work on its own, it just collects -# and calls out to other objects. - require 'puppet' require 'timeout' require 'puppet/rails' @@ -9,6 +5,9 @@ require 'puppet/util/methodhelper' require 'puppet/parser/parser' require 'puppet/parser/scope' +# The interpreter's job is to convert from a parsed file to the configuration +# for a given client. It really doesn't do any work on its own, it just collects +# and calls out to other objects. class Puppet::Parser::Interpreter class NodeDef include Puppet::Util::MethodHelper @@ -267,12 +266,65 @@ class Puppet::Parser::Interpreter # Find a class definition, relative to the current namespace. def findclass(namespace, name) - fqfind namespace, name, @classtable + find_or_load namespace, name, @classtable end # Find a component definition, relative to the current namespace. def finddefine(namespace, name) - fqfind namespace, name, @definetable + find_or_load namespace, name, @definetable + end + + # Attempt to find the requested object. If it's not yet loaded, + # attempt to load it. + def find_or_load(namespace, name, table) + if namespace == "" + fullname = name.gsub("::", File::SEPARATOR) + else + fullname = ("%s::%s" % [namespace, name]).gsub("::", File::SEPARATOR) + end + + # See if it's already loaded + if result = fqfind(namespace, name, table) + return result + end + + if fullname == "" + return nil + end + + # Nope. Try to load the module itself, to see if that + # loads it. + mod = fullname.scan(/^[\w-]+/).shift + # We couldn't find it, so try to load the base module + begin + @parser.import(mod) + Puppet.info "Autoloaded module %s" % mod + if result = fqfind(namespace, name, table) + return result + end + rescue Puppet::ImportError => detail + # We couldn't load the module + end + + + # If they haven't specified a subclass, then there's no point in looking for + # a separate file. + if ! fullname.include?("/") + return nil + end + + # Nope. Try to load the individual file + begin + @parser.import(fullname) + Puppet.info "Autloaded file %s from module %s" % [fullname, mod] + if result = fqfind(namespace, name, table) + return result + end + rescue Puppet::ImportError => detail + # We couldn't load the file + end + + return nil end # The recursive method used to actually look these objects up. diff --git a/test/language/interpreter.rb b/test/language/interpreter.rb index aababc1d0..1eeb65b1a 100755 --- a/test/language/interpreter.rb +++ b/test/language/interpreter.rb @@ -855,6 +855,8 @@ class TestInterpreter < PuppetTest::TestCase assert(scope.classlist.include?("other"), "NodeDef did not evaluate other class") end + # This can stay in the main test suite because it doesn't actually use ldapsearch, + # it just overrides the method so it behaves as though it were hitting ldap. def test_ldapnodes interp = mkinterp @@ -910,6 +912,114 @@ class TestInterpreter < PuppetTest::TestCase assert_equal(%w{one two three four five}.sort, node.classes.sort, "node classes were not set correctly with the top node") assert_equal({"base" => "true", "center" => "boo", "master" => "far"}, node.parameters, "node parameters were not set correctly with the top node") end + + # Setup a module. + def mk_module(name, files = {}) + mdir = File.join(@dir, name) + mandir = File.join(mdir, "manifests") + FileUtils.mkdir_p mandir + + if defs = files[:define] + files.delete(:define) + end + Dir.chdir(mandir) do + files.each do |file, classes| + File.open("%s.pp" % file, "w") do |f| + classes.each { |klass| + if defs + f.puts "define %s {}" % klass + else + f.puts "class %s {}" % klass + end + } + end + end + end + end + + # #596 - make sure classes and definitions load automatically if they're in modules, so we don't have to manually load each one. + def test_module_autoloading + @dir = tempfile + Puppet[:modulepath] = @dir + + FileUtils.mkdir_p @dir + + interp = mkinterp + + # Make sure we fail like normal for actually missing classes + assert_nil(interp.findclass("", "nosuchclass"), "Did not return nil on missing classes") + + # test the simple case -- the module class itself + name = "simple" + mk_module(name, :init => [name]) + + # Try to load the module automatically now + klass = interp.findclass("", name) + assert_instance_of(AST::HostClass, klass, "Did not autoload class from module init file") + assert_equal(name, klass.classname, "Incorrect class was returned") + + # Now try it with a definition as the base file + name = "simpdef" + mk_module(name, :define => true, :init => [name]) + + klass = interp.finddefine("", name) + assert_instance_of(AST::Component, klass, "Did not autoload class from module init file") + assert_equal(name, klass.classname, "Incorrect class was returned") + + # Now try it with namespace classes where both classes are in the init file + interp = mkinterp + modname = "both" + name = "sub" + mk_module(modname, :init => %w{both both::sub}) + + # First try it with a namespace + klass = interp.findclass("both", name) + assert_instance_of(AST::HostClass, klass, "Did not autoload sub class from module init file with a namespace") + assert_equal("both::sub", klass.classname, "Incorrect class was returned") + + # Now try it using the fully qualified name + interp = mkinterp + klass = interp.findclass("", "both::sub") + assert_instance_of(AST::HostClass, klass, "Did not autoload sub class from module init file with no namespace") + assert_equal("both::sub", klass.classname, "Incorrect class was returned") + + + # Now try it with the class in a different file + interp = mkinterp + modname = "separate" + name = "sub" + mk_module(modname, :init => %w{separate}, :sub => %w{separate::sub}) + + # First try it with a namespace + klass = interp.findclass("separate", name) + assert_instance_of(AST::HostClass, klass, "Did not autoload sub class from separate file with a namespace") + assert_equal("separate::sub", klass.classname, "Incorrect class was returned") + + # Now try it using the fully qualified name + interp = mkinterp + klass = interp.findclass("", "separate::sub") + assert_instance_of(AST::HostClass, klass, "Did not autoload sub class from separate file with no namespace") + assert_equal("separate::sub", klass.classname, "Incorrect class was returned") + + # Now make sure we don't get a failure when there's no module file + interp = mkinterp + modname = "alone" + name = "sub" + mk_module(modname, :sub => %w{alone::sub}) + + # First try it with a namespace + assert_nothing_raised("Could not autoload file when module file is missing") do + klass = interp.findclass("alone", name) + end + assert_instance_of(AST::HostClass, klass, "Did not autoload sub class from alone file with a namespace") + assert_equal("alone::sub", klass.classname, "Incorrect class was returned") + + # Now try it using the fully qualified name + interp = mkinterp + klass = interp.findclass("", "alone::sub") + assert_instance_of(AST::HostClass, klass, "Did not autoload sub class from alone file with no namespace") + assert_equal("alone::sub", klass.classname, "Incorrect class was returned") + end end class LdapNodeTest < PuppetTest::TestCase diff --git a/test/puppet/modules.rb b/test/puppet/modules.rb index e652182a8..15976c64a 100755 --- a/test/puppet/modules.rb +++ b/test/puppet/modules.rb @@ -2,7 +2,6 @@ $:.unshift("../lib").unshift("../../lib") if __FILE__ =~ /\.rb$/ -require 'puppet' require 'puppettest' class TestModules < Test::Unit::TestCase @@ -56,3 +55,5 @@ class TestModules < Test::Unit::TestCase assert_equal(templ_path, mod.template(templ)) end end + +# $Id$ -- cgit