dpkg provider cleanup

- :update and :install are now treated the same way and throw the
  same exceptions
- :remove and :purge don't require the source at all, so don't do
  any checking on that
- fix some convoluted side-effecty logic in load_current_resource
- load_current_resource now correctly gets the dpkg state on
  :remove and :purge when the file does not exist (pretty sure
  the old logic did not)
- fixed the FIXME about using en_US.UTF-8 (the default for shell_out!)
- just use shell_out! to throw exceptions
- clean up all the specs and remove all the instance vars from the code

1 files changed, 81 insertions, 60 deletions

diff --git a/lib/chef/provider/package/dpkg.rb b/lib/chef/provider/package/dpkg.rb
index 67e9b903c6..2de6226bb9 100644
--- a/lib/chef/provider/package/dpkg.rb
+++ b/lib/chef/provider/package/dpkg.rb
@@ -34,83 +34,61 @@ class Chef
 
         def define_resource_requirements
           super
-          requirements.assert(:install) do |a|
-            a.assertion{ not @new_resource.source.nil? }
-            a.failure_message Chef::Exceptions::Package, "Source for package #{@new_resource.name} required for action install"
+
+          requirements.assert(:install, :upgrade) do |a|
+            a.assertion { !new_resource.source.nil? }
+            a.failure_message Chef::Exceptions::Package, "#{new_resource} the source property is required for action :install or :upgrade"
           end
 
-          # TODO this was originally written for any action in which .source is provided
-          # but would it make more sense to only look at source if the action is :install?
-          requirements.assert(:all_actions) do |a|
-            a.assertion { @source_exists }
-            a.failure_message Chef::Exceptions::Package, "Package #{@new_resource.name} not found: #{@new_resource.source}"
-            a.whyrun "Assuming it would have been previously downloaded."
+          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."
           end
         end
 
         def load_current_resource
-          @source_exists = true
-          @current_resource = Chef::Resource::Package.new(@new_resource.name)
-          @current_resource.package_name(@new_resource.package_name)
-
-          if @new_resource.source
-            @source_exists = ::File.exists?(@new_resource.source)
-            if @source_exists
-              # Get information from the package if supplied
-              Chef::Log.debug("#{@new_resource} checking dpkg status")
-              status = shell_out_with_timeout("dpkg-deb -W #{@new_resource.source}")
-              pkginfo = status.stdout.split("\t")
-              unless pkginfo.empty?
-                @current_resource.package_name(pkginfo[0])
-                @candidate_version = pkginfo[1].strip
-              end
-            else
-              # Source provided but not valid means we can't safely do further processing
-              return
-            end
+          @current_resource = Chef::Resource::Package.new(new_resource.name)
+          current_resource.package_name(new_resource.package_name)
+
+          if source_file_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)
+          elsif !installing?
+            # we can't do this if we're installing with no source, because our package_name
+            # is probably not right.
+            #
+            # if we're removing or purging we don't use source, and our package_name must
+            # be right so we can do this.
+            #
+            # 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)
           end
 
-          # Check to see if it is installed
-          package_installed = nil
-          Chef::Log.debug("#{@new_resource} checking install state")
-          status = shell_out_with_timeout("dpkg -s #{@current_resource.package_name}")
-          status.stdout.each_line do |line|
-            case line
-            when DPKG_INSTALLED
-              package_installed = true
-            when DPKG_VERSION
-              if package_installed
-                Chef::Log.debug("#{@new_resource} current version is #{$1}")
-                @current_resource.version($1)
-              end
-            end
-          end
-
-          unless status.exitstatus == 0 || status.exitstatus == 1
-            raise Chef::Exceptions::Package, "dpkg failed - #{status.inspect}!"
-          end
-
-          @current_resource
+          current_resource
         end
 
         def install_package(name, version)
-          Chef::Log.info("#{@new_resource} installing #{@new_resource.source}")
+          Chef::Log.info("#{new_resource} installing #{new_resource.source}")
           run_noninteractive(
-            "dpkg -i#{expand_options(@new_resource.options)} #{@new_resource.source}"
+            "dpkg -i#{expand_options(new_resource.options)} #{new_resource.source}"
           )
         end
 
         def remove_package(name, version)
-          Chef::Log.info("#{@new_resource} removing #{@new_resource.package_name}")
+          Chef::Log.info("#{new_resource} removing #{new_resource.package_name}")
           run_noninteractive(
-            "dpkg -r#{expand_options(@new_resource.options)} #{@new_resource.package_name}"
+            "dpkg -r#{expand_options(new_resource.options)} #{new_resource.package_name}"
           )
         end
 
         def purge_package(name, version)
-          Chef::Log.info("#{@new_resource} purging #{@new_resource.package_name}")
+          Chef::Log.info("#{new_resource} purging #{new_resource.package_name}")
           run_noninteractive(
-            "dpkg -P#{expand_options(@new_resource.options)} #{@new_resource.package_name}"
+            "dpkg -P#{expand_options(new_resource.options)} #{new_resource.package_name}"
           )
         end
 
@@ -119,22 +97,65 @@ class Chef
         end
 
         def preseed_package(preseed_file)
-          Chef::Log.info("#{@new_resource} pre-seeding package installation instructions")
+          Chef::Log.info("#{new_resource} pre-seeding package installation instructions")
           run_noninteractive("debconf-set-selections #{preseed_file}")
         end
 
         def reconfig_package(name, version)
-          Chef::Log.info("#{@new_resource} reconfiguring")
+          Chef::Log.info("#{new_resource} reconfiguring")
           run_noninteractive("dpkg-reconfigure #{name}")
         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}")
+          package_installed = false
+          status.stdout.each_line do |line|
+            case line
+            when DPKG_INSTALLED
+              package_installed = true
+            when DPKG_VERSION
+              if package_installed
+                Chef::Log.debug("#{new_resource} current version is #{$1}")
+                return $1
+              end
+            end
+          end
+          return nil
+        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.
-        #
-        # FIXME: This should be "LC_ALL" => "en_US.UTF-8" in order to stabilize the output and get UTF-8
         def run_noninteractive(command)
-          shell_out_with_timeout!(command, :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil })
+          shell_out_with_timeout!(command, :env => { "DEBIAN_FRONTEND" => "noninteractive" })
+        end
+
+        def source_file_exist?
+          new_resource.source && ::File.exist?(new_resource.source)
+        end
+
+        def pkginfo
+          @pkginfo ||=
+            begin
+              Chef::Log.debug("#{new_resource} checking dpkg status")
+              status = shell_out_with_timeout!("dpkg-deb -W #{new_resource.source}")
+              status.stdout.split("\t")
+            end
+        end
+
+        def get_candidate_version
+          pkginfo[1].strip unless pkginfo.empty?
+        end
+
+        def get_package_name
+          pkginfo[0] unless pkginfo.empty?
+        end
+
+        def installing?
+          [:install, :upgrade].include?(action)
         end
 
       end