fix smartos_package for new "pkgin" output, fixes #3112

2 files changed, 55 insertions, 45 deletions

diff --git a/lib/chef/provider/package/smartos.rb b/lib/chef/provider/package/smartos.rb
index 7cef91953a..bba55ca39e 100644
--- a/lib/chef/provider/package/smartos.rb
+++ b/lib/chef/provider/package/smartos.rb
@@ -43,7 +43,7 @@ class Chef
         def check_package_state(name)
           Chef::Log.debug("#{@new_resource} checking package #{name}")
           version = nil
-          info = shell_out!("/opt/local/sbin/pkg_info -E \"#{name}*\"", :env => nil, :returns => [0,1])
+          info = shell_out!("/opt/local/sbin/pkg_info", "-E", "#{name}*", :env => nil, :returns => [0,1])
 
           if info.stdout
             version = info.stdout[/^#{@new_resource.package_name}-(.+)/, 1]
@@ -60,11 +60,11 @@ class Chef
           return @candidate_version if @candidate_version
           name = nil
           version = nil
-          pkg = shell_out!("/opt/local/bin/pkgin se #{new_resource.package_name}", :env => nil, :returns => [0,1])
+          pkg = shell_out!("/opt/local/bin/pkgin", "se", new_resource.package_name, :env => nil, :returns => [0,1])
           pkg.stdout.each_line do |line|
             case line
             when /^#{new_resource.package_name}/
-              name, version = line.split[0].split(/-([^-]+)$/)
+              name, version = line.split(/[; ]/)[0].split(/-([^-]+)$/)
             end
           end
           @candidate_version = version
@@ -74,7 +74,7 @@ class Chef
         def install_package(name, version)
           Chef::Log.debug("#{@new_resource} installing package #{name} version #{version}")
           package = "#{name}-#{version}"
-          out = shell_out!("/opt/local/bin/pkgin -y install #{package}", :env => nil)
+          out = shell_out!("/opt/local/bin/pkgin", "-y", "install", package, :env => nil)
         end
 
         def upgrade_package(name, version)
@@ -85,7 +85,7 @@ class Chef
         def remove_package(name, version)
           Chef::Log.debug("#{@new_resource} removing package #{name} version #{version}")
           package = "#{name}"
-          out = shell_out!("/opt/local/bin/pkgin -y remove #{package}", :env => nil)
+          out = shell_out!("/opt/local/bin/pkgin", "-y", "remove", package, :env => nil)
         end
 
       end
diff --git a/spec/unit/provider/package/smartos_spec.rb b/spec/unit/provider/package/smartos_spec.rb
index db39589b85..0b3f6a1896 100644
--- a/spec/unit/provider/package/smartos_spec.rb
+++ b/spec/unit/provider/package/smartos_spec.rb
@@ -29,45 +29,45 @@ describe Chef::Provider::Package::SmartOS, "load_current_resource" do
     @current_resource = Chef::Resource::Package.new("varnish")
 
 
-	  @status = double("Status", :exitstatus => 0)
-		@provider = Chef::Provider::Package::SmartOS.new(@new_resource, @run_context)
-		allow(Chef::Resource::Package).to receive(:new).and_return(@current_resource)
-		@stdin = StringIO.new
-		@stdout = "varnish-2.1.5nb2\n"
-		@stderr = StringIO.new
-		@pid = 10
-		@shell_out = OpenStruct.new(:stdout => @stdout, :stdin => @stdin, :stderr => @stderr, :status => @status, :exitstatus => 0)
+    @status = double("Status", :exitstatus => 0)
+    @provider = Chef::Provider::Package::SmartOS.new(@new_resource, @run_context)
+    allow(Chef::Resource::Package).to receive(:new).and_return(@current_resource)
+    @stdin = StringIO.new
+    @stdout = "varnish-2.1.5nb2\n"
+    @stderr = StringIO.new
+    @pid = 10
+    @shell_out = OpenStruct.new(:stdout => @stdout, :stdin => @stdin, :stderr => @stderr, :status => @status, :exitstatus => 0)
   end
 
-	describe "when loading current resource" do
+  describe "when loading current resource" do
 
     it "should create a current resource with the name of the new_resource" do
-			expect(@provider).to receive(:shell_out!).and_return(@shell_out)
-			expect(Chef::Resource::Package).to receive(:new).and_return(@current_resource)
-			@provider.load_current_resource
+      expect(@provider).to receive(:shell_out!).and_return(@shell_out)
+      expect(Chef::Resource::Package).to receive(:new).and_return(@current_resource)
+      @provider.load_current_resource
     end
 
-		it "should set the current resource package name" do
-			expect(@provider).to receive(:shell_out!).and_return(@shell_out)
-			expect(@current_resource).to receive(:package_name).with(@new_resource.package_name)
-			@provider.load_current_resource
-		end
+    it "should set the current resource package name" do
+      expect(@provider).to receive(:shell_out!).and_return(@shell_out)
+      expect(@current_resource).to receive(:package_name).with(@new_resource.package_name)
+      @provider.load_current_resource
+    end
 
-		it "should set the installed version if it is installed" do
-		  expect(@provider).to receive(:shell_out!).and_return(@shell_out)
-			@provider.load_current_resource
-			expect(@current_resource.version).to eq("2.1.5nb2")
-	  end
+    it "should set the installed version if it is installed" do
+      expect(@provider).to receive(:shell_out!).and_return(@shell_out)
+      @provider.load_current_resource
+      expect(@current_resource.version).to eq("2.1.5nb2")
+    end
 
-		it "should set the installed version to nil if it's not installed" do
-			out = OpenStruct.new(:stdout => nil)
-			expect(@provider).to receive(:shell_out!).and_return(out)
-			@provider.load_current_resource
-			expect(@current_resource.version).to eq(nil)
-		end
+    it "should set the installed version to nil if it's not installed" do
+      out = OpenStruct.new(:stdout => nil)
+      expect(@provider).to receive(:shell_out!).and_return(out)
+      @provider.load_current_resource
+      expect(@current_resource.version).to eq(nil)
+    end
 
 
-	end
+  end
 
   describe "candidate_version" do
     it "should return the candidate_version variable if already setup" do
@@ -76,27 +76,37 @@ describe Chef::Provider::Package::SmartOS, "load_current_resource" do
       @provider.candidate_version
     end
 
-    it "should lookup the candidate_version if the variable is not already set" do
+    it "should lookup the candidate_version if the variable is not already set (pkgin separated by spaces)" do
       search = double()
       expect(search).to receive(:each_line).
-        and_yield("something-varnish-1.1.1  something varnish like\n").
-        and_yield("varnish-2.3.4 actual varnish\n")
+        and_yield("something-varnish-1.1.1   something varnish like\n").
+        and_yield("varnish-2.3.4             actual varnish\n")
       @shell_out = double('shell_out!', :stdout => search)
-      expect(@provider).to receive(:shell_out!).with('/opt/local/bin/pkgin se varnish', :env => nil, :returns => [0,1]).and_return(@shell_out)
+      expect(@provider).to receive(:shell_out!).with('/opt/local/bin/pkgin', 'se', 'varnish', :env => nil, :returns => [0,1]).and_return(@shell_out)
+      expect(@provider.candidate_version).to eq("2.3.4")
+    end
+
+    it "should lookup the candidate_version if the variable is not already set (pkgin separated by semicolons)" do
+      search = double()
+      expect(search).to receive(:each_line).
+        and_yield("something-varnish-1.1.1;;something varnish like\n").
+        and_yield("varnish-2.3.4;;actual varnish\n")
+      @shell_out = double('shell_out!', :stdout => search)
+      expect(@provider).to receive(:shell_out!).with('/opt/local/bin/pkgin', 'se', 'varnish', :env => nil, :returns => [0,1]).and_return(@shell_out)
       expect(@provider.candidate_version).to eq("2.3.4")
     end
   end
 
-	describe "when manipulating a resource" do
+  describe "when manipulating a resource" do
 
-		it "run pkgin and install the package" do
-			out = OpenStruct.new(:stdout => nil)
-      expect(@provider).to receive(:shell_out!).with("/opt/local/sbin/pkg_info -E \"varnish*\"", {:env => nil, :returns=>[0,1]}).and_return(@shell_out)
-      expect(@provider).to receive(:shell_out!).with("/opt/local/bin/pkgin -y install varnish-2.1.5nb2", {:env=>nil}).and_return(out)
+    it "run pkgin and install the package" do
+      out = OpenStruct.new(:stdout => nil)
+      expect(@provider).to receive(:shell_out!).with("/opt/local/sbin/pkg_info", "-E", "varnish*", {:env => nil, :returns=>[0,1]}).and_return(@shell_out)
+      expect(@provider).to receive(:shell_out!).with("/opt/local/bin/pkgin", "-y", "install", "varnish-2.1.5nb2", {:env=>nil}).and_return(out)
       @provider.load_current_resource
       @provider.install_package("varnish", "2.1.5nb2")
-		end
+    end
 
-	end
+  end
 
 end