Fixing the package type so that :ensure is always used for version specification, rather than :version, which is now deprecated. This provides much more consistency. I have not tested on all platforms yet, but I want to enable testing on Gentoo, also.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1559 980ebf18-57e1-0310-9a29-db15c13687c0

11 files changed, 66 insertions, 57 deletions

diff --git a/lib/puppet/provider/package/darwinport.rb b/lib/puppet/provider/package/darwinport.rb
index ccf0ae82d..7c19a90a6 100755
--- a/lib/puppet/provider/package/darwinport.rb
+++ b/lib/puppet/provider/package/darwinport.rb
@@ -8,7 +8,7 @@ Puppet::Type.type(:package).provide :darwinport do
         # list out all of the packages
         open("| #{PORT} list installed") { |process|
             regex = %r{(\S+)\s+@(\S+)\s+(\S+)}
-            fields = [:name, :version, :location]
+            fields = [:name, :ensure, :location]
             hash = {}
 
             # now turn each returned line into a package object
@@ -21,7 +21,6 @@ Puppet::Type.type(:package).provide :darwinport do
                     }
 
                     hash.delete :location
-                    hash[:ensure] = hash[:version]
                     yield hash.dup
                 else
                     raise Puppet::DevError,
diff --git a/lib/puppet/provider/package/dpkg.rb b/lib/puppet/provider/package/dpkg.rb
index 690f1ca85..cb68cd97e 100755
--- a/lib/puppet/provider/package/dpkg.rb
+++ b/lib/puppet/provider/package/dpkg.rb
@@ -18,7 +18,7 @@ Puppet::Type.type(:package).provide :dpkg do
         open("| #{command(:dpkg)} -l") { |process|
             # our regex for matching dpkg output
             regex = %r{^(\S+)\s+(\S+)\s+(\S+)\s+(.+)$}
-            fields = [:status, :name, :version, :description]
+            fields = [:status, :name, :ensure, :description]
             hash = {}
 
             5.times { process.gets } # throw away the header
@@ -54,7 +54,7 @@ Puppet::Type.type(:package).provide :dpkg do
         # stupid stupid
         oldcol = ENV["COLUMNS"]
         ENV["COLUMNS"] = "500"
-        fields = [:desired, :status, :error, :name, :version, :description]
+        fields = [:desired, :status, :error, :name, :ensure, :description]
 
         hash = {}
         # list out our specific package
@@ -87,13 +87,12 @@ Puppet::Type.type(:package).provide :dpkg do
         if hash[:error] != " "
             raise Puppet::PackageError.new(
                 "Package %s, version %s is in error state: %s" %
-                    [hash[:name], hash[:version], hash[:error]]
+            [hash[:name], hash[:ensure], hash[:error]]
             )
         end
 
-        if hash[:status] == "i"
-            hash[:ensure] = :present
-        else
+        # DPKG can discuss packages that are no longer installed, so allow that.
+        if hash[:status] != "i"
             hash[:ensure] = :absent
         end
 
diff --git a/lib/puppet/provider/package/freebsd.rb b/lib/puppet/provider/package/freebsd.rb
index cd484d47a..8d44870e4 100755
--- a/lib/puppet/provider/package/freebsd.rb
+++ b/lib/puppet/provider/package/freebsd.rb
@@ -32,7 +32,7 @@ Puppet::Type.type(:package).provide :freebsd, :parent => :openbsd do
     def query
         self.class.list
 
-        if @model[:version]
+        if @model.is(:ensure)
             return :listed
         else
             return nil
@@ -40,7 +40,7 @@ Puppet::Type.type(:package).provide :freebsd, :parent => :openbsd do
     end
 
     def uninstall
-        cmd = "#{command(:delete)} %s-%s" % [@model[:name], @model[:version]]
+        cmd = "#{command(:delete)} %s-%s" % [@model[:name], @model[:ensure]]
         begin
             output = execute(cmd)
         rescue Puppet::ExecutionFailure
diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb
index 227541ea0..f6c84c23c 100755
--- a/lib/puppet/provider/package/gem.rb
+++ b/lib/puppet/provider/package/gem.rb
@@ -21,7 +21,6 @@ Puppet::Type.type(:package).provide :gem do
             list = execute(command).split("\n\n").collect do |set|
                 if gemhash = gemsplit(set)
                     gemhash[:provider] = :gem
-                    gemhash[:ensure] = gemhash[:version][0]
                     gemhash
                 else
                     nil
@@ -43,10 +42,10 @@ Puppet::Type.type(:package).provide :gem do
         when /^\*\*\*/: return nil
         when /^(\S+)\s+\((.+)\)\n/
             name = $1
-            version = $2.split(/,\s*/)
+            version = $2.split(/,\s*/)[0]
             return {
                 :name => name,
-                :version => version
+                :ensure => version
             }
         else
             Puppet.warning "Could not match %s" % desc
@@ -62,8 +61,8 @@ Puppet::Type.type(:package).provide :gem do
 
     def install(useversion = true)
         command = "#{command(:gem)} install "
-        if @model[:version] and useversion
-            command += "-v %s " % @model[:version]
+        if (! @model.should(:ensure).is_a? Symbol) and useversion
+            command += "-v %s " % @model[:ensure]
         end
         if source = @model[:source]
             command += source
@@ -82,7 +81,7 @@ Puppet::Type.type(:package).provide :gem do
         # This always gets the latest version available.
         hash = self.class.gemlist(:justme => @model[:name])
 
-        return hash[:version][0]
+        return hash[:ensure]
     end
 
     def query
diff --git a/lib/puppet/provider/package/openbsd.rb b/lib/puppet/provider/package/openbsd.rb
index 05dee8656..2a0cb87d8 100755
--- a/lib/puppet/provider/package/openbsd.rb
+++ b/lib/puppet/provider/package/openbsd.rb
@@ -13,7 +13,7 @@ Puppet::Type.type(:package).provide :openbsd do
             execpipe(listcmd()) do |process|
                 # our regex for matching pkg_info output
                 regex = %r{^(\S+)-([^-\s]+)\s+(.+)}
-                fields = [:name, :version, :description]
+                fields = [:name, :ensure, :description]
                 hash = {}
 
                 # now turn each returned line into a package object
@@ -25,7 +25,6 @@ Puppet::Type.type(:package).provide :openbsd do
                         }
                         yup = nil
                         name = hash[:name]
-                        hash[:ensure] = :present
 
                         hash[:provider] = self.name
 
@@ -83,8 +82,7 @@ Puppet::Type.type(:package).provide :openbsd do
 
         # Search for the version info
         if info =~ /Information for #{@model[:name]}-(\S+)/
-            hash[:version] = $1
-            hash[:ensure] = :present
+            hash[:ensure] = $1
         else
             return nil
         end
diff --git a/lib/puppet/provider/package/ports.rb b/lib/puppet/provider/package/ports.rb
index 6a46dcc03..c3d18141a 100755
--- a/lib/puppet/provider/package/ports.rb
+++ b/lib/puppet/provider/package/ports.rb
@@ -79,7 +79,7 @@ Puppet::Type.type(:package).provide :ports, :parent => :freebsd do
     def query
         self.class.list
 
-        if @model[:version] and @model.is(:ensure) != :absent
+        if @model.is(:ensure) and @model.is(:ensure) != :absent
             return :listed
         else
             return nil
diff --git a/lib/puppet/provider/package/rpm.rb b/lib/puppet/provider/package/rpm.rb
index b82700770..0c9c26d42 100755
--- a/lib/puppet/provider/package/rpm.rb
+++ b/lib/puppet/provider/package/rpm.rb
@@ -58,7 +58,7 @@ Puppet::Type.type(:package).provide :rpm do
 
         regex = %r{^(\S+)\s+(\S+)}
         #fields = [:name, :ensure, :description]
-        fields = [:name, :version]
+        fields = [:name, :ensure]
         hash = {}
         if match = regex.match(output)
             fields.zip(match.captures) { |field,value|
diff --git a/lib/puppet/provider/package/up2date.rb b/lib/puppet/provider/package/up2date.rb
index 51c01317e..8cceb3735 100644
--- a/lib/puppet/provider/package/up2date.rb
+++ b/lib/puppet/provider/package/up2date.rb
@@ -39,7 +39,7 @@ Puppet.type(:package).provide :up2date, :parent => :rpm do
         else
             # up2date didn't find updates, pretend the current
             # version is the latest
-            return @model[:version]
+            return @model[:ensure]
         end
     end
 
diff --git a/lib/puppet/provider/package/yum.rb b/lib/puppet/provider/package/yum.rb
index 587b1911e..5be838746 100755
--- a/lib/puppet/provider/package/yum.rb
+++ b/lib/puppet/provider/package/yum.rb
@@ -36,7 +36,7 @@ Puppet::Type.type(:package).provide :yum, :parent => :rpm do
         else
             # Yum didn't find updates, pretend the current
             # version is the latest
-            return @model[:version]
+            return @model[:ensure]
         end
     end
 
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index f8dcecdb5..b23ee2622 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -65,6 +65,26 @@ module Puppet
                 end
             end
 
+            newvalue(/./) do
+                unless provider.versionable?
+                    self.fail(
+                        "Package provider %s does not support specifying versions" %
+                        @parent[:provider]
+                     )
+                end
+                method = if self.is == :absent
+                             :install
+                         else
+                             :update
+                         end
+                begin
+                    provider.send(method)
+                rescue => detail
+                    self.fail "Could not update: %s" % detail
+                end
+            end
+
+
             defaultto :installed
 
             # Override the parent method, because we've got all kinds of
@@ -106,16 +126,14 @@ module Puppet
                                 self.fail "Could not get latest version: %s" % detail
                             end
                         end
+
                         case @is
                         when @latest:
                             return true
                         when :present:
-                            if @parent[:version] == @latest
-                                return true
-                            else
-                                self.debug "our version is %s and latest is %s" %
-                                    [@parent[:version], @latest]
-                            end
+                            # This will only happen on retarded packaging systems
+                            # that can't query versions.
+                            return true
                         else
                             self.debug "@is is %s, latest %s is %s" %
                                 [@is, @parent.name, @latest]
@@ -149,7 +167,11 @@ module Puppet
                 else
                     #self.info "updating from %s" % value
                     provider.update
-                    :package_updated
+                    if self.is == :absent
+                        return :package_installed
+                    else
+                        return :package_updated
+                    end
                 end
             end
         end
@@ -194,7 +216,9 @@ module Puppet
         end
 
         newparam(:source) do
-            desc "From where to retrieve the package."
+            desc "Where to find the actual package.  This must be a local file
+                (or on a network file system) or a URL that your specific
+                packaging type understands; Puppet will not retrieve files for you."
 
             validate do |value|
                 unless value =~ /^#{File::SEPARATOR}/ or value =~ /\w+:\/\//
@@ -212,10 +236,10 @@ module Puppet
         end
 
         newparam(:type) do
-            desc "Deprecated form of ``use``."
+            desc "Deprecated form of ``provider``."
 
             munge do |value|
-                warning "'type' is deprecated; use 'use' instead"
+                warning "'type' is deprecated; use 'provider' instead"
                 @parent[:provider] = value
 
                 @parent[:provider]
@@ -237,23 +261,6 @@ module Puppet
                 generally be a fully qualified path."
         end
 
-        # FIXME Version is screwy -- most package systems can't specify a
-        # version, but people will definitely want to query versions, so
-        # it almost seems like versions should be a read-only state,
-        # supporting syncing only in certain cases.
-        newparam(:version) do
-            desc "This is a read-only parameter set by the packaging system.
-                This parameter is present so that you can audit existing package
-                information."
-
-#            validate do |value|
-#                unless @parent.respond_to?(:versionable?) and @parent.versionable?
-#                    raise Puppet::Error,
-#                        "Package type %s does not support specifying versions." %
-#                            @parent.pkgtype
-#                end
-#            end
-        end
         newparam(:category) do
             desc "A read-only parameter set by the package."
         end
diff --git a/test/types/package.rb b/test/types/package.rb
index 779c693fe..63ab28f48 100644
--- a/test/types/package.rb
+++ b/test/types/package.rb
@@ -135,8 +135,10 @@ class TestPackages < Test::Unit::TestCase
                 obj.retrieve
             }
 
-            # Version is a parameter, not a state.
-            assert(obj[:version], "Could not retrieve package version")
+            assert_instance_of(String, obj[:ensure],
+                "Ensure did not return a version number")
+            assert(obj[:ensure] =~ /[0-9.]/,
+                "Ensure did not return a version number")
         }
     end
 
@@ -186,7 +188,13 @@ class TestPackages < Test::Unit::TestCase
         pkgtype = Puppet::Type.type(:package)
 
         assert_nothing_raised("Could not list packages") do
-            pkgtype.list
+            count = 0
+            pkgtype.list.each do |pkg|
+                assert_instance_of(Puppet::Type.type(:package), pkg)
+                count += 1
+            end
+
+            assert(count > 1, "Did not get any packages")
         end
     end
 
@@ -383,8 +391,7 @@ class TestPackages < Test::Unit::TestCase
         assert_nothing_raised {
             gem = Puppet::Type.newpackage(
                 :name => name,
-                :version => "0.0.2",
-                :ensure => "installed",
+                :ensure => "0.0.2",
                 :provider => :gem
             )
         }
@@ -413,7 +420,7 @@ class TestPackages < Test::Unit::TestCase
             latest = gem.provider.latest
         }
 
-        assert(latest != gem[:version], "Did not correctly find latest value")
+        assert(latest != gem[:ensure], "Did not correctly find latest value")
 
         gem[:ensure] = :latest
         assert_events([:package_changed], gem)