Merge pull request #9294 from joshcooper/PUP-11655

(PUP-11655) Use run_mode for better platform independence

lib/puppet/defaults.rb

@@ -47,29 +47,15 @@ def self.default_cadir
  end
 
  def self.default_basemodulepath
- if Puppet::Util::Platform.windows?
- path = ['$codedir/modules']
- installdir = ENV.fetch("FACTER_env_windows_installdir", nil)
- if installdir
- path << "#{installdir}/puppet/modules"
- end
- path.join(File::PATH_SEPARATOR)
- else
- '$codedir/modules:/opt/puppetlabs/puppet/modules'
+ path = ['$codedir/modules']
+ if (run_mode_dir = Puppet.run_mode.common_module_dir)
+ path << run_mode_dir
  end
+ path.join(File::PATH_SEPARATOR)
  end
 
  def self.default_vendormoduledir
- if Puppet::Util::Platform.windows?
- installdir = ENV.fetch("FACTER_env_windows_installdir", nil)
- if installdir
- "#{installdir}\\puppet\\vendor_modules"
- else
- nil
- end
- else
- '/opt/puppetlabs/puppet/vendor_modules'
- end
+ Puppet.run_mode.vendor_module_dir
  end
 
  ############################################################################################

lib/puppet/provider/package/gem.rb

@@ -83,6 +83,7 @@ def self.execute_gem_command(command, command_options, custom_environment = {})
  custom_environment[:PATH] = windows_path_without_puppet_bin
  end
 
+ # This uses an unusual form of passing the command and args as [<cmd>, [<arg1>, <arg2>, ...]]
  execute(cmd, { :failonfail => true, :combine => true, :custom_environment => custom_environment })
  end
 

lib/puppet/provider/package/puppet_gem.rb

@@ -8,20 +8,7 @@
 
  confine :true => Puppet.runtime[:facter].value(:aio_agent_version)
 
- def self.windows_gemcmd
- puppet_dir = ENV.fetch('PUPPET_DIR', nil)
- if puppet_dir
- File.join(puppet_dir.to_s, 'bin', 'gem.bat')
- else
- File.join(Gem.default_bindir, 'gem.bat')
- end
- end
-
- if Puppet::Util::Platform.windows?
- commands :gemcmd => windows_gemcmd
- else
- commands :gemcmd => "/opt/puppetlabs/puppet/bin/gem"
- end
+ commands :gemcmd => Puppet.run_mode.gem_cmd
 
  def uninstall
  super
@@ -30,7 +17,9 @@ def uninstall
  end
 
  def self.execute_gem_command(command, command_options, custom_environment = {})
- custom_environment['PKG_CONFIG_PATH'] = '/opt/puppetlabs/puppet/lib/pkgconfig' unless Puppet::Util::Platform.windows?
+ if (pkg_config_path = Puppet.run_mode.pkg_config_path)
+ custom_environment['PKG_CONFIG_PATH'] = pkg_config_path
+ end
  super(command, command_options, custom_environment)
  end
 end

lib/puppet/util/run_mode.rb

@@ -87,6 +87,22 @@ def run_dir
  def log_dir
  which_dir("/var/log/puppetlabs/puppet", "~/.puppetlabs/var/log")
  end
+
+ def pkg_config_path
+ '/opt/puppetlabs/puppet/lib/pkgconfig'
+ end
+
+ def gem_cmd
+ '/opt/puppetlabs/puppet/bin/gem'
+ end
+
+ def common_module_dir
+ '/opt/puppetlabs/puppet/modules'
+ end
+
+ def vendor_module_dir
+ '/opt/puppetlabs/puppet/vendor_modules'
+ end
  end
 
  class WindowsRunMode < RunMode
@@ -114,8 +130,32 @@ def log_dir
  which_dir(File.join(windows_common_base("puppet/var/log")), "~/.puppetlabs/var/log")
  end
 
+ def pkg_config_path
+ nil
+ end
+
+ def gem_cmd
+ if (puppet_dir = ENV.fetch('PUPPET_DIR', nil))
+ File.join(puppet_dir.to_s, 'bin', 'gem.bat')
+ else
+ File.join(Gem.default_bindir, 'gem.bat')
+ end
+ end
+
+ def common_module_dir
+ "#{installdir}/puppet/modules" if installdir
+ end
+
+ def vendor_module_dir
+ "#{installdir}\\puppet\\vendor_modules" if installdir
+ end
+
  private
 
+ def installdir
+ ENV.fetch('FACTER_env_windows_installdir', nil)
+ end
+
  def windows_common_base(*extra)
  [ENV.fetch('ALLUSERSPROFILE', nil), "PuppetLabs"] + extra
  end

spec/unit/provider/package/puppet_gem_spec.rb

@@ -20,42 +20,26 @@
  let(:provider_gem_cmd) { '/opt/puppetlabs/puppet/bin/gem' }
  end
 
- custom_environment = {"HOME"=>ENV["HOME"]}
- custom_environment['PKG_CONFIG_PATH'] = '/opt/puppetlabs/puppet/lib/pkgconfig'
-
- let(:execute_options) { {:failonfail => true, :combine => true, :custom_environment => custom_environment} }
+ let(:execute_options) do
+ {
+ failonfail: true,
+ combine: true,
+ custom_environment: {
+ 'HOME'=>ENV['HOME'],
+ 'PKG_CONFIG_PATH' => '/opt/puppetlabs/puppet/lib/pkgconfig'
+ }
+ }
+ end
 
  before :each do
  resource.provider = provider
- allow(described_class).to receive(:command).with(:gemcmd).and_return(provider_gem_cmd)
- allow(Puppet::Util::Platform).to receive(:windows?).and_return(false)
- end
-
-
- describe '.windows_gemcmd' do
- context 'when PUPPET_DIR is not set' do
- before do
- # allow(ENV).to receive(:fetch, anything).and_call_original
- allow(ENV).to receive(:fetch).with('PUPPET_DIR', anything).and_return(nil)
- allow(Gem).to receive(:default_bindir).and_return('default_gem_bin')
- end
-
- it 'uses Gem.default_bindir' do
- expected_path = File.join('default_gem_bin', 'gem.bat')
- expect(described_class.windows_gemcmd).to eql(expected_path)
- end
- end
-
- context 'when PUPPET_DIR is set' do
- before do
- allow(ENV).to receive(:fetch).with('PUPPET_DIR', anything).and_return('puppet_dir')
- end
-
- it 'uses Gem.default_bindir' do
- expected_path = File.join('puppet_dir', 'bin', 'gem.bat')
- expect(described_class.windows_gemcmd).to eql(expected_path)
- end
+ if Puppet::Util::Platform.windows?
+ # provider is loaded before we can stub, so stub the class we're testing
+ allow(provider.class).to receive(:command).with(:gemcmd).and_return(provider_gem_cmd)
+ else
+ allow(provider.class).to receive(:which).with(provider_gem_cmd).and_return(provider_gem_cmd)
  end
+ allow(File).to receive(:file?).with(provider_gem_cmd).and_return(true)
  end
 
  context "when installing" do
@@ -64,45 +48,43 @@
  end
 
  it "should use the path to the gem command" do
- allow(described_class).to receive(:validate_command).with(provider_gem_cmd)
- expect(described_class).to receive(:execute).with(be_a(Array), execute_options) { |args| expect(args[0]).to eq(provider_gem_cmd) }.and_return('')
+ expect(described_class).to receive(:execute).with([provider_gem_cmd, be_an(Array)], be_a(Hash)).and_return('')
  provider.install
  end
 
  it "should not append install_options by default" do
- expect(described_class).to receive(:execute_gem_command).with(provider_gem_cmd, %w{install --no-rdoc --no-ri myresource}).and_return('')
+ expect(described_class).to receive(:execute).with([provider_gem_cmd, %w{install --no-rdoc --no-ri myresource}], anything).and_return('')
  provider.install
  end
 
  it "should allow setting an install_options parameter" do
  resource[:install_options] = [ '--force', {'--bindir' => '/usr/bin' } ]
- expect(described_class).to receive(:execute_gem_command).with(provider_gem_cmd, %w{install --force --bindir=/usr/bin --no-rdoc --no-ri myresource}).and_return('')
+ expect(described_class).to receive(:execute).with([provider_gem_cmd, %w{install --force --bindir=/usr/bin --no-rdoc --no-ri myresource}], anything).and_return('')
  provider.install
  end
  end
 
  context "when uninstalling" do
  it "should use the path to the gem command" do
- allow(described_class).to receive(:validate_command).with(provider_gem_cmd)
- expect(described_class).to receive(:execute).with(be_a(Array), execute_options) { |args| expect(args[0]).to eq(provider_gem_cmd) }.and_return('')
+ expect(described_class).to receive(:execute).with([provider_gem_cmd, be_an(Array)], be_a(Hash)).and_return('')
  provider.uninstall
  end
 
  it "should not append uninstall_options by default" do
- expect(described_class).to receive(:execute_gem_command).with(provider_gem_cmd, %w{uninstall --executables --all myresource}).and_return('')
+ expect(described_class).to receive(:execute).with([provider_gem_cmd, %w{uninstall --executables --all myresource}], anything).and_return('')
  provider.uninstall
  end
 
  it "should allow setting an uninstall_options parameter" do
  resource[:uninstall_options] = [ '--force', {'--bindir' => '/usr/bin' } ]
- expect(described_class).to receive(:execute_gem_command).with(provider_gem_cmd, %w{uninstall --executables --all myresource --force --bindir=/usr/bin}).and_return('')
+ expect(described_class).to receive(:execute).with([provider_gem_cmd, %w{uninstall --executables --all myresource --force --bindir=/usr/bin}], anything).and_return('')
  provider.uninstall
  end
 
  it 'should invalidate the rubygems cache' do
  gem_source = double('gem_source')
  allow(Puppet::Util::Autoload).to receive(:gem_source).and_return(gem_source)
- expect(described_class).to receive(:execute_gem_command).with(provider_gem_cmd, %w{uninstall --executables --all myresource}).and_return('')
+ expect(described_class).to receive(:execute).with([provider_gem_cmd, %w{uninstall --executables --all myresource}], anything).and_return('')
  expect(gem_source).to receive(:clear_paths)
  provider.uninstall
  end
@@ -125,5 +107,4 @@
  it { is_expected.to be > 100 }
  end
  end
-
 end

spec/unit/provider/package/puppetserver_gem_spec.rb

@@ -16,57 +16,57 @@
 
  let(:provider_gem_cmd) { '/opt/puppetlabs/bin/puppetserver' }
 
- custom_environment = { HOME: ENV['HOME'] }
-
- let(:execute_options) { { failonfail: true, combine: true, custom_environment: custom_environment } }
+ let(:execute_options) do
+ { failonfail: true, combine: true, custom_environment: { 'HOME' => ENV['HOME'] } }
+ end
 
  before :each do
  resource.provider = provider
  allow(Puppet::Util).to receive(:which).with(provider_gem_cmd).and_return(provider_gem_cmd)
+ allow(File).to receive(:file?).with(provider_gem_cmd).and_return(true)
  end
 
  describe "#install" do
  it "uses the path to the gem command" do
- allow(described_class).to receive(:validate_command).with(provider_gem_cmd)
- expect(Puppet::Util::Execution).to receive(:execute).with(be_a(Array), execute_options) { |args| expect(args[0]).to eq(provider_gem_cmd) }.and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, be_an(Array)], be_a(Hash)).and_return('')
  provider.install
  end
 
  it "appends version if given" do
  resource[:ensure] = ['1.2.1']
- expect(described_class).to receive(:puppetservercmd).with(%w{gem install -v 1.2.1 --no-document myresource}).and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, %w{gem install -v 1.2.1 --no-document myresource}], anything).and_return('')
  provider.install
  end
 
  context "with install_options" do
  it "does not append the parameter by default" do
- expect(described_class).to receive(:puppetservercmd).with(%w{gem install --no-document myresource}).and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, %w{gem install --no-document myresource}], anything).and_return('')
  provider.install
  end
 
  it "allows setting the parameter" do
  resource[:install_options] = [ '--force', {'--bindir' => '/usr/bin' } ]
- expect(described_class).to receive(:puppetservercmd).with(%w{gem install --force --bindir=/usr/bin --no-document myresource}).and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, %w{gem install --force --bindir=/usr/bin --no-document myresource}], anything).and_return('')
  provider.install
  end
  end
 
  context "with source" do
  it "correctly sets http source" do
  resource[:source] = 'http://rubygems.com'
- expect(described_class).to receive(:puppetservercmd).with(%w{gem install --no-document --source http://rubygems.com myresource}).and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, %w{gem install --no-document --source http://rubygems.com myresource}], anything).and_return('')
  provider.install
  end
 
  it "correctly sets local file source" do
  resource[:source] = 'paint-2.2.0.gem'
- expect(described_class).to receive(:puppetservercmd).with(%w{gem install --no-document paint-2.2.0.gem}).and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, %w{gem install --no-document paint-2.2.0.gem}], anything).and_return('')
  provider.install
  end
 
  it "correctly sets local file source with URI scheme" do
  resource[:source] = 'file:///root/paint-2.2.0.gem'
- expect(described_class).to receive(:puppetservercmd).with(%w{gem install --no-document /root/paint-2.2.0.gem}).and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, %w{gem install --no-document /root/paint-2.2.0.gem}], anything).and_return('')
  provider.install
  end
 
@@ -84,20 +84,19 @@
 
  describe "#uninstall" do
  it "uses the path to the gem command" do
- allow(described_class).to receive(:validate_command).with(provider_gem_cmd)
- expect(Puppet::Util::Execution).to receive(:execute).with(be_a(Array), execute_options) { |args| expect(args[0]).to eq(provider_gem_cmd) }.and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, be_an(Array)], be_a(Hash)).and_return('')
  provider.uninstall
  end
 
  context "with uninstall_options" do
  it "does not append the parameter by default" do
- expect(described_class).to receive(:puppetservercmd).with(%w{gem uninstall --executables --all myresource}).and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, %w{gem uninstall --executables --all myresource}], anything).and_return('')
  provider.uninstall
  end
 
  it "allows setting the parameter" do
  resource[:uninstall_options] = [ '--force', {'--bindir' => '/usr/bin' } ]
- expect(described_class).to receive(:puppetservercmd).with(%w{gem uninstall --executables --all myresource --force --bindir=/usr/bin}).and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, %w{gem uninstall --executables --all myresource --force --bindir=/usr/bin}], anything).and_return('')
  provider.uninstall
  end
  end
@@ -106,14 +105,17 @@
  describe ".gemlist" do
  context "listing installed packages" do
  it "uses the puppet_gem provider_command to list local gems" do
+ allow(Puppet::Type::Package::ProviderPuppet_gem).to receive(:provider_command).and_return('/opt/puppetlabs/puppet/bin/gem')
+ allow(described_class).to receive(:validate_command).with('/opt/puppetlabs/puppet/bin/gem')
+
  expected = { name: 'world_airports', provider: :puppetserver_gem, ensure: ['1.1.3'] }
- expect(described_class).to receive(:execute_rubygems_list_command).with(['gem', 'list', '--local']).and_return(File.read(my_fixture('gem-list-local-packages')))
+ expect(Puppet::Util::Execution).to receive(:execute).with(['/opt/puppetlabs/puppet/bin/gem', %w[list --local]], anything).and_return(File.read(my_fixture('gem-list-local-packages')))
  expect(described_class.gemlist({ local: true })).to include(expected)
  end
  end
 
  it "appends the gem source if given" do
- expect(described_class).to receive(:puppetservercmd).with(%w{gem list --remote --source https://rubygems.com}).and_return('')
+ expect(Puppet::Util::Execution).to receive(:execute).with([provider_gem_cmd, %w{gem list --remote --source https://rubygems.com}], anything).and_return('')
  described_class.gemlist({ source: 'https://rubygems.com' })
  end
  end