From ebc642b2141cfed7d10a64ff0bb67a417f5f37be Mon Sep 17 00:00:00 2001
From: nfagerlund <nick.fagerlund@gmail.com>
Date: Thu, 19 May 2011 12:04:59 -0700
Subject: (#6962) Add "arguments" method to help API

Since some actions take arguments and some do not, action synopses were
incomplete and ambiguous.

This commit adds a method for explicitly declaring what argument(s) an
action takes, and places the arguments at the appropriate spot in the
action's synopsis. By convention, individual arguments should be wrapped
in angle brackets.
---
 lib/puppet/interface/action.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 3c377a49f..748888c2e 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -46,6 +46,7 @@ class Puppet::Interface::Action
   ########################################################################
   # Documentation...
   attr_doc :returns
+  attr_doc :arguments
   def synopsis
     output = PrettyPrint.format do |s|
       s.text("puppet #{@face.name}")
@@ -67,6 +68,7 @@ class Puppet::Interface::Action
           end
         end
       end
+      s.text(" #{arguments}") if arguments
     end
   end
 
-- 
cgit 


From 069a6b8e645255530abf308cfeeed1d7650211db Mon Sep 17 00:00:00 2001
From: nfagerlund <nick.fagerlund@gmail.com>
Date: Mon, 23 May 2011 17:26:59 -0700
Subject: Maint: Add ellipsis to generated short_descriptions.

Auto-generated short descriptions cut off at five lines with no
indication that they are truncated.

This commit adds ellipsis in brackets to indicate incompleteness.
---
 lib/puppet/interface/documentation.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/puppet/interface/documentation.rb b/lib/puppet/interface/documentation.rb
index 48e9a8b1a..aedcc1c24 100644
--- a/lib/puppet/interface/documentation.rb
+++ b/lib/puppet/interface/documentation.rb
@@ -78,7 +78,7 @@ class Puppet::Interface
         return nil if @description.nil?
         lines = @description.split("\n")
         grab  = [5, lines.index('') || 5].min
-        @short_description = lines[0, grab].join("\n")
+        @short_description = lines[0, grab].join("\n") + ' [...]'
       end
       @short_description
     end
-- 
cgit 


From 331d8128288f8c7bb3645149ff452372c0866429 Mon Sep 17 00:00:00 2001
From: nfagerlund <nick.fagerlund@gmail.com>
Date: Wed, 18 May 2011 16:20:05 -0700
Subject: (#7561) Complete help text for all faces and actions

Faces help output relies on input from the documentation methods in each
of the faces to be documented. This commit calls those methods in each of our
faces, with varying levels of detail depending on their complexity.
---
 lib/puppet/face/catalog.rb                     | 83 ++++++++++++++++----------
 lib/puppet/face/catalog/select.rb              | 25 +++++---
 lib/puppet/face/certificate.rb                 | 70 ++++++++++++++++++----
 lib/puppet/face/certificate_request.rb         | 40 ++++++++++---
 lib/puppet/face/certificate_revocation_list.rb | 38 ++++++++----
 lib/puppet/face/config.rb                      | 37 ++++++------
 lib/puppet/face/facts.rb                       | 67 ++++++++++++++++-----
 lib/puppet/face/file.rb                        | 45 ++++++++++++--
 lib/puppet/face/file/download.rb               | 18 ++++++
 lib/puppet/face/file/store.rb                  |  9 +++
 lib/puppet/face/help.rb                        | 11 +++-
 lib/puppet/face/help/action.erb                | 40 +++----------
 lib/puppet/face/key.rb                         | 18 +++---
 lib/puppet/face/node.rb                        | 40 ++++++++++---
 lib/puppet/face/parser.rb                      |  8 ++-
 lib/puppet/face/plugin.rb                      | 31 ++++++----
 lib/puppet/face/report.rb                      | 40 +++++++++----
 lib/puppet/face/resource.rb                    | 46 ++++++++++++--
 lib/puppet/face/resource_type.rb               | 71 ++++++++++++++++++++--
 lib/puppet/face/secret_agent.rb                | 40 +++++++++----
 lib/puppet/face/status.rb                      | 53 ++++++++++------
 lib/puppet/indirector/face.rb                  | 55 +++++++++--------
 22 files changed, 628 insertions(+), 257 deletions(-)

diff --git a/lib/puppet/face/catalog.rb b/lib/puppet/face/catalog.rb
index 4624313bc..c7501ff37 100644
--- a/lib/puppet/face/catalog.rb
+++ b/lib/puppet/face/catalog.rb
@@ -5,17 +5,17 @@ Puppet::Indirector::Face.define(:catalog, '0.0.1') do
   license   "Apache 2 license; see COPYING"
 
   summary "Compile, save, view, and convert catalogs."
-  description <<-EOT
+  description <<-'EOT'
     This face primarily interacts with the compiling subsystem. By default,
     it compiles a catalog using the default manifest and the hostname from
     `certname`, but you can choose to retrieve a catalog from the server by
-    specifying `--terminus rest`.  You can also choose to print any catalog
+    specifying '--terminus rest'.  You can also choose to print any catalog
     in 'dot' format (for easy graph viewing with OmniGraffle or Graphviz)
     with '--render-as dot'.
   EOT
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `active_record`
@@ -25,26 +25,35 @@ Puppet::Indirector::Face.define(:catalog, '0.0.1') do
     * `yaml`
   EOT
 
+  get_action(:destroy).summary "Invalid for this face."
+  get_action(:search).summary "Query format unknown; potentially invalid for this face."
+
   action(:apply) do
-    summary "Apply a Puppet::Resource::Catalog object"
-    description <<-EOT
-      Applies a catalog object retrieved with the `download` action. This
-      action cannot consume a serialized catalog, and is not intended for
-      command-line use."
+    summary "Apply a Puppet::Resource::Catalog object."
+    description <<-'EOT'
+      Finds and applies a catalog. This action takes no arguments, but
+      the source of the catalog can be managed with the --terminus option.
     EOT
-    notes <<-EOT
-      This action returns a Puppet::Transaction::Report object.
+    returns <<-'EOT'
+      A Puppet::Transaction::Report object.
     EOT
-    examples <<-EOT
-      From `secret_agent.rb`:
+    examples <<-'EOT'
+      Apply the locally cached catalog:
 
-          Puppet::Face[:plugin, '0.0.1'].download
+      $ puppet catalog apply --terminus yaml
+
+      Retrieve a catalog from the master and apply it, in one step:
+
+      $ puppet catalog apply --terminus rest
 
-          facts   = Puppet::Face[:facts, '0.0.1'].find(certname)
-          catalog = Puppet::Face[:catalog, '0.0.1'].download(certname, facts)
-          report  = Puppet::Face[:catalog, '0.0.1'].apply(catalog)
+      From `secret_agent.rb` (API example):
 
-          Puppet::Face[:report, '0.0.1'].submit(report)
+          # ...
+          Puppet::Face[:catalog, '0.0.1'].download
+          # (Termini are singletons; catalog.download has a side effect of
+          # setting the catalog terminus to yaml)
+          report  = Puppet::Face[:catalog, '0.0.1'].apply
+          # ...
     EOT
 
     when_invoked do |options|
@@ -71,23 +80,33 @@ Puppet::Indirector::Face.define(:catalog, '0.0.1') do
   end
 
   action(:download) do
-    summary "Download this node's catalog from the puppet master server"
-    description <<-EOT
-      Retrieves a catalog from the puppet master. Unlike the `find` action,
-      `download` submits facts to the master as part of the request. This
-      action is not intended for command-line use.
+    summary "Download this node's catalog from the puppet master server."
+    description <<-'EOT'
+      Retrieves a catalog from the puppet master and saves it to the
+      local yaml cache. The saved catalog can be used in subsequent
+      catalog actions by specifying '--terminus rest'.
+
+      This action always contacts the puppet master and will ignore
+      alternate termini.
     EOT
-    notes "This action returns a Puppet::Resource::Catalog object."
-    examples <<-EOT
-      From `secret_agent.rb`:
+    returns "Nothing."
+    notes <<-'EOT'
+      As termini are singletons, this action has a side effect of
+      exporting Puppet::Resource::Catalog.indirection.terminus_class =
+      yaml to the calling context when used with the Ruby Faces API. The
+      terminus must be explicitly re-set for subsequent catalog actions.
+    EOT
+    examples <<-'EOT'
+      Retrieve and store a catalog:
 
-          Puppet::Face[:plugin, '0.0.1'].download
+      $ puppet catalog download
 
-          facts   = Puppet::Face[:facts, '0.0.1'].find(certname)
-          catalog = Puppet::Face[:catalog, '0.0.1'].download(certname, facts)
-          report  = Puppet::Face[:catalog, '0.0.1'].apply(catalog)
+      From `secret_agent.rb` (API example):
 
-          Puppet::Face[:report, '0.0.1'].submit(report)
+          Puppet::Face[:plugin, '0.0.1'].download
+          Puppet::Face[:facts, '0.0.1'].upload
+          Puppet::Face[:catalog, '0.0.1'].download
+          # ...
     EOT
     when_invoked do |options|
       Puppet::Resource::Catalog.indirection.terminus_class = :rest
diff --git a/lib/puppet/face/catalog/select.rb b/lib/puppet/face/catalog/select.rb
index a6c97a627..d86963e75 100644
--- a/lib/puppet/face/catalog/select.rb
+++ b/lib/puppet/face/catalog/select.rb
@@ -2,21 +2,28 @@
 Puppet::Face.define(:catalog, '0.0.1') do
   action :select do
     summary "Select and show a list of resources of a given type"
-    description <<-EOT
+    arguments "<host> <resource_type>"
+    returns <<-'EOT'
+      An array of resource objects excised from a catalog. When used at
+      the command line, returns a list of resource references (Type[title]).
+    EOT
+    description <<-'EOT'
       Retrieves a catalog for the specified host and returns an array of
-      resources of the given type. This action is not intended for
-      command-line use.
+      resources of the given type.
     EOT
-    notes <<-NOTES
-      The type name for this action must be given in its capitalized form.
-      That is, calling `catalog select mynode file` will return an empty
-      array, whereas calling it with 'File' will return a list of the node's
-      file resources.
-
+    notes <<-'NOTES'
       By default, this action will retrieve a catalog from Puppet's compiler
       subsystem; you must call the action with `--terminus rest` if you wish
       to retrieve a catalog from the puppet master.
+
+      FORMATTING ISSUES: This action cannot currently render useful yaml;
+      instead, it returns an entire catalog. Use json instead.
     NOTES
+    examples <<-'EOT'
+      Ask the puppet master for a list of managed file resources for a node:
+
+      $ puppet catalog select --terminus rest somenode.magpie.lan file
+    EOT
     when_invoked do |host, type, options|
       # REVISIT: Eventually, type should have a default value that triggers
       # the non-specific behaviour.  For now, though, this will do.
diff --git a/lib/puppet/face/certificate.rb b/lib/puppet/face/certificate.rb
index 859946623..d77525ba8 100644
--- a/lib/puppet/face/certificate.rb
+++ b/lib/puppet/face/certificate.rb
@@ -6,16 +6,16 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do
   license   "Apache 2 license; see COPYING"
 
   summary "Provide access to the CA for certificate management"
-  description <<-EOT
+  description <<-'EOT'
     This face interacts with a local or remote Puppet certificate
-    authority. Currently, its behavior is not a full superset of puppet
-    cert; specifically, it is unable to mimic puppet cert's "clean" option,
+    authority. Currently, its behavior is not a full superset of `puppet
+    cert`; specifically, it is unable to mimic puppet cert's "clean" option,
     and its "generate" action submits a CSR rather than creating a
     signed certificate.
   EOT
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `ca`
@@ -25,7 +25,7 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do
 
   option "--ca-location LOCATION" do
     summary "The certificate authority to query"
-    description <<-EOT
+    description <<-'EOT'
       Whether to act on the local certificate authority or one provided by a
       remote puppet master. Allowed values are 'local' and 'remote.'
     EOT
@@ -36,16 +36,23 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do
   end
 
   action :generate do
-    summary "Generate a new certificate signing request for HOST"
-    description <<-EOT
+    summary "Generate a new certificate signing request for HOST."
+    arguments "<host>"
+    returns "Nothing."
+    description <<-'EOT'
       Generates and submits a certificate signing request (CSR) for the
-      provided host identifier. This CSR will then have to be signed by a user
+      specified host. This CSR will then have to be signed by a user
       with the proper authorization on the certificate authority.
 
-      Puppet agent handles CSR submission automatically. This action is
+      Puppet agent usually handles CSR submission automatically. This action is
       primarily useful for requesting certificates for individual users and
       external applications.
     EOT
+    examples <<-'EOT'
+      Request a certificate for "somenode" from the site's CA:
+
+      $ puppet certificate generate somenode.puppetlabs.lan --ca-location remote
+    EOT
 
     when_invoked do |name, options|
       host = Puppet::SSL::Host.new(name)
@@ -55,7 +62,11 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do
   end
 
   action :list do
-    summary "List all certificate signing requests"
+    summary "List all certificate signing requests."
+    returns <<-'EOT'
+      An array of CSR object #inspect strings. This output is currently messy,
+      but does contain the names of nodes requesting certificates.
+    EOT
 
     when_invoked do |options|
       Puppet::SSL::Host.indirection.search("*", {
@@ -65,7 +76,17 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do
   end
 
   action :sign do
-    summary "Sign a certificate signing request for HOST"
+    summary "Sign a certificate signing request for HOST."
+    arguments "<host>"
+    returns <<-'EOT'
+      A string that appears to be an x509 certificate, but is actually
+      not. Retrieve certificates using the `find` action.
+    EOT
+    examples <<-'EOT'
+      Sign somenode.puppetlabs.lan's certificate:
+
+      $ puppet certificate sign somenode.puppetlabs.lan --ca-location remote
+    EOT
 
     when_invoked do |name, options|
       host = Puppet::SSL::Host.new(name)
@@ -73,4 +94,27 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do
       Puppet::SSL::Host.indirection.save(host)
     end
   end
+
+  # Indirector action doc overrides
+  find = get_action(:find)
+  find.summary "Retrieve a certificate"
+  find.arguments "<host>"
+  find.returns <<-'EOT'
+    An x509 SSL certificate. You will usually want to render this as a
+    string ('--render-as s').
+
+    Note that this action has a side effect of caching a copy of the
+    certificate in Puppet's `ssldir`.
+  EOT
+
+  destroy = get_action(:destroy)
+  destroy.summary "Delete a local certificate."
+  destroy.arguments "<host>"
+  destroy.returns "Nothing."
+  destroy.description <<-'EOT'
+    Deletes a certificate. This action currently only works with a local CA.
+  EOT
+
+  get_action(:search).summary "Invalid for this face."
+  get_action(:save).summary "Invalid for this face."
 end
diff --git a/lib/puppet/face/certificate_request.rb b/lib/puppet/face/certificate_request.rb
index cc6021517..de52b7fb7 100644
--- a/lib/puppet/face/certificate_request.rb
+++ b/lib/puppet/face/certificate_request.rb
@@ -5,28 +5,50 @@ Puppet::Indirector::Face.define(:certificate_request, '0.0.1') do
   license   "Apache 2 license; see COPYING"
 
   summary "Manage certificate requests."
-  description <<-EOT
+  description <<-'EOT'
     Retrieves and submits certificate signing requests (CSRs). Invoke
-    `search` with an unread key to retrieve all outstanding CSRs, invoke
+    `search` with a dummy key to retrieve all outstanding CSRs, invoke
     `find` with a node certificate name to retrieve a specific request, and
     invoke `save` to submit a CSR.
   EOT
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `ca`
     * `file`
     * `rest`
   EOT
-  examples <<-EOT
-    Retrieve all CSRs from the local CA:
 
-        puppet certificate_request search no_key --terminus ca
+  # Per-action doc overrides
+  get_action(:destroy).summary "Invalid for this face."
 
+  get_action(:find).summary "Retrieve a single CSR."
+  get_action(:find).arguments "<host>"
+  get_action(:find).returns <<-'EOT'
+    A single certificate request. In most cases, you will want to render
+    this as a string ('--render-as s').
+  EOT
+  get_action(:find).examples <<-'EOT'
     Retrieve a single CSR from the puppet master's CA:
 
-        puppet certificate_request find mynode.puppetlabs.lan --terminus rest
+    $ puppet certificate_request find somenode.puppetlabs.lan --terminus rest
+  EOT
+
+  get_action(:search).summary "Retrieve all outstanding CSRs."
+  get_action(:search).arguments "<dummy_key>"
+  get_action(:search).returns <<-'EOT'
+    An array of certificate request objects. In most cases, you will
+    want to render this as a string ('--render-as s').
   EOT
+  get_action(:search).notes "This action always returns all CSRs, but requires a dummy search key."
+  get_action(:search).examples <<-'EOT'
+    Retrieve all CSRs from the local CA:
+
+    $ puppet certificate_request search x --terminus ca
+  EOT
+
+  get_action(:save).summary "Submit a certificate signing request."
+  get_action(:save).arguments "<x509_CSR>"
 end
diff --git a/lib/puppet/face/certificate_revocation_list.rb b/lib/puppet/face/certificate_revocation_list.rb
index 2722b20f2..db00ea3ca 100644
--- a/lib/puppet/face/certificate_revocation_list.rb
+++ b/lib/puppet/face/certificate_revocation_list.rb
@@ -5,26 +5,44 @@ Puppet::Indirector::Face.define(:certificate_revocation_list, '0.0.1') do
   license   "Apache 2 license; see COPYING"
 
   summary "Manage the list of revoked certificates."
-  description <<-EOT
+  description <<-'EOT'
     This face is primarily for retrieving the certificate revocation
     list from the CA. Although it exposes search/save/destroy methods,
     they shouldn't be used under normal circumstances.
   EOT
-  notes <<-EOT
-    Although the find action must be given an argument, this argument is
-    never read, and can contain the descriptive text of your choice.
-
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `ca`
     * `file`
     * `rest`
   EOT
-  examples <<-EXAMPLES
-    Retrieve the CRL:
 
-        puppet certificate_revocation_list find crl
+  get_action(:find).summary "Retrieve the certificate revocation list."
+  get_action(:find).arguments "<dummy_key>"
+  get_action(:find).returns <<-'EOT'
+    A certificate revocation list. You will usually want to render this
+    as a string ('--render-as s').
+  EOT
+  get_action(:find).examples <<-'EXAMPLES'
+    Retrieve a copy of the puppet master's CRL:
+
+    $ puppet certificate_revocation_list find crl --terminus rest
   EXAMPLES
+
+  get_action(:destroy).summary "Delete the certificate revocation list."
+  get_action(:destroy).arguments "<dummy_key>"
+  get_action(:destroy).returns "Nothing."
+  get_action(:destroy).description <<-'EOT'
+    Deletes the certificate revocation list. This cannot be done over
+    REST, but it is possible to both delete the locally cached copy of
+    the CA's CRL and delete the CA's own copy (if running on the CA
+    machine and invoked with '--terminus ca'). Needless to say, don't do
+    this unless you know what you're up to.
+  EOT
+
+  get_action(:search).summary "Invalid for this face."
+  get_action(:save).summary "Invalid for this face."
 end
diff --git a/lib/puppet/face/config.rb b/lib/puppet/face/config.rb
index 9ca41085e..6e5bff071 100644
--- a/lib/puppet/face/config.rb
+++ b/lib/puppet/face/config.rb
@@ -4,38 +4,37 @@ Puppet::Face.define(:config, '0.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
 
-  summary "Interact with Puppet configuration options"
+  summary "Interact with Puppet's configuration options."
 
   action(:print) do
-    summary "Examine Puppet's current configuration options"
-    description <<-EOT
+    summary "Examine Puppet's current configuration options."
+    arguments "(all | <option> [<option> ...]"
+    returns <<-'EOT'
+      When called with one option: a single value.
+
+      When called with "all" or multiple options: a list of options and
+      their values.
+    EOT
+    description <<-'EOT'
       Prints the value of a single configuration option or a list of
       configuration options.
 
       This action is an alternate interface to the information available with
-      `puppet agent --configprint`.
+      `puppet <subcommand> --configprint`.
     EOT
-    notes <<-EOT
-      The return data of this action varies depending on its arguments. When
-      called with "all," `print` will return a complete list of option names
-      and values. When called with a single configuration option name, it will
-      return the value of that option. When called with a list of
-      configuration option names, it will return the corresponding list of
-      option names and values.
-
-      By default, this action retrieves its configuration information in agent
-      mode. To examine the master's configuration, supply Puppet's global
-      `--mode master` option. To examine configurations from a specific
-      environment, you can use the `--environment` option.
+    notes <<-'EOT'
+      By default, this action reads the configuration in agent mode.
+      Use the '--mode' and '--environment' flags to examine other
+      configuration domains.
     EOT
-    examples <<-EOT
+    examples <<-'EOT'
       Get puppet's runfile directory:
 
-          puppet config print rundir
+      $ puppet config print rundir
 
       Get a list of important directories from the master's config:
 
-          puppet config print all --mode master | grep -E "(path|dir)"
+      $ puppet config print all --mode master | grep -E "(path|dir)"
     EOT
 
     when_invoked do |*args|
diff --git a/lib/puppet/face/facts.rb b/lib/puppet/face/facts.rb
index ecf4e371e..d7ddf1d51 100644
--- a/lib/puppet/face/facts.rb
+++ b/lib/puppet/face/facts.rb
@@ -5,10 +5,20 @@ Puppet::Indirector::Face.define(:facts, '0.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
 
-  summary "Retrieve, store, and view facts."
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  summary "Retrieve and store facts."
+  description <<-'EOT'
+    This face manages facts, the collections of normalized system
+    information used by Puppet. It can read facts directly from the
+    local system (using the default `facter` terminus), look up facts
+    reported by other systems, and submit facts to the puppet master.
+
+    When used with the `rest` terminus, this face is essentially a
+    front-end to the inventory service REST API. See the inventory
+    service documentation for more detail.
+  EOT
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `active_record`
@@ -21,18 +31,47 @@ Puppet::Indirector::Face.define(:facts, '0.0.1') do
     * `yaml`
   EOT
 
+  find = get_action(:find)
+  find.summary "Retrieve a host's facts."
+  find.arguments "<host>"
+  find.returns "A Puppet::Node::Facts object."
+  find.notes <<-'EOT'
+    When using the `facter` terminus, the host argument is essentially ignored.
+  EOT
+  find.examples <<-'EOT'
+    Get facts from the local system:
+
+    $ puppet facts find x
+
+    Ask the puppet master for facts for an arbitrary node:
+
+    $ puppet facts find somenode.puppetlabs.lan --terminus rest
+
+    Query a DB-backed inventory directly (bypassing the REST API):
+
+    $ puppet facts find somenode.puppetlabs.lan --terminus inventory_active_record --mode master
+  EOT
+
+  get_action(:destroy).summary "Invalid for this face."
+  get_action(:search).summary "Query format unknown; potentially invalid for this face."
+
   action(:upload) do
-    summary "Upload our facts to the puppet master."
-    description <<-EOT
-      Retrieves facts for the local system and saves them to the puppet master
-      server. This is essentially a shortcut action: it calls the `find`
-      action with the facter terminus, then passes the returned facts object
-      to the `save` action, which uses the rest terminus.
+    summary "Upload local facts to the puppet master."
+    description <<-'EOT'
+      Reads facts from the local system using the facter terminus, then
+      saves the returned facts using the rest terminus.
+    EOT
+    returns "Nothing."
+    notes <<-'EOT'
+      This action requires that the puppet master's `auth.conf` file
+      allow save access to the `facts` REST terminus. Puppet agent does
+      not use this facility, and it is turned off by default. See
+      <http://docs.puppetlabs.com/guides/rest_auth_conf.html> for more details.
     EOT
-    notes <<-EOT
-      This action uses the save action, which requires the puppet master's
-      auth.conf to allow save access to the `facts` REST terminus. See
-      `http://docs.puppetlabs.com/guides/rest_auth_conf.html` for more details.
+    examples <<-'EOT'
+      Upload facts:
+
+      $ puppet facts upload
     EOT
 
     render_as :yaml
diff --git a/lib/puppet/face/file.rb b/lib/puppet/face/file.rb
index 707ceafd4..70b44c38f 100644
--- a/lib/puppet/face/file.rb
+++ b/lib/puppet/face/file.rb
@@ -5,17 +5,50 @@ Puppet::Indirector::Face.define(:file, '0.0.1') do
   license   "Apache 2 license; see COPYING"
 
   summary "Retrieve and store files in a filebucket"
-  # TK this needs a description of how to find files in a filebucket, and
-  # some good use cases for retrieving/storing them. I can't write either
-  # of these yet.
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  description <<-'EOT'
+    This face interacts with objects stored in a local or remote
+    filebucket. File objects are accessed by their MD5 sum; see the
+    examples for the relevant syntax.
+  EOT
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `file`
     * `rest`
+
+    To retrieve the unmunged contents of a file, you must call find with
+    --render-as s. Rendering as yaml will return a hash of metadata
+    about the file, including its contents.
+
+    This face does not interact with the `clientbucketdir` (the default
+    local filebucket for puppet agent); it interacts with the primary
+    "master"-type filebucket located in the `bucketdir`. If you wish to
+    interact with puppet agent's default filebucket, you'll need to set
+    the <--bucketdir> option appropriately when invoking actions.
   EOT
 
+  file = get_action(:find)
+  file.summary "Retrieve a file from the filebucket."
+  file.arguments "md5/<md5sum>"
+  file.returns <<-'EOT'
+    The file object with the specified checksum.
+
+    RENDERING ISSUES: Rendering as a string returns the contents of the
+    file object; rendering as yaml returns a hash of metadata about said
+    file, including but not limited to its contents. Rendering as json
+    is currently broken, and returns a hash containing only the contents
+    of the file.
+  EOT
+  file.examples <<-'EOT'
+    Retrieve the contents of a file:
+
+    $ puppet file find md5/9aedba7f413c97dc65895b1cd9421f2c --render-as s
+  EOT
+
+  get_action(:search).summary "Invalid for this face."
+  get_action(:destroy).summary "Invalid for this face."
+
   set_indirection_name :file_bucket_file
 end
diff --git a/lib/puppet/face/file/download.rb b/lib/puppet/face/file/download.rb
index f5413d493..8a300fbf1 100644
--- a/lib/puppet/face/file/download.rb
+++ b/lib/puppet/face/file/download.rb
@@ -1,6 +1,24 @@
 # Download a specified file into the local filebucket.
 Puppet::Face.define(:file, '0.0.1') do
   action :download do |*args|
+    summary "Download a file into the local filebucket."
+    arguments "( {md5}<checksum> | puppet:///... )"
+    returns "Nothing"
+    description <<-EOT
+      Downloads a file from the puppet master's filebucket and
+      duplicates it in the local filebucket. This action's checksum
+      syntax differs from `find`'s, and it can accept a <puppet:///> URL.
+    EOT
+    examples <<-'EOT'
+      Download a file by URL:
+
+      $ puppet file download puppet:///modules/editors/vim/.vimrc
+
+      Download a file by MD5 sum:
+
+      $ puppet file download {md5}8f798d4e754db0ac89186bbaeaf0af18
+    EOT
+
     when_invoked do |sum, options|
       if sum =~ /^puppet:\/\// # it's a puppet url
         require 'puppet/file_serving'
diff --git a/lib/puppet/face/file/store.rb b/lib/puppet/face/file/store.rb
index 4c9523b6c..97dbd86b4 100644
--- a/lib/puppet/face/file/store.rb
+++ b/lib/puppet/face/file/store.rb
@@ -1,6 +1,15 @@
 # Store a specified file in our filebucket.
 Puppet::Face.define(:file, '0.0.1') do
   action :store do |*args|
+    summary "Store a file in the local filebucket."
+    arguments "<file>"
+    returns "Nothing."
+    examples <<-EOT
+      Store a file:
+
+      $ puppet file store /root/.bashrc
+    EOT
+
     when_invoked do |path, options|
       file = Puppet::FileBucket::File.new(File.read(path))
 
diff --git a/lib/puppet/face/help.rb b/lib/puppet/face/help.rb
index aef917447..7a382b8c9 100644
--- a/lib/puppet/face/help.rb
+++ b/lib/puppet/face/help.rb
@@ -7,13 +7,20 @@ Puppet::Face.define(:help, '0.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
 
-  summary "Displays help about puppet subcommands"
+  summary "Display Puppet help."
 
   action(:help) do
     summary "Display help about faces and their actions."
+    arguments "[<face>] [<action>]"
+    returns "Short help text for the specified face or action."
+    examples <<-'EOT'
+      Get help for an action:
+
+      $ puppet help
+    EOT
 
     option "--version VERSION" do
-      summary "which version of the interface to show help for"
+      summary "The version of the face for which to show help."
     end
 
     default
diff --git a/lib/puppet/face/help/action.erb b/lib/puppet/face/help/action.erb
index c26958a35..aab7f2564 100644
--- a/lib/puppet/face/help/action.erb
+++ b/lib/puppet/face/help/action.erb
@@ -1,48 +1,22 @@
-puppet <%= face.name %><%= action.default? ? '' : " #{action.name}" %>(1) -- <%= action.summary || face.summary %>
-<%= '=' * (_erbout.length - 1) %>
+puppet <%= face.name %><%= action.default? ? '' : " #{action.name}" %> - <%= action.summary || face.summary %>
 
 % if action.synopsis
-SYNOPSIS
---------
-
-<%= action.synopsis %>
+usage: <%= action.synopsis %>
 
 % end
-% if action.description
-DESCRIPTION
------------
-<%= action.description %>
+% if action.description or action.short_description
+<%= action.short_description %>
 
 %end
 % unless action.options.empty?
-OPTIONS
--------
+OPTIONS:
 %   action.options.sort.each do |name|
 %     option = action.get_option name
-<%= "  " + option.optparse.join(" |" ) %>
-<%= option.summary %>
-<%= option.description %>
+<%= "  " + option.optparse.join(" |" ) %> - <%= option.summary %>
 
 %   end
 % end
 % if action.examples
-EXAMPLES
---------
+EXAMPLES:
 <%= action.examples %>
 % end
-% if action.notes
-NOTES
------
-<%= action.notes %>
-
-% end
-% unless action.authors.empty?
-AUTHOR
-------
-<%= action.authors.map {|x| " * " + x } .join("\n") %>
-
-%end
-COPYRIGHT AND LICENSE
----------------------
-<%= action.copyright %>
-<%= action.license   %>
diff --git a/lib/puppet/face/key.rb b/lib/puppet/face/key.rb
index 67d775ca4..f19ae198f 100644
--- a/lib/puppet/face/key.rb
+++ b/lib/puppet/face/key.rb
@@ -4,16 +4,16 @@ Puppet::Indirector::Face.define(:key, '0.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
 
-  summary "Create, save, and remove certificate keys"
-
-  description <<-EOT
-    Keys are created for you automatically when certificate requests are
-    generated with 'puppet certificate generate'. You should not have to use
-    this action directly from the command line.
+  summary "Create, save, and remove certificate keys."
+  description <<-'EOT'
+    Manages certificate private keys. Keys are created for you
+    automatically when certificate requests are generated with 'puppet
+    certificate generate', and it should not be necessary to use this action
+    directly.
   EOT
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `ca`
diff --git a/lib/puppet/face/node.rb b/lib/puppet/face/node.rb
index be38ad388..c68d71846 100644
--- a/lib/puppet/face/node.rb
+++ b/lib/puppet/face/node.rb
@@ -3,16 +3,16 @@ Puppet::Indirector::Face.define(:node, '0.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
 
-  summary "View and manage node definitions"
-
-  description <<-EOT
-    This face interacts with node objects, which are what Puppet uses to
+  summary "View and manage node definitions."
+  description <<-'EOT'
+    This face interacts with node objects, which are used by Puppet to
     build a catalog. A node object consists of the node's facts,
-    environment, additional top-scope variables, and classes.
+    environment, node parameters (exposed in the parser as top-scope
+    variables), and classes.
   EOT
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `active_record`
@@ -23,4 +23,28 @@ Puppet::Indirector::Face.define(:node, '0.0.1') do
     * `rest`
     * `yaml`
   EOT
+
+  get_action(:destroy).summary "Invalid for this face."
+  get_action(:search).summary "Invalid for this face."
+  get_action(:save).summary "Invalid for this face."
+
+  find = get_action(:find)
+  find.summary "Retrieve a node object."
+  find.arguments "<host>"
+  find.returns <<-'EOT'
+    A Puppet::Node object.
+
+    RENDERING ISSUES: Rendering as string and json are currently broken;
+    node objects can only be rendered as yaml.
+  EOT
+  find.examples <<-'EOT'
+    Retrieve an "empty" (no classes, fact and bulit-in parameters only,
+    and an environment of "production") node:
+
+    $ puppet node find somenode.puppetlabs.lan --terminus plain --render-as yaml
+
+    Retrieve a node using the puppet master's configured ENC:
+
+    $ puppet node find somenode.puppetlabs.lan --terminus exec --mode master --render-as yaml
+  EOT
 end
diff --git a/lib/puppet/face/parser.rb b/lib/puppet/face/parser.rb
index e6a9503dd..8dd3efb75 100644
--- a/lib/puppet/face/parser.rb
+++ b/lib/puppet/face/parser.rb
@@ -5,11 +5,13 @@ Puppet::Face.define(:parser, '0.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
 
-  summary "Interact directly with the parser"
+  summary "Interact directly with the parser."
 
   action :validate do
-    summary "Validate the syntax of one or more Puppet manifests"
-    description <<-EOT
+    summary "Validate the syntax of one or more Puppet manifests."
+    arguments "[<manifest>] [<manifest> ...]"
+    returns "Nothing, or the first syntax error encountered."
+    description <<-'EOT'
       This action validates Puppet DSL syntax without compiling a catalog or
       syncing any resources. If no manifest files are provided, it will
       validate the default site manifest.
diff --git a/lib/puppet/face/plugin.rb b/lib/puppet/face/plugin.rb
index 969d42389..541468d39 100644
--- a/lib/puppet/face/plugin.rb
+++ b/lib/puppet/face/plugin.rb
@@ -3,29 +3,36 @@ Puppet::Face.define(:plugin, '0.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
 
-  summary "Interact with the Puppet plugin system"
-  description <<-EOT
+  summary "Interact with the Puppet plugin system."
+  description <<-'EOT'
     This face provides network access to the puppet master's store of
-    plugins. It is intended for use in other faces, rather than for direct
-    command line access.
+    plugins.
   EOT
-  notes <<-EOT
+  notes <<-'EOT'
     The puppet master can serve Ruby code collected from the lib directories
     of its modules. These plugins can be used on agent nodes to extend
     Facter and implement custom types and providers.
   EOT
 
   action :download do
-    summary "Download plugins from the configured master"
-    returns <<-EOT
-      An array containing the files actually downloaded. If all files
-      were in sync, this array will be empty.
+    summary "Download plugins from the puppet master."
+    description <<-'EOT'
+      Downloads plugins from the configured puppet master. Any plugins
+      downloaded in this way will be used in all subsequent Puppet activity.
     EOT
-    notes "This action modifies files on disk without returning any data."
-    examples <<-EOT
+    returns <<-'EOT'
+      A display-formatted list of the files downloaded. If all plugin
+      files were in sync, this list will be empty.
+    EOT
+    notes "This action modifies files on disk."
+    examples <<-'EOT'
       Retrieve plugins from the puppet master:
 
-          Puppet::Face[:plugin, '0.0.1'].download
+      $ puppet plugin download
+
+      Retrieve plugins from the puppet master (API example):
+
+      $ Puppet::Face[:plugin, '0.0.1'].download
     EOT
 
     when_invoked do |options|
diff --git a/lib/puppet/face/report.rb b/lib/puppet/face/report.rb
index c8549b14f..a5718bc0c 100644
--- a/lib/puppet/face/report.rb
+++ b/lib/puppet/face/report.rb
@@ -5,9 +5,9 @@ Puppet::Indirector::Face.define(:report, '0.0.1') do
   license   "Apache 2 license; see COPYING"
 
   summary "Create, display, and submit reports"
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `processor`
@@ -15,23 +15,41 @@ Puppet::Indirector::Face.define(:report, '0.0.1') do
     * `yaml`
   EOT
 
+  get_action(:find).summary "Invalid for this face."
+  get_action(:search).summary "Invalid for this face."
+  get_action(:destroy).summary "Invalid for this face."
+  save = get_action(:save)
+  save.summary "Submit a report."
+  save.arguments "<report>"
+  save.returns "Nothing."
+  save.examples <<-'EOT'
+    From the implementation of `puppet report submit` (API example):
+
+        begin
+          Puppet::Transaction::Report.indirection.terminus_class = :rest
+          Puppet::Face[:report, "0.0.1"].save(report)
+          Puppet.notice "Uploaded report for #{report.name}"
+        rescue => detail
+          puts detail.backtrace if Puppet[:trace]
+          Puppet.err "Could not send report: #{detail}"
+        end
+  EOT
+
   action(:submit) do
     summary "Submit a report object to the puppet master"
-    description <<-EOT
+    description <<-'EOT'
       This action is essentially a shortcut and wrapper for the `save` action
-      with a terminus of `rest`. It also can provide additional details in the
+      with the `rest` terminus, which provides additional details in the
       event of a report submission failure. It is not intended for use from
       a command line.
     EOT
-    examples <<-EOT
+    examples <<-'EOT'
       From secret_agent.rb:
-          Puppet::Face[:plugin, '0.0.1'].download
-
-          facts   = Puppet::Face[:facts, '0.0.1'].find(certname)
-          catalog = Puppet::Face[:catalog, '0.0.1'].download(certname, facts)
-          report  = Puppet::Face[:catalog, '0.0.1'].apply(catalog)
 
+          # ...
+          report  = Puppet::Face[:catalog, '0.0.1'].apply
           Puppet::Face[:report, '0.0.1'].submit(report)
+          return report
     EOT
     when_invoked do |report, options|
       begin
diff --git a/lib/puppet/face/resource.rb b/lib/puppet/face/resource.rb
index 87e587624..593776e53 100644
--- a/lib/puppet/face/resource.rb
+++ b/lib/puppet/face/resource.rb
@@ -5,19 +5,55 @@ Puppet::Indirector::Face.define(:resource, '0.0.1') do
   license   "Apache 2 license; see COPYING"
 
   summary "Interact directly with resources via the RAL, like ralsh"
-  description <<-EOT
+  description <<-'EOT'
     This face provides a Ruby API with functionality similar to the puppet
     resource (originally ralsh) command line application. It is not intended to be
     used from the command line.
   EOT
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `ral`
     * `rest`
   EOT
 
-  examples "TK we really need some examples for this one."
+  get_action(:destroy).summary "Invalid for this face."
+
+  search = get_action(:search)
+  search.summary "Get all resources of a single type."
+  search.arguments "<resource_type>"
+  search.returns "An array of resource objects."
+  search.examples <<-'EOT'
+    Get a list of all user resources (API example):
+
+        all_users = Puppet::Face[:resource, '0.0.1'].search("user")
+  EOT
+
+  find = get_action(:find)
+  find.summary "Get a single resource."
+  find.arguments "<type>/<title>"
+  find.returns "A resource object."
+  find.examples <<-'EOT'
+    Print information about a user on this system (API example):
+
+        puts Puppet::Face[:resource, '0.0.1'].find("user/luke").to_pson
+  EOT
+
+  save = get_action(:save)
+  save.summary "Create a new resource."
+  save.arguments "<resource_object>"
+  save.returns "The same resource object passed."
+  save.examples <<-'EOT'
+    Create a new file resource (API example):
+
+        my_resource = Puppet::Resource.new(
+          :file,
+          "/tmp/demonstration",
+          :parameters => {:ensure => :present, :content => "some\nthing\n"}
+        )
+
+        Puppet::Face[:resource, '0.0.1'].save(my_resource)
+  EOT
 end
diff --git a/lib/puppet/face/resource_type.rb b/lib/puppet/face/resource_type.rb
index 77ccefa8f..3973eb3b8 100644
--- a/lib/puppet/face/resource_type.rb
+++ b/lib/puppet/face/resource_type.rb
@@ -4,14 +4,75 @@ Puppet::Indirector::Face.define(:resource_type, '0.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
 
-  summary "View resource types, classes, and nodes from all manifests"
-  description "TK I have no idea what this does."
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  summary "View classes, defined resource types, and nodes from all manifests"
+  description <<-'EOT'
+    This face reads information about the resource collections (classes,
+    nodes, and defined types) available in Puppet's site manifest and
+    modules.
+
+    It will eventually be extended to examine native resource types.
+  EOT
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `parser`
     * `rest`
   EOT
+
+  # Action documentation overrides:
+  get_action(:save).summary = "Invalid for this face."
+  get_action(:destroy).summary = "Invalid for this face."
+
+  find = get_action(:find)
+  find.summary "Retrieve info about a resource collection."
+  find.arguments "<collection_name>"
+  find.returns <<-'EOT'
+    A hash of info about one resource collection. This hash will include the
+    following four keys:
+
+    * `file` (a string)
+    * `name` (a string)
+    * `type` (<hostclass>, <definition>, or <node>)
+    * `line` (an integer)
+
+    It may also include the following keys:
+
+    * `parent`    (<name_of_resource_collection>)
+    * `arguments` (a hash of parameters and default values)
+    * `doc`       (a string)
+
+    RENDERING ISSUES: yaml and string output for this indirection are
+    currently unusable; use json instead.
+  EOT
+  find.examples <<-'EOT'
+    Retrieve info about a specific locally-defined class:
+
+    $ puppet resource_type find ntp::disabled
+
+    Retrieve info from the puppet master about a specific class:
+
+    $ puppet resource_type find ntp --terminus rest
+  EOT
+
+  search = get_action(:search)
+  search.summary "Search for collections matching a regular expression."
+  search.arguments "<regular_expression>"
+  search.returns <<-'EOT'
+    An array of resource collection info hashes. (See "RETURNS" under `find`.)
+
+    RENDERING ISSUES: yaml and string output for this indirection are
+    currently unusable; use json instead.
+  EOT
+  search.examples <<-'EOT'
+    Retrieve all classes, nodes, and defined types:
+
+    $ puppet resource_type search '.*'
+
+    Search for classes related to Nagios:
+
+    $ puppet resource_type search nagios
+  EOT
+
 end
diff --git a/lib/puppet/face/secret_agent.rb b/lib/puppet/face/secret_agent.rb
index c8c8e6629..105850cd3 100644
--- a/lib/puppet/face/secret_agent.rb
+++ b/lib/puppet/face/secret_agent.rb
@@ -4,22 +4,36 @@ Puppet::Face.define(:secret_agent, '0.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
 
-  summary "Provides agent-like behavior, with no plugin downloading or reporting"
-  description <<-EOT
-    This face currently functions as a proof of concept, demonstrating how
-    Faces allows the separation of application logic from Puppet's internal
-    systems; compare the code for puppet agent. It will eventually replace
-    puppet agent entirely, and can provide a template for users who wish to
-    implement agent-like functionality with drastically different
-    application logic.
+  summary "Mimics puppet agent."
+  description <<-'EOT'
+    This face currently functions as a proof of concept, demonstrating
+    how Faces allows the separation of application logic from Puppet's
+    internal systems; compare the actual code for puppet agent. It will
+    eventually replace puppet agent entirely, and can provide a template
+    for users who wish to implement agent-like functionality with
+    non-standard application logic.
   EOT
 
   action(:synchronize) do
-    summary "Retrieve and apply a catalog from the puppet master"
-    description <<-EOT
-      This action mimics the behavior of the puppet agent application. It does
-      not currently daemonize, but can download plugins, submit facts,
-      retrieve and apply a catalog, and submit a report to the puppet master.
+    summary "Run secret_agent once."
+    arguments "[-v | --verbose] [-d | --debug]" # Remove this once options are introspectible
+    description <<-'EOT'
+      This action mimics a single run of the puppet agent application.
+      It does not currently daemonize, but can download plugins, submit
+      facts, retrieve and apply a catalog, and submit a report to the
+      puppet master.
+    EOT
+    returns "A Puppet::Transaction::Report object."
+    examples <<-'EOT'
+      Trigger a Puppet run with the configured puppet master:
+
+      $ puppet secret_agent
+    EOT
+    notes <<-'EOT'
+      This action requires that the puppet master's `auth.conf` file
+      allow save access to the `facts` REST terminus. Puppet agent does
+      not use this facility, and it is turned off by default. See
+      <http://docs.puppetlabs.com/guides/rest_auth_conf.html> for more details.
     EOT
 
     when_invoked do |options|
diff --git a/lib/puppet/face/status.rb b/lib/puppet/face/status.rb
index 6a29debdd..08379550c 100644
--- a/lib/puppet/face/status.rb
+++ b/lib/puppet/face/status.rb
@@ -4,27 +4,44 @@ Puppet::Indirector::Face.define(:status, '0.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
 
-  summary "View puppet server status"
-  description <<-EOT
-    This subcommand is only useful for determining whether a puppet master
-    server (or an agent node, if puppet was started with the `--listen`
-    option) is responding to requests.
-
-    Only the `find` action is valid. If the server is responding to
-    requests, `find` will retrieve a status object; if not, the connection
-    will be refused. When invoked with the `local` terminus, `find` will
-    always return true.
-
-    If you wish to query a server other than the master configured in
-    puppet.conf, you must set the `--server` and `--masterport` options on
-    the command line.
-  EOT
-  notes <<-EOT
-    This is an indirector face, which exposes find, search, save, and
-    destroy actions for an indirected subsystem of Puppet. Valid terminuses
+  summary "View puppet server status."
+  notes <<-'EOT'
+    This is an indirector face, which exposes `find`, `search`, `save`, and
+    `destroy` actions for an indirected subsystem of Puppet. Valid termini
     for this face include:
 
     * `local`
     * `rest`
   EOT
+
+  get_action(:destroy).summary "Invalid for this face."
+  get_action(:save).summary "Invalid for this face."
+  get_action(:search).summary "Invalid for this face."
+
+  find = get_action(:find)
+  find.summary "Check status of puppet master server."
+  find.arguments "<dummy_key>"
+  find.returns "A Puppet::Status object, or a low-level connection error."
+  find.description <<-'EOT'
+    Checks whether a Puppet server is properly receiving and processing
+    REST requests. This action is only useful when used with '--terminus
+    rest'; when invoked with the `local` terminus, `find` will always
+    return true.
+
+    This action will query the configured puppet master. To query other
+    servers, including puppet agent nodes started with the --listen
+    option, you can set set the global --server and --masterport options
+    on the command line; note that agent nodes listen on port 8139.
+  EOT
+  find.notes <<-'EOT'
+    This action requires that the server's `auth.conf` file allow find
+    access to the `status` REST terminus. Puppet agent does not use this
+    facility, and it is turned off by default. See
+    <http://docs.puppetlabs.com/guides/rest_auth_conf.html> for more details.
+  EOT
+  find.examples <<-'EOT'
+    Check the status of the configured puppet master:
+
+    $ puppet status find x --terminus rest
+  EOT
 end
diff --git a/lib/puppet/indirector/face.rb b/lib/puppet/indirector/face.rb
index 1371c647e..756306a2f 100644
--- a/lib/puppet/indirector/face.rb
+++ b/lib/puppet/indirector/face.rb
@@ -2,22 +2,22 @@ require 'puppet/face'
 
 class Puppet::Indirector::Face < Puppet::Face
   option "--terminus TERMINUS" do
-    summary "The indirector terminus to use for this action"
+    summary "The indirector terminus to use for this action."
     description <<-EOT
-Indirector faces expose indirected subsystems of Puppet. These
-subsystems are each able to retrieve and alter a specific type of data
-(with the familiar actions of `find`, `search`, `save`, and `destroy`)
-from an arbitrary number of pluggable backends. In Puppet parlance,
-these backends are called terminuses.
-
-Almost all indirected subsystems have a `rest` terminus that interacts
-with the puppet master's data. Most of them have additional terminuses
-for various local data models, which are in turn used by the indirected
-subsystem on the puppet master whenever it receives a remote request.
-
-The terminus for an action is often determined by context, but
-occasionally needs to be set explicitly. See the "Notes" section of this
-face's manpage for more details.
+      Indirector faces expose indirected subsystems of Puppet. These
+      subsystems are each able to retrieve and alter a specific type of data
+      (with the familiar actions of `find`, `search`, `save`, and `destroy`)
+      from an arbitrary number of pluggable backends. In Puppet parlance,
+      these backends are called terminuses.
+
+      Almost all indirected subsystems have a `rest` terminus that interacts
+      with the puppet master's data. Most of them have additional terminuses
+      for various local data models, which are in turn used by the indirected
+      subsystem on the puppet master whenever it receives a remote request.
+
+      The terminus for an action is often determined by context, but
+      occasionally needs to be set explicitly. See the "Notes" section of this
+      face's manpage for more details.
     EOT
 
     before_action do |action, args, options|
@@ -49,36 +49,39 @@ face's manpage for more details.
   end
 
   action :destroy do
-    summary "Delete an object"
+    summary "Delete an object."
+    arguments "<key>"
     when_invoked { |key, options| call_indirection_method(:destroy, key, options) }
   end
 
   action :find do
-    summary "Retrieve an object by name"
+    summary "Retrieve an object by name."
+    arguments "<key>"
     when_invoked { |key, options| call_indirection_method(:find, key, options) }
   end
 
   action :save do
-    summary "Create or modify an object"
-    notes <<-EOT
-      Save actions cannot currently be invoked from the command line, and are
-      for API use only.
+    summary "Create or overwrite an object."
+    arguments "<object>"
+    description <<-EOT
+      Create or overwrite an object. Save actions cannot currently be
+      invoked from the command line, and are for API use only.
     EOT
     when_invoked { |key, options| call_indirection_method(:save, key, options) }
   end
 
   action :search do
-    summary "Search for an object"
+    summary "Search for an object or retrieve multiple objects."
+    arguments "<query>"
     when_invoked { |key, options| call_indirection_method(:search, key, options) }
   end
 
   # Print the configuration for the current terminus class
   action :info do
-    summary "Print the default terminus class for this face"
+    summary "Print the default terminus class for this face."
     description <<-EOT
-      TK So this is per-face, right? No way to tell what the default terminus
-      is per-action, for subsystems that switch to REST for save but query
-      locally for find?
+      Prints the default terminus class for this face. Note that
+      different run modes may have different default terminuses.
     EOT
 
     when_invoked do |*args|
-- 
cgit 


From 4662f4d0aa57a8792b824338e82e89ca4ddb0960 Mon Sep 17 00:00:00 2001
From: nfagerlund <nick.fagerlund@gmail.com>
Date: Fri, 20 May 2011 18:38:13 -0700
Subject: (#7564) Finish templates

This is a rebase of the following commits:

* (#7563) Add a template for manpages

In order to generate manpages in a reasonably maintainable way, we need
to format text from the Faces help API for use with Ronn
(https://github.com/rtomayko/ronn/).

This commit adds a man template to the help templates folder; it has
been verified to generate Ronn-friendly input text. It isn't currently
called by any applications, but can be demonstrated by exchanging it for
face.erb.

* (#7634) Change ERB trim mode used in the Faces help API

<%= something -%> tags (note the minus) are unavoidably necessary at at
least one point in the Faces help templates. ERB objects instantiated
with the % trim mode will blow up horribly whenever one of these tags
appears.

This commit changes the trim mode to `-` and refactors all help
templates accordingly.

* (#7563) Refactor short help templates

This commit attempts to bring the short face and action help templates closer to the goals of fitting cleanly on one screen, fitting the prevailing *nix aesthetic, and being useful without overwhelming the user.
---
 lib/puppet/face/help.rb         |   2 +-
 lib/puppet/face/help/action.erb |  65 +++++++++++++++-----
 lib/puppet/face/help/face.erb   | 115 +++++++++++++++++++++++-----------
 lib/puppet/face/help/global.erb |  16 ++---
 lib/puppet/face/help/man.erb    | 132 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 270 insertions(+), 60 deletions(-)
 create mode 100644 lib/puppet/face/help/man.erb

diff --git a/lib/puppet/face/help.rb b/lib/puppet/face/help.rb
index 7a382b8c9..9376adf04 100644
--- a/lib/puppet/face/help.rb
+++ b/lib/puppet/face/help.rb
@@ -95,7 +95,7 @@ Puppet::Face.define(:help, '0.0.1') do
 
   def erb(name)
     template = (Pathname(__FILE__).dirname + "help" + name)
-    erb = ERB.new(template.read, nil, '%')
+    erb = ERB.new(template.read, nil, '-')
     erb.filename = template.to_s
     return erb
   end
diff --git a/lib/puppet/face/help/action.erb b/lib/puppet/face/help/action.erb
index aab7f2564..c788f34fd 100644
--- a/lib/puppet/face/help/action.erb
+++ b/lib/puppet/face/help/action.erb
@@ -1,22 +1,57 @@
-puppet <%= face.name %><%= action.default? ? '' : " #{action.name}" %> - <%= action.summary || face.summary %>
+<%= action.summary || face.summary || "unknown face..." %>
 
-% if action.synopsis
-usage: <%= action.synopsis %>
+<% if action.synopsis -%>
+USAGE: <%= action.synopsis %>
 
-% end
-% if action.description or action.short_description
-<%= action.short_description %>
+<% end -%>
+<% if action.short_description -%>
+<%= action.short_description.strip %>
 
-%end
-% unless action.options.empty?
+<% end -%>
+<% if action.returns -%>
+RETURNS: <%= action.returns.strip %>
+
+<% end -%>
 OPTIONS:
-%   action.options.sort.each do |name|
-%     option = action.get_option name
-<%= "  " + option.optparse.join(" |" ) %> - <%= option.summary %>
+<%# Remove these options once we can introspect them normally. -%>
+  --mode MODE                    - The run mode to use (`user`, `agent`, or
+                                   `master`).
+  --render-as FORMAT             - The rendering format to use.
+  --verbose                      - Whether to log verbosely.
+  --debug                        - Whether to log debug information.
+<% unless action.options.empty?
+     optionroom = 30
+     summaryroom = 80 - 5 - optionroom
+      action.options.sort.each do |name|
+        option = action.get_option name -%>
+<%= "  " + option.optparse.join(" | ")[0,(optionroom - 1)].ljust(optionroom) + ' - ' -%>
+<%     if !(option.summary) -%>
+unknown option...
+<%     elsif option.summary.length <= summaryroom -%>
+<%= option.summary %>
+<%
+        else
+          words = option.summary.split
+          wrapped = ['']
+          i = 0
+          words.each do |word|
+            if wrapped[i].length + word.length <= summaryroom
+              wrapped[i] << word + ' '
+            else
+              i += 1
+              wrapped[i] = word + ' '
+            end
+          end
+-%>
+<%= wrapped.shift.strip %>
+<%       wrapped.each do |line| -%>
+<%= (' ' * (optionroom + 5) ) + line.strip %>
+<%       end
+        end
+      end -%>
+<% end -%>
 
-%   end
-% end
-% if action.examples
+<% if action.examples -%>
 EXAMPLES:
 <%= action.examples %>
-% end
+<% end -%>
diff --git a/lib/puppet/face/help/face.erb b/lib/puppet/face/help/face.erb
index b249981de..2a0f0181c 100644
--- a/lib/puppet/face/help/face.erb
+++ b/lib/puppet/face/help/face.erb
@@ -1,48 +1,91 @@
-NAME
-  <%= face.name %> -- <%= face.summary || "unknown face..." %>
+<%= face.summary || "unknown face..." %>
 
-% if face.synopsis
-SYNOPSIS
-<%= face.synopsis.gsub(/^/, '  ') %>
+<% if face.synopsis -%>
+USAGE: <%= face.synopsis %>
 
-% end
-% if face.description
-DESCRIPTION
-<%= face.description.chomp.gsub(/^/, '  ') %>
+<% end -%>
+<% if face.short_description -%>
+<%= face.short_description %>
 
-%end
-% unless face.options.empty?
-OPTIONS
-%   face.options.sort.each do |name|
-%     option = face.get_option name
-<%= "  " + option.optparse.join(" |" ) %>
+<% end -%>
+OPTIONS:
+<%# Remove these options once we can introspect them normally. -%>
+  --mode MODE                    - The run mode to use (`user`, `agent`, or
+                                   `master`).
+  --render-as FORMAT             - The rendering format to use.
+  --verbose                      - Whether to log verbosely.
+  --debug                        - Whether to log debug information.
+<% unless face.options.empty?
+     optionroom = 30
+     summaryroom = 80 - 5 - optionroom
+      face.options.sort.each do |name|
+        option = face.get_option name -%>
+<%= "  " + option.optparse.join(" | ")[0,(optionroom - 1)].ljust(optionroom) + ' - ' -%>
+<%     if !(option.summary) -%>
+unknown option...
+<%     elsif option.summary.length <= summaryroom -%>
 <%= option.summary %>
-<%= option.description %>
+<%
+       else
+         words = option.summary.split
+         wrapped = ['']
+         i = 0
+         words.each do |word|
+           if wrapped[i].length + word.length <= summaryroom
+             wrapped[i] << word + ' '
+           else
+             i += 1
+             wrapped[i] = word + ' '
+           end
+         end
+-%>
+<%= wrapped.shift.strip %>
+<%       wrapped.each do |line| -%>
+<%= (' ' * (optionroom + 5) ) + line.strip %>
+<%       end
+        end
+      end -%>
+<% end -%>
 
-%   end
-% end
-ACTIONS
-% padding = face.actions.map{|x| x.to_s.length}.max + 2
-% face.actions.each do |actionname|
-%   action = face.get_action(actionname)
-  <%= action.name.to_s.ljust(padding) %>  <%= action.summary %>
-% end
+ACTIONS:
+<% padding = face.actions.map{|x| x.to_s.length}.max + 2
+   summaryroom = 80 - (padding + 4)
+   face.actions.each do |actionname|
+     action = face.get_action(actionname) -%>
+  <%= action.name.to_s.ljust(padding) + '  ' -%>
+<% if !(action.summary) -%>
+unknown action...
+<% elsif action.summary.length <= summaryroom -%>
+<%= action.summary %>
+<% else
+          words = action.summary.split
+          wrapped = ['']
+          i = 0
+          words.each do |word|
+            if wrapped[i].length + word.length <= summaryroom
+              wrapped[i] << word + ' '
+            else
+              i += 1
+              wrapped[i] = word + ' '
+            end
+          end
+-%>
+<%= wrapped.shift.strip %>
+<%       wrapped.each do |line| -%>
+<%= (' ' * (padding + 4) ) + line.strip %>
+<%       end
+        end
+end -%>
 
-% if face.examples
-EXAMPLES
-<%= face.examples %>
-% end
-% if face.notes
-NOTES
-<%= face.notes %>
+<% if face.respond_to? :indirection -%>
+TERMINI: <%= face.class.terminus_classes(face.indirection.name).join(", ") %>
 
-% end
-% unless face.authors.empty?
+<% end
+   unless face.authors.empty? -%>
 AUTHOR
 <%= face.authors.join("\n").gsub(/^/, ' * ') %>
 
-%end
-COPYRIGHT AND LICENSE
+<% end -%>
+COPYRIGHT AND LICENSE:
 <%= face.copyright.gsub(/^/, '  ') %>
 <%= face.license.gsub(/^/, '  ')   %>
-
diff --git a/lib/puppet/face/help/global.erb b/lib/puppet/face/help/global.erb
index 80c77ad26..c5a9ec9e0 100644
--- a/lib/puppet/face/help/global.erb
+++ b/lib/puppet/face/help/global.erb
@@ -1,18 +1,18 @@
 Usage: puppet <subcommand> [options] <action> [options]
 
 Available subcommands, from Puppet Faces:
-% Puppet::Face.faces.sort.each do |name|
-%   face = Puppet::Face[name, :current]
+<% Puppet::Face.faces.sort.each do |name|
+     face = Puppet::Face[name, :current] -%>
   <%= face.name.to_s.ljust(16) %>  <%= face.summary %>
-% end
+<% end -%>
 
-% unless legacy_applications.empty? then # great victory when this is true!
+<% unless legacy_applications.empty? then # great victory when this is true! -%>
 Available applications, soon to be ported to Faces:
-%   legacy_applications.each do |appname|
-%     summary = horribly_extract_summary_from appname
+<%   legacy_applications.each do |appname|
+       summary = horribly_extract_summary_from appname -%>
   <%= appname.to_s.ljust(16) %>  <%= summary %>
-%   end
-% end
+<%   end
+   end -%>
 
 See 'puppet help <subcommand> <action>' for help on a specific subcommand action.
 See 'puppet help <subcommand>' for help on a specific subcommand.
diff --git a/lib/puppet/face/help/man.erb b/lib/puppet/face/help/man.erb
new file mode 100644
index 000000000..6f21fe413
--- /dev/null
+++ b/lib/puppet/face/help/man.erb
@@ -0,0 +1,132 @@
+puppet-<%= face.name %>(8) -- <%= face.summary || "Unknown face." %>
+<%= '=' * (_erbout.length - 1) %>
+
+<% if face.synopsis -%>
+SYNOPSIS
+--------
+<%= face.synopsis %>
+
+<% end
+   if face.description -%>
+DESCRIPTION
+-----------
+<%= face.description.strip %>
+
+<% end -%>
+OPTIONS
+-------
+Note that any configuration parameter that's valid in the configuration
+file is also a valid long argument, although it may or may not be
+relevant to the present action. For example, `server` is a valid
+configuration parameter, so you can specify `--server <servername>` as
+an 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 with
+`--genconfig`.
+
+* --mode MODE:
+  The run mode to use for the current action. Valid modes are `user`, `agent`,
+  and `master`.
+* --render-as FORMAT:
+  The format in which to render output. The most common formats are `json`,
+  `s` (string), and `yaml`, but other options such as `dot` are
+  sometimes available.
+* --verbose:
+  Whether to log verbosely.
+* --debug:
+  Whether to log debug information.
+<% unless face.options.empty?
+     face.options.sort.each do |name|
+       option = face.get_option name -%>
+<%= "* " + option.optparse.join(" | " ) %>:
+<%= option.description.gsub(/^/, '  ') || '  ' + option.summary %>
+<%   end
+   end -%>
+
+ACTIONS
+-------
+<% face.actions.each do |actionname|
+     action = face.get_action(actionname) -%>
+* `<%= action.name.to_s %>` - <%= action.summary %>:
+<%   if action.synopsis -%>
+  `SYNOPSIS`
+
+  <%= action.synopsis %>
+
+<%   end -%>
+<%   if action.description -%>
+  `DESCRIPTION`
+
+<%= action.description.gsub(/^/, '  ') %>
+<%   end
+     unique_options = action.options - face.options
+     unless unique_options.empty? -%>
+  `OPTIONS`
+
+<%     unique_options.sort.each do |name|
+         option = action.get_option name
+         text = option.description || option.summary -%>
+  <%= '<' + option.optparse.join("> | <") + '>' %> -
+<%= text.gsub(/^/, '  ') %>
+<%     end -%>
+
+<%   end -%>
+<%   if action.returns -%>
+  `RETURNS`
+
+<%= action.returns.gsub(/^/, '  ') %>
+
+<%   end
+     if action.notes -%>
+  `NOTES`
+
+<%= action.notes.gsub(/^/, '  ') %>
+
+<%   end
+   end
+   if face.examples or face.actions.any? {|actionname| face.get_action(actionname).examples} -%>
+EXAMPLES
+--------
+<% end
+   if face.examples -%>
+<%= face.examples %>
+
+<% end
+   face.actions.each do |actionname|
+     action = face.get_action(actionname)
+     if action.examples -%>
+`<%= action.name.to_s %>`
+
+<%= action.examples.strip %>
+<%   end
+   end -%>
+
+<% if face.notes or face.respond_to? :indirection -%>
+NOTES
+-----
+<% if face.notes -%>
+<%= face.notes.strip %>
+
+<% end # notes
+if face.respond_to? :indirection -%>
+This is an indirector face, which exposes `find`, `search`, `save`, and
+`destroy` actions for an indirected subsystem of Puppet. Valid termini
+for this face include:
+
+* `<%= face.class.terminus_classes(face.indirection.name).join("`\n* `") %>`
+
+<% end # indirection
+   end # notes or indirection
+   unless face.authors.empty? -%>
+AUTHOR
+------
+<%= face.authors.join("\n").gsub(/^/, ' * ') %>
+
+<% end -%>
+COPYRIGHT AND LICENSE
+---------------------
+<%= face.copyright %>
+<%= face.license %>
-- 
cgit 


From 76887064b7691eb6f6751739df5bc83beee76a80 Mon Sep 17 00:00:00 2001
From: nfagerlund <nick.fagerlund@gmail.com>
Date: Tue, 24 May 2011 10:22:00 -0700
Subject: (#7563) DRY: Remove indirector boilerplate from individual faces

Termini lists are now being generated in the help templates. This commit
removes the hardcoded lists from each of the affected faces.
---
 lib/puppet/face/catalog.rb                     | 11 -----------
 lib/puppet/face/certificate.rb                 |  9 ---------
 lib/puppet/face/certificate_request.rb         |  9 ---------
 lib/puppet/face/certificate_revocation_list.rb |  9 ---------
 lib/puppet/face/facts.rb                       | 14 --------------
 lib/puppet/face/file.rb                        |  7 -------
 lib/puppet/face/key.rb                         |  8 --------
 lib/puppet/face/node.rb                        | 13 -------------
 lib/puppet/face/report.rb                      |  9 ---------
 lib/puppet/face/resource.rb                    |  8 --------
 lib/puppet/face/resource_type.rb               |  8 --------
 lib/puppet/face/status.rb                      |  8 --------
 12 files changed, 113 deletions(-)

diff --git a/lib/puppet/face/catalog.rb b/lib/puppet/face/catalog.rb
index c7501ff37..13351540a 100644
--- a/lib/puppet/face/catalog.rb
+++ b/lib/puppet/face/catalog.rb
@@ -13,17 +13,6 @@ Puppet::Indirector::Face.define(:catalog, '0.0.1') do
     in 'dot' format (for easy graph viewing with OmniGraffle or Graphviz)
     with '--render-as dot'.
   EOT
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `active_record`
-    * `compiler`
-    * `queue`
-    * `rest`
-    * `yaml`
-  EOT
 
   get_action(:destroy).summary "Invalid for this face."
   get_action(:search).summary "Query format unknown; potentially invalid for this face."
diff --git a/lib/puppet/face/certificate.rb b/lib/puppet/face/certificate.rb
index d77525ba8..cab8817e3 100644
--- a/lib/puppet/face/certificate.rb
+++ b/lib/puppet/face/certificate.rb
@@ -13,15 +13,6 @@ Puppet::Indirector::Face.define(:certificate, '0.0.1') do
     and its "generate" action submits a CSR rather than creating a
     signed certificate.
   EOT
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `ca`
-    * `file`
-    * `rest`
-  EOT
 
   option "--ca-location LOCATION" do
     summary "The certificate authority to query"
diff --git a/lib/puppet/face/certificate_request.rb b/lib/puppet/face/certificate_request.rb
index de52b7fb7..29cf7dc78 100644
--- a/lib/puppet/face/certificate_request.rb
+++ b/lib/puppet/face/certificate_request.rb
@@ -11,15 +11,6 @@ Puppet::Indirector::Face.define(:certificate_request, '0.0.1') do
     `find` with a node certificate name to retrieve a specific request, and
     invoke `save` to submit a CSR.
   EOT
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `ca`
-    * `file`
-    * `rest`
-  EOT
 
   # Per-action doc overrides
   get_action(:destroy).summary "Invalid for this face."
diff --git a/lib/puppet/face/certificate_revocation_list.rb b/lib/puppet/face/certificate_revocation_list.rb
index db00ea3ca..0525a601f 100644
--- a/lib/puppet/face/certificate_revocation_list.rb
+++ b/lib/puppet/face/certificate_revocation_list.rb
@@ -10,15 +10,6 @@ Puppet::Indirector::Face.define(:certificate_revocation_list, '0.0.1') do
     list from the CA. Although it exposes search/save/destroy methods,
     they shouldn't be used under normal circumstances.
   EOT
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `ca`
-    * `file`
-    * `rest`
-  EOT
 
   get_action(:find).summary "Retrieve the certificate revocation list."
   get_action(:find).arguments "<dummy_key>"
diff --git a/lib/puppet/face/facts.rb b/lib/puppet/face/facts.rb
index d7ddf1d51..05028a435 100644
--- a/lib/puppet/face/facts.rb
+++ b/lib/puppet/face/facts.rb
@@ -16,20 +16,6 @@ Puppet::Indirector::Face.define(:facts, '0.0.1') do
     front-end to the inventory service REST API. See the inventory
     service documentation for more detail.
   EOT
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `active_record`
-    * `couch`
-    * `facter`
-    * `inventory_active_record`
-    * `memory`
-    * `network_device`
-    * `rest`
-    * `yaml`
-  EOT
 
   find = get_action(:find)
   find.summary "Retrieve a host's facts."
diff --git a/lib/puppet/face/file.rb b/lib/puppet/face/file.rb
index 70b44c38f..1cf9bc7f3 100644
--- a/lib/puppet/face/file.rb
+++ b/lib/puppet/face/file.rb
@@ -11,13 +11,6 @@ Puppet::Indirector::Face.define(:file, '0.0.1') do
     examples for the relevant syntax.
   EOT
   notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `file`
-    * `rest`
-
     To retrieve the unmunged contents of a file, you must call find with
     --render-as s. Rendering as yaml will return a hash of metadata
     about the file, including its contents.
diff --git a/lib/puppet/face/key.rb b/lib/puppet/face/key.rb
index f19ae198f..54c4975c5 100644
--- a/lib/puppet/face/key.rb
+++ b/lib/puppet/face/key.rb
@@ -11,13 +11,5 @@ Puppet::Indirector::Face.define(:key, '0.0.1') do
     certificate generate', and it should not be necessary to use this action
     directly.
   EOT
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `ca`
-    * `file`
-  EOT
 
 end
diff --git a/lib/puppet/face/node.rb b/lib/puppet/face/node.rb
index c68d71846..d244127a4 100644
--- a/lib/puppet/face/node.rb
+++ b/lib/puppet/face/node.rb
@@ -10,19 +10,6 @@ Puppet::Indirector::Face.define(:node, '0.0.1') do
     environment, node parameters (exposed in the parser as top-scope
     variables), and classes.
   EOT
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `active_record`
-    * `exec`
-    * `ldap`
-    * `memory`
-    * `plain`
-    * `rest`
-    * `yaml`
-  EOT
 
   get_action(:destroy).summary "Invalid for this face."
   get_action(:search).summary "Invalid for this face."
diff --git a/lib/puppet/face/report.rb b/lib/puppet/face/report.rb
index a5718bc0c..1345d6359 100644
--- a/lib/puppet/face/report.rb
+++ b/lib/puppet/face/report.rb
@@ -5,15 +5,6 @@ Puppet::Indirector::Face.define(:report, '0.0.1') do
   license   "Apache 2 license; see COPYING"
 
   summary "Create, display, and submit reports"
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `processor`
-    * `rest`
-    * `yaml`
-  EOT
 
   get_action(:find).summary "Invalid for this face."
   get_action(:search).summary "Invalid for this face."
diff --git a/lib/puppet/face/resource.rb b/lib/puppet/face/resource.rb
index 593776e53..59f628675 100644
--- a/lib/puppet/face/resource.rb
+++ b/lib/puppet/face/resource.rb
@@ -10,14 +10,6 @@ Puppet::Indirector::Face.define(:resource, '0.0.1') do
     resource (originally ralsh) command line application. It is not intended to be
     used from the command line.
   EOT
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `ral`
-    * `rest`
-  EOT
 
   get_action(:destroy).summary "Invalid for this face."
 
diff --git a/lib/puppet/face/resource_type.rb b/lib/puppet/face/resource_type.rb
index 3973eb3b8..9d7639fc5 100644
--- a/lib/puppet/face/resource_type.rb
+++ b/lib/puppet/face/resource_type.rb
@@ -12,14 +12,6 @@ Puppet::Indirector::Face.define(:resource_type, '0.0.1') do
 
     It will eventually be extended to examine native resource types.
   EOT
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `parser`
-    * `rest`
-  EOT
 
   # Action documentation overrides:
   get_action(:save).summary = "Invalid for this face."
diff --git a/lib/puppet/face/status.rb b/lib/puppet/face/status.rb
index 08379550c..4ae24e81b 100644
--- a/lib/puppet/face/status.rb
+++ b/lib/puppet/face/status.rb
@@ -5,14 +5,6 @@ Puppet::Indirector::Face.define(:status, '0.0.1') do
   license   "Apache 2 license; see COPYING"
 
   summary "View puppet server status."
-  notes <<-'EOT'
-    This is an indirector face, which exposes `find`, `search`, `save`, and
-    `destroy` actions for an indirected subsystem of Puppet. Valid termini
-    for this face include:
-
-    * `local`
-    * `rest`
-  EOT
 
   get_action(:destroy).summary "Invalid for this face."
   get_action(:save).summary "Invalid for this face."
-- 
cgit 


From 149a12f882252150ef9d642c6cf0345306f772ee Mon Sep 17 00:00:00 2001
From: nfagerlund <nick.fagerlund@gmail.com>
Date: Thu, 26 May 2011 10:32:22 -0700
Subject: Maint: Fix ellipses for short descriptions

Previously, we were adding ellipsis to any short_description, which was
misleading; they were only necessary when we were truncating in the _middle_ of
a paragraph.

This commit changes the behavior, and updates the tests to show when we expect
ellipsis.

Paired-with: Matt Robinson <matt@puppetlabs.com>
---
 lib/puppet/interface/documentation.rb            |  4 +++-
 spec/shared_behaviours/documentation_on_faces.rb | 11 +++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/interface/documentation.rb b/lib/puppet/interface/documentation.rb
index 48e9a8b1a..fcaec2568 100644
--- a/lib/puppet/interface/documentation.rb
+++ b/lib/puppet/interface/documentation.rb
@@ -77,8 +77,10 @@ class Puppet::Interface
       if @short_description.nil? then
         return nil if @description.nil?
         lines = @description.split("\n")
-        grab  = [5, lines.index('') || 5].min
+        first_paragraph_break = lines.index('') || 5
+        grab  = [5, first_paragraph_break].min
         @short_description = lines[0, grab].join("\n")
+        @short_description += ' [...]' if (grab < lines.length and first_paragraph_break >= 5)
       end
       @short_description
     end
diff --git a/spec/shared_behaviours/documentation_on_faces.rb b/spec/shared_behaviours/documentation_on_faces.rb
index 3cfb178f7..204b173a5 100644
--- a/spec/shared_behaviours/documentation_on_faces.rb
+++ b/spec/shared_behaviours/documentation_on_faces.rb
@@ -109,11 +109,18 @@ shared_examples_for "documentation on faces" do
       subject.short_description.should == "hello\ngoodbye"
     end
 
-    it "should trim a very, very long first paragraph" do
+    it "should trim a very, very long first paragraph and add ellipsis" do
       line = "this is a very, very, very long long line full of text\n"
       subject.description = line * 20 + "\n\nwhatever, dude."
 
-      subject.short_description.should == (line * 5).chomp
+      subject.short_description.should == (line * 5).chomp + ' [...]'
+    end
+
+    it "should trim a very very long only paragraph even if it is followed by a new paragraph" do
+      line = "this is a very, very, very long long line full of text\n"
+      subject.description = line * 20
+
+      subject.short_description.should == (line * 5).chomp + ' [...]'
     end
   end
 
-- 
cgit 


From 9447cb957b8c01ba7d368baebc1061c3dbfc9399 Mon Sep 17 00:00:00 2001
From: Matt Robinson <matt@puppetlabs.com>
Date: Thu, 26 May 2011 14:36:19 -0700
Subject: maint: Dedup the loadpath so we don't have to walk it multiple times

If the user's path has duplicate entries, we end up looking at them
multiple times.  This has bitten people in the past in that if you get a
lot of duplication it can make autloading a lot slower.  Really the user
shouldn't be duplicating their path, but since we can't control that,
this is a safe fix to prevent them from having autoload slowness.

Paired-with: Max Martin <max@puppetlabs.com>
---
 lib/puppet/util/autoload.rb     | 2 +-
 spec/unit/util/autoload_spec.rb | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb
index f0dd0a5c5..0e7025aef 100644
--- a/lib/puppet/util/autoload.rb
+++ b/lib/puppet/util/autoload.rb
@@ -124,7 +124,7 @@ class Puppet::Util::Autoload
 
   # The list of directories to search through for loadable plugins.
   def searchpath(env=nil)
-    search_directories(env).collect { |d| File.join(d, @path) }.find_all { |d| FileTest.directory?(d) }
+    search_directories(env).uniq.collect { |d| File.join(d, @path) }.find_all { |d| FileTest.directory?(d) }
   end
 
   def module_directories(env=nil)
diff --git a/spec/unit/util/autoload_spec.rb b/spec/unit/util/autoload_spec.rb
index 512f06c75..d61b7689e 100755
--- a/spec/unit/util/autoload_spec.rb
+++ b/spec/unit/util/autoload_spec.rb
@@ -51,9 +51,9 @@ describe Puppet::Util::Autoload do
       @autoload.search_directories.should == %w{/one /two /libdir1 /lib/dir/two /third/lib/dir} + $LOAD_PATH
     end
 
-    it "should include in its search path all of the search directories that have a subdirectory matching the autoload path" do
+    it "should include in its search path all of the unique search directories that have a subdirectory matching the autoload path" do
       @autoload = Puppet::Util::Autoload.new("foo", "loaddir")
-      @autoload.expects(:search_directories).returns %w{/one /two /three}
+      @autoload.expects(:search_directories).returns %w{/one /two /three /three}
       FileTest.expects(:directory?).with("/one/loaddir").returns true
       FileTest.expects(:directory?).with("/two/loaddir").returns false
       FileTest.expects(:directory?).with("/three/loaddir").returns true
-- 
cgit 


From 2aa9f2fe390c15806cce5532afed2fc7ecf51b8b Mon Sep 17 00:00:00 2001
From: Matt Robinson <matt@puppetlabs.com>
Date: Thu, 26 May 2011 14:54:11 -0700
Subject: (#7690) Don't blow up when listing terminuses available for faces

Previously, in order to list the available terminuses for an indirected
face we loaded all the the terminuses in order to list them.  This meant
that if a terminus like active_record didn't have the dependencies it
needed, the documentation would raise errors and not list terminuses.

    <Puppet::Error: Could not autoload filename uninitialized constant Object::ActiveRecord>

Now we just list the terminuses available in the load path without
trying to load them.  The terminus will still raise an error if you try
to use it without its dependencies being met.

Paired-with: Max Martin <max@puppetlabs.com>
---
 lib/puppet/application/faces.rb       |  6 +-----
 lib/puppet/indirector/terminus.rb     |  9 +++------
 lib/puppet/util/autoload.rb           | 28 +++++++++++++++-------------
 spec/unit/indirector/terminus_spec.rb |  6 ++++++
 4 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/lib/puppet/application/faces.rb b/lib/puppet/application/faces.rb
index e7fce66b1..3145da821 100644
--- a/lib/puppet/application/faces.rb
+++ b/lib/puppet/application/faces.rb
@@ -66,7 +66,7 @@ Copyright (c) 2011 Puppet Labs, LLC Licensed under the Apache 2.0 License
       str = "#{name}:\n"
       if arguments.include?("terminuses")
         begin
-          terms = terminus_classes(name.to_sym)
+          terms = Puppet::Indirector::Face.terminus_classes(name.to_sym)
           str << "\tTerminuses: #{terms.join(", ")}\n"
         rescue => detail
           puts detail.backtrace if Puppet[:trace]
@@ -107,10 +107,6 @@ Copyright (c) 2011 Puppet Labs, LLC Licensed under the Apache 2.0 License
     Puppet::Face.faces
   end
 
-  def terminus_classes(indirection)
-    Puppet::Indirector::Terminus.terminus_classes(indirection).collect { |t| t.to_s }.sort
-  end
-
   def actions(indirection)
     return [] unless face = Puppet::Face[indirection, '0.0.1']
     face.load_actions
diff --git a/lib/puppet/indirector/terminus.rb b/lib/puppet/indirector/terminus.rb
index 4ebd0d004..d488869d1 100644
--- a/lib/puppet/indirector/terminus.rb
+++ b/lib/puppet/indirector/terminus.rb
@@ -111,12 +111,9 @@ class Puppet::Indirector::Terminus
     # Return all terminus classes for a given indirection.
     def terminus_classes(indirection_name)
       setup_instance_loading indirection_name
-
-      # Load them all.
-      instance_loader(indirection_name).loadall
-
-      # And return the list of names.
-      loaded_instances(indirection_name)
+      instance_loader(indirection_name).files_to_load.map do |file|
+        File.basename(file).chomp(".rb").intern
+      end
     end
 
     private
diff --git a/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb
index 0e7025aef..6537a4a4e 100644
--- a/lib/puppet/util/autoload.rb
+++ b/lib/puppet/util/autoload.rb
@@ -105,23 +105,25 @@ class Puppet::Util::Autoload
   # so that already-loaded files don't get reloaded unnecessarily.
   def loadall
     # Load every instance of everything we can find.
-    searchpath.each do |dir|
-      Dir.glob("#{dir}/*.rb").each do |file|
-        name = File.basename(file).sub(".rb", '').intern
-        next if loaded?(name)
-        begin
-          Kernel.require file
-          loaded(name, file)
-        rescue SystemExit,NoMemoryError
-          raise
-        rescue Exception => detail
-          puts detail.backtrace if Puppet[:trace]
-          raise Puppet::Error, "Could not autoload #{file}: #{detail}"
-        end
+    files_to_load.each do |file|
+      name = File.basename(file).chomp(".rb").intern
+      next if loaded?(name)
+      begin
+        Kernel.require file
+        loaded(name, file)
+      rescue SystemExit,NoMemoryError
+        raise
+      rescue Exception => detail
+        puts detail.backtrace if Puppet[:trace]
+        raise Puppet::Error, "Could not autoload #{file}: #{detail}"
       end
     end
   end
 
+  def files_to_load
+    searchpath.map { |dir| Dir.glob("#{dir}/*.rb") }.flatten
+  end
+
   # The list of directories to search through for loadable plugins.
   def searchpath(env=nil)
     search_directories(env).uniq.collect { |d| File.join(d, @path) }.find_all { |d| FileTest.directory?(d) }
diff --git a/spec/unit/indirector/terminus_spec.rb b/spec/unit/indirector/terminus_spec.rb
index 33932cfca..2f37c1ff5 100755
--- a/spec/unit/indirector/terminus_spec.rb
+++ b/spec/unit/indirector/terminus_spec.rb
@@ -242,3 +242,9 @@ describe Puppet::Indirector::Terminus, " when creating terminus class types", :'
   end
 end
 
+describe Puppet::Indirector::Terminus, " when listing terminus classes" do
+  it "should list the terminus files available to load" do
+    Puppet::Util::Autoload.any_instance.stubs(:files_to_load).returns ["/foo/bar/baz", "/max/runs/marathon"]
+    Puppet::Indirector::Terminus.terminus_classes('my_stuff').should == [:baz, :marathon]
+  end
+end
-- 
cgit 


From ea7908b8d6dc2092b7a7ff4b92452821a3afd86b Mon Sep 17 00:00:00 2001
From: Matt Robinson <matt@puppetlabs.com>
Date: Thu, 26 May 2011 15:44:43 -0700
Subject: maint: Fix order dependent spec failure for face indirection

An indirected face is being created in
spec/unit/application/indirection_base_spec.rb that uses a stubbed out
indirection.  This stub didn't have a name method defined, which caused
the documentation_spec.rb, that does checks against every available
face, to blow up with:

    expected no Exception, got #<NoMethodError: undefined method `to_sym' for nil:NilClass>

in lib/puppet/face/indirector/face.rb when listing the indirections for
the help text.

I toyed with creating a real indirection for the test, but that was
harder than expected.

Paired-with: Max Martin <max@puppetlabs.com>
---
 spec/unit/application/indirection_base_spec.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/unit/application/indirection_base_spec.rb b/spec/unit/application/indirection_base_spec.rb
index d72def6cf..8a5eee2c6 100755
--- a/spec/unit/application/indirection_base_spec.rb
+++ b/spec/unit/application/indirection_base_spec.rb
@@ -19,7 +19,6 @@ face.instance_variable_set('@version', :current)
 Puppet::Face.register(face)
 ########################################################################
 
-
 describe Puppet::Application::IndirectionBase do
   subject { Puppet::Application::TestIndirection.new }
 
@@ -27,6 +26,8 @@ describe Puppet::Application::IndirectionBase do
     # It would be nice not to have to stub this, but whatever... writing an
     # entire indirection stack would cause us more grief. --daniel 2011-03-31
     terminus = stub_everything("test indirection terminus")
+    terminus.stubs(:name).returns(:testindirection)
+
     Puppet::Indirector::Indirection.expects(:instance).
       with(:testindirection).returns(terminus)
 
-- 
cgit 


From a5a78a52bb2345759f5703554e17e08703bca01e Mon Sep 17 00:00:00 2001
From: Matt Robinson <matt@puppetlabs.com>
Date: Thu, 26 May 2011 17:05:28 -0700
Subject: (#7557) Remove Faces Application

It used to be that to list the faces available and their actions and
options, you had this legacy based face application.  This functionality
has all been moved into the puppet help face application.

Reviewed-by: Daniel Pittman <daniel@puppetlabs.com>
---
 lib/puppet/application/faces.rb     | 122 ------------------------------------
 spec/unit/application/faces_spec.rb |  14 -----
 2 files changed, 136 deletions(-)
 delete mode 100644 lib/puppet/application/faces.rb
 delete mode 100755 spec/unit/application/faces_spec.rb

diff --git a/lib/puppet/application/faces.rb b/lib/puppet/application/faces.rb
deleted file mode 100644
index 3145da821..000000000
--- a/lib/puppet/application/faces.rb
+++ /dev/null
@@ -1,122 +0,0 @@
-require 'puppet/application'
-require 'puppet/face'
-
-class Puppet::Application::Faces < Puppet::Application
-
-  should_parse_config
-  run_mode :agent
-
-  option("--debug", "-d") do |arg|
-    Puppet::Util::Log.level = :debug
-  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}
-    end
-    faces.each do |name|
-      str = "#{name}:\n"
-      if arguments.include?("terminuses")
-        begin
-          terms = Puppet::Indirector::Face.terminus_classes(name.to_sym)
-          str << "\tTerminuses: #{terms.join(", ")}\n"
-        rescue => detail
-          puts detail.backtrace if Puppet[:trace]
-          $stderr.puts "Could not load terminuses for #{name}: #{detail}"
-        end
-      end
-
-      if arguments.include?("actions")
-        begin
-          actions = actions(name.to_sym)
-          str << "\tActions: #{actions.join(", ")}\n"
-        rescue => detail
-          puts detail.backtrace if Puppet[:trace]
-          $stderr.puts "Could not load actions for #{name}: #{detail}"
-        end
-      end
-
-      print str
-    end
-  end
-
-  attr_accessor :name, :arguments
-
-  def main
-    list(*arguments)
-  end
-
-  def setup
-    Puppet::Util::Log.newdestination :console
-
-    load_applications # Call this to load all of the apps
-
-    @arguments = command_line.args
-    @arguments ||= []
-  end
-
-  def faces
-    Puppet::Face.faces
-  end
-
-  def actions(indirection)
-    return [] unless face = Puppet::Face[indirection, '0.0.1']
-    face.load_actions
-    return face.actions.sort { |a, b| a.to_s <=> b.to_s }
-  end
-
-  def load_applications
-    command_line.available_subcommands.each do |app|
-      command_line.require_application app
-    end
-  end
-end
-
diff --git a/spec/unit/application/faces_spec.rb b/spec/unit/application/faces_spec.rb
deleted file mode 100755
index cc159b6a5..000000000
--- a/spec/unit/application/faces_spec.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-#!/usr/bin/env rspec
-require 'spec_helper'
-require 'puppet/application/faces'
-
-describe Puppet::Application::Faces do
-  it "should be an application" do
-    Puppet::Application::Faces.superclass.should equal(Puppet::Application)
-  end
-
-  it "should always call 'list'" do
-    subject.expects(:list)
-    subject.main
-  end
-end
-- 
cgit 


From 68c106e3ef192d64eb5a1e8daa1e070774909728 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Fri, 27 May 2011 10:12:34 -0700
Subject: (#5318) Always notice changes to manifests when compiling.

When we are asked to compile a catalog we need to update the set of known
resource types, along with the node declaration, from the pool of manifests on
disk.  This is, obviously, a per-environment pool of resource types.

To reduce the scope of the race where an update to those manifest files on
disk can be updated during a compilation, we tried to cache the set of known
resources in the current thread at the start of compilation, then flush it
after compile finished   and used the cache unconditionally if it was set.

Theoretically, this would assure us that we would parse the set of manifests
once, use them for the entire compile, flush the cache, and then carry on.

Practically, this was enforced as described: flush *after* the compile, assume
this would mean that it was clear at the start of the next compile.

That presumably worked more or less right until a change was made to push
extra data into the catalog at the start of the 2.6 series; that made
serialization of the catalog depend on the pool of known resource types.

When that happened we would reload the cache (and reparse the manifests)
during serialization, but after compilation ... and leave that in the thread
cache, so the precondition for the compiler was no longer true.  It would see
the cache as of the end of the previous compile run, not the start of the next
compile run.

This, in turn, was what made Puppet wait for multiple runs of the agent before
showing you a change in your manifests under Passenger, but *not* under
Webrick: Passenger would reuse the same thread for the next request, cache in
place, while Webrick would create a new thread and (by side-effect) "flush"
the cache as the compiler expected.

To minimally fix this, with as little change of side-effect as possible, we
move the cache flush from after compile runs to before compile runs.

This might have minimal memory cost until another compile request runs in the
same thread, because we cache the data longer, but most models won't cause
that to be too much trouble.

Reviewed-By: Matt Robinson <matt@puppetlabs.com>
---
 lib/puppet/parser/compiler.rb | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index a891f1a11..bc0c6a15d 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -15,15 +15,19 @@ class Puppet::Parser::Compiler
   include Puppet::Resource::TypeCollectionHelper
 
   def self.compile(node)
+    # We get these from the environment and only cache them in a thread
+    # variable for the duration of the compilation.  If nothing else is using
+    # the thread, though, we can leave 'em hanging round with no ill effects,
+    # and this is safer than cleaning them at the end and assuming that will
+    # stick until the next entry to this function.
+    Thread.current[:known_resource_types] = nil
+    Thread.current[:env_module_directories] = nil
+
+    # ...and we actually do the compile now we have caching ready.
     new(node).compile.to_resource
   rescue => detail
     puts detail.backtrace if Puppet[:trace]
     raise Puppet::Error, "#{detail} on node #{node.name}"
-  ensure
-    # We get these from the environment and only cache them in a thread
-    # variable for the duration of the compilation.
-    Thread.current[:known_resource_types] = nil
-    Thread.current[:env_module_directories] = nil
  end
 
   attr_reader :node, :facts, :collections, :catalog, :node_scope, :resources, :relationships
-- 
cgit 


From 3facc3393378ef7848374621a4674c977e82f884 Mon Sep 17 00:00:00 2001
From: Dominic Maraglia <dominic@puppetlabs.com>
Date: Tue, 24 May 2011 11:28:40 -0700
Subject: add puppet master polling step for ticket 7117

The test for ticket 7117 producedes spurious failures due to timing:
a curl command is executed from an agent to a freshly started
Puppet Master; if the Puppet Master is not ready to accept the connection
the test will fail.  Added an until loop that issues  simple curl
command to see if the Puppet Master is up and ready
---
 acceptance/tests/ticket_7117_broke_env_criteria_authconf.rb | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/acceptance/tests/ticket_7117_broke_env_criteria_authconf.rb b/acceptance/tests/ticket_7117_broke_env_criteria_authconf.rb
index 3f762bcca..37c71893c 100644
--- a/acceptance/tests/ticket_7117_broke_env_criteria_authconf.rb
+++ b/acceptance/tests/ticket_7117_broke_env_criteria_authconf.rb
@@ -17,9 +17,17 @@ on master, "ps -U puppet | awk '/puppet/ { print \$1 }' | xargs kill || echo \"P
 step "Master: Start Puppet Master"
 on master, puppet_master("--certdnsnames=\"puppet:$(hostname -s):$(hostname -f)\" --verbose --noop")
 # allow Master to start and initialize environment
-sleep 1
-
 
+step "Verify Puppet Master is ready to accept connections"
+host=agents.first
+time1 = Time.new
+until
+  on(host, "curl -k https://#{master}:8140") do
+    sleep 1
+  end
+time2 = Time.new
+elapsed = time2 - time1
+Log.notify "Slept for #{elapsed} seconds waiting for Puppet Master to become ready"
 
 # Run test on Agents
 step "Agent: agent --test"
-- 
cgit 


From 2935feb648f7945175ebd11a243155ab088a8efe Mon Sep 17 00:00:00 2001
From: nfagerlund <nick.fagerlund@gmail.com>
Date: Fri, 27 May 2011 16:00:17 -0700
Subject: (#7707) Document signals in puppet agent and puppet master help

Previously, the signals accepted by the agent and master daemons were only
documented in the configuration reference, which didn't make any particular
sense. This commit moves their documentation to a blurb in the relevant
man pages.

This is a doc string only commit.
---
 lib/puppet/application/agent.rb  | 12 ++++++++++++
 lib/puppet/application/master.rb | 10 ++++++++++
 2 files changed, 22 insertions(+)

diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb
index fc8616817..e8f2f1bc5 100644
--- a/lib/puppet/application/agent.rb
+++ b/lib/puppet/application/agent.rb
@@ -281,6 +281,18 @@ EXAMPLE
     $ puppet agent --server puppet.domain.com
 
 
+DIAGNOSTICS
+-----------
+
+Puppet agent accepts the following signals:
+
+* SIGHUP:
+  Restart the puppet agent daemon.
+* SIGINT and SIGTERM:
+  Shut down the puppet agent daemon.
+* SIGUSR1: 
+  Immediately retrieve and apply configurations from the puppet master.
+
 AUTHOR
 ------
 Luke Kanies
diff --git a/lib/puppet/application/master.rb b/lib/puppet/application/master.rb
index a90829ae0..18425c8bc 100644
--- a/lib/puppet/application/master.rb
+++ b/lib/puppet/application/master.rb
@@ -102,6 +102,16 @@ EXAMPLE
 -------
   puppet master
 
+DIAGNOSTICS
+-----------
+
+When running as a standalone daemon, puppet master accepts the
+following signals:
+
+* SIGHUP:
+  Restart the puppet master server.
+* SIGINT and SIGTERM:
+  Shut down the puppet master server.
 
 AUTHOR
 ------
-- 
cgit 


From 7df1fa448c9eb5f23f8ea481bbea18e09f8fff29 Mon Sep 17 00:00:00 2001
From: nfagerlund <nick.fagerlund@gmail.com>
Date: Fri, 27 May 2011 16:03:36 -0700
Subject: (#7708) Delete extended documentation from configuration reference

The auto-generated references are meant to be pithy, dense, and fast -- to be
references, not guides. The configuration reference was front-loaded with
several pages of explanatory text that properly belongs in the guides on the
docs site. This commit removes that text in preparation for a reorganization of
the docs.

This is a doc string only commit.
---
 lib/puppet/reference/configuration.rb | 127 +++++++---------------------------
 1 file changed, 26 insertions(+), 101 deletions(-)

diff --git a/lib/puppet/reference/configuration.rb b/lib/puppet/reference/configuration.rb
index 6581427ff..18efb6fe7 100644
--- a/lib/puppet/reference/configuration.rb
+++ b/lib/puppet/reference/configuration.rb
@@ -41,104 +41,29 @@ config = Puppet::Util::Reference.newreference(:configuration, :depth => 1, :doc
   return str
 end
 
-config.header = "
-## Specifying Configuration Parameters
-
-### On The Command-Line
-
-Every Puppet executable (with the exception of `puppetdoc`) accepts all of
-the parameters below, but not all of the arguments make sense for every executable.
-
-I have tried to be as thorough as possible in the descriptions of the
-arguments, so it should be obvious whether an argument is appropriate or not.
-
-These parameters can be supplied to the executables either as command-line
-options or in the configuration file.  For instance, the command-line
-invocation below would set the configuration directory to `/private/puppet`:
-
-    $ puppet agent --confdir=/private/puppet
-
-Note that boolean options are turned on and off with a slightly different
-syntax on the command line:
-
-    $ puppet agent --storeconfigs
-
-    $ puppet agent --no-storeconfigs
-
-The invocations above will enable and disable, respectively, the storage of
-the client configuration.
-
-### Configuration Files
-
-As mentioned above, the configuration parameters can also be stored in a
-configuration file, located in the configuration directory.  As root, the
-default configuration directory is `/etc/puppet`, and as a regular user, the
-default configuration directory is `~user/.puppet`.  As of 0.23.0, all
-executables look for `puppet.conf` in their configuration directory
-(although they previously looked for separate files).  For example,
-`puppet.conf` is located at `/etc/puppet/puppet.conf` as `root` and
-`~user/.puppet/puppet.conf` as a regular user by default.
-
-All executables will set any parameters set within the `[main]` section,
-and each executable will also use one of the `[master]`, `[agent]`.
-
-#### File Format
-
-The file follows INI-style formatting.  Here is an example of a very simple
-`puppet.conf` file:
-
-    [main]
-      confdir = /private/puppet
-      storeconfigs = true
-
-Note that boolean parameters must be explicitly specified as `true` or
-`false` as seen above.
-
-If you need to change file or directory parameters (e.g., reset the mode or owner), do
-so within curly braces on the same line:
-
-    [main]
-      vardir = /new/vardir {owner = root, mode = 644}
-
-If you're starting out with a fresh configuration, you may wish to let
-the executable generate a template configuration file for you by invoking
-the executable in question with the `--genconfig` command.  The executable
-will print a template configuration to standard output, which can be
-redirected to a file like so:
-
-    $ puppet agent --genconfig > /etc/puppet/puppet.conf
-
-Note that this invocation will replace the contents of any pre-existing
-`puppet.conf` file, so make a backup of your present config if it contains
-valuable information.
-
-Like the `--genconfig` argument, the executables also accept a `--genmanifest`
-argument, which will generate a manifest that can be used to manage all of
-Puppet's directories and files and prints it to standard output.  This can
-likewise be redirected to a file:
-
-    $ puppet agent --genmanifest > /etc/puppet/manifests/site.pp
-
-Puppet can also create user and group accounts for itself (one `puppet` group
-and one `puppet` user) if it is invoked as `root` with the `--mkusers` argument:
-
-    $ puppet master --mkusers
-
-## Signals
-
-The `puppet agent` and `puppet master` executables catch some signals for special
-handling.  Both daemons catch (`SIGHUP`), which forces the server to restart
-tself.  Predictably, interrupt and terminate (`SIGINT` and `SIGTERM`) will shut
-down the server, whether it be an instance of `puppet agent` or `puppet master`.
-
-Sending the `SIGUSR1` signal to an instance of `puppet agent` will cause it to
-immediately begin a new configuration transaction with the server.  This
-signal has no effect on `puppet master`.
-
-## Configuration Parameter Reference
-
-Below is a list of all documented parameters.  Not all of them are valid with all
-Puppet executables, but the executables will ignore any inappropriate values.
-
-"
-
+config.header = <<EOT
+## Configuration Settings
+
+* Each of these settings can be specified in `puppet.conf` or on the
+  command line.
+* When using boolean settings on the command line, use `--setting` and
+  `--no-setting` instead of `--setting (true|false)`.
+* Settings can be interpolated as `$variables` in other settings; `$environment`
+  is special, in that puppet master will interpolate each agent node's
+  environment instead of its own.
+* Multiple values should be specified as comma-separated lists; multiple
+  directories should be separated with the system path separator (usually
+  a colon).
+* Settings that take a single file or directory can optionally set the owner,
+  group, and mode for their value: `rundir = $vardir/run { owner = puppet,
+  group = puppet, mode = 644 }`
+* The Puppet executables will ignore any setting that isn't relevant to
+  their function.
+
+See the [configuration guide][confguide] for more details.
+
+[confguide]: http://docs.puppetlabs.com/guides/configuring.html
+
+* * *
+
+EOT
-- 
cgit 


From 618495c7b70671ff1da1653c76d1524ff3f615b7 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Tue, 31 May 2011 15:02:41 -0700
Subject: #7211: more helpful error messages in various cases.

We were emitting a bunch of unhelpful failure messages, surrounding invalid
actions and especially default actions interacting with the command-line.

This cleans those up, to give a helpful, informative, and correct message in
all cases.  Notably, we no longer report that there is no "default" action
when you specify an unknown action on a face.

This change revealed some other weaknesses in our unit tests, now correctly,
that result in slightly more robust code.
---
 lib/puppet/application/face_base.rb     | 26 ++++++++++++++++++++------
 spec/lib/puppet/face/basetest.rb        |  5 +++++
 spec/unit/application/face_base_spec.rb | 25 +++++++++++++++++++++----
 3 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
index 7a5ce3400..2f07faae1 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -66,9 +66,9 @@ class Puppet::Application::FaceBase < Puppet::Application
     # Now, walk the command line and identify the action.  We skip over
     # arguments based on introspecting the action and all, and find the first
     # non-option word to use as the action.
-    action = nil
-    index  = -1
-    until @action or (index += 1) >= command_line.args.length do
+    action_name = nil
+    index       = -1
+    until action_name or (index += 1) >= command_line.args.length do
       item = command_line.args[index]
       if item =~ /^-/ then
         option = @face.options.find do |name|
@@ -96,7 +96,11 @@ class Puppet::Application::FaceBase < Puppet::Application
           raise OptionParser::InvalidOption.new(item.sub(/=.*$/, ''))
         end
       else
-        @action = @face.get_action(item.to_sym)
+        # Stash away the requested action name for later, and try to fetch the
+        # action object it represents; if this is an invalid action name that
+        # will be nil, and handled later.
+        action_name = item.to_sym
+        @action = @face.get_action(action_name)
       end
     end
 
@@ -104,8 +108,18 @@ class Puppet::Application::FaceBase < Puppet::Application
       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)
+        # REVISIT: ...and this horror thanks to our log setup, which doesn't
+        # initialize destinations until the setup method, which we will never
+        # reach.  We could also just print here, but that is actually a little
+        # uglier and nastier in the long term, in which we should do log setup
+        # earlier if at all possible. --daniel 2011-05-31
+        Puppet::Util::Log.newdestination(:console)
+
+        face   = @face.name
+        action = action_name.nil? ? 'default' : "'#{action_name}'"
+        msg = "'#{face}' has no #{action} action.  See `puppet help #{face}`."
+        Puppet.err(msg)
+
         exit false
       end
     end
diff --git a/spec/lib/puppet/face/basetest.rb b/spec/lib/puppet/face/basetest.rb
index 9398f5b2d..e5c9a9dc5 100755
--- a/spec/lib/puppet/face/basetest.rb
+++ b/spec/lib/puppet/face/basetest.rb
@@ -38,4 +38,9 @@ Puppet::Face.define(:basetest, '0.0.1') do
     when_invoked do |options| "this is not the hook you are looking for" end
     when_rendering :s do |value| "you invoked the 's' rendering hook" end
   end
+
+  action :count_args do
+    summary "return the count of arguments given"
+    when_invoked do |*args| args.length - 1 end
+  end
 end
diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb
index 3318e061e..ae8c7733c 100755
--- a/spec/unit/application/face_base_spec.rb
+++ b/spec/unit/application/face_base_spec.rb
@@ -52,6 +52,12 @@ describe Puppet::Application::FaceBase do
       end
     end
 
+    it "should stop if the first thing found is not an action" do
+      app.command_line.stubs(:args).returns %w{banana count_args}
+      expect { app.run }.to exit_with 1
+      @logs.first.message.should =~ /has no 'banana' action/
+    end
+
     it "should use the default action if not given any arguments" do
       app.command_line.stubs(:args).returns []
       action = stub(:options => [], :render_as => nil)
@@ -77,7 +83,18 @@ describe Puppet::Application::FaceBase do
       Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(nil)
       app.stubs(:main)
       expect { app.run }.to exit_with 1
-      @logs.first.message.should =~ /does not have a default action/
+      @logs.first.message.should =~ /has no 'bar' action./
+    end
+
+    [%w{something_I_cannot_do},
+     %w{something_I_cannot_do argument}].each do |input|
+      it "should report unknown actions nicely" do
+        app.command_line.stubs(:args).returns input
+        Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(nil)
+        app.stubs(:main)
+        expect { app.run }.to exit_with 1
+        @logs.first.message.should =~ /has no 'something_I_cannot_do' action/
+      end
     end
 
     it "should report a sensible error when options with = fail" do
@@ -149,7 +166,7 @@ describe Puppet::Application::FaceBase do
     end
 
     it "should handle application-level options", :'fails_on_ruby_1.9.2' => true do
-      app.command_line.stubs(:args).returns %w{basetest --verbose return_true}
+      app.command_line.stubs(:args).returns %w{--verbose return_true}
       app.preinit
       app.parse_options
       app.face.name.should == :basetest
@@ -304,7 +321,7 @@ EOT
     end
 
     it "should fail early if asked to render an invalid format" do
-      app.command_line.stubs(:args).returns %w{--render-as interpretive-dance help help}
+      app.command_line.stubs(:args).returns %w{--render-as interpretive-dance return_true}
       # We shouldn't get here, thanks to the exception, and our expectation on
       # it, but this helps us fail if that slips up and all. --daniel 2011-04-27
       Puppet::Face[:help, :current].expects(:help).never
@@ -315,7 +332,7 @@ EOT
     end
 
     it "should work if asked to render a NetworkHandler format" do
-      app.command_line.stubs(:args).returns %w{dummy find dummy --render-as yaml}
+      app.command_line.stubs(:args).returns %w{count_args a b c --render-as yaml}
       expect {
         expect { app.run }.to exit_with 0
       }.to have_printed(/--- 3/)
-- 
cgit 


From dd8108cd8f48d399ab728c3437e3fef38f0eb46c Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Tue, 31 May 2011 15:05:54 -0700
Subject: #7211: nasty logic error with global Face options taking arguments.

A logic error meant that global Face options that took arguments were
mishandled: we never consumed the argument, so we read this:

    puppet facts --render-as json find $(hostname)

...as meaning "invoke the 'json' action on the 'facts' face"...

This fixes that problem, so we now correctly handle both optional and
non-optional arguments to global Face options.
---
 lib/puppet/application/face_base.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
index 2f07faae1..ea5ba4aaf 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -91,7 +91,7 @@ class Puppet::Application::FaceBase < Puppet::Application
             index += 1          # ...so skip the argument.
           end
         elsif option = find_application_argument(item) then
-          index += 1 if (option[:argument] and option[:optional])
+          index += 1 if (option[:argument] and not option[:optional])
         else
           raise OptionParser::InvalidOption.new(item.sub(/=.*$/, ''))
         end
-- 
cgit 


From 8072b4be084637b65846b662e4307b3d5a3a2f79 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Tue, 31 May 2011 15:18:42 -0700
Subject: #7211: Test unknown options don't shadow unknown actions.

Unknown options used to shadow unknown actions:

    $ puppet node something_I_cannot_do --unknown-option
    Could not parse options: invalid option: --unknown-option

The earlier changes have fixed this problem, but we now add extra testing to
make sure that we don't regress to the earlier state of play.
---
 spec/unit/application/face_base_spec.rb | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb
index ae8c7733c..0a4a86be6 100755
--- a/spec/unit/application/face_base_spec.rb
+++ b/spec/unit/application/face_base_spec.rb
@@ -97,6 +97,17 @@ describe Puppet::Application::FaceBase do
       end
     end
 
+    [%w{something_I_cannot_do --unknown-option},
+     %w{something_I_cannot_do argument --unknown-option}].each do |input|
+      it "should report unknown actions even if there are unknown options" do
+        app.command_line.stubs(:args).returns input
+        Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(nil)
+        app.stubs(:main)
+        expect { app.run }.to exit_with 1
+        @logs.first.message.should =~ /has no 'something_I_cannot_do' action/
+      end
+    end
+
     it "should report a sensible error when options with = fail" do
       app.command_line.stubs(:args).returns %w{--action=bar foo}
       expect { app.preinit; app.parse_options }.
-- 
cgit 


From 3f47b0cce4436aa638e0b914ff8ab1c9fbf67552 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 1 Jun 2011 10:34:22 -0700
Subject: (#7717) Layout cleanup for subcommand extraction.

This transforms the layout of the code, to make it easier to work with, but
makes no functional changes.  Done separately to make clearer the functional
changes vs the non-functional changes.
---
 lib/puppet/util/command_line.rb | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb
index 8190f8ac1..c475dacc4 100644
--- a/lib/puppet/util/command_line.rb
+++ b/lib/puppet/util/command_line.rb
@@ -93,13 +93,19 @@ module Puppet
 
         if zero == 'puppet'
           case argv.first
-          when nil;              [ stdin.tty? ? nil : "apply", argv] # ttys get usage info
-          when "--help", "-h";         [nil,     argv] # help should give you usage, not the help for `puppet apply`
-          when /^-|\.pp$|\.rb$/; ["apply", argv]
-          else [ argv.first, argv[1..-1] ]
+          when nil then
+            # ttys get usage info
+            [stdin.tty? ? nil : "apply", argv]
+          when "--help", "-h" then
+            # help should give you usage, not the help for `puppet apply`
+            [nil, argv]
+          when /^-|\.pp$|\.rb$/ then
+            ["apply", argv]
+          else
+            [argv.first, argv[1..-1]]
           end
         else
-          [ zero, argv ]
+          [zero, argv]
         end
       end
 
-- 
cgit 


From a23cfd869f90ae4456dded6e5a1c82719b128f01 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 1 Jun 2011 11:04:18 -0700
Subject: (#7177) Deprecate implicit 'puppet apply' for 2.7.0

Back in prehistory (eg: 0.25 era), 'puppet' was the name for the agent, and
could be used directly to apply a manifest as well as to communicate with the
puppet master process.

During the 2.6 series we moved to a single binary, but continued to support
older scripts by detecting invocations that looked like the traditional
scripting uses and implicitly turning those into a call to 'puppet apply'.

Now, with the 2.7.0 release, we are moving to deprecate that behaviour.  We
still do the same detection, and still run the old manifests, but we now
emit a deprecation warning directing people to use 'puppet apply' directly.

We intend to remove the behaviour entirely in the 2.8 release, which also
paves the way to nicer handling of the command line.

Reviewed-By: Randall Hansen <randall@puppetlabs.com>
Reviewed-By: Nick Fagerlund <nick.fagerlund@puppetlabs.com>
---
 lib/puppet/util/command_line.rb | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb
index c475dacc4..372aa5122 100644
--- a/lib/puppet/util/command_line.rb
+++ b/lib/puppet/util/command_line.rb
@@ -94,12 +94,27 @@ module Puppet
         if zero == 'puppet'
           case argv.first
           when nil then
-            # ttys get usage info
-            [stdin.tty? ? nil : "apply", argv]
+            if stdin.tty? then
+              [nil, argv]       # ttys get usage info
+            else
+              # Killed for 2.7.0 --daniel 2011-06-01
+              Puppet.deprecation_warning <<EOM
+Implicit invocation of 'puppet apply' by redirection into 'puppet' is deprecated,
+and will be removed in the 2.8 series. Please invoke 'puppet apply' directly
+in the future.
+EOM
+              ["apply", argv]
+            end
           when "--help", "-h" then
             # help should give you usage, not the help for `puppet apply`
             [nil, argv]
           when /^-|\.pp$|\.rb$/ then
+            # Killed for 2.7.0 --daniel 2011-06-01
+            Puppet.deprecation_warning <<EOM
+Implicit invocation of 'puppet apply' by passing files (or flags) directly
+to 'puppet' is deprecated, and will be removed in the 2.8 series.  Please
+invoke 'puppet apply' directly in the future.
+EOM
             ["apply", argv]
           else
             [argv.first, argv[1..-1]]
@@ -112,3 +127,10 @@ module Puppet
     end
   end
 end
+
+# REVISIT: Without this, we can't use the full Puppet suite inside the code,
+# but we can't include it before we get this far through defining things as
+# this code is the product of incestuous union with the global code; global
+# code at load scope depends on methods we define, while we only depend on
+# them at runtime scope.  --daniel 2011-06-01
+require 'puppet'
-- 
cgit 


From 796900b04e234b61a77fe4b64c006f24e15a7360 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 1 Jun 2011 11:53:40 -0700
Subject: (#7699) Don't duplicate inherited action names on faces.

We earlier moved to duplicating Action objects in the Faces subsystem to
ensure they had the correct binding context during execution and introspection.

This was correct, but introduced a bug where we would report both the parent
and child binding as separate entries with duplicate names, in the list of
actions.

This flowed on to the help output, where it would cause every inherited action
to be listed twice: once on the parent, once on the child.  (This was actually
worse if the inheritance was deeper: we would duplicate once for every level
between the instance and the origin of the action.)
---
 lib/puppet/interface/action_manager.rb     |  5 ++-
 spec/unit/interface/action_manager_spec.rb | 50 +++++++++++++++++++++++-------
 2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/lib/puppet/interface/action_manager.rb b/lib/puppet/interface/action_manager.rb
index c5eb8e08a..fbf588d7d 100644
--- a/lib/puppet/interface/action_manager.rb
+++ b/lib/puppet/interface/action_manager.rb
@@ -34,7 +34,10 @@ module Puppet::Interface::ActionManager
     elsif self.class.respond_to?(:actions)
       result += self.class.actions
     end
-    result.sort
+    # We need to uniq the result, because we duplicate actions when they are
+    # fetched to ensure that they have the correct bindings; they shadow the
+    # parent, and uniq implements that. --daniel 2011-06-01
+    result.uniq.sort
   end
 
   def get_action(name)
diff --git a/spec/unit/interface/action_manager_spec.rb b/spec/unit/interface/action_manager_spec.rb
index e357a5fa1..3a84e4f83 100755
--- a/spec/unit/interface/action_manager_spec.rb
+++ b/spec/unit/interface/action_manager_spec.rb
@@ -186,23 +186,49 @@ describe Puppet::Interface::ActionManager do
       @instance.should be_action(:foo)
     end
 
-    it "should list actions defined in superclasses" do
-      @subclass = Class.new(@klass)
-      @instance = @subclass.new
+    context "with actions defined in superclass" do
+      before :each do
+        @subclass = Class.new(@klass)
+        @instance = @subclass.new
+
+        @klass.action(:parent) do
+          when_invoked { |options| "a" }
+        end
+        @subclass.action(:sub) do
+          when_invoked { |options| "a" }
+        end
+        @instance.action(:instance) do
+          when_invoked { |options| "a" }
+        end
+      end
+
+      it "should list actions defined in superclasses" do
+        @instance.should be_action(:parent)
+        @instance.should be_action(:sub)
+        @instance.should be_action(:instance)
+      end
 
-      @klass.action(:parent) do
-        when_invoked { |options| "a" }
+      it "should list inherited actions" do
+        @instance.actions.should =~ [:instance, :parent, :sub]
       end
-      @subclass.action(:sub) do
-        when_invoked { |options| "a" }
+
+      it "should not duplicate instance actions after fetching them (#7699)" do
+        @instance.actions.should =~ [:instance, :parent, :sub]
+        @instance.get_action(:instance)
+        @instance.actions.should =~ [:instance, :parent, :sub]
       end
-      @instance.action(:instance) do
-        when_invoked { |options| "a" }
+
+      it "should not duplicate subclass actions after fetching them (#7699)" do
+        @instance.actions.should =~ [:instance, :parent, :sub]
+        @instance.get_action(:sub)
+        @instance.actions.should =~ [:instance, :parent, :sub]
       end
 
-      @instance.should be_action(:parent)
-      @instance.should be_action(:sub)
-      @instance.should be_action(:instance)
+      it "should not duplicate superclass actions after fetching them (#7699)" do
+        @instance.actions.should =~ [:instance, :parent, :sub]
+        @instance.get_action(:parent)
+        @instance.actions.should =~ [:instance, :parent, :sub]
+      end
     end
 
     it "should create an instance method when an action is defined in a superclass" do
-- 
cgit 


From e647f70cbde1735af0710d07fc704448f5a8dcd8 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 1 Jun 2011 14:15:33 -0700
Subject: maint: remove obsolete work-around code from help face.

The application wrapper for help used to disable the inherited 'render' method
from FaceBase - specifically, to avoid the behaviour of rendering strings
poorly.

Now that we have support for good output in the upstream method, this is
unnecessary, so we can eliminate the stub method entirely and use the default
behaviour.

(This also enables rendering the help into JSON or YAML, against the odds that
 someone actually cares about that. ;)
---
 lib/puppet/application/help.rb | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/puppet/application/help.rb b/lib/puppet/application/help.rb
index 4829a2036..66baa462e 100644
--- a/lib/puppet/application/help.rb
+++ b/lib/puppet/application/help.rb
@@ -1,7 +1,4 @@
 require 'puppet/application/face_base'
 
 class Puppet::Application::Help < Puppet::Application::FaceBase
-  # Meh.  Disable the default behaviour, which is to inspect the
-  # string and return that – not so helpful. --daniel 2011-04-11
-  def render(result) result end
 end
-- 
cgit 


From 2389bdf85bf0ae26110666ff34680d057acc32ff Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 1 Jun 2011 14:17:11 -0700
Subject: (#7683) Add a 'man' face and subcommand to Puppet.

This is the minimal wrapper, cloning a good deal of the logic from help, that
runs our face through the 'man' template and returns ronn-formatted Markdown.

This provides the crudest baseline possible for getting man-style output, but
lets us move forward to improve behaviour.

Reviewed-By: Nick Fagerlund <nick.fagerlund@puppetlabs.com>
---
 lib/puppet/application/man.rb |  4 ++++
 lib/puppet/face/man.rb        | 47 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 lib/puppet/application/man.rb
 create mode 100644 lib/puppet/face/man.rb

diff --git a/lib/puppet/application/man.rb b/lib/puppet/application/man.rb
new file mode 100644
index 000000000..1ecc4d691
--- /dev/null
+++ b/lib/puppet/application/man.rb
@@ -0,0 +1,4 @@
+require 'puppet/application/face_base'
+
+class Puppet::Application::Man < Puppet::Application::FaceBase
+end
diff --git a/lib/puppet/face/man.rb b/lib/puppet/face/man.rb
new file mode 100644
index 000000000..44a339490
--- /dev/null
+++ b/lib/puppet/face/man.rb
@@ -0,0 +1,47 @@
+require 'puppet/face'
+require 'pathname'
+require 'erb'
+
+Puppet::Face.define(:man, '0.0.1') do
+  copyright "Puppet Labs", 2011
+  license   "Apache 2 license; see COPYING"
+
+  summary "Display Puppet subcommand manual pages."
+
+  action(:man) do
+    summary "Display the manual page for a face."
+    arguments "<face>"
+    returns "The man data, in markdown format, suitable for consumption by Ronn."
+    examples <<-'EOT'
+      Get the manual page for a face:
+
+      $ puppet man facts
+    EOT
+
+    default
+    when_invoked do |name, options|
+      if legacy_applications.include? name then
+        return Puppet::Application[name].help
+      end
+
+      face = Puppet::Face[name.to_sym, :current]
+
+      file = (Pathname(__FILE__).dirname + "help" + 'man.erb')
+      erb = ERB.new(file.read, nil, '-')
+      erb.filename = file.to_s
+
+      # Run the ERB template in our current binding, including all the local
+      # variables we established just above. --daniel 2011-04-11
+      return erb.result(binding)
+    end
+  end
+
+  def legacy_applications
+    # The list of applications, less those that are duplicated as a face.
+    Puppet::Util::CommandLine.available_subcommands.reject do |appname|
+      Puppet::Face.face? appname.to_sym, :current or
+        # ...this is a nasty way to exclude non-applications. :(
+        %w{face_base indirection_base}.include? appname
+    end
+  end
+end
-- 
cgit 


From be233c33a8f857b0edc07e24f74db85d469983a4 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 1 Jun 2011 14:49:00 -0700
Subject: (#7683) Use ronn, when available, to render the output.

We now look for ronn(1), and if it is available ask it to generate the *roff
output, delegate that to man(1) directly, and show the result to the user in
their pager.

If ronn(1) isn't available we delegate to $MANPAGER, $PAGER, less, most, or
more, in that order, to paginate the raw markdown.  Not nearly so nice, but
better than doing nothing.

Finally, if none of those pagers are available we fall through to the default
behaviour of puppet rendering the output, which more or less results in a
direct dump to the console.  Nice.

Reviewed-By: Nick Fagerlund <nick.fagerlund@puppetlabs.com>
---
 lib/puppet/face/man.rb | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/lib/puppet/face/man.rb b/lib/puppet/face/man.rb
index 44a339490..38b9202eb 100644
--- a/lib/puppet/face/man.rb
+++ b/lib/puppet/face/man.rb
@@ -1,4 +1,5 @@
 require 'puppet/face'
+require 'puppet/util'
 require 'pathname'
 require 'erb'
 
@@ -8,6 +9,26 @@ Puppet::Face.define(:man, '0.0.1') do
 
   summary "Display Puppet subcommand manual pages."
 
+  description <<-EOT
+    The man face, when invoked from the command line, tries very hard to
+    behave nicely for interactive use.  If possible, it delegates to the
+    ronn(1) command to format the output as a real manual page.
+
+    If ronn(1) is not available, it will use the first of `$MANPAGER`,
+    `$PAGER`, `less`, `most`, or `more` to paginate the (human-readable)
+    input text for the manual page.
+
+    We do try hard to ensure that this behaves correctly when used as
+    part of a pipeline.  (Well, we delegate to tools that do the right
+    thing, which is more or less the same.)
+  EOT
+
+  notes <<-EOT
+    We strongly encourage you to install the `ronn` gem on your system,
+    or otherwise make it available, so that we can display well structured
+    output from this face.
+  EOT
+
   action(:man) do
     summary "Display the manual page for a face."
     arguments "<face>"
@@ -34,6 +55,33 @@ Puppet::Face.define(:man, '0.0.1') do
       # variables we established just above. --daniel 2011-04-11
       return erb.result(binding)
     end
+
+
+    when_rendering :console do |text|
+      # OK, if we have Ronn on the path we can delegate to it and override the
+      # normal output process.  Otherwise delegate to a pager on the raw text,
+      # otherwise we finally just delegate to our parent.  Oh, well.
+      ENV['LESS'] ||= 'FRSX'    # emulate git...
+
+      ronn  = Puppet::Util.which('ronn')
+      pager = [ENV['MANPAGER'], ENV['PAGER'], 'less', 'most', 'more'].
+        detect {|x| x and x.length > 0 and Puppet::Util.which(x) }
+
+      if ronn then
+        # ronn is a stupid about pager selection, we can be smarter. :)
+        if pager then ENV['PAGER'] = pager end
+
+        args  = "--man --manual='Puppet Manual' --organization='Puppet Labs, LLC'"
+        IO.popen("#{ronn} #{args}", 'w') do |fh| fh.write text end
+
+        ''                      # suppress local output, neh?
+      elsif pager then
+        IO.popen(pager, 'w') do |fh| fh.write text end
+        ''
+      else
+        text
+      end
+    end
   end
 
   def legacy_applications
-- 
cgit 


From b70f32a45a5dd5777c341e43816d54c37707cf27 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 1 Jun 2011 17:51:25 -0700
Subject: (#7746) Fix bootstrap issues from #7717 fix.

So, turns out the fix in #7717 introduced a failure when starting the Puppet
master on some versions of Ruby.  Weird stuff.  I figured it out, eventually.

You see, the process of bootstrapping Puppet is ... complex.  This file, like
many of our early initialization files, has an incestuous relationship between
the order of files loaded, code executed at load time, and code executed in
other files at runtime.

When we construct this object we have not yet actually loaded the global
puppet object, so we can't use any methods in it.  That includes all the
logging stuff, which is used by the deprecation warning subsystem.

On the other hand, we can't just load the logging system, because that depends
on the top level Puppet module being bootstrapped.  It doesn't actually load
the stuff it uses, though, for hysterical raisins.

Finally, we can't actually just load the top level Puppet module.  This one is
precious: it turns out that some of the code loaded in the top level Puppet
module has a dependency on the run mode values.

Run mode is set correctly *only* when the application is loaded, and if it is
wrong when the top level code is brought in we end up with the wrong settings
scattered through some of the defaults.

Which means that we have a dependency cycle that runs:
1. The binary creates an instance of P::U::CL.
2. That identifies the application to load.
3. It does, then instantiates the application.
   4. That sets the run-mode.
   5. That then loads the top level Puppet module.
6. Finally, we get to where we can use the top level stuff

So, essentially, we see a dependency between runtime code in this file,
run-time code in the application, and load-time code in the top level module.

Which leads me to our current horrible hack: we stash away the message we
wanted to log about deprecation, then send it to our logging system once we
have done enough bootstrapping that it will, y'know, actually work.

I would have liked to fix this, but that is going to be a whole pile of work
digging through and decrufting all the global state from the local state, and
working out what depends on what else in the product.

Oh, and we use a global because we have *two* instances of a P::U::CL object
during the startup sequence.  I don't know why.

Reviewed-By: Nigel Kersten <nigel@puppetlabs.com>
Reviewed-By: Nick Lewis <nick@puppetlabs.com>
Reviewed-By: Jacob Helwig <jacob@puppetlabs.com>
---
 lib/puppet/util/command_line.rb | 71 +++++++++++++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 9 deletions(-)

diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb
index 372aa5122..244e2c2c9 100644
--- a/lib/puppet/util/command_line.rb
+++ b/lib/puppet/util/command_line.rb
@@ -59,6 +59,13 @@ module Puppet
           require_application subcommand_name
           app = Puppet::Application.find(subcommand_name).new(self)
           Puppet::Plugins.on_application_initialization(:appliation_object => self)
+
+          # See the note in 'warn_later' down below. --daniel 2011-06-01
+          if $delayed_deprecation_warning_for_p_u_cl.is_a? String then
+            Puppet.deprecation_warning($delayed_deprecation_warning_for_p_u_cl)
+            $delayed_deprecation_warning_for_p_u_cl = true
+          end
+
           app.run
         elsif execute_external_subcommand then
           # Logically, we shouldn't get here, but we do, so whatever.  We just
@@ -98,7 +105,7 @@ module Puppet
               [nil, argv]       # ttys get usage info
             else
               # Killed for 2.7.0 --daniel 2011-06-01
-              Puppet.deprecation_warning <<EOM
+              warn_later <<EOM
 Implicit invocation of 'puppet apply' by redirection into 'puppet' is deprecated,
 and will be removed in the 2.8 series. Please invoke 'puppet apply' directly
 in the future.
@@ -110,7 +117,7 @@ EOM
             [nil, argv]
           when /^-|\.pp$|\.rb$/ then
             # Killed for 2.7.0 --daniel 2011-06-01
-            Puppet.deprecation_warning <<EOM
+            warn_later <<EOM
 Implicit invocation of 'puppet apply' by passing files (or flags) directly
 to 'puppet' is deprecated, and will be removed in the 2.8 series.  Please
 invoke 'puppet apply' directly in the future.
@@ -124,13 +131,59 @@ EOM
         end
       end
 
+      # So, this is more than a little bit of a horror.  You see, the process
+      # of bootstrapping Puppet is ... complex.  This file, like many of our
+      # early initialization files, has an incestuous relationship between the
+      # order of files loaded, code executed at load time, and code executed
+      # in other files at runtime.
+      #
+      # When we construct this object we have not yet actually loaded the
+      # global puppet object, so we can't use any methods in it.  That
+      # includes all the logging stuff, which is used by the deprecation
+      # warning subsystem.
+      #
+      # On the other hand, we can't just load the logging system, because that
+      # depends on the top level Puppet module being bootstrapped.  It doesn't
+      # actually load the stuff it uses, though, for hysterical raisins.
+      #
+      # Finally, we can't actually just load the top level Puppet module.
+      # This one is precious: it turns out that some of the code loaded in the
+      # top level Puppet module has a dependency on the run mode values.
+      #
+      # Run mode is set correctly *only* when the application is loaded, and
+      # if it is wrong when the top level code is brought in we end up with
+      # the wrong settings scattered through some of the defaults.
+      #
+      # Which means that we have a dependency cycle that runs:
+      # 1. The binary creates an instance of P::U::CL.
+      # 2. That identifies the application to load.
+      # 3. It does, then instantiates the application.
+      #    4. That sets the run-mode.
+      #    5. That then loads the top level Puppet module.
+      # 6. Finally, we get to where we can use the top level stuff
+      #
+      # So, essentially, we see a dependency between runtime code in this
+      # file, run-time code in the application, and load-time code in the top
+      # level module.
+      #
+      # Which leads me to our current horrible hack: we stash away the message
+      # we wanted to log about deprecation, then send it to our logging system
+      # once we have done enough bootstrapping that it will, y'know, actually
+      # work.
+      #
+      # I would have liked to fix this, but that is going to be a whole pile
+      # of work digging through and decrufting all the global state from the
+      # local state, and working out what depends on what else in the product.
+      #
+      # Oh, and we use a global because we have *two* instances of a P::U::CL
+      # object during the startup sequence.  I don't know why.
+      #
+      # Maybe, one day, when I am not behind a deadline to ship code.
+      # --daniel 2011-06-01
+      def warn_later(text)
+        return if $delayed_deprecation_warning_for_p_u_cl
+        $delayed_deprecation_warning_for_p_u_cl = text
+      end
     end
   end
 end
-
-# REVISIT: Without this, we can't use the full Puppet suite inside the code,
-# but we can't include it before we get this far through defining things as
-# this code is the product of incestuous union with the global code; global
-# code at load scope depends on methods we define, while we only depend on
-# them at runtime scope.  --daniel 2011-06-01
-require 'puppet'
-- 
cgit