From 48cd11446ab14c425b3da0fa744d4f4ed11b2f73 Mon Sep 17 00:00:00 2001 From: Thom May Date: Mon, 15 May 2017 10:52:15 +0100 Subject: Ensure that we check the embedded gem binary last Previously we injected our expected paths in to ENV['PATH'] but we stopped doing that in Chef 13; for gem_package, we need to ensure that if all else fails we'll use the Omnibus provided gem binary, but we should never pick that if we can find a different one. Closes: #6103 Signed-off-by: Thom May --- lib/chef/mixin/which.rb | 2 +- lib/chef/provider/package/rubygems.rb | 8 +++----- spec/unit/provider/package/rubygems_spec.rb | 25 ++++++++++++++----------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/lib/chef/mixin/which.rb b/lib/chef/mixin/which.rb index fd386241a0..8a825f55c9 100644 --- a/lib/chef/mixin/which.rb +++ b/lib/chef/mixin/which.rb @@ -25,7 +25,7 @@ class Chef def where(*cmds, extra_path: nil, &block) # NOTE: unnecessarily duplicates function of path_sanity extra_path ||= [ "/bin", "/usr/bin", "/sbin", "/usr/sbin" ] - paths = env_path.split(File::PATH_SEPARATOR) + extra_path + paths = env_path.split(File::PATH_SEPARATOR) + Array(extra_path) cmds.map do |cmd| paths.map do |path| filename = Chef.path_to(File.join(path, cmd)) diff --git a/lib/chef/provider/package/rubygems.rb b/lib/chef/provider/package/rubygems.rb index 69936b3d58..161f790c31 100644 --- a/lib/chef/provider/package/rubygems.rb +++ b/lib/chef/provider/package/rubygems.rb @@ -21,6 +21,7 @@ require "uri" require "chef/provider/package" require "chef/resource/package" require "chef/mixin/get_source_from_package" +require "chef/mixin/which" # Class methods on Gem are defined in rubygems require "rubygems" @@ -359,6 +360,7 @@ class Chef provides :gem_package include Chef::Mixin::GetSourceFromPackage + include Chef::Mixin::Which def initialize(new_resource, run_context = nil) super @@ -410,11 +412,7 @@ class Chef end def find_gem_by_path - Chef::Log.debug("#{new_resource} searching for 'gem' binary in path: #{ENV['PATH']}") - separator = ::File::ALT_SEPARATOR ? ::File::ALT_SEPARATOR : ::File::SEPARATOR - path_to_first_gem = ENV["PATH"].split(::File::PATH_SEPARATOR).find { |path| ::File.exist?(path + separator + "gem") } - raise Chef::Exceptions::FileNotFound, "Unable to find 'gem' binary in path: #{ENV['PATH']}" if path_to_first_gem.nil? - path_to_first_gem + separator + "gem" + which("gem", extra_path: RbConfig::CONFIG["bindir"]) end def gem_dependency diff --git a/spec/unit/provider/package/rubygems_spec.rb b/spec/unit/provider/package/rubygems_spec.rb index c40eed50cf..9f19e3602f 100644 --- a/spec/unit/provider/package/rubygems_spec.rb +++ b/spec/unit/provider/package/rubygems_spec.rb @@ -338,7 +338,7 @@ describe Chef::Provider::Package::Rubygems do let(:target_version) { nil } let(:gem_name) { "rspec-core" } let(:gem_binary) { nil } - let(:bindir) { "/usr/bin/ruby" } + let(:bindir) { "/usr/bin" } let(:options) { nil } let(:source) { nil } let(:include_default_source) { true } @@ -372,6 +372,8 @@ describe Chef::Provider::Package::Rubygems do allow(RbConfig::CONFIG).to receive(:[]).with("bindir").and_return(bindir) # Rubygems uses this interally allow(RbConfig::CONFIG).to receive(:[]).with("arch").and_call_original + allow(File).to receive(:executable?).and_return false + allow(File).to receive(:executable?).with("#{bindir}/gem").and_return true end describe "when new_resource version is nil" do @@ -439,9 +441,9 @@ describe Chef::Provider::Package::Rubygems do it "searches for a gem binary when running on Omnibus on Unix" do platform_mock :unix do allow(ENV).to receive(:[]).with("PATH").and_return("/usr/bin:/usr/sbin:/opt/chef/embedded/bin") - allow(File).to receive(:exist?).with("/usr/bin/gem").and_return(false) - allow(File).to receive(:exist?).with("/usr/sbin/gem").and_return(true) - allow(File).to receive(:exist?).with("/opt/chef/embedded/bin/gem").and_return(true) # should not get here + allow(File).to receive(:executable?).with("/usr/bin/gem").and_return(false) + allow(File).to receive(:executable?).with("/usr/sbin/gem").and_return(true) + allow(File).to receive(:executable?).with("/opt/chef/embedded/bin/gem").and_return(true) # should not get here expect(provider.gem_env.gem_binary_location).to eq("/usr/sbin/gem") end end @@ -451,13 +453,14 @@ describe Chef::Provider::Package::Rubygems do it "searches for a gem binary when running on Omnibus on Windows" do platform_mock :windows do - allow(ENV).to receive(:[]).with("PATH").and_return('C:\windows\system32;C:\windows;C:\Ruby186\bin;d:\opscode\chef\embedded\bin') - allow(File).to receive(:exist?).with('C:\\windows\\system32\\gem').and_return(false) - allow(File).to receive(:exist?).with('C:\\windows\\gem').and_return(false) - allow(File).to receive(:exist?).with('C:\\Ruby186\\bin\\gem').and_return(true) - allow(File).to receive(:exist?).with('d:\\opscode\\chef\\bin\\gem').and_return(false) # should not get here - allow(File).to receive(:exist?).with('d:\\opscode\\chef\\embedded\\bin\\gem').and_return(false) # should not get here - expect(provider.gem_env.gem_binary_location).to eq('C:\Ruby186\bin\gem') + allow(ENV).to receive(:[]).with("PATH").and_return('C:\windows\system32;C:\windows;C:\Ruby186\bin') + allow(File).to receive(:executable?).with('C:\\windows\\system32/gem').and_return(false) + allow(File).to receive(:executable?).with('C:\\windows/gem').and_return(false) + allow(File).to receive(:executable?).with('C:\\Ruby186\\bin/gem').and_return(true) + allow(File).to receive(:executable?).with('d:\\opscode\\chef\\bin/gem').and_return(false) # should not get here + allow(File).to receive(:executable?).with('d:\\opscode\\chef\\bin/gem').and_return(false) # should not get here + allow(File).to receive(:executable?).with("d:/opscode/chef/embedded/bin/gem").and_return(false) # should not get here + expect(provider.gem_env.gem_binary_location).to eq('C:\Ruby186\bin/gem') end end end -- cgit v1.2.1