Adding Sun support and fixing the last remaining bugs related to the daemon changes i just made

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

9 files changed, 465 insertions, 389 deletions

diff --git a/lib/puppet.rb b/lib/puppet.rb
index 932ee9e1c..518c143df 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -75,6 +75,7 @@ PUPPETVERSION = '0.10.0'
         :logdir         => [:puppetvar,      "log"],
         :bucketdir      => [:puppetvar,      "bucket"],
         :statedir       => [:puppetvar,      "state"],
+        :rundir         => [:puppetvar,      "run"],
 
         # then the files},
         :manifestdir    => [:puppetconf,     "manifests"],
diff --git a/lib/puppet/daemon.rb b/lib/puppet/daemon.rb
index 68b6ff3d9..d4014f995 100755
--- a/lib/puppet/daemon.rb
+++ b/lib/puppet/daemon.rb
@@ -5,6 +5,15 @@ require 'puppet'
 module Puppet # :nodoc:
     # A module that handles operations common to all daemons.
     module Daemon
+        def daemonname
+            $0.sub(/.+#{File::SEPARATOR}/,'')
+        end
+
+        # The path to the pid file for this server
+        def pidfile
+            File.join(Puppet[:rundir], daemonname() + ".pid")
+        end
+
         # Put the daemon into the background.
         def daemonize
             if pid = fork()
@@ -12,6 +21,8 @@ module Puppet # :nodoc:
                 exit(0)
             end
 
+            setpidfile()
+
             # Get rid of console logging
             Puppet::Log.close(:console)
 
@@ -29,8 +40,6 @@ module Puppet # :nodoc:
                 Puppet.err "Could not start %s: %s" % [$0, detail]
                 exit(12)
             end
-
-            setpidfile()
         end
 
         def fqdn
@@ -171,8 +180,7 @@ module Puppet # :nodoc:
 
         # Create the pid file.
         def setpidfile
-            name = $0.gsub(/.+#{File::SEPARATOR}/,'')
-            @pidfile = File.join(Puppet[:puppetvar], "run", name + ".pid")
+            @pidfile = self.pidfile
             if FileTest.exists?(@pidfile)
                 Puppet.info "Deleting old pid file"
                 begin
@@ -183,10 +191,17 @@ module Puppet # :nodoc:
                 end
             end
 
+            unless FileTest.exists?(Puppet[:rundir])
+                Puppet.recmkdir(Puppet[:rundir])
+                File.chmod(01777, Puppet[:rundir])
+            end
+
+            Puppet.info "Setting pidfile to %s" % @pidfile
             begin
                 File.open(@pidfile, "w") { |f| f.puts $$ }
             rescue => detail
                 Puppet.err "Could not create PID file: %s" % detail
+                exit(74)
             end
             Puppet.info "pid file is %s" % @pidfile
         end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index dc7f0dab9..ad76cfe8e 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -935,8 +935,7 @@ class Type < Puppet::Element
         private :new
     end
 
-    # initialize the type instance
-    def initialize(hash)
+    def initvars
         @children = []
         @evalcount = 0
 
@@ -973,6 +972,15 @@ class Type < Puppet::Element
         @syncedchanges = 0
         @failedchanges = 0
 
+        @inited = true
+    end
+
+    # initialize the type instance
+    def initialize(hash)
+        unless defined? @inited
+            self.initvars
+        end
+
         # Before anything else, set our parent if it was included
         if hash.include?(:parent)
             @parent = hash[:parent]
@@ -1024,8 +1032,11 @@ class Type < Puppet::Element
 
     # For any parameters or states that have defaults and have not yet been
     # set, set them now.
-    def setdefaults
-        self.class.allattrs.each { |attr|
+    def setdefaults(*ary)
+        if ary.empty?
+            ary = self.class.allattrs
+        end
+        ary.each { |attr|
             type = self.class.attrtype(attr)
             next if self.attrset?(type, attr)
 
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index 5f1c37939..8b57c4833 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -4,11 +4,6 @@
 # systems.
 
 require 'puppet/type/state'
-require 'puppet/type/package/dpkg.rb'
-require 'puppet/type/package/apt.rb'
-require 'puppet/type/package/rpm.rb'
-require 'puppet/type/package/yum.rb'
-require 'puppet/type/package/sun.rb'
 
 module Puppet
     class PackageError < Puppet::Error; end
@@ -17,6 +12,55 @@ module Puppet
             from remote sources but currently only supports packaging
             systems which can retrieve their own packages, like ``apt``."
 
+        # Create a new packaging type
+        def self.newpkgtype(name, parent = nil, &block)
+            @pkgtypes ||= {}
+
+            if @pkgtypes.include?(name)
+                raise Puppet::DevError, "Package type %s already defined" % name
+            end
+
+            mod = Module.new
+
+            # Add our parent, if it exists
+            if parent
+                unless @pkgtypes.include?(parent)
+                    raise Puppet::DevError, "No parent type %s for package type %s" %
+                        [parent, name]
+                end
+                mod.send(:include, @pkgtypes[parent])
+            end
+
+            # And now define the support methods
+            code = %{
+                def self.name
+                    "#{name}"
+                end
+
+                def self.to_s
+                    "PkgType(#{name})"
+                end
+
+                def pkgtype
+                    "#{name}"
+                end
+            }
+
+            mod.module_eval(code)
+
+            mod.module_eval(&block)
+
+            @pkgtypes[name] = mod
+        end
+
+        def self.pkgtype(name)
+            @pkgtypes[name]
+        end
+
+        def self.pkgtypes
+            @pkgtypes.keys
+        end
+
         newstate(:install) do
             desc "What state the package should be in.  Specifying *true* will
                 only result in a change if the package is not installed at all;
@@ -32,8 +76,8 @@ module Puppet
                     unless @parent.respond_to?(:latest)
                         self.err @parent.inspect
                         raise Puppet::Error,
-                            "Package type %s does not support querying versions" %
-                            @parent[:type]
+                            "Package type %s cannot install later versions" %
+                            @parent[:type].name
                     end
                     return :latest
                 when true, :installed:
@@ -135,37 +179,32 @@ module Puppet
         # into the general package object...
         attr_reader :version, :pkgtype
 
-        @pkgtypes = [
-            Puppet::PackagingType::APT,
-            Puppet::PackagingType::DPKG,
-            Puppet::PackagingType::RPM,
-            Puppet::PackagingType::Yum,
-            Puppet::PackagingType::Sun
-        ]
-
-        pkgstr = @pkgtypes.collect {|t|
-                "``" + t.name.to_s + "``"
-            }.join(", ")
-
-        @pkgtypehash = {}
-
-        # Now collect the name of each type and store it thusly
-        @pkgtypes.each { |type|
-            if type.respond_to?(:typename)
-                @pkgtypehash[type.typename] = type
-            else
-                name = type.to_s.sub(/.+::/, '').downcase.intern
-                @pkgtypehash[name] = type
-            end
-        }
-
         newparam(:name) do
             desc "The package name."
             isnamevar
         end
 
         newparam(:type) do
-            desc "The package format.  Currently supports " + pkgstr
+            desc "The package format, e.g., rpm or dpkg."
+
+            defaultto { @parent.class.default }
+
+            # We cannot log in this routine, because this gets called before
+            # there's a name for the package.
+            munge do |type|
+                @parent.type2module(type)
+            end
+        end
+
+        newparam(:source) do
+            desc "From where to retrieve the package."
+
+            validate do |value|
+                unless value =~ /^#{File::SEPARATOR}/
+                    raise Puppet::Error,
+                        "Package sources must be fully qualified files"
+                end
+            end
         end
         newparam(:instance) do
             desc "A read-only parameter set by the package."
@@ -228,7 +267,7 @@ module Puppet
                 )
             end
             case @platform
-            when "solaris": @default = :sun
+            when "solaris": @default = :sunpkg
             when "gentoo":
                 Puppet.notice "No support for gentoo yet"
                 @default = nil
@@ -316,26 +355,27 @@ module Puppet
             end
         end
 
-        # Retrieve a package type by name; names are symbols.
-        def self.pkgtype(pkgtype)
-            if pkgtype.is_a?(String)
-                pkgtype = pkgtype.intern
-            end
-            return @pkgtypehash[pkgtype]
-        end
-
         # okay, there are two ways that a package could be created...
         # either through the language, in which case the hash's values should
         # be set in 'should', or through comparing against the system, in which
         # case the hash's values should be set in 'is'
         def initialize(hash)
-            type = hash["type"] || hash[:type] || self.class.default
-            self.type2module(type)
+            self.initvars
+            type = nil
+            [:type, "type"].each { |label|
+                if hash.include?(label)
+                    type = hash[label]
+                    hash.delete(label)
+                end
+            }
+            if type
+                self[:type] = type
+            else
+                self.setdefaults(:type)
+            end
 
             super
 
-            self.debug "Extending to package type %s" % [@type]
-
             unless @states.include?(:install)
                 self.debug "Defaulting to installing a package"
                 self[:install] = true
@@ -367,8 +407,9 @@ module Puppet
         # Extend the package with the appropriate package type.
         def type2module(typename)
             if type = self.class.pkgtype(typename)
-                @type = type
                 self.extend(type)
+
+                return type
             else
                 raise Puppet::Error, "Invalid package type %s" % typename
             end
@@ -420,4 +461,11 @@ module Puppet
     }
 end
 
+# The order these are loaded is important.
+require 'puppet/type/package/dpkg.rb'
+require 'puppet/type/package/apt.rb'
+require 'puppet/type/package/rpm.rb'
+require 'puppet/type/package/yum.rb'
+require 'puppet/type/package/sun.rb'
+
 # $Id$
diff --git a/lib/puppet/type/package/apt.rb b/lib/puppet/type/package/apt.rb
index dbc764df5..8b77ca38f 100755
--- a/lib/puppet/type/package/apt.rb
+++ b/lib/puppet/type/package/apt.rb
@@ -1,70 +1,67 @@
 module Puppet
-    module PackagingType
+    Puppet.type(:package).newpkgtype(:apt, :dpkg) do
         # A derivative of DPKG; this is how most people actually manage
         # Debian boxes, and the only thing that differs is that it can
         # install packages from remote sites.
-        module APT
-            include DPKG
 
-            # Install a package using 'apt-get'.  This function needs to support
-            # installing a specific version.
-            def install
-                should = self.should(:install)
+        # Install a package using 'apt-get'.  This function needs to support
+        # installing a specific version.
+        def install
+            should = self.should(:install)
 
-                str = self.name
-                case should
-                when true, false, Symbol
-                    # pass
-                else
-                    # Add the package version
-                    str += "=%s" % should
-                end
-                cmd = "apt-get -q -y install %s" % str
+            str = self.name
+            case should
+            when true, false, Symbol
+                # pass
+            else
+                # Add the package version
+                str += "=%s" % should
+            end
+            cmd = "apt-get -q -y install %s" % str
 
-                self.info "Executing %s" % cmd.inspect
-                output = %x{#{cmd} 2>&1}
+            self.info "Executing %s" % cmd.inspect
+            output = %x{#{cmd} 2>&1}
 
-                unless $? == 0
-                    raise Puppet::PackageError.new(output)
-                end
+            unless $? == 0
+                raise Puppet::PackageError.new(output)
             end
+        end
 
-            # What's the latest package version available?
-            def latest
-                cmd = "apt-cache showpkg %s" % self.name 
-                output = %x{#{cmd} 2>&1}
-
-                unless $? == 0
-                    raise Puppet::PackageError.new(output)
-                end
+        # What's the latest package version available?
+        def latest
+            cmd = "apt-cache showpkg %s" % self.name 
+            output = %x{#{cmd} 2>&1}
 
-                if output =~ /Versions:\s*\n((\n|.)+)^$/
-                    versions = $1
-                    version = versions.split(/\n/).collect { |version|
-                        if version =~ /^([^\(]+)\(/
-                            $1
-                        else
-                            self.warning "Could not match version '%s'" % version
-                            nil
-                        end
-                    }.reject { |vers| vers.nil? }.sort[-1]
+            unless $? == 0
+                raise Puppet::PackageError.new(output)
+            end
 
-                    unless version
-                        self.debug "No latest version"
-                        if Puppet[:debug]
-                            print output
-                        end
+            if output =~ /Versions:\s*\n((\n|.)+)^$/
+                versions = $1
+                version = versions.split(/\n/).collect { |version|
+                    if version =~ /^([^\(]+)\(/
+                        $1
+                    else
+                        self.warning "Could not match version '%s'" % version
+                        nil
                     end
+                }.reject { |vers| vers.nil? }.sort[-1]
 
-                    return version
-                else
-                    self.err "Could not match string"
+                unless version
+                    self.debug "No latest version"
+                    if Puppet[:debug]
+                        print output
+                    end
                 end
-            end
 
-            def update
-                self.install
+                return version
+            else
+                self.err "Could not match string"
             end
         end
+
+        def update
+            self.install
+        end
     end
 end
diff --git a/lib/puppet/type/package/dpkg.rb b/lib/puppet/type/package/dpkg.rb
index 48a1a497b..e33ba5a73 100755
--- a/lib/puppet/type/package/dpkg.rb
+++ b/lib/puppet/type/package/dpkg.rb
@@ -1,105 +1,102 @@
 module Puppet
-    module PackagingType
-        # The packaging system for Debian systems.
-        module DPKG
-            def query
-                packages = []
-
-                # dpkg only prints as many columns as you have available
-                # which means we don't get all of the info
-                # stupid stupid
-                oldcol = ENV["COLUMNS"]
-                ENV["COLUMNS"] = "500"
-                fields = [:desired, :status, :error, :name, :version, :description]
+    Puppet.type(:package).newpkgtype(:dpkg) do
+        def query
+            packages = []
+
+            # dpkg only prints as many columns as you have available
+            # which means we don't get all of the info
+            # stupid stupid
+            oldcol = ENV["COLUMNS"]
+            ENV["COLUMNS"] = "500"
+            fields = [:desired, :status, :error, :name, :version, :description]
+
+            hash = {}
+            # list out our specific package
+            open("| dpkg -l %s 2>/dev/null" % self.name) { |process|
+                # our regex for matching dpkg output
+                regex = %r{^(.)(.)(.)\s(\S+)\s+(\S+)\s+(.+)$}
+
+                # we only want the last line
+                lines = process.readlines
+                # we've got four header lines, so we should expect all of those
+                # plus our output
+                if lines.length < 5
+                    return nil
+                end
+
+                line = lines[-1]
+
+                if match = regex.match(line)
+                    fields.zip(match.captures) { |field,value|
+                        hash[field] = value
+                    }
+                    #packages.push Puppet.type(:package).installedpkg(hash)
+                else
+                    raise Puppet::DevError,
+                        "failed to match dpkg line %s" % line
+                end
+            }
+            ENV["COLUMNS"] = oldcol
+
+            if hash[:error] != " "
+                raise Puppet::PackageError.new(
+                    "Package %s, version %s is in error state: %s" %
+                        [hash[:name], hash[:install], hash[:error]]
+                )
+            end
 
+            if hash[:status] == "i"
+                hash[:install] = hash[:version]
+            else
+                hash[:install] = :notinstalled
+            end
+
+            return hash
+        end
+
+        def list
+            packages = []
+
+            # dpkg only prints as many columns as you have available
+            # which means we don't get all of the info
+            # stupid stupid
+            oldcol = ENV["COLUMNS"]
+            ENV["COLUMNS"] = "500"
+
+            # list out all of the packages
+            open("| dpkg -l") { |process|
+                # our regex for matching dpkg output
+                regex = %r{^(\S+)\s+(\S+)\s+(\S+)\s+(.+)$}
+                fields = [:status, :name, :install, :description]
                 hash = {}
-                # list out our specific package
-                open("| dpkg -l %s 2>/dev/null" % self.name) { |process|
-                    # our regex for matching dpkg output
-                    regex = %r{^(.)(.)(.)\s(\S+)\s+(\S+)\s+(.+)$}
-
-                    # we only want the last line
-                    lines = process.readlines
-                    # we've got four header lines, so we should expect all of those
-                    # plus our output
-                    if lines.length < 5
-                        return nil
-                    end
 
-                    line = lines[-1]
+                5.times { process.gets } # throw away the header
 
+                # now turn each returned line into a package object
+                process.each { |line|
                     if match = regex.match(line)
+                        hash.clear
+
                         fields.zip(match.captures) { |field,value|
                             hash[field] = value
                         }
-                        #packages.push Puppet.type(:package).installedpkg(hash)
+                        packages.push Puppet.type(:package).installedpkg(hash)
                     else
                         raise Puppet::DevError,
-                            "failed to match dpkg line %s" % line
+                            "Failed to match dpkg line %s" % line
                     end
                 }
-                ENV["COLUMNS"] = oldcol
-
-                if hash[:error] != " "
-                    raise Puppet::PackageError.new(
-                        "Package %s, version %s is in error state: %s" %
-                            [hash[:name], hash[:install], hash[:error]]
-                    )
-                end
-
-                if hash[:status] == "i"
-                    hash[:install] = hash[:version]
-                else
-                    hash[:install] = :notinstalled
-                end
-
-                return hash
-            end
-
-            def list
-                packages = []
-
-                # dpkg only prints as many columns as you have available
-                # which means we don't get all of the info
-                # stupid stupid
-                oldcol = ENV["COLUMNS"]
-                ENV["COLUMNS"] = "500"
-
-                # list out all of the packages
-                open("| dpkg -l") { |process|
-                    # our regex for matching dpkg output
-                    regex = %r{^(\S+)\s+(\S+)\s+(\S+)\s+(.+)$}
-                    fields = [:status, :name, :install, :description]
-                    hash = {}
-
-                    5.times { process.gets } # throw away the header
-
-                    # now turn each returned line into a package object
-                    process.each { |line|
-                        if match = regex.match(line)
-                            hash.clear
-
-                            fields.zip(match.captures) { |field,value|
-                                hash[field] = value
-                            }
-                            packages.push Puppet.type(:package).installedpkg(hash)
-                        else
-                            raise Puppet::DevError,
-                                "Failed to match dpkg line %s" % line
-                        end
-                    }
-                }
-                ENV["COLUMNS"] = oldcol
+            }
+            ENV["COLUMNS"] = oldcol
 
-                return packages
-            end
+            return packages
+        end
 
-            def remove
-                cmd = "dpkg -r %s" % self.name
-                output = %x{#{cmd} 2>&1}
-                if $? != 0
-                    raise Puppet::PackageError.new(output)
-                end
+        def remove
+            cmd = "dpkg -r %s" % self.name
+            output = %x{#{cmd} 2>&1}
+            if $? != 0
+                raise Puppet::PackageError.new(output)
             end
         end
     end
diff --git a/lib/puppet/type/package/rpm.rb b/lib/puppet/type/package/rpm.rb
index af340ff40..1cb7fc5ec 100755
--- a/lib/puppet/type/package/rpm.rb
+++ b/lib/puppet/type/package/rpm.rb
@@ -1,80 +1,78 @@
 module Puppet
-    module PackagingType
-        module RPM
-            def query
-                fields = {
-                    :name => "NAME",
-                    :install => "VERSION",
-                    :description => "DESCRIPTION"
-                }
-
-                cmd = "rpm -q #{self.name} --qf '%s\n'" %
-                    "%{NAME} %{VERSION}-%{RELEASE}"
+    Puppet.type(:package).newpkgtype(:rpm) do
+        def query
+            fields = {
+                :name => "NAME",
+                :install => "VERSION",
+                :description => "DESCRIPTION"
+            }
 
-                # list out all of the packages
-                output = %x{#{cmd} 2>/dev/null}.chomp
+            cmd = "rpm -q #{self.name} --qf '%s\n'" %
+                "%{NAME} %{VERSION}-%{RELEASE}"
 
-                if $? != 0
-                    return nil
-                end
+            # list out all of the packages
+            output = %x{#{cmd} 2>/dev/null}.chomp
 
-                regex = %r{^(\S+)\s+(\S+)}
-                #fields = [:name, :install, :description]
-                fields = [:name, :install]
-                hash = {}
-                if match = regex.match(output)
-                    fields.zip(match.captures) { |field,value|
-                        hash[field] = value
-                    }
-                else
-                    raise Puppet::DevError,
-                        "Failed to match rpm output '%s'" %
-                        output
-                end
+            if $? != 0
+                return nil
+            end
 
-                return hash
+            regex = %r{^(\S+)\s+(\S+)}
+            #fields = [:name, :install, :description]
+            fields = [:name, :install]
+            hash = {}
+            if match = regex.match(output)
+                fields.zip(match.captures) { |field,value|
+                    hash[field] = value
+                }
+            else
+                raise Puppet::DevError,
+                    "Failed to match rpm output '%s'" %
+                    output
             end
 
-            def list
-                packages = []
+            return hash
+        end
 
-                # list out all of the packages
-                open("| rpm -q -a --qf '%{NAME} %{VERSION}\n'") { |process|
-                    # our regex for matching dpkg output
-                    regex = %r{^(\S+)\s+(\S+)}
-                    fields = [:name, :install]
-                    hash = {}
+        def list
+            packages = []
 
-                    # now turn each returned line into a package object
-                    process.each { |line|
-                        if match = regex.match(line)
-                            hash.clear
+            # list out all of the packages
+            open("| rpm -q -a --qf '%{NAME} %{VERSION}\n'") { |process|
+                # our regex for matching dpkg output
+                regex = %r{^(\S+)\s+(\S+)}
+                fields = [:name, :install]
+                hash = {}
 
-                            fields.zip(match.captures) { |field,value|
-                                hash[field] = value
-                            }
-                            packages.push Puppet.type(:package).installedpkg(hash)
-                        else
-                            raise "failed to match rpm line %s" % line
-                        end
-                    }
+                # now turn each returned line into a package object
+                process.each { |line|
+                    if match = regex.match(line)
+                        hash.clear
+
+                        fields.zip(match.captures) { |field,value|
+                            hash[field] = value
+                        }
+                        packages.push Puppet.type(:package).installedpkg(hash)
+                    else
+                        raise "failed to match rpm line %s" % line
+                    end
                 }
+            }
 
-                return packages
-            end
+            return packages
+        end
 
-            # we need package retrieval mechanisms before we can have package
-            # installation mechanisms...
-            #type.install = proc { |pkg|
-            #    raise "installation not implemented yet"
-            #}
+        # we need package retrieval mechanisms before we can have package
+        # installation mechanisms...
+        #type.install = proc { |pkg|
+        #    raise "installation not implemented yet"
+        #}
 
-            def remove
-                cmd = "rpm -e %s" % self.name
-                output = %x{#{cmd}}
-                if $? != 0
-                    raise output
-                end
+        def remove
+            cmd = "rpm -e %s" % self.name
+            output = %x{#{cmd}}
+            if $? != 0
+                raise output
             end
         end
     end
diff --git a/lib/puppet/type/package/sun.rb b/lib/puppet/type/package/sun.rb
index 90e5c579e..3e58fc2e5 100755
--- a/lib/puppet/type/package/sun.rb
+++ b/lib/puppet/type/package/sun.rb
@@ -1,116 +1,128 @@
 module Puppet
-    module PackagingType
-        module Sun
-            def query
-                names = {
-                    "PKGINST" => :name,
-                    "NAME" => nil,
-                    "CATEGORY" => :category,
-                    "ARCH" => :platform,
-                    "VERSION" => :install,
-                    "BASEDIR" => :root,
-                    "HOTLINE" => nil,
-                    "EMAIL" => nil,
-                    "VENDOR" => :vendor,
-                    "DESC" => :description,
-                    "PSTAMP" => nil,
-                    "INSTDATE" => nil,
-                    "STATUS" => nil,
-                    "FILES" => nil
-                }
+    Puppet.type(:package).newpkgtype(:sunpkg) do
+        def install
+            unless self[:source]
+                raise Puppet::Error, "Sun packages must specify a package source"
+            end
+            cmd = "pkgadd -d %s -n %s 2>&1" % [self[:source], self[:name]]
+
+            self.info "Executing %s" % cmd.inspect
+            output = %x{#{cmd} 2>&1}
+
+            unless $? == 0
+                raise Puppet::PackageError.new(output)
+            end
+        end
 
-                hash = {}
+        def query
+            names = {
+                "PKGINST" => :name,
+                "NAME" => nil,
+                "CATEGORY" => :category,
+                "ARCH" => :platform,
+                "VERSION" => :install,
+                "BASEDIR" => :root,
+                "HOTLINE" => nil,
+                "EMAIL" => nil,
+                "VENDOR" => :vendor,
+                "DESC" => :description,
+                "PSTAMP" => nil,
+                "INSTDATE" => nil,
+                "STATUS" => nil,
+                "FILES" => nil
+            }
 
-                # list out all of the packages
-                open("| pkginfo -l %s 2>/dev/null" % self.name) { |process|
-                    # we're using the long listing, so each line is a separate
-                    # piece of information
-                    process.each { |line|
-                        case line
-                        when /^$/:  # ignore
-                        when /\s*([A-Z]+):\s+(.+)/:
-                            name = $1
-                            value = $2
-                            if names.include?(name)
-                                unless names[name].nil?
-                                    hash[names[name]] = value
-                                end
-                            else
-                                self.err "'pkginfo' returned invalid name %s" %
-                                    name
+            hash = {}
+
+            # list out all of the packages
+            open("| pkginfo -l %s 2>/dev/null" % self.name) { |process|
+                # we're using the long listing, so each line is a separate
+                # piece of information
+                process.each { |line|
+                    case line
+                    when /^$/:  # ignore
+                    when /\s*([A-Z]+):\s+(.+)/:
+                        name = $1
+                        value = $2
+                        if names.include?(name)
+                            unless names[name].nil?
+                                hash[names[name]] = value
                             end
-                        when /\s+\d+.+/:
-                            # nothing; we're ignoring the FILES info
+                        else
+                            self.err "'pkginfo' returned invalid name %s" %
+                                name
                         end
-                    }
+                    when /\s+\d+.+/:
+                        # nothing; we're ignoring the FILES info
+                    end
                 }
+            }
 
-                if hash.empty?
-                    return nil
-                else
-                    return hash
-                end
+            if hash.empty?
+                return nil
+            else
+                return hash
             end
+        end
 
-            def list
-                packages = []
-                hash = {}
-                names = {
-                    "PKGINST" => :name,
-                    "NAME" => nil,
-                    "CATEGORY" => :category,
-                    "ARCH" => :platform,
-                    "VERSION" => :install,
-                    "BASEDIR" => :root,
-                    "HOTLINE" => nil,
-                    "EMAIL" => nil,
-                    "VENDOR" => :vendor,
-                    "DESC" => :description,
-                    "PSTAMP" => nil,
-                    "INSTDATE" => nil,
-                    "STATUS" => nil,
-                    "FILES" => nil
-                }
+        def list
+            packages = []
+            hash = {}
+            names = {
+                "PKGINST" => :name,
+                "NAME" => nil,
+                "CATEGORY" => :category,
+                "ARCH" => :platform,
+                "VERSION" => :install,
+                "BASEDIR" => :root,
+                "HOTLINE" => nil,
+                "EMAIL" => nil,
+                "VENDOR" => :vendor,
+                "DESC" => :description,
+                "PSTAMP" => nil,
+                "INSTDATE" => nil,
+                "STATUS" => nil,
+                "FILES" => nil
+            }
 
-                # list out all of the packages
-                open("| pkginfo -l") { |process|
-                    # we're using the long listing, so each line is a separate
-                    # piece of information
-                    process.each { |line|
-                        case line
-                        when /^$/:
-                            packages.push Puppet.type(:package).installedpkg(hash)
-                            hash.clear
-                        when /\s*(\w+):\s+(.+)/:
-                            name = $1
-                            value = $2
-                            if names.include?(name)
-                                unless names[name].nil?
-                                    hash[names[name]] = value
-                                end
-                            else
-                                raise "Could not find %s" % name
+            # list out all of the packages
+            open("| pkginfo -l 2>&1") { |process|
+                # we're using the long listing, so each line is a separate
+                # piece of information
+                process.each { |line|
+                    case line
+                    when /^$/:
+                        packages.push Puppet.type(:package).installedpkg(hash)
+                        hash.clear
+                    when /\s*(\w+):\s+(.+)/:
+                        name = $1
+                        value = $2
+                        if names.include?(name)
+                            unless names[name].nil?
+                                hash[names[name]] = value
                             end
-                        when /\s+\d+.+/:
-                            # nothing; we're ignoring the FILES info
+                        else
+                            raise "Could not find %s" % name
                         end
-                    }
+                    when /\s+\d+.+/:
+                        # nothing; we're ignoring the FILES info
+                    end
                 }
-                return packages
-            end
+            }
+            return packages
+        end
 
-            # we need package retrieval mechanisms before we can have package
-            # installation mechanisms...
-            #type.install = proc { |pkg|
-            #    raise "installation not implemented yet"
-            #}
+        # we need package retrieval mechanisms before we can have package
+        # installation mechanisms...
+        #type.install = proc { |pkg|
+        #    raise "installation not implemented yet"
+        #}
 
-            def remove
-                cmd = "pkgrm -n %s" % self.name
-                output = %x{#{cmd}}
-                if $? != 0
-                    raise output
-                end
+        def remove
+            cmd = "pkgrm -n %s 2>&1" % self.name
+            output = %x{#{cmd}}
+            if $? != 0
+                raise Puppet::Error, "Removal of %s failed: %s" % [self.name, output]
             end
         end
     end
diff --git a/lib/puppet/type/package/yum.rb b/lib/puppet/type/package/yum.rb
index beb356f89..b7b833e9c 100755
--- a/lib/puppet/type/package/yum.rb
+++ b/lib/puppet/type/package/yum.rb
@@ -1,58 +1,55 @@
 module Puppet
-    module PackagingType
+    Puppet.type(:package).newpkgtype(:yum, :rpm) do
         # A derivative of DPKG; this is how most people actually manage
         # Debian boxes, and the only thing that differs is that it can
         # install packages from remote sites.
-        module Yum
-            include RPM
 
-            # Install a package using 'apt-get'.
-            def install
-                cmd = "yum -y install %s" % self.name
+        # Install a package using 'apt-get'.
+        def install
+            cmd = "yum -y install %s" % self.name
 
-                self.info "Executing %s" % cmd.inspect
-                output = %x{#{cmd} 2>&1}
+            self.info "Executing %s" % cmd.inspect
+            output = %x{#{cmd} 2>&1}
 
-                unless $? == 0
-                    raise Puppet::PackageError.new(output)
-                end
+            unless $? == 0
+                raise Puppet::PackageError.new(output)
             end
+        end
 
-            # What's the latest package version available?
-            def latest
-                cmd = "yum list %s" % self.name 
-                output = %x{#{cmd} 2>&1}
+        # What's the latest package version available?
+        def latest
+            cmd = "yum list %s" % self.name 
+            output = %x{#{cmd} 2>&1}
 
-                unless $? == 0
-                    raise Puppet::PackageError.new(output)
-                end
-
-                if output =~ /#{self.name}\S+\s+(\S+)\s/
-                    return $1
-                else
-                    self.debug "No version"
-                    if Puppet[:debug]
-                        print output
-                    end
+            unless $? == 0
+                raise Puppet::PackageError.new(output)
+            end
 
-                    return nil
+            if output =~ /#{self.name}\S+\s+(\S+)\s/
+                return $1
+            else
+                self.debug "No version"
+                if Puppet[:debug]
+                    print output
                 end
+
+                return nil
             end
+        end
 
-            def update
-                cmd = "yum -y update %s" % self.name
+        def update
+            cmd = "yum -y update %s" % self.name
 
-                self.info "Executing %s" % cmd.inspect
-                output = %x{#{cmd} 2>&1}
+            self.info "Executing %s" % cmd.inspect
+            output = %x{#{cmd} 2>&1}
 
-                unless $? == 0
-                    raise Puppet::PackageError.new(output)
-                end
+            unless $? == 0
+                raise Puppet::PackageError.new(output)
             end
+        end
 
-            def versionable?
-                false
-            end
+        def versionable?
+            false
         end
     end
 end