Fix for #565: Final merge of changes from source:branches/execute-refactor into source:trunk

Generated with
svn merge -r 2378:HEAD https://reductivelabs.com/svn/puppet/branches/execute-refactor trunk

CHANGES:
 - Puppet::Util#execute now takes hash key/value pairs as arguments after the command array.
 - Processes executed from the base service provider are now silenced.  That is, their
   standard input, output, and error pipes are all directed to /dev/null.


git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2385 980ebf18-57e1-0310-9a29-db15c13687c0

2 files changed, 63 insertions, 26 deletions

diff --git a/lib/puppet/provider/service/base.rb b/lib/puppet/provider/service/base.rb
index c615c30c0..c736c20c2 100755
--- a/lib/puppet/provider/service/base.rb
+++ b/lib/puppet/provider/service/base.rb
@@ -114,12 +114,12 @@ Puppet::Type.type(:service).provide :base do
     # A simple wrapper so execution failures are a bit more informative.
     def texecute(type, command, fof = true)
         begin
-            output = execute(command, fof)
+            # #565: Services generally produce no output, so squelch them.
+            execute(command, :failonfail => fof, :squelch => true)
         rescue Puppet::ExecutionFailure => detail
             @model.fail "Could not %s %s: %s" % [type, @model.ref, detail]
         end
-
-        return output
+        return nil
     end
 
     # Use either a specified command or the default for our provider.
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 65c48fff9..8e07a27a1 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -267,7 +267,8 @@ module Util
     end
 
     # Execute the desired command, and return the status and output.
-    def execute(command, failonfail = true, uid = nil, gid = nil)
+    # def execute(command, failonfail = true, uid = nil, gid = nil)
+    def execute(command, arguments = {:failonfail => true})
         if command.is_a?(Array)
             command = command.flatten.collect { |i| i.to_s }
             str = command.join(" ")
@@ -284,30 +285,35 @@ module Util
             Puppet.debug "Executing '%s'" % str
         end
         
-        if uid
-            uid = Puppet::Util::SUIDManager.convert_xid(:uid, uid)
+        if arguments[:uid]
+            arguments[:uid] = Puppet::Util::SUIDManager.convert_xid(:uid, arguments[:uid])
         end
-        if gid
-            gid = Puppet::Util::SUIDManager.convert_xid(:gid, gid)
+        if arguments[:gid]
+            arguments[:gid] = Puppet::Util::SUIDManager.convert_xid(:gid, arguments[:gid])
         end
         
         @@os ||= Facter.value(:operatingsystem)
         output = nil
-        IO.popen("-") do |f|
-            if f
-                output = f.read
+        child_pid, child_status = nil
+        # The idea here is to avoid IO#read whenever possible.
+        if arguments[:squelch]
+            child_pid = Kernel.fork
+            if child_pid
+                # Parent process executes this
+                child_status = Process.waitpid2(child_pid)[1]
             else
+                # Child process executes this
                 begin
                     $stdin.reopen("/dev/null")
-                    $stderr.close
-                    $stderr = $stdout.dup
-                    if gid
-                        Process.egid = gid
-                        Process.gid = gid unless @@os == "Darwin"
+                    $stdout.reopen("/dev/null")
+                    $stderr.reopen("/dev/null")
+                    if arguments[:gid]
+                        Process.egid = arguments[:gid]
+                        Process.gid = arguments[:gid] unless @@os == "Darwin"
                     end
-                    if uid
-                        Process.euid = uid
-                        Process.uid = uid unless @@os == "Darwin"
+                    if arguments[:uid]
+                        Process.euid = arguments[:uid]
+                        Process.uid = arguments[:uid] unless @@os == "Darwin"
                     end
                     if command.is_a?(Array)
                         Kernel.exec(*command)
@@ -317,13 +323,44 @@ module Util
                 rescue => detail
                     puts detail.to_s
                     exit!(1)
-                end
-            end
-        end
-
-        if failonfail
-            unless $? == 0
-                raise ExecutionFailure, "Execution of '%s' returned %s: %s" % [str, $?.exitstatus, output]
+                end # begin; rescue
+            end # if child_pid; else
+        else
+            IO.popen("-") do |f|
+                if f
+                    # Parent process executes this
+                    output = f.read
+                else
+                    # Parent process executes this
+                    begin
+                        $stdin.reopen("/dev/null")
+                        $stderr.close
+                        $stderr = $stdout.dup
+                        if arguments[:gid]
+                            Process.egid = arguments[:gid]
+                            Process.gid = arguments[:gid] unless @@os == "Darwin"
+                        end
+                        if arguments[:uid]
+                            Process.euid = arguments[:uid]
+                            Process.uid = arguments[:uid] unless @@os == "Darwin"
+                        end
+                        if command.is_a?(Array)
+                            Kernel.exec(*command)
+                        else
+                            Kernel.exec(command)
+                        end
+                    rescue => detail
+                        puts detail.to_s
+                        exit!(1)
+                    end # begin; rescue
+                end # if f; else
+            end # IO.popen do |f|
+            child_status = $?
+        end # if arguments[:squelch]; else
+
+        if arguments[:failonfail]
+            unless child_status == 0
+                raise ExecutionFailure, "Execution of '%s' returned %s: %s" % [str, child_status.inspect, output]
             end
         end