Merge pull request #2922 from chef/lcg/fix-multipackage

Lcg/fix multipackage

2 files changed, 104 insertions, 90 deletions

diff --git a/lib/chef/provider/package/apt.rb b/lib/chef/provider/package/apt.rb
index c960806e8f..e426b51992 100644
--- a/lib/chef/provider/package/apt.rb
+++ b/lib/chef/provider/package/apt.rb
@@ -27,12 +27,18 @@ class Chef
 
         provides :apt_package, os: "linux"
 
+        # return [Hash] mapping of package name to Boolean value
         attr_accessor :is_virtual_package
 
+        def initialize(new_resource, run_context)
+          super
+          @is_virtual_package = {}
+        end
+
         def load_current_resource
           @current_resource = Chef::Resource::Package.new(@new_resource.name)
           @current_resource.package_name(@new_resource.package_name)
-          check_package_state(@new_resource.package_name)
+          check_all_packages_state(@new_resource.package_name)
           @current_resource
         end
 
@@ -50,86 +56,91 @@ class Chef
           "-o APT::Default-Release=#{@new_resource.default_release}" if @new_resource.respond_to?(:default_release) && @new_resource.default_release
         end
 
-        def check_package_state(package)
-          final_installed_version = []
-          final_candidate_version = []
-          final_installed = []
-          final_virtual = []
-
-          [package].flatten.each do |pkg|
-            installed = virtual = false
-            installed_version = candidate_version = nil
-            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
-                if installed_version == '(none)'
-                  Chef::Log.debug("#{@new_resource} current version is nil")
-                  installed_version = nil
-                else
-                  Chef::Log.debug("#{@new_resource} current version is #{installed_version}")
-                  installed = true
-                end
-              when /^\s{2}Candidate: (.+)$/
-                candidate_version = $1
-                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}", {:timeout => 900}).stdout
-                  providers = Hash.new
-                  showpkg.rpartition(/Reverse Provides: ?#{$/}/)[2].each_line do |line|
-                    provider, version = line.split
-                    providers[provider] = version
-                  end
-                  # Check if the package providing this virtual package is installed
-                  num_providers = providers.length
-                  raise Chef::Exceptions::Package, "#{@new_resource.package_name} has no candidate in the apt-cache" if num_providers == 0
-                  # apt will only install a virtual package if there is a single providing package
-                  raise Chef::Exceptions::Package, "#{@new_resource.package_name} is a virtual package provided by #{num_providers} packages, you must explicitly select one to install" if num_providers > 1
-                  # Check if the package providing this virtual package is installed
-                  Chef::Log.info("#{@new_resource} is a virtual package, actually acting on package[#{providers.keys.first}]")
-                  installed = check_package_state(providers.keys.first)
-                else
-                  Chef::Log.debug("#{@new_resource} candidate version is #{$1}")
+        def check_package_state(pkg)
+          is_virtual_package = false
+          installed          = false
+          installed_version  = nil
+          candidate_version  = nil
+
+          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
+              if installed_version == '(none)'
+                Chef::Log.debug("#{@new_resource} current version is nil")
+                installed_version = nil
+              else
+                Chef::Log.debug("#{@new_resource} current version is #{installed_version}")
+                installed = true
+              end
+            when /^\s{2}Candidate: (.+)$/
+              candidate_version = $1
+              if candidate_version == '(none)'
+                # This may not be an appropriate assumption, but it shouldn't break anything that already worked -- btm
+                is_virtual_package = true
+                showpkg = shell_out!("apt-cache showpkg #{pkg}", {:timeout => 900}).stdout
+                providers = Hash.new
+                showpkg.rpartition(/Reverse Provides: ?#{$/}/)[2].each_line do |line|
+                  provider, version = line.split
+                  providers[provider] = version
                 end
+                # Check if the package providing this virtual package is installed
+                num_providers = providers.length
+                raise Chef::Exceptions::Package, "#{@new_resource.package_name} has no candidate in the apt-cache" if num_providers == 0
+                # apt will only install a virtual package if there is a single providing package
+                raise Chef::Exceptions::Package, "#{@new_resource.package_name} is a virtual package provided by #{num_providers} packages, you must explicitly select one to install" if num_providers > 1
+                # Check if the package providing this virtual package is installed
+                Chef::Log.info("#{@new_resource} is a virtual package, actually acting on package[#{providers.keys.first}]")
+                ret = check_package_state(providers.keys.first)
+                installed = ret[:installed]
+                installed_version = ret[:installed_version]
+              else
+                Chef::Log.debug("#{@new_resource} candidate version is #{$1}")
               end
             end
-            if package.is_a?(Array)
-              final_installed_version << installed_version
-              final_candidate_version << candidate_version
-              final_installed << installed
-              final_virtual << virtual
-            else
-              final_installed_version = installed_version
-              final_candidate_version = candidate_version
-              final_installed = installed
-              final_virtual = virtual
-            end
           end
-          @candidate_version = final_candidate_version
-          @current_resource.version(final_installed_version)
-          @is_virtual_package = final_virtual
- 
-          return final_installed.is_a?(Array) ? final_installed.any? : final_installed
+
+          return {
+            installed_version:   installed_version,
+            installed:           installed,
+            candidate_version:   candidate_version,
+            is_virtual_package:  is_virtual_package,
+          }
         end
 
-        def install_package(name, version)
-          if name.is_a?(Array)
-            index = 0
-            package_name = name.zip(version).map do |x, y|
-              namestr = nil
-              if @is_virtual_package[index]
-                namestr = x
-              else
-                namestr = "#{x}=#{y}"
-              end
-              index += 1
-              namestr
-            end.join(' ')
+        def check_all_packages_state(package)
+          installed_version = {}
+          candidate_version = {}
+          installed = {}
+
+          [package].flatten.each do |pkg|
+            ret = check_package_state(pkg)
+            is_virtual_package[pkg] = ret[:is_virtual_package]
+            installed[pkg]          = ret[:installed]
+            installed_version[pkg]  = ret[:installed_version]
+            candidate_version[pkg]  = ret[:candidate_version]
+          end
+
+          if package.is_a?(Array)
+            @candidate_version = []
+            final_installed_version = []
+            [package].flatten.each do |pkg|
+              @candidate_version << candidate_version[pkg]
+              final_installed_version << installed_version[pkg]
+            end
+            @current_resource.version(final_installed_version)
           else
-            package_name = "#{name}=#{version}"
-            package_name = name if @is_virtual_package
+            @candidate_version = candidate_version[package]
+            @current_resource.version(installed_version[package])
           end
+        end
+
+        def install_package(name, version)
+          name_array = [ name ].flatten
+          version_array = [ version ].flatten
+          package_name = name_array.zip(version_array).map do |n, v|
+            is_virtual_package[n] ? n : "#{n}=#{v}"
+          end.join(' ')
           run_noninteractive("apt-get -q -y#{expand_options(default_release_options)}#{expand_options(@new_resource.options)} install #{package_name}")
         end
 
@@ -138,20 +149,12 @@ class Chef
         end
 
         def remove_package(name, version)
-          if name.is_a?(Array)
-            package_name = name.join(' ')
-          else
-            package_name = name
-          end
+          package_name = [ name ].flatten.join(' ')
           run_noninteractive("apt-get -q -y#{expand_options(@new_resource.options)} remove #{package_name}")
         end
 
         def purge_package(name, version)
-          if name.is_a?(Array)
-            package_name = name.join(' ')
-          else
-            package_name = "#{name}"
-          end
+          package_name = [ name ].flatten.join(' ')
           run_noninteractive("apt-get -q -y#{expand_options(@new_resource.options)} purge #{package_name}")
         end
 
@@ -161,11 +164,7 @@ class Chef
         end
 
         def reconfig_package(name, version)
-          if name.is_a?(Array)
-            package_name = name.join(' ')
-          else
-            package_name = "#{name}"
-          end
+          package_name = [ name ].flatten.join(' ')
           Chef::Log.info("#{@new_resource} reconfiguring")
           run_noninteractive("dpkg-reconfigure #{package_name}")
         end
diff --git a/spec/unit/provider/package/apt_spec.rb b/spec/unit/provider/package/apt_spec.rb
index acf0707bbf..8528480689 100644
--- a/spec/unit/provider/package/apt_spec.rb
+++ b/spec/unit/provider/package/apt_spec.rb
@@ -334,7 +334,7 @@ mpg123 1.12.1-0ubuntu1
           end
 
           it "should not run debconf-set-selections if the preseed file has not changed" do
-            allow(@provider).to receive(:check_package_state)
+            allow(@provider).to receive(:check_all_packages_state)
             @current_resource.version "0.8.11"
             @new_resource.response_file "/tmp/file"
             allow(@provider).to receive(:get_preseed_file).and_return(false)
@@ -356,7 +356,7 @@ mpg123 1.12.1-0ubuntu1
 
         describe "when installing a virtual package" do
           it "should install the package without specifying a version" do
-            @provider.is_virtual_package = true
+            @provider.is_virtual_package['libmysqlclient-dev'] = true
             expect(@provider).to receive(:shell_out!).with(
               "apt-get -q -y install libmysqlclient-dev",
               :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil },
@@ -365,6 +365,21 @@ mpg123 1.12.1-0ubuntu1
             @provider.install_package("libmysqlclient-dev", "not_a_real_version")
           end
         end
+
+        describe "when installing multiple packages" do
+          it "can install a virtual package followed by a non-virtual package" do
+            # https://github.com/chef/chef/issues/2914
+            @provider.is_virtual_package['libmysqlclient-dev'] = true
+            @provider.is_virtual_package['irssi'] = false
+            expect(@provider).to receive(:shell_out!).with(
+              "apt-get -q -y install libmysqlclient-dev irssi=0.8.12-7",
+              :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil },
+              :timeout => @timeout
+            )
+            @provider.install_package(["libmysqlclient-dev", "irssi"], ["not_a_real_version", "0.8.12-7"])
+          end
+        end
+
       end
     end
   end