From 38571f51b4adca0d9cb4d4b84829bbabe31b00ad Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Wed, 5 Dec 2012 15:47:47 -0800 Subject: [PATCH] (#7316) List faces using an autoloader Previously, calling Puppet::Interface.faces generated a different list of face applications than those included in Puppet::Application.available_application_names, since the former only considered the $LOAD_PATH, and the latter used an Autoloader, which includes gem directories and the current environment's modulepath. From what I can tell, the method is never used, as puppet itself accesses faces via Puppet::Interface[name, version] method. However, if someone had called `faces` it would have excluded faces that could be loaded via require. --- lib/puppet/interface.rb | 6 +++--- lib/puppet/interface/face_collection.rb | 13 ++++++------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb index e9e2e4a994e..527247a26a9 100644 --- a/lib/puppet/interface.rb +++ b/lib/puppet/interface.rb @@ -20,10 +20,9 @@ class Puppet::Interface include Puppet::Util class << self - # This is just so we can search for actions. We only use its - # list of directories to search. - # Can't we utilize an external autoloader, or simply use the $LOAD_PATH? -pvb + # @deprecated def autoloader + Puppet.deprecation_warning("Puppet::Interface.autoloader is deprecated; please use Puppet::Interface#loader instead") @autoloader ||= Puppet::Util::Autoload.new(:application, "puppet/face") end @@ -56,6 +55,7 @@ def face?(name, version) def [](name, version) unless face = Puppet::Interface::FaceCollection[name, version] + # REVISIT (#18042) no sense in rechecking if version == :current -- josh if current = Puppet::Interface::FaceCollection[name, :current] raise Puppet::Error, "Could not find version #{version} of #{name}" else diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index d2e3dbe01a0..11056588cd2 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -3,14 +3,13 @@ module Puppet::Interface::FaceCollection @faces = Hash.new { |hash, key| hash[key] = {} } + @loader = Puppet::Util::Autoload.new(:application, 'puppet/face') + def self.faces unless @loaded @loaded = true - $LOAD_PATH.each do |dir| - Dir.glob("#{dir}/puppet/face/*.rb"). - collect {|f| File.basename(f, '.rb') }. - each {|name| self[name, :current] } - end + names = @loader.files_to_load.map {|fn| ::File.basename(fn, '.rb')}.uniq + names.each {|name| self[name, :current]} end @faces.keys.select {|name| @faces[name].length > 0 } end @@ -58,7 +57,7 @@ def self.load_face(name, version) # other Ruby library that we might want to use. --daniel 2011-04-06 if safely_require name then # If we wanted :current, we need to index to find that; direct version - # requests just work™ as they go. --daniel 2011-04-06 + # requests just work as they go. --daniel 2011-04-06 if version == :current then # We need to find current out of this. This is the largest version # number that doesn't have a dedicated on-disk file present; those @@ -99,7 +98,7 @@ def self.load_face(name, version) end def self.safely_require(name, version = nil) - path = File.join 'puppet' ,'face', version.to_s, name.to_s + path = @loader.expand(version ? ::File.join(version.to_s, name.to_s) : name) require path true