Greetings!
Please review the pull request #168: Ticket/2.7.x/9636 onlyif unless windows exec parameters opened by (joshcooper)
Some more information about the pull request:
- Opened: Wed Oct 12 22:34:00 UTC 2011
- Based on: puppetlabs:2.7.x (5e3962c5b149c1eeaf62db9090b665b45ad8a128)
- Requested merge: joshcooper:ticket/2.7.x/9636-onlyif-unless-windows-exec (5721ab9d0effb56cfc2d498966712fa8efc42eb8)
Description:
This patch series fixes issues with Puppet::Util.which and Puppet::Util.execute on Windows. In doing so it resolves, issue #9636 whereby the onlyif and unless exec parameters were not working on windows.
Thanks!
The Pull Request Bot
Diff follows:
diff --git a/lib/puppet/provider/exec/windows.rb b/lib/puppet/provider/exec/windows.rb
index 9ce29f1..09eb883 100644
--- a/lib/puppet/provider/exec/windows.rb
+++ b/lib/puppet/provider/exec/windows.rb
@@ -23,11 +23,10 @@ only directly calls the command with the arguments given."
return
end
- path = resource[:path] || []
-
- exts = [".exe", ".ps1", ".bat", ".com", ""]
- withenv :PATH => path.join(File::PATH_SEPARATOR) do
- return if exts.any? {|ext| which(exe + ext) }
+ if resource[:path]
+ withenv :PATH => resource[:path].join(File::PATH_SEPARATOR) do
+ return if which(exe)
+ end
end
raise ArgumentError, "Could not find command '#{exe}'"
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 51fd4d7..d69eb77 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -196,8 +196,8 @@ module Util
exts = ENV['PATHEXT']
exts = exts ? exts.split(File::PATH_SEPARATOR) : %w[.COM .EXE .BAT .CMD]
exts.each do |ext|
- bin = File.expand_path(dest + ext)
- return bin if FileTest.file? bin and FileTest.executable? bin
+ destext = File.expand_path(dest + ext)
+ return destext if FileTest.file? destext and FileTest.executable? destext
end
end
return dest if FileTest.file? dest and FileTest.executable? dest
@@ -358,17 +358,21 @@ module Util
stdout = arguments[:squelch] ? File.open(null_file, 'w') : Tempfile.new('puppet')
stderr = arguments[:combine] ? stdout : File.open(null_file, 'w')
-
exec_args = [command, arguments, stdin, stdout, stderr]
if execution_stub = Puppet::Util::ExecutionStub.current_value
return execution_stub.call(*exec_args)
elsif Puppet.features.posix?
child_pid = execute_posix(*exec_args)
- child_status = Process.waitpid2(child_pid).last.exitstatus
+ exit_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
+ exit_status = Process.waitpid2(child_pid).last
+ # $CHILD_STATUS is not set when calling win32/process Process.create
+ # and since it's read-only, we can't set it. But we can execute a
+ # a shell that simply returns the desired exit status, which has the
+ # desired effect.
+ %x{#{ENV['COMSPEC']} /c exit #{exit_status}}
end
[stdin, stdout, stderr].each {|io| io.close rescue nil}
@@ -379,8 +383,8 @@ module Util
Puppet.warning "Could not get output" unless output
end
- if arguments[:failonfail] and child_status != 0
- raise ExecutionFailure, "Execution of '#{str}' returned #{child_status}: #{output}"
+ if arguments[:failonfail] and exit_status != 0
+ raise ExecutionFailure, "Execution of '#{str}' returned #{exit_status}: #{output}"
end
output
diff --git a/spec/integration/type/exec_spec.rb b/spec/integration/type/exec_spec.rb
new file mode 100755
index 0000000..3a29dea
--- /dev/null
+++ b/spec/integration/type/exec_spec.rb
@@ -0,0 +1,77 @@
+#!/usr/bin/env rspec
+require 'spec_helper'
+
+require 'puppet_spec/files'
+
+describe Puppet::Type.type(:exec) do
+ include PuppetSpec::Files
+
+ let(:catalog) { Puppet::Resource::Catalog.new }
+ let(:path) { tmpfile('exec_provider') }
+ let(:command) { "ruby -e 'File.open(\"#{path}\", \"w\") { |f| f.print \"foo\" }'" }
+
+ before :each do
+ catalog.host_config = false
+ end
+
+ it "should execute the command" do
+ exec = described_class.new :command => command, :path => ENV['PATH']
+
+ catalog.add_resource exec
+ catalog.apply
+
+ File.read(path).should == 'foo'
+ end
+
+ it "should not execute the command if onlyif returns non-zero" do
+ exec = described_class.new(
+ :command => command,
+ :_onlyif_ => "ruby -e 'exit 44'",
+ :path => ENV['PATH']
+ )
+
+ catalog.add_resource exec
+ catalog.apply
+
+ File.should_not be_exist(path)
+ end
+
+ it "should execute the command if onlyif returns zero" do
+ exec = described_class.new(
+ :command => command,
+ :_onlyif_ => "ruby -e 'exit 0'",
+ :path => ENV['PATH']
+ )
+
+ catalog.add_resource exec
+ catalog.apply
+
+ File.read(path).should == 'foo'
+ end
+
+ it "should execute the command if unless returns non-zero" do
+ exec = described_class.new(
+ :command => command,
+ :unless => "ruby -e 'exit 45'",
+ :path => ENV['PATH']
+ )
+
+ catalog.add_resource exec
+ catalog.apply
+
+ File.read(path).should == 'foo'
+ end
+
+ it "should not execute the command if unless returns zero" do
+ exec = described_class.new(
+ :command => command,
+ :unless => "ruby -e 'exit 0'",
+ :path => ENV['PATH']
+ )
+
+ catalog.add_resource exec
+ catalog.apply
+
+ File.should_not be_exist(path)
+ end
+end
diff --git a/spec/integration/util_spec.rb b/spec/integration/util_spec.rb
old mode 100644
new mode 100755
index a50f783..2b596fd
--- a/spec/integration/util_spec.rb
+++ b/spec/integration/util_spec.rb
@@ -9,5 +9,21 @@ describe Puppet::Util do
Puppet::Util.execute(command, :combine => true).split.should =~ [*'1'..'10']
end
+
+ it "should return output and set $CHILD_STATUS" do
+ command = "ruby -e 'puts \"foo\"; exit 42'"
+
+ output = Puppet::Util.execute(command, {:failonfail => false})
+
+ output.should == "foo\n"
+ $CHILD_STATUS.exitstatus.should == 42
+ end
+
+ it "should raise an error if non-zero exit status is returned" do
+ command = "ruby -e 'exit 43'"
+
+ expect { Puppet::Util.execute(command) }.to raise_error(Puppet::ExecutionFailure, /Execution of '#{command}' returned 43: /)
+ $CHILD_STATUS.exitstatus.should == 43
+ end
end
end
diff --git a/spec/unit/provider/exec/windows_spec.rb b/spec/unit/provider/exec/windows_spec.rb
old mode 100644
new mode 100755
index f566ad2..28438de
--- a/spec/unit/provider/exec/windows_spec.rb
+++ b/spec/unit/provider/exec/windows_spec.rb
@@ -77,19 +77,12 @@ describe Puppet::Type.type(:exec).provider(:windows) do
end
it "should search for executables with no extension" do
+ provider.resource[:path] = [File.expand_path('/bogus/bin')]
provider.expects(:which).with('foo').returns('foo')
provider.checkexe('foo')
end
- %w[bat com ps1 exe].each do |ext|
- it "should search for executables with the extension '#{ext}'" do
- provider.expects(:which).with("foo.#{ext}").returns("foo.#{ext}")
-
- provider.checkexe('foo')
- end
- end
-
it "should fail if the command isn't in the path" do
expect { provider.checkexe('foo') }.to raise_error(ArgumentError, "Could not find command 'foo'")
end
diff --git a/spec/unit/util_spec.rb b/spec/unit/util_spec.rb
index 8ac8017..baf3250 100755
--- a/spec/unit/util_spec.rb
+++ b/spec/unit/util_spec.rb
@@ -498,10 +498,13 @@ describe Puppet::Util do
describe "when a file extension is not specified" do
it "should walk each extension in PATHEXT until an executable is found" do
- ENV.stubs(:[]).with('PATH').returns(base)
+ bar = File.expand_path('/bar')
+ ENV.stubs(:[]).with('PATH').returns("#{bar}#{File::PATH_SEPARATOR}#{base}")
ENV.stubs(:[]).with('PATHEXT').returns(".EXE#{File::PATH_SEPARATOR}.CMD")
exts = sequence('extensions')
+ FileTest.expects(:file?).in_sequence(exts).with(File.join(bar, 'foo.EXE')).returns false
+ FileTest.expects(:file?).in_sequence(exts).with(File.join(bar, 'foo.CMD')).returns false
FileTest.expects(:file?).in_sequence(exts).with(File.join(base, 'foo.EXE')).returns false
FileTest.expects(:file?).in_sequence(exts).with(path).returns 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.
