Make yum always use arrays internally

Also handle a lot more corner casses consistently and cleanup more code.

3 files changed, 92 insertions, 74 deletions

diff --git a/lib/chef/provider/package/apt.rb b/lib/chef/provider/package/apt.rb
index 099a4a9b61..b523cdc7cc 100644
--- a/lib/chef/provider/package/apt.rb
+++ b/lib/chef/provider/package/apt.rb
@@ -51,19 +51,17 @@ class Chef
         end
 
         def check_package_state(package)
-          if package.is_a?(Array)
-            final_installed_version = []
-            final_candidate_version = []
-            final_installed = []
-            final_virtual = []
-          end
+          final_installed_version = []
+          final_candidate_version = []
+          final_installed = []
+          final_virtual = []
           installed = virtual = false
           installed_version = candidate_version = nil
 
           [package].flatten.each do |pkg|
             installed = virtual = false
             installed_version = candidate_version = nil
-            shell_out!("apt-cache#{expand_options(default_release_options)} policy #{pkg}").stdout.each_line do |line|
+            shell_out!("apt-cache#{expand_options(default_release_options)} policy #{pkg}", {:timeout=>900}).stdout.each_line do |line|
               case line
               when /^\s{2}Installed: (.+)$/
                 installed_version = $1
@@ -79,9 +77,9 @@ class Chef
                 if candidate_version == '(none)'
                   # This may not be an appropriate assumption, but it shouldn't break anything that already worked -- btm
                   virtual = true
-                  showpkg = shell_out!("apt-cache showpkg #{package}").stdout
+                  showpkg = shell_out!("apt-cache showpkg #{package}", {:timeout => 900}).stdout
                   providers = Hash.new
-                  showpkg.rpartition(/Reverse Provides:? #{$/}/)[2].each_line do |line|
+                  showpkg.rpartition(/Reverse Provides: ?#{$/}/)[2].each_line do |line|
                     provider, version = line.split
                     providers[provider] = version
                   end
diff --git a/lib/chef/provider/package/yum.rb b/lib/chef/provider/package/yum.rb
index c85cfd20c8..670437c297 100644
--- a/lib/chef/provider/package/yum.rb
+++ b/lib/chef/provider/package/yum.rb
@@ -1068,6 +1068,9 @@ class Chef
             parse_arch
           end
 
+          # normalize internal representation as an array
+          @new_resource.package_name(as_array(@new_resource.package_name))
+
           @current_resource = Chef::Resource::Package.new(@new_resource.name)
           @current_resource.package_name(@new_resource.package_name)
 
@@ -1100,12 +1103,12 @@ class Chef
             installed_version << @yum.installed_version(pkg, arch)
             @candidate_version << @yum.candidate_version(pkg, arch)
           end
-          if installed_version.size == 1
-            @current_resource.version(installed_version[0])
-            @candidate_version = @candidate_version[0]
-          else
-            @current_resource.version(installed_version)
-          end
+          #if installed_version.size == 1
+          #  @current_resource.version(installed_version[0])
+          #  @candidate_version = @candidate_version[0]
+          #else
+            @current_resource.version(installed_version.flatten)
+          #end
 
           Chef::Log.debug("#{@new_resource} installed version: #{installed_version || "(none)"} candidate version: " +
                           "#{@candidate_version || "(none)"}")
@@ -1116,45 +1119,62 @@ class Chef
         def install_remote_package(name, version)
           # Work around yum not exiting with an error if a package doesn't exist
           # for CHEF-2062
-          if !name.is_a?(Array) && @yum.version_available?(name, version, arch)
-            method = "install"
-            log_method = "installing"
-
+          all_avail = as_array(name).zip(as_array(version)).any? do |n, v|
+            @yum.version_available?(n, v, arch)
+          end
+          method = log_method = nil
+          methods = []
+          if all_avail
             # More Yum fun:
             #
             # yum install of an old name+version will exit(1)
             # yum install of an old name+version+arch will exit(0) for some reason
             #
             # Some packages can be installed multiple times like the kernel
-            unless @yum.allow_multi_install.include?(name)
-              if RPMVersion.parse(@current_resource.version) > RPMVersion.parse(version)
-                # Unless they want this...
-                if allow_downgrade
-                  method = "downgrade"
-                  log_method = "downgrading"
-                else
-                  # we bail like yum when the package is older
-                  raise Chef::Exceptions::Package, "Installed package #{name}-#{@current_resource.version} is newer " +
-                                                   "than candidate package #{name}-#{version}"
+            as_array(name).zip(as_array(version)).each do |n, v|
+              method = "install"
+              log_method = "installing"
+              idx = package_name_array.index(n)
+              unless @yum.allow_multi_install.include?(n)
+                if RPMVersion.parse(current_version_array[idx]) > RPMVersion.parse(v)
+                  # We allow downgrading only in the evenit of single-package
+                  # rules where the user explicitly allowed it
+                  if allow_downgrade
+                    method = "downgrade"
+                    log_method = "downgrading"
+                  else
+                    # we bail like yum when the package is older
+                    raise Chef::Exceptions::Package, "Installed package #{name}-#{@current_resource.version} is newer " +
+                                                     "than candidate package #{name}-#{version}"
+                  end
                 end
               end
+              # methods don't count for packages we won't be touching
+              next if RPMVersion.parse(current_version_array[idx]) == RPMVersion.parse(v)
+              methods << method
+            end
+            # We could split this up into two commands if we wanted to, but
+            # for now, just don't support this.
+            if methods.uniq.length > 1
+              raise Chef::Exceptions::Package, "Multipackage rule #{name} has a mix of upgrade and downgrade packages. Cannot proceed."
             end
 
-            repo = @yum.package_repository(name, version, arch)
-            Chef::Log.info("#{@new_resource} #{log_method} #{name}-#{version}#{yum_arch} from #{repo} repository")
-
-            yum_command("yum -d0 -e0 -y#{expand_options(@new_resource.options)} #{method} #{name}-#{version}#{yum_arch}")
-          elsif name.is_a?(Array)
+            repos = []
+            pkg_string_bits = []
             index = 0
-            pkg_string = name.zip(version).map do |x|
+            as_array(name).zip(as_array(version)).each do |n, v|
               s = ''
-              unless x[1] == @current_resource.version[index]
-                s = "#{x.join('-')}#{yum_arch}"
+              unless v == current_version_array[index]
+                s = "#{n}-#{v}#{yum_arch}"
+                repo = @yum.package_repository(n, v, arch)
+                repos << "#{s} from #{repo} repository"
+                pkg_string_bits << s
               end
               index += 1
-              s
-            end.join(' ')
-            yum_command("yum -d0 -e0 -y#{expand_options(@new_resource.options)} install #{pkg_string}")
+            end
+            pkg_string = pkg_string_bits.join(' ')
+            Chef::Log.info("#{@new_resource} #{log_method} #{repos.join(' ')}")
+            yum_command("yum -d0 -e0 -y#{expand_options(@new_resource.options)} #{method} #{pkg_string}")
           else
             raise Chef::Exceptions::Package, "Version #{version} of #{name} not found. Did you specify both version " +
                                              "and release? (version-release, e.g. 1.84-10.fc6)"
diff --git a/spec/unit/provider/package/yum_spec.rb b/spec/unit/provider/package/yum_spec.rb
index 0d2a44f3ae..160deb964b 100644
--- a/spec/unit/provider/package/yum_spec.rb
+++ b/spec/unit/provider/package/yum_spec.rb
@@ -50,23 +50,23 @@ describe Chef::Provider::Package::Yum do
 
     it "should set the current resources package name to the new resources package name" do
       @provider.load_current_resource
-      expect(@provider.current_resource.package_name).to eq("cups")
+      expect(@provider.current_resource.package_name).to eq(["cups"])
     end
 
     it "should set the installed version to nil on the current resource if no installed package" do
-      allow(@yum_cache).to receive(:installed_version).and_return(nil)
+      allow(@yum_cache).to receive(:installed_version).and_return([nil])
       @provider.load_current_resource
-      expect(@provider.current_resource.version).to be_nil
+      expect(@provider.current_resource.version).to eq([nil])
     end
 
     it "should set the installed version if yum has one" do
       @provider.load_current_resource
-      expect(@provider.current_resource.version).to eq("1.2.4-11.18.el5")
+      expect(@provider.current_resource.version).to eq(["1.2.4-11.18.el5"])
     end
 
     it "should set the candidate version if yum info has one" do
       @provider.load_current_resource
-      expect(@provider.candidate_version).to eql("1.2.4-11.18.el5_2.3")
+      expect(@provider.candidate_version).to eql(["1.2.4-11.18.el5_2.3"])
     end
 
     it "should return the current resouce" do
@@ -96,14 +96,14 @@ describe Chef::Provider::Package::Yum do
         allow(Chef::Provider::Package::Yum::YumCache).to receive(:instance).and_return(@yum_cache)
         @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
         @provider.load_current_resource
-        expect(@provider.new_resource.package_name).to eq("testing")
+        expect(@provider.new_resource.package_name).to eq(["testing"])
         expect(@provider.new_resource.arch).to eq("noarch")
         expect(@provider.arch).to eq("noarch")
 
         @new_resource = Chef::Resource::YumPackage.new('testing.more.noarch')
         @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
         @provider.load_current_resource
-        expect(@provider.new_resource.package_name).to eq("testing.more")
+        expect(@provider.new_resource.package_name).to eq(["testing.more"])
         expect(@provider.new_resource.arch).to eq("noarch")
         expect(@provider.arch).to eq("noarch")
       end
@@ -131,14 +131,14 @@ describe Chef::Provider::Package::Yum do
         @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
         # annoying side effect of the fun stub'ing above
         @provider.load_current_resource
-        expect(@provider.new_resource.package_name).to eq("testing.beta3")
+        expect(@provider.new_resource.package_name).to eq(["testing.beta3"])
         expect(@provider.new_resource.arch).to eq(nil)
         expect(@provider.arch).to eq(nil)
 
         @new_resource = Chef::Resource::YumPackage.new('testing.beta3.more')
         @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
         @provider.load_current_resource
-        expect(@provider.new_resource.package_name).to eq("testing.beta3.more")
+        expect(@provider.new_resource.package_name).to eq(["testing.beta3.more"])
         expect(@provider.new_resource.arch).to eq(nil)
         expect(@provider.arch).to eq(nil)
       end
@@ -161,14 +161,14 @@ describe Chef::Provider::Package::Yum do
         allow(Chef::Provider::Package::Yum::YumCache).to receive(:instance).and_return(@yum_cache)
         @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
         @provider.load_current_resource
-        expect(@provider.new_resource.package_name).to eq("testing.beta3")
+        expect(@provider.new_resource.package_name).to eq(["testing.beta3"])
         expect(@provider.new_resource.arch).to eq(nil)
         expect(@provider.arch).to eq(nil)
 
         @new_resource = Chef::Resource::YumPackage.new('testing.beta3.more')
         @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
         @provider.load_current_resource
-        expect(@provider.new_resource.package_name).to eq("testing.beta3.more")
+        expect(@provider.new_resource.package_name).to eq(["testing.beta3.more"])
         expect(@provider.new_resource.arch).to eq(nil)
         expect(@provider.arch).to eq(nil)
       end
@@ -196,7 +196,7 @@ describe Chef::Provider::Package::Yum do
         allow(Chef::Provider::Package::Yum::YumCache).to receive(:instance).and_return(@yum_cache)
         @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
         @provider.load_current_resource
-        expect(@provider.new_resource.package_name).to eq("testing.i386")
+        expect(@provider.new_resource.package_name).to eq(["testing.i386"])
         expect(@provider.new_resource.arch).to eq("x86_64")
       end
     end
@@ -247,7 +247,7 @@ describe Chef::Provider::Package::Yum do
       expect(@yum_cache).to receive(:packages_from_require).and_return([pkg])
       @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
       @provider.load_current_resource
-      expect(@new_resource.package_name).to eq("test-package")
+      expect(@new_resource.package_name).to eq(["test-package"])
     end
 
     it "should search provides if package name can't be found, warn about multiple matches, but use the first one" do
@@ -268,7 +268,7 @@ describe Chef::Provider::Package::Yum do
       expect(Chef::Log).to receive(:warn).exactly(1).times.with(%r{matched multiple Provides})
       @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
       @provider.load_current_resource
-      expect(@new_resource.package_name).to eq("test-package-x")
+      expect(@new_resource.package_name).to eq(["test-package-x"])
     end
 
     it "should search provides if no package is available - if no match in installed provides then load the complete set" do
@@ -328,7 +328,7 @@ describe Chef::Provider::Package::Yum do
       expect(@yum_cache).to receive(:packages_from_require).twice.and_return([])
       @provider = Chef::Provider::Package::Yum.new(@new_resource, @run_context)
       @provider.load_current_resource
-      expect(@new_resource.package_name).to eq("cups")
+      expect(@new_resource.package_name).to eq(["cups"])
     end
   end
 
@@ -337,9 +337,9 @@ describe Chef::Provider::Package::Yum do
       @provider.load_current_resource
       allow(Chef::Provider::Package::Yum::RPMUtils).to receive(:rpmvercmp).and_return(-1)
       expect(@provider).to receive(:yum_command).with(
-        "yum -d0 -e0 -y install emacs-1.0"
+        "yum -d0 -e0 -y install cups-1.2.4-11.19.el5"
       )
-      @provider.install_package("emacs", "1.0")
+      @provider.install_package(["cups"], ["1.2.4-11.19.el5"])
     end
 
     it "should run yum localinstall if given a path to an rpm" do
@@ -347,7 +347,7 @@ describe Chef::Provider::Package::Yum do
       expect(@provider).to receive(:yum_command).with(
         "yum -d0 -e0 -y localinstall /tmp/emacs-21.4-20.el5.i386.rpm"
       )
-      @provider.install_package("emacs", "21.4-20.el5")
+      @provider.install_package(["emacs"], ["21.4-20.el5"])
     end
 
     it "should run yum localinstall if given a path to an rpm as the package" do
@@ -358,7 +358,7 @@ describe Chef::Provider::Package::Yum do
       expect(@provider).to receive(:yum_command).with(
         "yum -d0 -e0 -y localinstall /tmp/emacs-21.4-20.el5.i386.rpm"
       )
-      @provider.install_package("/tmp/emacs-21.4-20.el5.i386.rpm", "21.4-20.el5")
+      @provider.install_package(["/tmp/emacs-21.4-20.el5.i386.rpm"], ["21.4-20.el5"])
     end
 
     it "should run yum install with the package name, version and arch" do
@@ -366,14 +366,14 @@ describe Chef::Provider::Package::Yum do
       allow(@new_resource).to receive(:arch).and_return("i386")
       allow(Chef::Provider::Package::Yum::RPMUtils).to receive(:rpmvercmp).and_return(-1)
       expect(@provider).to receive(:yum_command).with(
-        "yum -d0 -e0 -y install emacs-21.4-20.el5.i386"
+        "yum -d0 -e0 -y install cups-1.2.4-11.19.el5.i386"
       )
-      @provider.install_package("emacs", "21.4-20.el5")
+      @provider.install_package(["cups"], ["1.2.4-11.19.el5"])
     end
 
     it "installs the package with the options given in the resource" do
       @provider.load_current_resource
-      @provider.candidate_version = '11'
+      @provider.candidate_version = ['11']
       allow(@new_resource).to receive(:options).and_return("--disablerepo epmd")
       allow(Chef::Provider::Package::Yum::RPMUtils).to receive(:rpmvercmp).and_return(-1)
       expect(@provider).to receive(:yum_command).with(
@@ -467,10 +467,10 @@ describe Chef::Provider::Package::Yum do
       @provider.load_current_resource
       allow(Chef::Provider::Package::Yum::RPMUtils).to receive(:rpmvercmp).and_return(-1)
       expect(@provider).to receive(:yum_command).with(
-        "yum -d0 -e0 -y install emacs-1.0"
+        "yum -d0 -e0 -y install cups-1.2.4-11.15.el5"
       )
       expect(@yum_cache).to receive(:reload).once
-      @provider.install_package("emacs", "1.0")
+      @provider.install_package("cups", "1.2.4-11.15.el5")
     end
 
     it "should run yum install then not flush the cache if :after is false" do
@@ -478,10 +478,10 @@ describe Chef::Provider::Package::Yum do
       @provider.load_current_resource
       allow(Chef::Provider::Package::Yum::RPMUtils).to receive(:rpmvercmp).and_return(-1)
       expect(@provider).to receive(:yum_command).with(
-        "yum -d0 -e0 -y install emacs-1.0"
+        "yum -d0 -e0 -y install cups-1.2.4-11.15.el5"
       )
       expect(@yum_cache).not_to receive(:reload)
-      @provider.install_package("emacs", "1.0")
+      @provider.install_package("cups", "1.2.4-11.15.el5")
     end
   end
 
@@ -530,10 +530,10 @@ describe Chef::Provider::Package::Yum do
       allow(@yum_cache).to receive(:installed_version).and_return(nil)
       @provider.load_current_resource
       @current_resource = Chef::Resource::Package.new('cups')
-      @provider.candidate_version = '11'
+      @provider.candidate_version = ['11']
       expect(@provider).to receive(:upgrade_package).with(
-        "cups",
-        "11"
+        ["cups"],
+        ["11"]
       )
       @provider.action_upgrade
     end
@@ -541,7 +541,7 @@ describe Chef::Provider::Package::Yum do
     it "should call action_upgrade in the parent if the candidate version is nil" do
       @provider.load_current_resource
       @current_resource = Chef::Resource::Package.new('cups')
-      @provider.candidate_version = nil
+      @provider.candidate_version = [nil]
       expect(@provider).not_to receive(:upgrade_package)
       @provider.action_upgrade
     end
@@ -549,10 +549,10 @@ describe Chef::Provider::Package::Yum do
     it "should call action_upgrade in the parent if the candidate is newer" do
       @provider.load_current_resource
       @current_resource = Chef::Resource::Package.new('cups')
-      @provider.candidate_version = '11'
+      @provider.candidate_version = ['11']
       expect(@provider).to receive(:upgrade_package).with(
-        "cups",
-        "11"
+        ["cups"],
+        ["11"]
       )
       @provider.action_upgrade
     end
@@ -561,7 +561,7 @@ describe Chef::Provider::Package::Yum do
       allow(@yum_cache).to receive(:installed_version).and_return("12")
       @provider.load_current_resource
       @current_resource = Chef::Resource::Package.new('cups')
-      @provider.candidate_version = '11'
+      @provider.candidate_version = ['11']
       expect(@provider).not_to receive(:upgrade_package)
       @provider.action_upgrade
     end