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

[Tim Lauridsen]

Florian Festi wrote:
> Panu Matilainen wrote:
>> On Thu, 4 Oct 2007, Tim Lauridsen wrote:
>>
>>> Florian Festi wrote:
>>>> Hi!
>>>>
>>>> While creating the new test frame work I came across several minor 
>>>> issues. One problem is the existence of the 
>>>> RPMDBPackageSack.installed() method. It is currently only supported 
>>>> by the RPMDBPackageSack class. This doesn't allow to replace the 
>>>> rpmdb with an inmemory PackageSack. Seth already rejected the idea 
>>>> of just moving the method to the base class as "installed" doesn't 
>>>> make any sense for non RpmDB sacks.
>>>>
>>>> As I really dislike inconsistent APIs I'd like to suggest to just 
>>>> remove the usage of that method from the code. There are just 7 
>>>> places in yum and one place in yum-utils where it is used and it is 
>>>> nothing more than a tiny convenience wrapper. In fact it can be 
>>>> replaced quite easily by .searchNevra() - one of our work horse 
>>>> methods. The attached patches remove the usage of the method and 
>>>> add an deprecation warning (feel free to ignore that if we want to 
>>>> keep the method).
>>>>
>>>> Florian
>>>>
>>>> PS: The patch also fixes a small issue when running without 
>>>> repositories which simplifies setting up the test cases.
>>>> ------------------------------------------------------------------------ 
>>>>
>>>>
>>>> _______________________________________________
>>>> Yum-devel mailing list
>>>> > Yum-devel at linux.duke.edu
>>>> https://lists.dulug.duke.edu/mailman/listinfo/yum-devel
>>> RPMDBPackageSack.installed() make the code look better and make it 
>>> easier to understand.
>>>
>>> self.rpmdb.installed(po)
>>>
>>> Look much better then self.rpmdb.searchNevra(po.name, po.epoch, 
>>> po.version, po.release, po.arch)
>
> Agreed. What about adding that po param to the searchNevra method(s)? 
> This shouldn't break any code and would also be more compact. It would 
> also make more clear what really happens. There always is the 
> ambiguity between identity and equality - in other words: "Is there a 
> packages with the same Nevra in the Sack?" or "Is exactly this 
> packages (from the same origin) in the Sack?". searchNevra() makes 
> clear that we are checking for equality while installed() is a bit 
> unclear.
self.rpmdb.instralled(po=po) is typical used if i what to test if a 
available package is installed on the system (exists() is a better name 
as Panu suggested), there self.rpmdb.searchNevra is used to find 
packages packages matching Nevra.
In side the depsolver code, it might make more sense to use 
self.rpmdb.searchNevra sometimes, but if you are writing a plugin or a 
util using yum then 'if self.rpmdb.instralled(po=po):" make it easier to 
read the code than 'if self.rpmdb.searchNevra(n,e,v,r,a):'

Adding po param is a good idea to avoid  searchNevra(po.name, po.epoch, 
po.version, po.release, po.arch) and just use searchNevra(po=po)
>
>>> Removing the installed() method is a API breakage and there is other 
>>> API users out there (yumex, pirut, pungi, revisor, PackageKit etc)
>
> As you can see there currently isn't any patch removing the method 
> itself. But the yum API in not in such a good shape that we can 
> reasonably ban changes forever. Of course the method would be 
> deprecated for quite some time before we can remove it. In fact I 
> don't mind if we still keep the method without deprecating it. But I 
> do mind that all Sack classes should be replaceable by a PackageSack 
> (expect more stuff in that direction to come). We simply cannot build 
> proper unit tests without.
>
Yes, i know we can ban changes forever, and i don't what that, but when 
coding an application til yumex, there shall work with
yum 3.0.x and yum 3.2.x and later, it is really hurting is the API 
changes to much, and for other not following the yum development so 
close, it  is even more painful.
I have no problems changing the installed to searchNevra inside the 
depsolver, if i have some benfits, but there is no need to deprecate 
installed method used by highlevel API users like the yum-cli, yumex, 
pirut and other, and make the code in these application harder to read.
Adding the po param to searchNevra and replacing  rpmdb.installed with 
rpmdb.searchNevra inside the depsolver is fine to me, but there is no 
need to do it in yum-cli.

Tim