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

[Florian Festi]

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.

>> 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.

Florian