Merge branch '2.7.x' into 2.7.next

Conflicts:
 * spec/unit/node/facts_spec.rb

Updates:
 * spec/unit/interface/action{,_builder}_spec.rb
   => update for 'when_invoked' block being required.

5 files changed, 122 insertions, 66 deletions

diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
index 7bebd18bb..7a5ce3400 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -15,8 +15,8 @@ class Puppet::Application::FaceBase < Puppet::Application
     Puppet::Util::Log.level = :info
   end
 
-  option("--render-as FORMAT") do |arg|
-    @render_as = arg.to_sym
+  option("--render-as FORMAT") do |format|
+    self.render_as = format.to_sym
   end
 
   option("--mode RUNMODE", "-r") do |arg|
@@ -27,55 +27,23 @@ class Puppet::Application::FaceBase < Puppet::Application
 
 
   attr_accessor :face, :action, :type, :arguments, :render_as
-  attr_writer :exit_code
 
-  # This allows you to set the exit code if you don't want to just exit
-  # immediately but you need to indicate a failure.
-  def exit_code
-    @exit_code || 0
+  def render_as=(format)
+    if format == :json then
+      @render_as = Puppet::Network::FormatHandler.format(:pson)
+    else
+      @render_as = Puppet::Network::FormatHandler.format(format)
+    end
+    @render_as or raise ArgumentError, "I don't know how to render '#{format}'"
   end
 
   def render(result)
-    format = render_as || action.render_as || :for_humans
-
     # Invoke the rendering hook supplied by the user, if appropriate.
-    if hook = action.when_rendering(format) then
+    if hook = action.when_rendering(render_as.name)
       result = hook.call(result)
     end
 
-    if format == :for_humans then
-      render_for_humans(result)
-    else
-      render_method = Puppet::Network::FormatHandler.format(format).render_method
-      if render_method == "to_pson"
-        PSON::pretty_generate(result, :allow_nan => true, :max_nesting => false)
-      else
-        result.send(render_method)
-      end
-    end
-  end
-
-  def render_for_humans(result)
-    # String to String
-    return result if result.is_a? String
-    return result if result.is_a? Numeric
-
-    # Simple hash to table
-    if result.is_a? Hash and result.keys.all? { |x| x.is_a? String or x.is_a? Numeric }
-      output = ''
-      column_a = result.map do |k,v| k.to_s.length end.max + 2
-      column_b = 79 - column_a
-      result.sort_by { |k,v| k.to_s } .each do |key, value|
-        output << key.to_s.ljust(column_a)
-        output << PP.pp(value, '', column_b).
-          chomp.gsub(/\n */) { |x| x + (' ' * column_a) }
-        output << "\n"
-      end
-      return output
-    end
-
-    # ...or pretty-print the inspect outcome.
-    return result.pretty_inspect
+    render_as.render(result)
   end
 
   def preinit
@@ -133,8 +101,13 @@ class Puppet::Application::FaceBase < Puppet::Application
     end
 
     if @action.nil?
-      @action = @face.get_default_action()
-      @is_default_action = true
+      if @action = @face.get_default_action() then
+        @is_default_action = true
+      else
+        Puppet.err "#{face.name} does not have a default action, and no action was given"
+        Puppet.err Puppet::Face[:help, :current].help(@face.name)
+        exit false
+      end
     end
 
     # Now we can interact with the default option code to build behaviour
@@ -142,7 +115,7 @@ class Puppet::Application::FaceBase < Puppet::Application
     @action.options.each do |option|
       option = @action.get_option(option) # make it the object.
       self.class.option(*option.optparse) # ...and make the CLI parse it.
-    end if @action
+    end
 
     # ...and invoke our parent to parse all the command line options.
     super
@@ -194,23 +167,66 @@ class Puppet::Application::FaceBase < Puppet::Application
     # would invoke the action with options set as global state in the
     # interface object.  --daniel 2011-03-28
     @arguments << options
+
+    # If we don't have a rendering format, set one early.
+    self.render_as ||= (@action.render_as || :console)
   end
 
 
   def main
+    status = false
+
     # Call the method associated with the provided action (e.g., 'find').
-    if @action
-      result = @face.send(@action.name, *arguments)
-      puts render(result) unless result.nil?
-    else
-      if arguments.first.is_a? Hash
-        puts "#{@face} does not have a default action"
-      else
-        puts "#{@face} does not respond to action #{arguments.first}"
-      end
+    unless @action
+      puts Puppet::Face[:help, :current].help(@face.name)
+      raise "#{face} does not respond to action #{arguments.first}"
+    end
 
-      puts Puppet::Face[:help, :current].help(@face.name, *arguments)
+    # We need to do arity checking here because this is generic code
+    # calling generic methods – that have argument defaulting.  We need to
+    # make sure we don't accidentally pass the options as the first
+    # argument to a method that takes one argument.  eg:
+    #
+    #   puppet facts find
+    #   => options => {}
+    #      @arguments => [{}]
+    #   => @face.send :bar, {}
+    #
+    #   def face.bar(argument, options = {})
+    #   => bar({}, {})  # oops!  we thought the options were the
+    #                   # positional argument!!
+    #
+    # We could also fix this by making it mandatory to pass the options on
+    # every call, but that would make the Ruby API much more annoying to
+    # work with; having the defaulting is a much nicer convention to have.
+    #
+    # We could also pass the arguments implicitly, by having a magic
+    # 'options' method that was visible in the scope of the action, which
+    # returned the right stuff.
+    #
+    # That sounds attractive, but adds complications to all sorts of
+    # things, especially when you think about how to pass options when you
+    # are writing Ruby code that calls multiple faces.  Especially if
+    # faces are involved in that. ;)
+    #
+    # --daniel 2011-04-27
+    if (arity = @action.positional_arg_count) > 0
+      unless (count = arguments.length) == arity then
+        s = arity == 2 ? '' : 's'
+        raise ArgumentError, "puppet #{@face.name} #{@action.name} takes #{arity-1} argument#{s}, but you gave #{count-1}"
+      end
     end
-    exit(exit_code)
+
+    result = @face.send(@action.name, *arguments)
+    puts render(result) unless result.nil?
+    status = true
+
+  rescue Exception => detail
+    puts detail.backtrace if Puppet[:trace]
+    Puppet.err detail.to_s
+    Puppet.err "Try 'puppet help #{@face.name} #{@action.name}' for usage"
+
+  ensure
+    exit status
   end
 end
diff --git a/lib/puppet/application/faces.rb b/lib/puppet/application/faces.rb
index 3dd3f0312..e7fce66b1 100644
--- a/lib/puppet/application/faces.rb
+++ b/lib/puppet/application/faces.rb
@@ -10,16 +10,54 @@ class Puppet::Application::Faces < Puppet::Application
     Puppet::Util::Log.level = :debug
   end
 
-  option("--help", "-h") do |arg|
-    puts "Usage: puppet faces [actions|terminuses]
-Lists all available faces, and by default includes all available terminuses and actions.
-"
-  end
-
   option("--verbose", "-v") do
     Puppet::Util::Log.level = :info
   end
 
+  def help
+    <<-HELP
+puppet-faces(8) -- List available Faces and actions
+========
+
+SYNOPSIS
+--------
+Lists the available subcommands (with applicable terminuses and/or actions)
+provided by the Puppet Faces API. This information is automatically read
+from the Puppet code present on the system. By default, the output includes
+all terminuses and actions.
+
+USAGE
+-----
+puppet faces [-d|--debug] [-v|--verbose] [actions|terminuses]
+
+OPTIONS
+-------
+Note that any configuration option valid in the configuration file is also
+a valid long argument. See the configuration file documentation at
+http://docs.puppetlabs.com/references/stable/configuration.html for the
+full list of acceptable parameters. A commented list of all
+configuration options can also be generated by running puppet agent with
+'--genconfig'.
+
+* --verbose:
+  Sets the log level to "info." This option has no tangible effect at the time
+  of this writing.
+
+* --debug:
+  Sets the log level to "debug." This option has no tangible effect at the time
+  of this writing.
+
+AUTHOR
+------
+Puppet Labs
+
+COPYRIGHT
+---------
+Copyright (c) 2011 Puppet Labs, LLC Licensed under the Apache 2.0 License
+
+    HELP
+  end
+
   def list(*arguments)
     if arguments.empty?
       arguments = %w{terminuses actions}
diff --git a/lib/puppet/application/help.rb b/lib/puppet/application/help.rb
index 0d7767632..4829a2036 100644
--- a/lib/puppet/application/help.rb
+++ b/lib/puppet/application/help.rb
@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 require 'puppet/application/face_base'
 
 class Puppet::Application::Help < Puppet::Application::FaceBase
diff --git a/lib/puppet/application/plugin.rb b/lib/puppet/application/plugin.rb
new file mode 100644
index 000000000..2d0402e43
--- /dev/null
+++ b/lib/puppet/application/plugin.rb
@@ -0,0 +1,3 @@
+require 'puppet/application/face_base'
+class Puppet::Application::Plugin < Puppet::Application::FaceBase
+end
diff --git a/lib/puppet/application/configurer.rb b/lib/puppet/application/secret_agent.rb
index 6e86cd2d4..d704d14b2 100644
--- a/lib/puppet/application/configurer.rb
+++ b/lib/puppet/application/secret_agent.rb
@@ -1,7 +1,7 @@
 require 'puppet/application'
 require 'puppet/face'
 
-class Puppet::Application::Configurer < Puppet::Application
+class Puppet::Application::Secret_agent < Puppet::Application
   should_parse_config
   run_mode :agent
 
@@ -17,7 +17,7 @@ class Puppet::Application::Configurer < Puppet::Application
   end
 
   def run_command
-    report = Puppet::Face[:configurer, '0.0.1'].synchronize(Puppet[:certname])
+    report = Puppet::Face[:secret_agent, '0.0.1'].synchronize(Puppet[:certname])
     Puppet::Face[:report, '0.0.1'].submit(report)
   end
 end