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
> [email protected]
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
_______________________________________________
Yum-devel mailing list
[email protected]
https://lists.dulug.duke.edu/mailman/listinfo/yum-devel
