On Thu, Jul 12, 2007 at 09:09:41PM +0200, ngaba@petra.hos.u-szeged.hu wrote:
Wouldn't it be better to use **list instead then ? AFAIK, if we'd use **list, that assumption could be dropped. Bye, ngaba
I see, that's also the problem you had with your previous patch for removedeps. I took it back, and made a few changes, including usage of **targs instead of targs. I also renamed removedeps to recursedeps, added an include_explicit argument, and added some comments. Does this look ok to you ?
From e9ec64516957efb3eba4a9226ac52131636fa013 Mon Sep 17 00:00:00 2001 From: Chantry Xavier <shiningxc@gmail.com> Date: Fri, 13 Jul 2007 15:30:37 +0200 Subject: [PATCH] libalpm/deps.c : fix for remove044 pactest.
Patch from Nagy that makes removedeps use alpm_depcmp. I also renamed removedeps to recursedeps, as it can have a more general usage, and added an include_explicit argument, so we can control if packages explictly installed are added or not. Signed-off-by: Chantry Xavier <shiningxc@gmail.com> --- lib/libalpm/deps.c | 100 +++++++++++++++++++++++-------------------------- lib/libalpm/deps.h | 2 +- lib/libalpm/remove.c | 10 ++-- 3 files changed, 53 insertions(+), 59 deletions(-) diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 591e5a8..fa99630 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -484,8 +484,10 @@ pmdepend_t SYMEXPORT *alpm_splitdep(const char *depstring) /* These parameters are messy. We check if this package, given a list of * targets (and a db), is safe to remove. We do NOT remove it if it is in the - * target list */ -static int can_remove_package(pmdb_t *db, pmpkg_t *pkg, alpm_list_t *targets) + * target list. We do not remove it either if the package was + * explictly installed, and include_explicit == 0 */ +static int can_remove_package(pmdb_t *db, pmpkg_t *pkg, alpm_list_t *targets, + int include_explicit) { alpm_list_t *i; @@ -493,13 +495,21 @@ static int can_remove_package(pmdb_t *db, pmpkg_t *pkg, alpm_list_t *targets) return(0); } - /* see if it was explicitly installed */ - if(alpm_pkg_get_reason(pkg) == PM_PKG_REASON_EXPLICIT) { - _alpm_log(PM_LOG_DEBUG, "excluding %s -- explicitly installed", - alpm_pkg_get_name(pkg)); - return(0); + if(!include_explicit) { + /* see if it was explicitly installed */ + if(alpm_pkg_get_reason(pkg) == PM_PKG_REASON_EXPLICIT) { + _alpm_log(PM_LOG_DEBUG, "excluding %s -- explicitly installed", + alpm_pkg_get_name(pkg)); + return(0); + } } + /* TODO: checkdeps could be used there, it handles multiple providers + * better, but that also makes it slower. + * Also this would require to first add the package to the targets list, + * then call checkdeps with it, then remove the package from the targets list + * if checkdeps detected it would break something */ + /* see if other packages need it */ for(i = alpm_pkg_get_requiredby(pkg); i; i = i->next) { pmpkg_t *reqpkg = _alpm_db_get_pkgfromcache(db, i->data); @@ -512,71 +522,55 @@ static int can_remove_package(pmdb_t *db, pmpkg_t *pkg, alpm_list_t *targets) return(1); } -/* return a new alpm_list_t target list containing all packages in the original - * target list, as well as all their un-needed dependencies. By un-needed, - * I mean dependencies that are *only* required for packages in the target - * list, so they can be safely removed. This function is recursive. +/* Adds un-needed dependencies in a list of packages + * By un-needed, I mean dependencies that are *only* required for packages in the target + * list, so they can be safely removed. + * @param targs pointer to a list of packages + * @param include_explicit if set to 0, then explictly installed packages are not added */ -alpm_list_t *_alpm_removedeps(pmdb_t *db, alpm_list_t *targs) +void _alpm_recursedeps(pmdb_t *db, alpm_list_t **targs, int include_explicit) { alpm_list_t *i, *j, *k; - alpm_list_t *newtargs = targs; ALPM_LOG_FUNC; - if(db == NULL) { - return(newtargs); + if(db == NULL || targs == NULL) { + return; } - for(i = targs; i; i = i->next) { - pmpkg_t *pkg = i->data; - for(j = alpm_pkg_get_depends(pkg); j; j = j->next) { - pmdepend_t *depend = alpm_splitdep(j->data); - pmpkg_t *deppkg; - if(depend == NULL) { - continue; - } - - deppkg = _alpm_db_get_pkgfromcache(db, depend->name); - if(deppkg == NULL) { - /* package not found... look for a provision instead */ - alpm_list_t *provides = _alpm_db_whatprovides(db, depend->name); - if(!provides) { - /* Not found, that's fine, carry on */ - _alpm_log(PM_LOG_DEBUG, "cannot find package \"%s\" or anything that provides it!", depend->name); + /* TODO: the while loop should be removed if we can assume + * that alpm_list_add (or another function) adds to the end of the list, + * and that the target list is topo sorted (by _alpm_sortbydeps()). + */ + int ready = 0; + while(!ready) { + ready = 1; + for(i = *targs; i; i = i->next) { + pmpkg_t *pkg = i->data; + for(j = alpm_pkg_get_depends(pkg); j; j = j->next) { + pmdepend_t *depend = alpm_splitdep(j->data); + if(depend == NULL) { continue; } - for(k = provides; k; k = k->next) { - pmpkg_t *provpkg = k->data; - if(can_remove_package(db, provpkg, newtargs)) { - pmpkg_t *pkg = _alpm_pkg_dup(provpkg); - - _alpm_log(PM_LOG_DEBUG, "adding '%s' to the targets", - alpm_pkg_get_name(pkg)); + for(k = _alpm_db_get_pkgcache(db); k; k = k->next) { + pmpkg_t *deppkg = k->data; + if(alpm_depcmp(deppkg,depend) + && can_remove_package(db, deppkg, *targs, include_explicit)) { + _alpm_log(PM_LOG_DEBUG, "adding '%s' to the targets", + alpm_pkg_get_name(deppkg)); /* add it to the target list */ - newtargs = alpm_list_add(newtargs, pkg); - newtargs = _alpm_removedeps(db, newtargs); + *targs = alpm_list_add(*targs, _alpm_pkg_dup(deppkg)); + ready = 0; } } - alpm_list_free(provides); - } else if(can_remove_package(db, deppkg, newtargs)) { - pmpkg_t *pkg = _alpm_pkg_dup(deppkg); - - _alpm_log(PM_LOG_DEBUG, "adding '%s' to the targets", - alpm_pkg_get_name(pkg)); - - /* add it to the target list */ - newtargs = alpm_list_add(newtargs, pkg); - newtargs = _alpm_removedeps(db, newtargs); + free(depend); } - free(depend); } } - - return(newtargs); } + /* populates *list with packages that need to be installed to satisfy all * dependencies (recursive) for syncpkg * diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 2edbb50..f11a19a 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -58,7 +58,7 @@ int _alpm_depmiss_isin(pmdepmissing_t *needle, alpm_list_t *haystack); alpm_list_t *_alpm_sortbydeps(alpm_list_t *targets, pmtranstype_t mode); alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, alpm_list_t *packages); -alpm_list_t *_alpm_removedeps(pmdb_t *db, alpm_list_t *targs); +void _alpm_recursedeps(pmdb_t *db, alpm_list_t **targs, int include_explicit); int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *syncpkg, alpm_list_t *list, alpm_list_t *trail, pmtrans_t *trans, alpm_list_t **data); diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 57e452d..9876f41 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -131,11 +131,6 @@ int _alpm_remove_prepare(pmtrans_t *trans, pmdb_t *db, alpm_list_t **data) } } - if(trans->flags & PM_TRANS_FLAG_RECURSE) { - _alpm_log(PM_LOG_DEBUG, "finding removable dependencies"); - trans->packages = _alpm_removedeps(db, trans->packages); - } - /* re-order w.r.t. dependencies */ _alpm_log(PM_LOG_DEBUG, "sorting by dependencies"); lp = _alpm_sortbydeps(trans->packages, PM_TRANS_TYPE_REMOVE); @@ -143,6 +138,11 @@ int _alpm_remove_prepare(pmtrans_t *trans, pmdb_t *db, alpm_list_t **data) alpm_list_free(trans->packages); trans->packages = lp; + if(trans->flags & PM_TRANS_FLAG_RECURSE) { + _alpm_log(PM_LOG_DEBUG, "finding removable dependencies"); + _alpm_recursedeps(db, &trans->packages, 0); + } + EVENT(trans, PM_TRANS_EVT_CHECKDEPS_DONE, NULL, NULL); } -- 1.5.2.2