[pacman-dev] Please use _alpm_depcmp!
Hi! We have this nice function, but it is very rarely used in libalpm source. In most cases it is reimplemented (see trans.c/_alpm_trans_update_depends for example); but there is a bigger problem: in many cases only a package-name comparison is used to check dependencies, which may lead problems (I concerned about this in my previous mails: http://archlinux.org/pipermail/pacman-dev/2007-March/002155.html for example). So the whole source should be checked, and this would lead to a much cleaner and more bug-free code. Bye, Nagy Gabor
On 4/1/07, Nagy Gabor
Hi! We have this nice function, but it is very rarely used in libalpm source. In most cases it is reimplemented (see trans.c/_alpm_trans_update_depends for example); but there is a bigger problem: in many cases only a package-name comparison is used to check dependencies, which may lead problems (I concerned about this in my previous mails: http://archlinux.org/pipermail/pacman-dev/2007-March/002155.html for example). So the whole source should be checked, and this would lead to a much cleaner and more bug-free code.
patches are welcome
Hi! To be continued... ;-) ----------------------- diff -Naur pacman-lib/lib/libalpm/package.c pacman-lib.new/lib/libalpm/package.c --- pacman-lib/lib/libalpm/package.c 2007-03-12 05:47:58.000000000 +0100 +++ pacman-lib.new/lib/libalpm/package.c 2007-04-10 23:33:57.000000000 +0200 @@ -536,11 +536,12 @@ return(0); } - +/* This function scans the whole local db to fill in the + 'requiredby' field of a package + (this is useful if we want to install this package) */ void _alpm_pkg_update_requiredby(pmpkg_t *pkg) { - alpm_list_t *i, *j, *k; - const char *pkgname = alpm_pkg_get_name(pkg); + alpm_list_t *i, *j; pmdb_t *localdb = alpm_option_get_localdb(); for(i = _alpm_db_get_pkgcache(localdb); i; i = i->next) { @@ -555,38 +556,20 @@ if(!j->data) { continue; } - dep = alpm_splitdep(j->data); - if(dep == NULL) { - continue; - } - - /* check the actual package itself */ - if(strcmp(dep->name, pkgname) == 0) { + dep = alpm_splitdep(j->data); + if(dep == NULL) { + continue; + } + int satisfies = alpm_depcmp(pkg, dep); + free(dep); + if(satisfies) { alpm_list_t *reqs = alpm_pkg_get_requiredby(pkg); - - if(!alpm_list_find_str(reqs, cachepkgname)) { - _alpm_log(PM_LOG_DEBUG, _("adding '%s' in requiredby field for '%s'"), - cachepkgname, pkg->name); - reqs = alpm_list_add(reqs, strdup(cachepkgname)); - pkg->requiredby = reqs; - } + _alpm_log(PM_LOG_DEBUG, _("adding '%s' in requiredby field for '%s'"), + cachepkgname, pkg->name); + reqs = alpm_list_add(reqs, strdup(cachepkgname)); + pkg->requiredby = reqs; + break; // we add an entry only once } - - /* check for provisions as well */ - for(k = alpm_pkg_get_provides(pkg); k; k = k->next) { - const char *provname = k->data; - if(strcmp(dep->name, provname) == 0) { - alpm_list_t *reqs = alpm_pkg_get_requiredby(pkg); - - if(!alpm_list_find_str(reqs, cachepkgname)) { - _alpm_log(PM_LOG_DEBUG, _("adding '%s' in requiredby field for '%s' (provides: %s)"), - cachepkgname, pkgname, provname); - reqs = alpm_list_add(reqs, strdup(cachepkgname)); - pkg->requiredby = reqs; - } - } - } - free(dep); } } } ------------------------ Bye, Nagy Gabor
On 4/10/07, Nagy Gabor
Hi! To be continued... ;-) ----------------------- diff -Naur pacman-lib/lib/libalpm/package.c pacman-lib.new/lib/libalpm/package.c --- pacman-lib/lib/libalpm/package.c 2007-03-12 05:47:58.000000000 +0100 +++ pacman-lib.new/lib/libalpm/package.c 2007-04-10 23:33:57.000000000 +0200
Hey Nagy, Cleaning out the email inbox, so we'll start at the earliest emails I have. Can you explain exactly what this patch does before I commit it? I made the changes locally on my GIT tree and it seems to work without problems (at least in pactest's eyes). However, I want to know WHY it works, and why we should change it. In addition, this makes a good commit message. Thanks! -Dan
Can you explain exactly what this patch does before I commit it? Hi! -this is cleaner;-) -_alpm_checkdeps is sophisticated and if we do some changes in dependency checking (for example we add version number for provisions .-), we need to change the _alpm_checkdeps function only. -the old version didn't take care of version numbers, just the package names..., I provide you a requiredby000.py test, which fails without the patch. Bye, ngaba ----requiredby.py---- self.description = "A package was removed with -Rd, then we downgrade it...: requiredby test"
lp1 = pmpkg("pkg1") lp1.depends = ["pkg2=1.0-2"] self.addpkg2db("local", lp1) p = pmpkg("pkg2", "1.0-1") self.addpkg(p) self.args = "-A %s" % p.filename() self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=pkg1") self.addrule("PKG_VERSION=pkg2|1.0-1") self.addrule("PKG_EXIST=pkg2") self.addrule("!PKG_REQUIREDBY=pkg2|pkg1")
participants (3)
-
Aaron Griffin
-
Dan McGee
-
Nagy Gabor