Greetings!
Please review the pull request #112: (#8410) Fix child exit status on Windows opened by (joshcooper)
Some more information about the pull request:
- Opened: Wed Sep 14 16:19:26 UTC 2011
- Based on: puppetlabs:2.7.x (a20551f4343765b850562060e4f41d4393cdf31e)
- Requested merge: joshcooper:ticket/2.7.x/8410-waitpid2-on-windows (b27b013060169f372adb43c71ed8623d60556d69)
Description:
The Process.waitpid2 method in the win32-process gem returns an array,
with the child's exit status as the last element. This is different
than ruby's implementation where the last element is a Process::Status
object, which has an exitstatus method.
As a result, if a child process returned a non-zero exit status on
Windows, then Puppet::Util.execute would raise an error while trying
to call the exitstatus method on a Fixnum (the actual exit status).
This commit ensures we consistently retrieve the exit status
across all platforms.
Thanks!
The Pull Request Bot
Diff follows:
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 80ce963..2ce2ad8 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -309,12 +309,12 @@ module Util
return execution_stub.call(*exec_args)
elsif Puppet.features.posix?
child_pid = execute_posix(*exec_args)
+ child_status = Process.waitpid2(child_pid).last.exitstatus
elsif Puppet.features.microsoft_windows?
child_pid = execute_windows(*exec_args)
+ child_status = Process.waitpid2(child_pid).last
end
- child_status = Process.waitpid2(child_pid).last
-
[stdin, stdout, stderr].each {|io| io.close rescue nil}
# read output in if required
@@ -324,7 +324,7 @@ module Util
end
if arguments[:failonfail] and child_status != 0
- raise ExecutionFailure, "Execution of '#{str}' returned #{child_status.exitstatus}: #{output}"
+ raise ExecutionFailure, "Execution of '#{str}' returned #{child_status}: #{output}"
end
output
diff --git a/spec/unit/util_spec.rb b/spec/unit/util_spec.rb
old mode 100644
new mode 100755
index 26a9bd7..8644c9b
--- a/spec/unit/util_spec.rb
+++ b/spec/unit/util_spec.rb
@@ -3,6 +3,12 @@
require 'spec_helper'
describe Puppet::Util do
+ def process_status(exitstatus)
+ return exitstatus if Puppet.features.microsoft_windows?
+
+ stub('child_status', :exitstatus => exitstatus)
+ end
+
describe "#absolute_path?" do
it "should default to the platform of the local system" do
Puppet.features.stubs(:posix?).returns(true)
@@ -122,7 +128,7 @@ describe Puppet::Util do
before :each do
Process.stubs(:create).returns(proc_info_stub)
- Process.stubs(:waitpid2).with(pid).returns([pid, 0])
+ Process.stubs(:waitpid2).with(pid).returns([pid, process_status(0)])
@stdin = File.open(null_file, 'r')
@stdout = Tempfile.new('stdout')
@@ -153,7 +159,7 @@ describe Puppet::Util do
describe "#execute" do
before :each do
- Process.stubs(:waitpid2).with(pid).returns([pid, 0])
+ Process.stubs(:waitpid2).with(pid).returns([pid, process_status(0)])
end
describe "when an execution stub is specified" do
@@ -251,8 +257,9 @@ describe Puppet::Util do
describe "after execution" do
let(:executor) { Puppet.features.microsoft_windows? ? 'execute_windows' : 'execute_posix' }
+
before :each do
- Process.stubs(:waitpid2).with(pid).returns([pid, 0])
+ Process.stubs(:waitpid2).with(pid).returns([pid, process_status(0)])
Puppet::Util.stubs(executor).returns(pid)
end
@@ -260,7 +267,7 @@ describe Puppet::Util do
it "should wait for the child process to exit" do
Puppet::Util.stubs(:wait_for_output)
- Process.expects(:waitpid2).with(pid).returns([pid, 0])
+ Process.expects(:waitpid2).with(pid).returns([pid, process_status(0)])
Puppet::Util.execute('test command')
end
@@ -306,9 +313,7 @@ describe Puppet::Util do
end
it "should raise an error if failonfail is true and the child failed" do
- child_status = stub('child_status', :exitstatus => 1)
-
- Process.expects(:waitpid2).with(pid).returns([pid, child_status])
+ Process.expects(:waitpid2).with(pid).returns([pid, process_status(1)])
expect {
Puppet::Util.execute('fail command', :failonfail => true)
@@ -316,7 +321,7 @@ describe Puppet::Util do
end
it "should not raise an error if failonfail is false and the child failed" do
- Process.expects(:waitpid2).with(pid).returns([pid, 1])
+ Process.expects(:waitpid2).with(pid).returns([pid, process_status(1)])
expect {
Puppet::Util.execute('fail command', :failonfail => false)
@@ -324,7 +329,7 @@ describe Puppet::Util do
end
it "should not raise an error if failonfail is true and the child succeeded" do
- Process.expects(:waitpid2).with(pid).returns([pid, 0])
+ Process.expects(:waitpid2).with(pid).returns([pid, process_status(0)])
expect {
Puppet::Util.execute('fail command', :failonfail => true)
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
