Don't blindly check source on candidate_version calls.

Signed-off-by: Julian C. Dunn <jdunn@chef.io>

2 files changed, 34 insertions, 31 deletions

diff --git a/lib/chef/provider/package/aix.rb b/lib/chef/provider/package/aix.rb
index 728f181055..12bf11ad0f 100644
--- a/lib/chef/provider/package/aix.rb
+++ b/lib/chef/provider/package/aix.rb
@@ -38,7 +38,7 @@ class Chef
             a.failure_message Chef::Exceptions::Package, "Source for package #{@new_resource.name} required for action install"
           end
           requirements.assert(:all_actions) do |a|
-            a.assertion { !@new_resource.source || @package_source_found }
+            a.assertion { !@new_resource.source || package_source_found? }
             a.failure_message Chef::Exceptions::Package, "Package #{@new_resource.name} not found: #{@new_resource.source}"
             a.whyrun "would assume #{@new_resource.source} would be have previously been made available"
           end
@@ -48,24 +48,21 @@ class Chef
           @current_resource = Chef::Resource::Package.new(@new_resource.name)
           @current_resource.package_name(@new_resource.package_name)
 
-          if @new_resource.source
-            @package_source_found = ::File.exists?(@new_resource.source)
-            if @package_source_found
-              Chef::Log.debug("#{@new_resource} checking pkg status")
-              ret = shell_out_with_timeout("installp -L -d #{@new_resource.source}")
-              ret.stdout.each_line do |line|
-                case line
-                when /:#{@new_resource.package_name}:/
-                  fields = line.split(":")
-                  @new_resource.version(fields[2])
-                when /^#{@new_resource.package_name}:/
-                  Chef::Log.warn("You are installing a bff package by product name. For idempotent installs, please install individual filesets")
-                  fields = line.split(":")
-                  @new_resource.version(fields[2])
-                end
+          if package_source_found?
+            Chef::Log.debug("#{@new_resource} checking pkg status")
+            ret = shell_out_with_timeout("installp -L -d #{@new_resource.source}")
+            ret.stdout.each_line do |line|
+              case line
+              when /:#{@new_resource.package_name}:/
+                fields = line.split(":")
+                @new_resource.version(fields[2])
+              when /^#{@new_resource.package_name}:/
+                Chef::Log.warn("You are installing a bff package by product name. For idempotent installs, please install individual filesets")
+                fields = line.split(":")
+                @new_resource.version(fields[2])
               end
-              raise Chef::Exceptions::Package, "package source #{@new_resource.source} does not provide package #{@new_resource.package_name}" unless @new_resource.version
             end
+            raise Chef::Exceptions::Package, "package source #{@new_resource.source} does not provide package #{@new_resource.package_name}" unless @new_resource.version
           end
 
           Chef::Log.debug("#{@new_resource} checking install state")
@@ -88,18 +85,20 @@ class Chef
 
         def candidate_version
           return @candidate_version if @candidate_version
-          ret = shell_out_with_timeout("installp -L -d #{@new_resource.source}")
-          ret.stdout.each_line do |line|
-            case line
-            when /\w:#{Regexp.escape(@new_resource.package_name)}:(.*)/
-              fields = line.split(":")
-              @candidate_version = fields[2]
-              @new_resource.version(fields[2])
-              Chef::Log.debug("#{@new_resource} setting install candidate version to #{@candidate_version}")
+          if package_source_found?
+            ret = shell_out_with_timeout("installp -L -d #{@new_resource.source}")
+            ret.stdout.each_line do |line|
+              case line
+              when /\w:#{Regexp.escape(@new_resource.package_name)}:(.*)/
+                fields = line.split(":")
+                @candidate_version = fields[2]
+                @new_resource.version(fields[2])
+                Chef::Log.debug("#{@new_resource} setting install candidate version to #{@candidate_version}")
+              end
+            end
+            unless ret.exitstatus == 0
+              raise Chef::Exceptions::Package, "installp -L -d #{@new_resource.source} - #{ret.format_for_exception}!"
             end
-          end
-          unless ret.exitstatus == 0
-            raise Chef::Exceptions::Package, "installp -L -d #{@new_resource.source} - #{ret.format_for_exception}!"
           end
           @candidate_version
         end
@@ -134,6 +133,10 @@ class Chef
           end
         end
 
+        def package_source_found?
+          @package_source_found ||= @new_resource.source && ::File.exists?(@new_resource.source)
+        end
+
       end
     end
   end
diff --git a/spec/unit/provider/package/aix_spec.rb b/spec/unit/provider/package/aix_spec.rb
index 2b574bb7df..dba56b186d 100644
--- a/spec/unit/provider/package/aix_spec.rb
+++ b/spec/unit/provider/package/aix_spec.rb
@@ -28,7 +28,7 @@ describe Chef::Provider::Package::Aix do
     @new_resource.source("/tmp/samba.base")
 
     @provider = Chef::Provider::Package::Aix.new(@new_resource, @run_context)
-    allow(::File).to receive(:exists?).and_return(true)
+    allow(::File).to receive(:exists?).with(@new_resource.source).and_return(true)
   end
 
   describe "assessing the current package status" do
@@ -57,7 +57,7 @@ describe Chef::Provider::Package::Aix do
 
     it "should raise an exception if a source is supplied but not found" do
       allow(@provider).to receive(:shell_out).and_return(@empty_status)
-      allow(::File).to receive(:exists?).and_return(false)
+      allow(::File).to receive(:exists?).with(@new_resource.source).and_return(false)
       @provider.load_current_resource
       @provider.define_resource_requirements
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Package)
@@ -154,7 +154,7 @@ describe Chef::Provider::Package::Aix do
       @provider.install_package("samba.base", "3.3.12.0")
     end
 
-    it "should run  when the package is a path to install" do
+    it "should run installp -aYF -d when the package is a path to install" do
       @new_resource = Chef::Resource::Package.new("/tmp/samba.base")
       @provider = Chef::Provider::Package::Aix.new(@new_resource, @run_context)
       expect(@new_resource.source).to eq("/tmp/samba.base")