dpkg multipackage and bonus fixes

- multipackages dpkg_package
- fixes edge conditions in load-current-resource around purging/removing
  packages that threw errors before
- fixes the ability to purge packages that have been removed
- adds a lot of functional tests for dpkg_package

5 files changed, 214 insertions, 64 deletions

diff --git a/lib/chef/mixin/get_source_from_package.rb b/lib/chef/mixin/get_source_from_package.rb
index 2ed251854a..cb5583b431 100644
--- a/lib/chef/mixin/get_source_from_package.rb
+++ b/lib/chef/mixin/get_source_from_package.rb
@@ -1,5 +1,5 @@
 # Author:: Lamont Granquist (<lamont@opscode.com>)
-# Copyright:: Copyright (c) 2008 Opscode, Inc.
+# Copyright:: Copyright (c) 2008-2015 Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -27,6 +27,12 @@
 class Chef
   module Mixin
     module GetSourceFromPackage
+      # FIXME:  this is some bad code that I wrote a long time ago.
+      #  - it does too much in the initializer
+      #  - it mutates the new_resource
+      #  - it does not support multipackage arrays
+      # this code is deprecated, check out the :use_package_names_for_source
+      # subclass directive instead
       def initialize(new_resource, run_context)
         super
         return if new_resource.package_name.is_a?(Array)
@@ -40,4 +46,3 @@ class Chef
     end
   end
 end
-
diff --git a/lib/chef/mixin/subclass_directive.rb b/lib/chef/mixin/subclass_directive.rb
new file mode 100644
index 0000000000..c0c7aa0328
--- /dev/null
+++ b/lib/chef/mixin/subclass_directive.rb
@@ -0,0 +1,42 @@
+#
+# Author:: Adam Jacob (<adam@opscode.com>)
+# Copyright:: Copyright (c) 2008-2015 Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+class Chef
+  module Mixin
+    module SubclassDirective
+      def self.included(base)
+        base.extend(ClassMethods)
+      end
+      module ClassMethods
+        def subclass_directive(sym)
+          define_singleton_method sym do
+            instance_variable_set(:"@#{sym}", true)
+          end
+
+          define_singleton_method :"#{sym}?" do
+            !!instance_variable_get(:"@#{sym}")
+          end
+
+          define_method :"#{sym}?" do
+            self.class.send(:"#{sym}?")
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/chef/provider/package.rb b/lib/chef/provider/package.rb
index 888b171fa4..c78d125f96 100644
--- a/lib/chef/provider/package.rb
+++ b/lib/chef/provider/package.rb
@@ -18,6 +18,7 @@
 
 require 'chef/mixin/shell_out'
 require 'chef/mixin/command'
+require 'chef/mixin/subclass_directive'
 require 'chef/log'
 require 'chef/file_cache'
 require 'chef/platform'
@@ -27,6 +28,12 @@ class Chef
     class Package < Chef::Provider
       include Chef::Mixin::Command
       include Chef::Mixin::ShellOut
+      include Chef::Mixin::SubclassDirective
+
+      # subclasses declare this if they want all their arguments as arrays of packages and names
+      subclass_directive :use_multipackage_api
+      # subclasses declare this if they want sources (filenames) pulled from their package names
+      subclass_directive :use_package_name_for_source
 
       #
       # Hook that subclasses use to populate the candidate_version(s)
@@ -44,6 +51,8 @@ class Chef
       end
 
       def check_resource_semantics!
+        # FIXME: this is not universally true and subclasses are needing to override this and no-ops it.  It should be turned into
+        # another "subclass_directive" and the apt and yum providers should declare that they need this behavior.
         if new_resource.package_name.is_a?(Array) && new_resource.source != nil
           raise Chef::Exceptions::InvalidResourceSpecification, "You may not specify both multipackage and source"
         end
@@ -211,7 +220,7 @@ class Chef
       # @todo use composition rather than inheritance
 
       def multipackage_api_adapter(name, version)
-        if supports_arrays?
+        if use_multipackage_api?
           yield [name].flatten, [version].flatten
         else
           yield name, version
@@ -242,7 +251,7 @@ class Chef
         raise( Chef::Exceptions::UnsupportedAction, "#{self.to_s} does not support :reconfig" )
       end
 
-      # this is heavily used by subclasses
+      # used by subclasses.  deprecated.  use #a_to_s instead.
       def expand_options(options)
         options ? " #{options}" : ""
       end
@@ -298,24 +307,6 @@ class Chef
         [ thing ].flatten
       end
 
-      class << self
-        attr_accessor :supports_arrays
-
-        def supports_arrays?
-          !!@supports_arrays
-        end
-
-        private
-
-        def use_multipackage_api
-          @supports_arrays = true
-        end
-      end
-
-      def supports_arrays?
-        self.class.supports_arrays?
-      end
-
       private
 
       # Returns the package names which need to be modified.  If the resource was called with an array of packages
@@ -528,6 +519,16 @@ class Chef
         end
         args
       end
+
+      # Helper for sublcasses to convert an array of string args into a string.  It
+      # will compact nil or empty strings in the array and will join the array elements
+      # with spaces, without introducing any double spaces for nil/empty elements.
+      #
+      # @param args [String] variable number of string arguments
+      # @return [String] nicely concatenated string or empty string
+      def a_to_s(*args)
+        args.reject {|i| i.nil? || i == "" }.join(" ")
+      end
     end
   end
 end
diff --git a/lib/chef/provider/package/dpkg.rb b/lib/chef/provider/package/dpkg.rb
index 2de6226bb9..35b6f4beee 100644
--- a/lib/chef/provider/package/dpkg.rb
+++ b/lib/chef/provider/package/dpkg.rb
@@ -19,31 +19,40 @@
 require 'chef/provider/package'
 require 'chef/mixin/command'
 require 'chef/resource/package'
-require 'chef/mixin/get_source_from_package'
 
 class Chef
   class Provider
     class Package
       class Dpkg < Chef::Provider::Package
+        DPKG_REMOVED = /^Status: deinstall ok config-files/
         DPKG_INSTALLED = /^Status: install ok installed/
         DPKG_VERSION = /^Version: (.+)$/
 
         provides :dpkg_package, os: "linux"
 
-        include Chef::Mixin::GetSourceFromPackage
+        use_multipackage_api
+        use_package_name_for_source
+
+        # semantics of dpkg properties:
+        #
+        # new_resource.name is always an array for this resource
+        # new_resource.package_name is always an array for this resource
+        # new_resource.source is always an array and may be [ nil ] for this resource.  properly use #sources or
+        #    #name_sources to also get the automatic package-name-to-source-conversion.  this will never be nil?
+        #
 
         def define_resource_requirements
           super
 
           requirements.assert(:install, :upgrade) do |a|
-            a.assertion { !new_resource.source.nil? }
+            a.assertion { !sources.compact.empty? }
             a.failure_message Chef::Exceptions::Package, "#{new_resource} the source property is required for action :install or :upgrade"
           end
 
           requirements.assert(:install, :upgrade) do |a|
-            a.assertion { source_file_exist? }
-            a.failure_message Chef::Exceptions::Package, "#{new_resource} source file does not exist: #{new_resource.source}"
-            a.whyrun "Assuming it would have been previously created."
+            a.assertion { source_files_exist? }
+            a.failure_message Chef::Exceptions::Package, "#{new_resource} source file(s) do not exist: #{missing_sources}"
+            a.whyrun "Assuming they would have been previously created."
           end
         end
 
@@ -51,11 +60,11 @@ class Chef
           @current_resource = Chef::Resource::Package.new(new_resource.name)
           current_resource.package_name(new_resource.package_name)
 
-          if source_file_exist?
+          if source_files_exist?
             @candidate_version = get_candidate_version
             current_resource.package_name(get_package_name)
             # if the source file exists then our package_name is right
-            current_resource.version(get_current_version)
+            current_resource.version(get_current_version_from(current_package_name_array))
           elsif !installing?
             # we can't do this if we're installing with no source, because our package_name
             # is probably not right.
@@ -65,31 +74,26 @@ class Chef
             #
             # we don't error here on the dpkg command since we'll handle the exception or
             # the why-run message in define_resource_requirements.
-            current_resource.version(get_current_version)
+            current_resource.version(get_current_version_from(current_package_name_array))
           end
 
           current_resource
         end
 
         def install_package(name, version)
-          Chef::Log.info("#{new_resource} installing #{new_resource.source}")
-          run_noninteractive(
-            "dpkg -i#{expand_options(new_resource.options)} #{new_resource.source}"
-          )
+          sources = name.map { |n| name_sources[n] }
+          Chef::Log.info("#{new_resource} installing package(s): #{name.join(' ')}")
+          run_noninteractive("dpkg -i", new_resource.options, *sources)
         end
 
         def remove_package(name, version)
-          Chef::Log.info("#{new_resource} removing #{new_resource.package_name}")
-          run_noninteractive(
-            "dpkg -r#{expand_options(new_resource.options)} #{new_resource.package_name}"
-          )
+          Chef::Log.info("#{new_resource} removing package(s): #{name.join(' ')}")
+          run_noninteractive("dpkg -r", new_resource.options, *name)
         end
 
         def purge_package(name, version)
-          Chef::Log.info("#{new_resource} purging #{new_resource.package_name}")
-          run_noninteractive(
-            "dpkg -P#{expand_options(new_resource.options)} #{new_resource.package_name}"
-          )
+          Chef::Log.info("#{new_resource} purging packages(s): #{name.join(' ')}")
+          run_noninteractive("dpkg -P", new_resource.options, *name)
         end
 
         def upgrade_package(name, version)
@@ -98,22 +102,29 @@ class Chef
 
         def preseed_package(preseed_file)
           Chef::Log.info("#{new_resource} pre-seeding package installation instructions")
-          run_noninteractive("debconf-set-selections #{preseed_file}")
+          run_noninteractive("debconf-set-selections", *preseed_file)
         end
 
         def reconfig_package(name, version)
           Chef::Log.info("#{new_resource} reconfiguring")
-          run_noninteractive("dpkg-reconfigure #{name}")
+          run_noninteractive("dpkg-reconfigure", *name)
+        end
+
+        # Override the superclass check.  Multiple sources are required here.
+        def check_resource_semantics!
         end
 
         private
 
-        def get_current_version
-          Chef::Log.debug("#{new_resource} checking install state")
-          status = shell_out_with_timeout("dpkg -s #{current_resource.package_name}")
+        def read_current_version_of_package(package_name)
+          Chef::Log.debug("#{new_resource} checking install state of #{package_name}")
+          status = shell_out_with_timeout("dpkg -s #{package_name}")
           package_installed = false
           status.stdout.each_line do |line|
             case line
+            when DPKG_REMOVED
+              # if we are 'purging' then we consider 'removed' to be 'installed'
+              package_installed = true if action == :purge
             when DPKG_INSTALLED
               package_installed = true
             when DPKG_VERSION
@@ -126,34 +137,124 @@ class Chef
           return nil
         end
 
+        def get_current_version_from(array)
+          array.map do |name|
+            read_current_version_of_package(name)
+          end
+        end
+
         # Runs command via shell_out_with_timeout with magic environment to disable
-        # interactive prompts. Command is run with default localization rather
-        # than forcing locale to "C", so command output may not be stable.
-        def run_noninteractive(command)
-          shell_out_with_timeout!(command, :env => { "DEBIAN_FRONTEND" => "noninteractive" })
+        # interactive prompts.
+        def run_noninteractive(*command)
+          shell_out_with_timeout!(a_to_s(*command), :env => { "DEBIAN_FRONTEND" => "noninteractive" })
+        end
+
+        # Returns true if all sources exist.  Returns false if any do not, or if no
+        # sources were specified.
+        #
+        # @return [Boolean] True if all sources exist
+        def source_files_exist?
+          sources.all? {|s| s && ::File.exist?(s) }
         end
 
-        def source_file_exist?
-          new_resource.source && ::File.exist?(new_resource.source)
+        # Helper to return all the nanes of the missing sources for error messages.
+        #
+        # @return [Array<String>] Array of missing sources
+        def missing_sources
+          sources.select {|s| s.nil? || !::File.exist?(s) }
+        end
+
+        def current_package_name_array
+          [ current_resource.package_name ].flatten
+        end
+
+        def source_array
+          if new_resource.source.nil?
+            package_name_array.map { nil }
+          else
+            [ new_resource.source ].flatten
+          end
+        end
+
+        # Helper to construct Array of sources.  If the new_resource.source is nil it
+        # will return an array filled will nil the same size as the package_name array
+        # For all the nil source values, if a file exists on the filesystem that
+        # matches the package name it will use that name as the source.
+        #
+        # @return [Array] Array of normalized sources with package_names converted to sources
+        def sources
+          @sources ||=
+            begin
+              source_array.each_with_index.map do |source, i|
+                package_name = package_name_array[i]
+                # we require at least one '/' in the package_name to avoid dpkg_package 'foo' breaking due to a random 'foo' file in cwd
+                if use_package_name_for_source? && source.nil? && package_name.match(/#{::File::SEPARATOR}/) && ::File.exist?(package_name)
+                  Chef::Log.debug("No package source specified, but #{package_name} exists on filesystem, using #{package_name} as source.")
+                  package_name
+                else
+                  source
+                end
+              end
+            end
+        end
+
+        # Helper to construct Hash of names-to-sources.
+        #
+        # @return [Hash] Mapping of package names to sources
+        def name_sources
+          @name_sources =
+            begin
+              Hash[*package_name_array.zip(sources).flatten]
+            end
+        end
+
+        # Helper to construct Hash of names-to-package-information.
+        #
+        # @return [Hash] Mapping of package names to package information
+        def name_pkginfo
+          @name_pkginfo ||=
+            begin
+              pkginfos = sources.map do |src|
+                Chef::Log.debug("#{new_resource} checking #{src} dpkg status")
+                status = shell_out_with_timeout!("dpkg-deb -W #{src}")
+                status.stdout
+              end
+              Hash[*package_name_array.zip(pkginfos).flatten]
+            end
+        end
+
+        def name_candidate_version
+          @name_candidate_version ||=
+            begin
+              Hash[name_pkginfo.map {|k, v| [k, v ? v.split("\t")[1].strip : nil] }]
+            end
         end
 
-        def pkginfo
-          @pkginfo ||=
+        def name_package_name
+          @name_package_name ||=
             begin
-              Chef::Log.debug("#{new_resource} checking dpkg status")
-              status = shell_out_with_timeout!("dpkg-deb -W #{new_resource.source}")
-              status.stdout.split("\t")
+              Hash[name_pkginfo.map {|k, v| [k, v ? v.split("\t")[0] : nil] }]
             end
         end
 
+        # Return candidate version array from pkg-deb -W against the source file(s).
+        #
+        # @return [Array] Array of candidate versions read from the source files
         def get_candidate_version
-          pkginfo[1].strip unless pkginfo.empty?
+          package_name_array.map { |name| name_candidate_version[name] }
         end
 
+        # Return package names from the candidate source file(s).
+        #
+        # @return [Array] Array of actual package names read from the source files
         def get_package_name
-          pkginfo[0] unless pkginfo.empty?
+          package_name_array.map { |name| name_package_name[name] }
         end
 
+        # Since upgrade just calls install, this is a helper to determine
+        # if our action means that we'll be calling install_package.
+        #
+        # @return [Boolean] true if we're doing :install or :upgrade
         def installing?
           [:install, :upgrade].include?(action)
         end
diff --git a/lib/chef/resource/dpkg_package.rb b/lib/chef/resource/dpkg_package.rb
index 38adf24cf6..9288c18632 100644
--- a/lib/chef/resource/dpkg_package.rb
+++ b/lib/chef/resource/dpkg_package.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Adam Jacob (<adam@opscode.com>)
-# Copyright:: Copyright (c) 2008 Opscode, Inc.
+# Copyright:: Copyright (c) 2008-2015 Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -17,14 +17,15 @@
 #
 
 require 'chef/resource/package'
-require 'chef/provider/package/dpkg'
 
 class Chef
   class Resource
     class DpkgPackage < Chef::Resource::Package
-
       provides :dpkg_package, os: "linux"
 
+      resource_name :dpkg_package
+
+      property :source, [String, Array, nil]
     end
   end
 end