Improve for CHEF-1575 based on ticket discussion

danp · danielsdeleo · commit 33d73a5c0801 · 2010-12-15T22:50:29.000-08:00
diff --git a/chef/lib/chef/provider/package/portage.rb b/chef/lib/chef/provider/package/portage.rb
@@ -24,44 +24,59 @@ class Chef
   class Provider
     class Package
       class Portage < Chef::Provider::Package
+        PACKAGE_NAME_PATTERN = %r{(([^/]+)/)?([^/]+)}
 
         def load_current_resource
           @current_resource = Chef::Resource::Package.new(@new_resource.name)
           @current_resource.package_name(@new_resource.package_name)
 
           @current_resource.version(nil)
 
-          _, category_with_slash, category, pkg = %r{^(([^/]+)+/)?([^/]+)$}.match(@new_resource.package_name).to_a
+          _, category_with_slash, category, pkg = %r{^#{PACKAGE_NAME_PATTERN}$}.match(@new_resource.package_name).to_a
 
-          possibilities = Dir["/var/db/pkg/#{category || "*"}/#{pkg}-*"].map {|d| ::File.basename(d) }
-          possibilities.each do |entry|
-            if(entry =~ /^#{Regexp.escape(pkg)}\-(\d[\.\d]*((_(alpha|beta|pre|rc|p)\d*)*)?(-r\d+)?)/)
-              @current_resource.version($1)
-              Chef::Log.debug("Got current version #{$1}")
-              break
+          possibilities = Dir["/var/db/pkg/#{category || "*"}/#{pkg}-*"].map {|d| d.sub(%r{/var/db/pkg/}, "") }
+          versions = possibilities.map do |entry|
+            if(entry =~ %r{[^/]+/#{Regexp.escape(pkg)}\-(\d[\.\d]*((_(alpha|beta|pre|rc|p)\d*)*)?(-r\d+)?)})
+              [$&, $1]
             end
+          end.compact
+
+          if versions.size > 1
+            atoms = versions.map {|v| v.first }.sort
+            raise Chef::Exceptions::Package, "Multiple packages found for #{@new_resource.package_name}: #{atoms.join(" ")}. Specify a category."
+          elsif versions.size == 1
+            @current_resource.version(versions.first.last)
+            Chef::Log.debug("Got current version #{$1}")
           end
 
           @current_resource
         end
 
 
         def parse_emerge(package, txt)
-          available, installed, pkg = nil
+          availables = {}
+          package_without_category = package.split("/").last
+          found_package_name = nil
+
           txt.each do |line|
-            if line =~ /\*(.*)/
-              pkg = $1.strip
-            end
-            if (pkg == package) || (pkg.split('/').last == package rescue false)
-              if line =~ /Latest version available: (.*)/
-                available = $1
-              elsif line =~ /Latest version installed: (.*)/
-                installed = $1
+            if line =~ /\*\s+#{PACKAGE_NAME_PATTERN}/
+              found_package_name = $&.strip
+              if found_package_name == package || found_package_name.split("/").last == package_without_category
+                availables[found_package_name] = nil
               end
             end
+
+            if line =~ /Latest version available: (.*)/ && availables.has_key?(found_package_name)
+              availables[found_package_name] = $1.strip
+            end
           end
-          available = installed unless available
-          [available, installed]
+
+          if availables.size > 1
+            # shouldn't happen if a category is specified so just use `package`
+            raise Chef::Exceptions::Package, "Multiple emerge results found for #{package}: #{availables.keys.join(" ")}. Specify a category."
+          end
+
+          availables.values.first
         end
 
         def candidate_version
diff --git a/chef/spec/unit/provider/package/portage_spec.rb b/chef/spec/unit/provider/package/portage_spec.rb
@@ -74,27 +74,144 @@
       @provider.load_current_resource
       @provider.current_resource.version.should be_nil
     end
+
+    it "should throw an exception if a category isn't specified and multiple packages are found" do
+      ::Dir.stub!(:[]).with("/var/db/pkg/*/git-*").and_return(["/var/db/pkg/dev-util/git-1.0.0", "/var/db/pkg/funny-words/git-1.0.0"])
+      @provider = Chef::Provider::Package::Portage.new(@new_resource_without_category, @run_context)
+      lambda { @provider.load_current_resource }.should raise_error(Chef::Exceptions::Package)
+    end
   end
 
   describe "once the state of the package is known" do
 
     describe Chef::Provider::Package::Portage, "candidate_version" do
-      it "should return the candidate_version variable if already setup" do
+      it "should return the candidate_version variable if already set" do
         @provider.candidate_version = "1.0.0"
         @provider.should_not_receive(:popen4)
         @provider.candidate_version
       end
 
-      it "should lookup the candidate_version if the variable is not already set" do
-        @status = mock("Status", :exitstatus => 0)
+      it "should throw an exception if the exitstatus is not 0" do
+        @status = mock("Status", :exitstatus => 1)
         @provider.stub!(:popen4).and_return(@status)
-        @provider.should_receive(:popen4)
-        @provider.candidate_version
+        lambda { @provider.candidate_version }.should raise_error(Chef::Exceptions::Package)
       end
 
-      it "should throw and exception if the exitstatus is not 0" do
-        @status = mock("Status", :exitstatus => 1)
-        @provider.stub!(:popen4).and_return(@status)
+      it "should find the candidate_version if a category is specifed and there are no duplicates" do
+        output = <<EOF
+Searching...
+[ Results for search key : git ]
+[ Applications found : 14 ]
+
+*  app-misc/digitemp [ Masked ]
+      Latest version available: 3.5.0
+      Latest version installed: [ Not Installed ]
+      Size of files: 261 kB
+      Homepage:      http://www.digitemp.com/ http://www.ibutton.com/
+      Description:   Temperature logging and reporting using Dallas Semiconductor's iButtons and 1-Wire protocol
+      License:       GPL-2
+
+*  dev-util/git
+      Latest version available: 1.6.0.6
+      Latest version installed: ignore
+      Size of files: 2,725 kB
+      Homepage:      http://git.or.cz/
+      Description:   GIT - the stupid content tracker, the revision control system heavily used by the Linux kernel team
+      License:       GPL-2
+
+*  dev-util/gitosis [ Masked ]
+      Latest version available: 0.2_p20080825
+      Latest version installed: [ Not Installed ]
+      Size of files: 31 kB
+      Homepage:      http://eagain.net/gitweb/?p=gitosis.git;a=summary
+      Description:   gitosis -- software for hosting git repositories
+      License:       GPL-2
+EOF
+
+        @status = mock("Status", :exitstatus => 0)
+        @provider.should_receive(:popen4).and_yield(nil, nil, StringIO.new(output), nil).and_return(@status)
+        @provider.candidate_version.should == "1.6.0.6"
+      end
+
+      it "should find the candidate_version if a category is not specifed and there are no duplicates" do
+        output = <<EOF
+Searching...
+[ Results for search key : git ]
+[ Applications found : 14 ]
+
+*  app-misc/digitemp [ Masked ]
+      Latest version available: 3.5.0
+      Latest version installed: [ Not Installed ]
+      Size of files: 261 kB
+      Homepage:      http://www.digitemp.com/ http://www.ibutton.com/
+      Description:   Temperature logging and reporting using Dallas Semiconductor's iButtons and 1-Wire protocol
+      License:       GPL-2
+
+*  dev-util/git
+      Latest version available: 1.6.0.6
+      Latest version installed: ignore
+      Size of files: 2,725 kB
+      Homepage:      http://git.or.cz/
+      Description:   GIT - the stupid content tracker, the revision control system heavily used by the Linux kernel team
+      License:       GPL-2
+
+*  dev-util/gitosis [ Masked ]
+      Latest version available: 0.2_p20080825
+      Latest version installed: [ Not Installed ]
+      Size of files: 31 kB
+      Homepage:      http://eagain.net/gitweb/?p=gitosis.git;a=summary
+      Description:   gitosis -- software for hosting git repositories
+      License:       GPL-2
+EOF
+
+        @status = mock("Status", :exitstatus => 0)
+        @provider = Chef::Provider::Package::Portage.new(@new_resource_without_category, @run_context)
+        @provider.should_receive(:popen4).and_yield(nil, nil, StringIO.new(output), nil).and_return(@status)
+        @provider.candidate_version.should == "1.6.0.6"
+      end
+
+      it "should throw an exception if a category is not specified and there are duplicates" do
+        output = <<EOF
+Searching...
+[ Results for search key : git ]
+[ Applications found : 14 ]
+
+*  app-misc/digitemp [ Masked ]
+      Latest version available: 3.5.0
+      Latest version installed: [ Not Installed ]
+      Size of files: 261 kB
+      Homepage:      http://www.digitemp.com/ http://www.ibutton.com/
+      Description:   Temperature logging and reporting using Dallas Semiconductor's iButtons and 1-Wire protocol
+      License:       GPL-2
+
+*  app-misc/git
+      Latest version available: 4.3.20
+      Latest version installed: [ Not Installed ]
+      Size of files: 416 kB
+      Homepage:      http://www.gnu.org/software/git/
+      Description:   GNU Interactive Tools - increase speed and efficiency of most daily task
+      License:       GPL-2
+
+*  dev-util/git
+      Latest version available: 1.6.0.6
+      Latest version installed: ignore
+      Size of files: 2,725 kB
+      Homepage:      http://git.or.cz/
+      Description:   GIT - the stupid content tracker, the revision control system heavily used by the Linux kernel team
+      License:       GPL-2
+
+*  dev-util/gitosis [ Masked ]
+      Latest version available: 0.2_p20080825
+      Latest version installed: [ Not Installed ]
+      Size of files: 31 kB
+      Homepage:      http://eagain.net/gitweb/?p=gitosis.git;a=summary
+      Description:   gitosis -- software for hosting git repositories
+      License:       GPL-2
+EOF
+
+        @status = mock("Status", :exitstatus => 0)
+        @provider = Chef::Provider::Package::Portage.new(@new_resource_without_category, @run_context)
+        @provider.should_receive(:popen4).and_yield(nil, nil, StringIO.new(output), nil).and_return(@status)
         lambda { @provider.candidate_version }.should raise_error(Chef::Exceptions::Package)
       end
 
