Revert "Merge branch 'fix/3.x/7316_load_faces_from_modulepath_try3' i…

…nto 3.x"

This reverts commit b277bb3, reversing
changes made to bdf1936.

These changes are being backed out because they referenced a undefined
methods (`default_global_config_dir` and `default_global_var_dir`).
After fixing that locally, there was still a problem with `puppet help`
printing out a large number of warnings because it could not load face
code correctly.

I think we need to step back from this, re-evaluate what is happening,
and try a new approach.

lib/puppet/application.rb

@@ -219,64 +219,17 @@ def option_parser_commands
       @option_parser_commands
     end
 
-    ##
-    # Load an application from disk using the convention of mapping
-    # `Puppet::Application::Foo` to the Ruby library `puppet/application/foo`.
-    # If the library cannot be loaded using `require "puppet/application/foo"`
-    # then the Puppet modulepath will be searched for
-    # `<modulepath>/*/lib/puppet/application/foo.rb`.
-    #
-    # Note, for the purposes of error handling, application files are expected
-    # to always load.  Faces, however, have better exception handling.  As a
-    # result the application stub should be a simple as possible to ensure it
-    # always loads.
-    #
-    # @param [String] file_name the file name (without .rb extension) of an
-    # application to load, e.g. "catalog" or "minicat"
-    #
-    # @param [String] face_app_name the name of the expected face application
-    # class, e.g. `Catalog` or `Agent`.
-    #
-    # @return [Boolean] result of the `require` statement of the relative or
-    # absolute file path
-    def load_application_file(file_name, face_app_name)
-      path = "puppet/application/#{file_name}"
+    def find(file_name)
+      # This should probably be using the autoloader, but due to concerns about the fact that
+      #  the autoloader currently considers the modulepath when looking for things to load,
+      #  we're delaying that for now.
       begin
-        require path
-        return true
-      rescue LoadError => detail
-        first_error = detail
-        Puppet.debug "Could not `require \"puppet/application/#{file_name}\"` (Using the $LOAD_PATH)"
+        require ::File.join('puppet', 'application', file_name.to_s.downcase)
+      rescue LoadError => e
+        Puppet.log_and_raise(e, "Unable to find application '#{file_name}'.  #{e}")
       end
 
-      module_apps = Puppet::Util::CommandLine.module_applications(Puppet.settings[:modulepath])
-
-      if absolute_path = module_apps[file_name]['app'] then
-        begin
-          require absolute_path
-          Puppet.debug "Loaded '#{absolute_path}' (Using absolute path)"
-        rescue LoadError => detail
-          Puppet.log_and_raise(detail, "Unable to find application '#{face_app_name}'.  #{detail}")
-        end
-      else
-        Puppet.log_and_raise(first_error, "Unable to find application '#{face_app_name}'.  #{first_error}")
-      end
-    end
-
-    ##
-    # Find a Puppet face application, loading the face script from the
-    # filesystem if necessary.
-    #
-    # @param [String] face_app_name the name of the application to find, e.g.
-    # "Agent" or "Catalog"
-    #
-    # @return [Class] the class of the face application, e.g.
-    # `Puppet::Application::Catalog`.
-    def find(face_app_name)
-      file_name = face_app_name.to_s.downcase
-      load_application_file(file_name, face_app_name)
-
-      class_name = Puppet::Util::ConstantInflector.file2constant(face_app_name.to_s)
+      class_name = Puppet::Util::ConstantInflector.file2constant(file_name.to_s)
 
       clazz = try_load_class(class_name)
 
@@ -286,7 +239,7 @@ def find(face_app_name)
       ####  and then get rid of this stanza in a subsequent release.
       ################################################################
       if (clazz.nil?)
-        class_name = face_app_name.capitalize
+        class_name = file_name.capitalize
         clazz = try_load_class(class_name)
       end
       ################################################################
@@ -349,7 +302,9 @@ def clear_everything_for_tests
   end
 
   def app_defaults()
-    Puppet::Settings.app_defaults_for_run_mode(self.class.run_mode)
+    Puppet::Settings.app_defaults_for_run_mode(self.class.run_mode).merge(
+        :name => name
+    )
   end
 
   def initialize_app_defaults()

lib/puppet/application/master.rb

@@ -126,14 +126,7 @@ def help
   end
 
   def app_defaults()
-    super.merge({
-        :facts_terminus => 'yaml',
-        # when we are running master, we want to default to the global/system
-        #  config and vardirs (usually /etc), rather than defaulting to user
-        #  directories (which is what we do for other applications)
-        :confdir => Puppet::Settings.default_global_config_dir,
-        :vardir => Puppet::Settings.default_global_var_dir,
-      })
+    super.merge :facts_terminus => 'yaml'
   end
 
   def preinit

lib/puppet/interface/face_collection.rb

@@ -1,11 +1,5 @@
 require 'puppet/interface'
 
-# XXX The Puppet::Util::CommandLine.module_applications method needs to be
-# extracted out into its own class or module or something that both
-# Puppet::Application, Puppet::Util::CommandLine _and_
-# Puppet::Interface::FaceCollection can all use cleanly.
-require 'puppet/util/command_line'
-
 module Puppet::Interface::FaceCollection
   @faces = Hash.new { |hash, key| hash[key] = {} }
 
@@ -104,48 +98,20 @@ def self.load_face(name, version)
     return get_face(name, version)
   end
 
-  ##
-  # Load a _face_ application from disk.  Face applications are usually have a
-  # reflected "legacy" application.  This method is similar to
-  # `Puppet::Application.load_application_file`.
-  #
-  # @param [Symbol] name the name of the face to load, e.g. "catalog" or "minicat"
-  #
-  # @param [String] version the specific version to load.  The face application
-  # file must reside in a subdirectory if specified.  e.g.
-  # `puppet/face/1.2.3/catalog`.  If the version is not specified, the face
-  # must reside in the `face` subdirectory, e.g. `puppet/face/face`.
-  #
-  # @return [Boolean] true if the face application is loaded, false if there
-  # were errors while loading the file.
-  #
-  # @see Puppet::Application.load_application_file
   def self.safely_require(name, version = nil)
     path = File.join 'puppet' ,'face', version.to_s, name.to_s
-    begin
-      require path
-      return true
-    rescue ScriptError => detail
-      if not detail.message =~ %r{file -- puppet/face/([^/]+/)?#{name}$}
-        Puppet.err("Failed to load face #{name}:\n#{detail}")
-      end
-    end
-
-    module_apps = Puppet::Util::CommandLine.module_applications(Puppet.settings[:modulepath])
-
-    if absolute_path = module_apps[name.to_s]['face'] then
-      begin
-        if require absolute_path then
-          Puppet.debug "Loaded '#{absolute_path}' (Using absolute path)"
-        end
-        return true
-      rescue LoadError => detail
-        Puppet.debug "Unable to find face '#{name}'.  #{detail}"
-      rescue ScriptError => detail
-        Puppet.err("Failed to load face #{name}:\n#{detail}")
-      end
-    end
-    return nil
+    require path
+    true
+
+  rescue LoadError => e
+    raise unless e.message =~ %r{-- #{path}$}
+    # ...guess we didn't find the file; return a much better problem.
+    nil
+  rescue SyntaxError => e
+    raise unless e.message =~ %r{#{path}\.rb:\d+: }
+    Puppet.err "Failed to load face #{name}:\n#{e}"
+    # ...but we just carry on after complaining.
+    nil
   end
 
   def self.register(face)

lib/puppet/settings.rb

@@ -22,16 +22,17 @@ class Puppet::Settings
   attr_accessor :files
   attr_reader :timer
 
-  # These are the settings that every app is required to specify; there are
-  # reasonable defaults defined in application.rb.
-  REQUIRED_APP_SETTINGS = [:logdir]
+  # These are the settings that every app is required to specify; there are reasonable defaults defined in application.rb.
+  REQUIRED_APP_SETTINGS = [:logdir, :confdir, :vardir]
 
   # This method is intended for puppet internal use only; it is a convenience method that
   # returns reasonable application default settings values for a given run_mode.
   def self.app_defaults_for_run_mode(run_mode)
     {
         :name     => run_mode.to_s,
         :run_mode => run_mode.name,
+        :confdir  => run_mode.conf_dir,
+        :vardir   => run_mode.var_dir,
         :rundir   => run_mode.run_dir,
         :logdir   => run_mode.log_dir,
     }
@@ -108,7 +109,7 @@ def clear
   # Remove all set values, potentially skipping cli values.
   def unsafe_clear(clear_cli = true, clear_application_defaults = false)
     @values.each do |name, values|
-      next if (([:application_defaults, :global_defaults].include?(name)) and !clear_application_defaults)
+      next if ((name == :application_defaults) and !clear_application_defaults)
       next if ((name == :cli) and !clear_cli)
       @values.delete(name)
     end
@@ -130,7 +131,6 @@ def clearused
     @used = []
   end
 
-
   def global_defaults_initialized?()
     @global_defaults_initialized
   end
@@ -142,26 +142,10 @@ def initialize_global_settings(args = [])
     # 1) Parse the command line options and handle any of them that are
     #    registered, defined "global" puppet settings (mostly from defaults.rb).
     # 2) Parse the puppet config file(s).
-    #
-    # These 2 steps are being handled explicitly here.  If there ever arises a
-    #  situation where they need to be triggered from outside of this class,
-    #  without triggering the rest of the lifecycle--we might want to move them
-    #  out into a separate method that we call from here.  However, this seems
-    #  to be sufficient for now.
-    #  --cprice 2012-03-16
 
     parse_global_options(args)
-
-    # Here's step 2.  NOTE: this is a change in behavior where we are now
-    #  parsing the config file on every run; before, there were several apps
-    #  that specifically registered themselves as not requiring anything from
-    #  the config file.  The fact that we're always parsing it now might be a
-    #  small performance hit, but it was necessary in order to make sure that
-    #  we can resolve the libdir before we look for the available applications.
     parse_config_files
 
-    initialize_default_dir_settings
-
     @global_defaults_initialized = true
   end
 
@@ -200,35 +184,6 @@ def parse_global_options(args)
   end
   private :parse_global_options
 
-  ##
-  # Initialize the settings object with the default values of confdir and
-  # vardir.  This method should depend only on the effective UID of the
-  # process.  If the EUID is root, then use the agent runmode to determine
-  # confdir and vardir.  If EUID is not root, then use the user runmode to
-  # determine conf_dir and var_dir.
-  def initialize_default_dir_settings
-    if (Puppet.features.root?)
-      set_value(:confdir, Puppet::Util::RunMode[:agent].conf_dir, :global_defaults)
-      set_value(:vardir, Puppet::Util::RunMode[:agent].var_dir, :global_defaults)
-    else
-      set_value(:confdir, Puppet::Util::RunMode[:user].conf_dir, :global_defaults)
-      set_value(:vardir, Puppet::Util::RunMode[:user].var_dir, :global_defaults)
-    end
-  end
-
-  ## Private utility method; this is the callback that the OptionParser will use when it finds
-  ## an option that was defined in Puppet.settings.  All that this method does is a little bit
-  ## of clanup to get the option into the exact format that Puppet.settings expects it to be in,
-  ## and then passes it along to Puppet.settings.
-  ##
-  ## @param [String] opt the command-line option that was matched
-  ## @param [String, TrueClass, FalseClass] the value for the setting (as determined by the OptionParser)
-  #def handlearg(opt, val)
-  #  opt, val = self.class.clean_opt(opt, val)
-  #  Puppet.settings.handlearg(opt, val)
-  #end
-  #private :handlearg
-
   # A utility method (public, is used by application.rb and perhaps elsewhere) that munges a command-line
   # option string into the format that Puppet.settings expects.  (This mostly has to deal with handling the
   # "no-" prefix on flag/boolean options).
@@ -250,15 +205,6 @@ def app_defaults_initialized?
     @app_defaults_initialized
   end
 
-  ##
-  # Initialize the settings object given a Hash of application specific
-  # defaults.  The Hash must contain at least the keys specified in the
-  # REQUIRED_APP_SETTINGS constant.
-  #
-  # @param [Hash] app_defaults containing key/value pairs of default values for
-  #   specific settings.
-  #
-  # @return [Boolean] the value of `@app_defaults_initialized`
   def initialize_app_defaults(app_defaults)
     raise Puppet::DevError, "Attempting to initialize application default settings more than once!" if app_defaults_initialized?
     REQUIRED_APP_SETTINGS.each do |key|
@@ -740,9 +686,9 @@ def reuse
   # The order in which to search for values.
   def searchpath(environment = nil)
     if environment
-      [:cli, :memory, environment, :run_mode, :main, :application_defaults, :global_defaults]
+      [:cli, :memory, environment, :run_mode, :main, :application_defaults]
     else
-      [:cli, :memory, :run_mode, :main, :application_defaults, :global_defaults]
+      [:cli, :memory, :run_mode, :main, :application_defaults]
     end
   end
 
@@ -1204,9 +1150,9 @@ def parse_file(file)
       case line
       when /^\s*\[(\w+)\]\s*$/
         section = $1.intern # Section names
-        #disallow application_defaults and global_defaults in config file
-        if [:application_defaults, :global_defaults].include?(section)
-          raise Puppet::Error, "Illegal section '#{section}' in config file #{file} at line #{line}"
+        #disallow application_defaults in config file
+        if section == :application_defaults
+          raise Puppet::Error, "Illegal section 'application_defaults' in config file #{file} at line #{line}"
         end
         # Add a meta section
         result[section][:_meta] ||= {}

lib/puppet/util/autoload.rb

@@ -133,32 +133,32 @@ def module_directories(env=nil)
       # that we ship with puppet, and thus the module path is irrelevant.
       #
       # In the long term, I think the way that we want to handle this is to
-      # have the autoloader ignore the module path in all cases where it is not
-      # specifically requested (e.g., by a constructor param or something)...
-      # because there are very few cases where we should actually be loading
-      # code from the module path.  However, until that happens, we at least
-      # need a way to prevent the autoloader from attempting to access the
-      # module path before it is initialized.  For now we are accomplishing
-      # that by calling the "global_defaults_initialized?" method on the main
-      # puppet Settings object.
-      if Puppet.settings.global_defaults_initialized?
+      # have the autoloader ignore the module path in all cases where it is
+      # not specifically requested (e.g., by a constructor param or
+      # something)... because there are very few cases where we should
+      # actually be loading code from the module path.  However, until that
+      # happens, we at least need a way to prevent the autoloader from
+      # attempting to access the module path before it is initialized.  For
+      # now we are accomplishing that by calling the
+      # "app_defaults_initialized?" method on the main puppet Settings object.
+      # --cprice 2012-03-16
+      if Puppet.settings.app_defaults_initialized?
         # if the app defaults have been initialized then it should be safe to access the module path setting.
         Thread.current[:env_module_directories][real_env] ||= real_env.modulepath.collect do |dir|
           Dir.entries(dir).reject { |f| f =~ /^\./ }.collect { |f| File.join(dir, f) }
         end.flatten.collect { |d| File.join(d, "lib") }.find_all do |d|
           FileTest.directory?(d)
         end
       else
-        # if we get here, the global defaults have not been initialized, so we
-        # basically use an empty module path.
+        # if we get here, the app defaults have not been initialized, so we basically use an empty module path.
         []
       end
     end
 
     def libdirs()
       # See the comments in #module_directories above.  Basically, we need to be careful not to try to access the
       # libdir before we know for sure that all of the settings have been initialized (e.g., during bootstrapping).
-      if (Puppet.settings.global_defaults_initialized?)
+      if (Puppet.settings.app_defaults_initialized?)
         Puppet[:libdir].split(File::PATH_SEPARATOR)
       else
         []