[pacman-dev] [PATCH] libalpm/sync.c : conflicts resolving cleanup.
The original patch from Nagy tried to resolve target vs target conflicts, and so broke the following pactests : sync040, sync041 and sync990 Nagy's proposal to solve this situation was to choose the interactive way, ask the user how to deal with it: either remove pkg1 or remove pkg2 or stop here. So he left this as a TODO. But instead of trying to resolve these conflicts or asking the user, I simply made pacman fail in this case. It's simpler, and still acceptable in my opinion. However, this broke the following pactests : sync893, sync897, sync898, sync992, sync999 But some of them are rather strange.. Reference: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009745.html Signed-off-by: Chantry Xavier <shiningxc@gmail.com> --- lib/libalpm/sync.c | 222 +++++++++++++++------------------------------------- 1 files changed, 62 insertions(+), 160 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index f6fa318..a2bc39c 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -385,20 +385,6 @@ int _alpm_sync_addtarget(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sy return(0); } -/* Helper functions for alpm_list_remove - */ -static int syncpkg_cmp(const void *s1, const void *s2) -{ - const pmsyncpkg_t *sp1 = s1; - const pmsyncpkg_t *sp2 = s2; - pmpkg_t *p1, *p2; - - p1 = alpm_sync_get_pkg(sp1); - p2 = alpm_sync_get_pkg(sp2); - - return(strcmp(alpm_pkg_get_name(p1), alpm_pkg_get_name(p2))); -} - int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync, alpm_list_t **data) { alpm_list_t *deps = NULL; @@ -490,170 +476,86 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync EVENT(trans, PM_TRANS_EVT_INTERCONFLICTS_START, NULL, NULL); _alpm_log(PM_LOG_DEBUG, "looking for conflicts\n"); - deps = _alpm_checkconflicts(db_local, list); + + /* 1. check for conflicts in the target list */ + _alpm_log(PM_LOG_DEBUG, "check targets vs targets\n"); + deps = _alpm_innerconflicts(list); + + /* TODO : should we try to resolve targets vs targets conflicts? */ if(deps) { - int errorout = 0; - alpm_list_t *asked = NULL; - pmconflict_t *conflict = NULL; - - for(i = deps; i && !errorout; i = i->next) { - pmsyncpkg_t *sync; - pmpkg_t *found = NULL; - - conflict = i->data; - _alpm_log(PM_LOG_DEBUG, "package '%s' conflicts with '%s'\n", - conflict->package1, conflict->package2); - /* check if the conflicting package is about to be removed/replaced. - * if so, then just ignore it. */ - for(j = trans->packages; j && !found; j = j->next) { - sync = j->data; - if(sync->type == PM_SYNC_TYPE_REPLACE) { - found = _alpm_pkg_find(conflict->package2, sync->data); - } - } - if(found) { - _alpm_log(PM_LOG_DEBUG, "'%s' is already elected for removal -- skipping\n", - alpm_pkg_get_name(found)); - continue; - } + pm_errno = PM_ERR_CONFLICTING_DEPS; + ret = -1; + *data = deps; + goto cleanup; + } - sync = _alpm_sync_find(trans->packages, conflict->package1); - if(sync == NULL) { - _alpm_log(PM_LOG_DEBUG, "'%s' not found in transaction set -- skipping\n", - conflict->package1); - continue; - } - pmpkg_t *local = _alpm_db_get_pkgfromcache(db_local, conflict->package2); - /* check if this package provides the package it's conflicting with */ - if(alpm_list_find(alpm_pkg_get_provides(sync->pkg), - conflict->package2, _alpm_prov_cmp)) { - /* treat like a replaces item so requiredby fields are - * inherited properly. */ - _alpm_log(PM_LOG_DEBUG, "package '%s' provides its own conflict\n", - conflict->package1); - if(!local) { - char *rmpkg = NULL; - int target, depend; - /* hmmm, package2 isn't installed, so it must be conflicting - * with another package in our final list. For example: - * - * pacman -S blackbox xfree86 - * - * If no x-servers are installed and blackbox pulls in xorg, then - * xorg and xfree86 will conflict with each other. In this case, - * we should follow the user's preference and rip xorg out of final, - * opting for xfree86 instead. - */ - - /* figure out which one was requested in targets. If they both - * were, then it's still an unresolvable conflict. */ - target = alpm_list_find_str(trans->targets, conflict->package1); - depend = alpm_list_find_str(trans->targets, conflict->package2); - if(depend && !target) { - _alpm_log(PM_LOG_DEBUG, "'%s' is in the target list -- keeping it\n", - conflict->package2); - /* remove conflict->package1 */ - rmpkg = conflict->package1; - } else if(target && !depend) { - _alpm_log(PM_LOG_DEBUG, "'%s' is in the target list -- keeping it\n", - conflict->package1); - /* remove conflict->package2 */ - rmpkg = conflict->package2; - } else { - /* miss->target2 is not needed, miss->target already provides - * it, let's resolve the conflict */ - rmpkg = conflict->package2; - } - if(rmpkg) { - pmsyncpkg_t *rsync = _alpm_sync_find(trans->packages, rmpkg); - void *vpkg; - _alpm_log(PM_LOG_DEBUG, "removing '%s' from target list\n", - rsync->pkg->name); - trans->packages = alpm_list_remove(trans->packages, rsync, - syncpkg_cmp, &vpkg); - _alpm_sync_free(vpkg); - continue; - } + /* 2. we check for target vs db conflicts (and resolve)*/ + _alpm_log(PM_LOG_DEBUG, "check targets vs db and db vs targets\n"); + deps = _alpm_outerconflicts(db_local, list); + + for(i = deps; i; i = i->next) { + pmconflict_t *conflict = i->data; + + /* if conflict->package2 (the local package) is not elected for removal, + we ask the user */ + int found = 0; + for(j = trans->packages; j && !found; j = j->next) { + pmsyncpkg_t *sync = j->data; + if(sync->type == PM_SYNC_TYPE_REPLACE) { + if(_alpm_pkg_find(conflict->package2, sync->data)) { + found = 1; } } - /* It's a conflict -- see if they want to remove it */ - _alpm_log(PM_LOG_DEBUG, "resolving package '%s' conflict\n", - conflict->package1); - if(local) { - int doremove = 0; - if(!alpm_list_find_str(asked, conflict->package2)) { - QUESTION(trans, PM_TRANS_CONV_CONFLICT_PKG, conflict->package1, + } + if(found) { + continue; + } + + _alpm_log(PM_LOG_DEBUG, "package '%s' conflicts with '%s'\n", + conflict->package1, conflict->package2); + + pmsyncpkg_t *sync = _alpm_sync_find(trans->packages, conflict->package1); + pmpkg_t *local = _alpm_db_get_pkgfromcache(db_local, conflict->package2); + int doremove = 0; + QUESTION(trans, PM_TRANS_CONV_CONFLICT_PKG, conflict->package1, conflict->package2, NULL, &doremove); - asked = alpm_list_add(asked, strdup(conflict->package2)); - if(doremove) { - pmpkg_t *q = _alpm_pkg_dup(local); - if(sync->type != PM_SYNC_TYPE_REPLACE) { - /* switch this sync type to REPLACE */ - sync->type = PM_SYNC_TYPE_REPLACE; - _alpm_pkg_free(sync->data); - sync->data = NULL; - } - /* append to the replaces list */ - _alpm_log(PM_LOG_DEBUG, "electing '%s' for removal\n", - conflict->package2); - sync->data = alpm_list_add(sync->data, q); - /* see if the package is in the current target list */ - pmsyncpkg_t *rsync = _alpm_sync_find(trans->packages, - conflict->package2); - if(rsync) { - /* remove it from the target list */ - void *vpkg; - _alpm_log(PM_LOG_DEBUG, "removing '%s' from target list\n", - conflict->package2); - trans->packages = alpm_list_remove(trans->packages, rsync, - syncpkg_cmp, &vpkg); - _alpm_sync_free(vpkg); - } - } else { - /* abort */ - _alpm_log(PM_LOG_ERROR, _("unresolvable package conflicts detected\n")); - errorout = 1; - } - } - } else { - _alpm_log(PM_LOG_ERROR, _("unresolvable package conflicts detected\n")); - errorout = 1; + if(doremove) { + pmpkg_t *q = _alpm_pkg_dup(local); + if(sync->type != PM_SYNC_TYPE_REPLACE) { + /* switch this sync type to REPLACE */ + sync->type = PM_SYNC_TYPE_REPLACE; + _alpm_pkg_free(sync->data); + sync->data = NULL; } - } - if(errorout) { - /* The last conflict was unresolvable, so we duplicate it and add it to *data */ - pm_errno = PM_ERR_CONFLICTING_DEPS; + /* append to the replaces list */ + _alpm_log(PM_LOG_DEBUG, "electing '%s' for removal\n", conflict->package2); + sync->data = alpm_list_add(sync->data, q); + } else { /* abort */ + _alpm_log(PM_LOG_ERROR, _("unresolvable package conflicts detected\n")); if(data) { - pmconflict_t *lastconflict = conflict; - if((conflict = malloc(sizeof(pmconflict_t))) == NULL) { - _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %zd bytes\n"), - sizeof(pmconflict_t)); + pmconflict_t *retconflict; + if((retconflict = malloc(sizeof(pmconflict_t))) == NULL) { + _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %zd bytes\n"), sizeof(pmconflict_t)); FREELIST(*data); pm_errno = PM_ERR_MEMORY; - } else { - *conflict = *lastconflict; - *data = alpm_list_add(*data, conflict); + ret = -1; + goto cleanup; } + *retconflict = *conflict; + *data = alpm_list_add(*data, retconflict); } - FREELIST(asked); - FREELIST(deps); + pm_errno = PM_ERR_CONFLICTING_DEPS; ret = -1; + FREELIST(deps); goto cleanup; } - FREELIST(asked); - FREELIST(deps); } EVENT(trans, PM_TRANS_EVT_INTERCONFLICTS_DONE, NULL, NULL); + FREELIST(deps); } if(!(trans->flags & PM_TRANS_FLAG_NODEPS)) { - /* rebuild remove and list */ - alpm_list_free(list); - list = NULL; - for(i = trans->packages; i; i = i->next) { - pmsyncpkg_t *sync = i->data; - list = alpm_list_add(list, sync->pkg); - } + /* rebuild remove list */ alpm_list_free(remove); remove = NULL; for(i = trans->packages; i; i = i->next) { @@ -670,7 +572,7 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync if(deps) { pm_errno = PM_ERR_UNSATISFIED_DEPS; ret = -1; - FREELIST(deps); + *data = deps; goto cleanup; } } -- 1.5.3.6
On Mon, Nov 26, 2007 at 05:04:09PM +0100, Nagy Gabor wrote:
The original patch from Nagy tried to resolve target vs target conflicts, and so broke the following pactests : sync040, sync041 and sync990
Nagy's proposal to solve this situation was to choose the interactive way, ask the user how to deal with it: either remove pkg1 or remove pkg2 or stop here. So he left this as a TODO.
But instead of trying to resolve these conflicts or asking the user, I simply made pacman fail in this case. It's simpler, and still acceptable in my opinion. However, this broke the following pactests : sync893, sync897, sync898, sync992, sync999 But some of them are rather strange.. Reference: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009745.html
Signed-off-by: Chantry Xavier <shiningxc@gmail.com>
I hope it's not too confusing :) I wrote the git log from my POV, but let Nagy the author of the commit :p
Idézés Xavier <shiningxc@gmail.com>:
On Mon, Nov 26, 2007 at 05:04:09PM +0100, Nagy Gabor wrote:
The original patch from Nagy tried to resolve target vs target conflicts, and so broke the following pactests : sync040, sync041 and sync990
Nagy's proposal to solve this situation was to choose the interactive way, ask the user how to deal with it: either remove pkg1 or remove pkg2 or stop here. So he left this as a TODO.
But instead of trying to resolve these conflicts or asking the user, I simply made pacman fail in this case. It's simpler, and still acceptable in my opinion. However, this broke the following pactests : sync893, sync897, sync898, sync992, sync999 But some of them are rather strange.. Reference: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009745.html
Signed-off-by: Chantry Xavier <shiningxc@gmail.com>
I hope it's not too confusing :) I wrote the git log from my POV, but let Nagy the author of the commit :p
Thanks a lot again. Sometimes I feel that you should use 'Chantry Xavier and Nagy Gabor' [or 'Chantry Xavier' only ;-] as the author ;-) Bye ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
participants (3)
-
Nagy Gabor
-
Nagy Gabor
-
Xavier