[Yum-devel] [PATCH] remove RPMDBPackageSack.installed()

[seth vidal]

On Thu, 2007-10-04 at 13:23 +0200, Tim Lauridsen wrote:
> Panu Matilainen wrote:
> > On Thu, 4 Oct 2007, Tim Lauridsen wrote:
> >> Panu Matilainen wrote:
> >>>
> >>> The problem with installed() is just that it's the wrong term for 
> >>> this - make it exists() and it'll make sense for all the package 
> >>> sack types. Whether a package is installed or not is just a question 
> >>> whether it exists in rpmdb or not, if you want to keep API compat 
> >>> just make rpmdb.installed() a wrapper for exists().
> >> Sound like a good idea, it still keeps the api simple.
> >>
> >>> Somewhat related, IMO the installed() method belongs to the package, 
> >>> not sack object. So something like
> >>>
> >>> for pkg in installable:
> >>> if self.rpmdb.installed(po=pkg):
> >>> # do stuff
> >>>
> >>> would become
> >>>
> >>> for pkg in installable:
> >>> if pkg.installed():
> >>> # do stuff
> >>>
> >>> ..but that's another story, dunno how feasible it'd be within yum 
> >>> currently (sorry I haven't been paying that much attention lately)
> >> Sound like a great idea, but can be tricky to implement.
> >> a YumInstalledPackage know it is installed, but a YumAvailablePackage 
> >> dont know if it is installed, I know it is available in a repo, but 
> >> it dont know if it is installed on the system.
> >
> > Yup, the package object would have to know about rpmdb then which is 
> > probably not that clean. A simple solution would be to move the 
> > installed() method to a higher level object that knows about rpmdb, 
> > such as YumBase. Basically
> >
> > class YumBase:
> > ...
> > def installed(self, po):
> > return self.rpmdb.exists(po)
> >
> > and the above example would become
> >
> > for pkg in installable:
> > if self.installed(po):
> > # ... do stuff
> >
> Sound like a good idea.
> 

-1

We don't need to pollute the top level name space for this. If installed
is only a mode of something in the rpmdb then why are we moving it
outside of the rpmsack?

When Florian proposed this whole thing on irc originally I said: 
1. it breaks the api if we move installed()
2. installed() makes no sense on PackageSackBase
3. if you want to check to see if a package exists inside whatever sack
object you have then we should wrap installed() in the rpmdb around
something like exists() in PackageSackBase and have exists take the same
args that installed() takes now.

Finally, unless there is a compelling reason we need to I do not want to
break the api right before we're finished with Fedora 8 and while folks
are still thinking about 3.2.X for rhel5/centos5 future releases.

-sv