Greetings!
Please review the pull request #128: (#9461) 'puppet resource package' fails on Windows opened by (ChaseMotorman)
Some more information about the pull request:
- Opened: Tue Sep 20 22:56:03 UTC 2011
- Based on: puppetlabs:2.7.x (18afb3a5262b1447c6bb2f21e5ecb3262d004777)
- Requested merge: ChaseMotorman:ticket/2.7.x/9491-windows-package-failure (062197842c8e617cc1f16144f0a91a5325b20836)
Description:
This commit corrects a basic failure to enumerate package resources
on Windows. In previous versions, 'package' enumeration failed due to
the provider subsystem's inability to exec any process neccesary to
do the enumerations.
The general fix expands the Puppet::Util.which command when running
on Windows to search for EXE variants if the target is not found.
For example, a search for 'gem' fails, because 'gem' (like all other
Ruby and Puppet binaries) is wrapped in a batch file, creating 'gem.bat'.
If a lookup fails on Windows, and the passed value doesn't contain a file
extension, 'which' will examine the value of the environment 'PATHEXT'
setting, which contains a delimited list of known extensions. 'which'
will append each extension to the value (in order of appearance) and
reevaluate. If no PATHEXT value exists, the code defaiults to the
basic Windows binary extensions (.bat;.ps1;.exe;.com). The first match
found wins, and the translated value is cached against future lookups.
Spec tests have been updated to reflect this change.
(#9607) MSI provider fails to enumerate instances
The second, specific change corrects resource enumeration in the Windows
MSI provider. Installed MSI package state files were not examined for
the required 'source' value. This commit now examines the YAML state
file, caches the yaml object, and adds a get-method to return the
'source' value. The 'package' type base class was also fixed to only
validate source properties for providers that extend the 'source' method.
Spec tests have been updated to reflect new and fixed code.
Depends on resolution #9461:puppet resource package' fails on Windows
Thanks!
The Pull Request Bot
Diff follows:
diff --git a/lib/puppet/provider/package/msi.rb b/lib/puppet/provider/package/msi.rb
index fc57b7c..8e2af23 100644
--- a/lib/puppet/provider/package/msi.rb
+++ b/lib/puppet/provider/package/msi.rb
@@ -20,6 +20,10 @@ Puppet::Type.type(:package).provide(:msi, :parent => Puppet::Provider::Package)
end
end
+ def source
+ yaml.nil? ? resource[:source] : yaml['source']
+ end
+
def query
{:name => resource[:name], :ensure => :installed} if FileTest.exists?(state_file)
end
@@ -79,4 +83,9 @@ Puppet::Type.type(:package).provide(:msi, :parent => Puppet::Provider::Package)
def shell_quote(value)
value.include?(' ') ? %Q["#{value.gsub(/"/, '\"')}"] : value
end
+
+ def yaml
+ @yaml ||= YAML.load_file( File.expand_path( state_file ) ) rescue nil
+ @yaml
+ end
end
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index 903834f..59a1569 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -301,7 +301,7 @@ module Puppet
end
validate do
- provider.validate_source(self[:source])
+ provider.validate_source(self.provider.source) if self.provider.respond_to?(:source)
end
autorequire(:file) do
@@ -312,9 +312,11 @@ module Puppet
end
}
- if source = self[:source]
- if source =~ /^#{File::SEPARATOR}/
- autos << source
+ if self.provider.respond_to?( :source )
+ if source = self.provider.source
+ if source =~ /^#{File::SEPARATOR}/
+ autos << source
+ end
end
end
autos
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 80ce963..8b02fbc 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -185,12 +185,29 @@ module Util
end
end
+ # keep a cache of the translated windows binaries
+ @@winbin_cache = {}
+
def which(bin)
if absolute_path?(bin)
return bin if FileTest.file? bin and FileTest.executable? bin
else
ENV['PATH'].split(File::PATH_SEPARATOR).each do |dir|
- dest=File.join(dir, bin)
+ dest=File.expand_path( File.join(dir, bin) )
+ if Puppet.features.microsoft_windows? && File.extname( dest ).empty?
+ pexts = ENV[ 'PATHEXT' ].split( ';' )
+ # Ruby sometimes converts ENV['PATHENV'] into ENV['%PATHEXT%;.rb;.rw']
+ pexts = [ ".ps1", ".bat", ".exe", ".com" ] if( pexts.empty? || ENV['PATHEXT'] =~ /\%PATHEXT\%/ )
+ pexts.each{ |ext|
+ wdest = File.expand_path( dest + ext )
+ return @@winbin_cache[ dest ] if @@winbin_cache.include? dest
+ if FileTest.file? wdest and FileTest.executable? wdest
+ # cache the value for subsequent lookups
+ @@winbin_cache[ dest ] = wdest
+ return wdest
+ end
+ }
+ end
return dest if FileTest.file? dest and FileTest.executable? dest
end
end
diff --git a/spec/unit/provider/package/msi_spec.rb b/spec/unit/provider/package/msi_spec.rb
index 88e44ef..74cf712 100644
--- a/spec/unit/provider/package/msi_spec.rb
+++ b/spec/unit/provider/package/msi_spec.rb
@@ -15,6 +15,7 @@ describe 'Puppet::Provider::Package::Msi' do
:name => 'mysql-5.1.58-winx64',
:source => 'E:\mysql-5.1.58-winx64.msi'
)
+
resource.provider.stubs(:execute)
resource.provider.install
@@ -90,6 +91,7 @@ describe 'Puppet::Provider::Package::Msi' do
end
it 'should remove the state file' do
+ Puppet::Type.type(:package).provider(:msi).any_instance.stubs(:source).returns('E:\mysql-5.1.58-winx64.msi')
resource = Puppet::Type.type(:package).new(
:name => 'mysql-5.1.58-winx64',
:source => 'E:\mysql-5.1.58-winx64.msi'
@@ -101,10 +103,12 @@ describe 'Puppet::Provider::Package::Msi' do
end
it 'should leave the state file if uninstalling fails' do
+ Puppet::Type.type(:package).provider(:msi).any_instance.stubs(:source).returns('E:\mysql-5.1.58-winx64.msi')
resource = Puppet::Type.type(:package).new(
:name => 'mysql-5.1.58-winx64',
:source => 'E:\mysql-5.1.58-winx64.msi'
)
+
resource.provider.stubs(:msiexec).raises(Puppet::ExecutionFailure.new("Execution of 'msiexec.exe' returned 128: Blargle"))
expect { resource.provider.uninstall }.to raise_error(Puppet::ExecutionFailure, /msiexec\.exe/)
@@ -112,6 +116,7 @@ describe 'Puppet::Provider::Package::Msi' do
end
it 'should fail if the source parameter is not set' do
+ Puppet::Type.type(:package).provider(:msi).any_instance.stubs(:source).returns(nil)
expect do
resource = Puppet::Type.type(:package).new(
:name => 'mysql-5.1.58-winx64'
@@ -120,6 +125,7 @@ describe 'Puppet::Provider::Package::Msi' do
end
it 'should fail if the source parameter is empty' do
+ Puppet::Type.type(:package).provider(:msi).any_instance.stubs(:source).returns('')
expect do
resource = Puppet::Type.type(:package).new(
:name => 'mysql-5.1.58-winx64',
@@ -148,6 +154,7 @@ describe 'Puppet::Provider::Package::Msi' do
FileUtils.mkdir_p(@state_dir)
File.open(File.join(@state_dir, 'mysql-5.1.58-winx64.yml'), 'w') {|f| f.puts 'Hello'}
+ Puppet::Type.type(:package).provider(:msi).any_instance.stubs(:source).returns('E:\mysql-5.1.58-winx64.msi')
resource = Puppet::Type.type(:package).new(
:name => 'mysql-5.1.58-winx64',
:source => 'E:\mysql-5.1.58-winx64.msi'
@@ -167,4 +174,13 @@ describe 'Puppet::Provider::Package::Msi' do
resource.provider.query.should be_nil
end
+
+ it "should return the package name as the 'source' property" do
+ resource = Puppet::Type.type(:package).new(
+ :name => 'mysql-5.1.58-winx64',
+ :source => 'E:\mysql-5.1.58-winx64.msi'
+ )
+
+ resource.provider.source.should == 'E:\mysql-5.1.58-winx64.msi'
+ end
end
diff --git a/spec/unit/util_spec.rb b/spec/unit/util_spec.rb
index 26a9bd7..9b3c322 100644
--- a/spec/unit/util_spec.rb
+++ b/spec/unit/util_spec.rb
@@ -332,4 +332,14 @@ describe Puppet::Util do
end
end
end
+
+ describe "#which" do
+ it 'should search for Windows binary when passed a filename without extension' do
+ Puppet.features.stubs(:posix?).returns(false)
+ Puppet.features.stubs(:microsoft_windows?).returns(true)
+ Puppet::Util.expects( 'which' ).with( 'foo' ).returns( 'foo.bat' )
+ Puppet::Util.which( 'foo' ).should == 'foo.bat'
+ end
+ end
+
end
-- 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.
