On Sun, Aug 05, 2007 at 05:29:39PM +0200, Nagy Gabor wrote:

Hi! If you want to apply this patch, please follow my patch flow ;-) I think my alpm_sync_prepare is much cleaner than the old one. As you see, sync040.py and sync041.py now fails, because patched pacman autoresolves target vs target conflict now (I marked this as TODO: user should be asked to 'remove pkg1' or 'remove pkg2' or don't resolve this conflict.) Bye, ngaba Ps: Both the old and patched version of sync_prepare have a problem: If conflict resolution induces a replace, we can loose the PM_SYNC_TYPE_DEPEND information!!

diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 46d1f22..5817094 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -98,7 +98,7 @@ static void add_conflict(alpm_list_t **baddeps, const char *pkg1, * @param *baddeps list to store conflicts * @param order if >= 0 the conflict order is preserved, if < 0 it's reversed */ -static void check_conflict(alpm_list_t *list1, alpm_list_t *list2, +void check_conflict(alpm_list_t *list1, alpm_list_t *list2, alpm_list_t **baddeps, int order) { alpm_list_t *i, *j, *k;

diff --git a/lib/libalpm/conflict.h b/lib/libalpm/conflict.h index 8928de8..c7ddf8c 100644 --- a/lib/libalpm/conflict.h +++ b/lib/libalpm/conflict.h @@ -34,6 +34,8 @@ struct __pmconflict_t { char ctarget[PKG_NAME_LEN]; };

+void check_conflict(alpm_list_t *list1, alpm_list_t *list2, + alpm_list_t **baddeps, int order); alpm_list_t *_alpm_checkconflicts(pmdb_t *db, alpm_list_t *packages); alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root);

diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 6f48699..30a4580 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -467,177 +467,106 @@ 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"); - deps = _alpm_checkconflicts(db_local, list); - if(deps) { - int errorout = 0; - alpm_list_t *asked = NULL; - - for(i = deps; i && !errorout; i = i->next) { - pmdepmissing_t *miss = i->data; - pmsyncpkg_t *sync; - pmpkg_t *found = NULL; - - _alpm_log(PM_LOG_DEBUG, "package '%s' conflicts with '%s'", - miss->target, miss->depend.name); - /* 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(miss->depend.name, sync->data); - } - } - if(found) { - _alpm_log(PM_LOG_DEBUG, "'%s' is already elected for removal -- skipping", - alpm_pkg_get_name(found)); - continue; - }

- sync = _alpm_sync_find(trans->packages, miss->target); - if(sync == NULL) { - _alpm_log(PM_LOG_DEBUG, "'%s' not found in transaction set -- skipping", - miss->target); - continue; - } - pmpkg_t *local = _alpm_db_get_pkgfromcache(db_local, miss->depend.name); - /* check if this package provides the package it's conflicting with */ - if(alpm_list_find_str(alpm_pkg_get_provides(sync->pkg), - miss->depend.name)) { - /* treat like a replaces item so requiredby fields are - * inherited properly. */ - _alpm_log(PM_LOG_DEBUG, "package '%s' provides its own conflict", - miss->target); - if(!local) { - char *rmpkg = NULL; - int target, depend; - /* hmmm, depend.name 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. - */ + /* 1. check for conflicts in the target list (and resolve) */ + _alpm_log(PM_LOG_DEBUG, "check targets vs targets"); + check_conflict(list, list, &deps, 0);

- /* 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, miss->target); - depend = alpm_list_find_str(trans->targets, miss->depend.name); - if(depend && !target) { - _alpm_log(PM_LOG_DEBUG, "'%s' is in the target list -- keeping it", - miss->depend.name); - /* remove miss->target */ - rmpkg = miss->target; - } else if(target && !depend) { - _alpm_log(PM_LOG_DEBUG, "'%s' is in the target list -- keeping it", - miss->target); - /* remove miss->depend.name */ - rmpkg = miss->depend.name; - } else { - /* miss->depend.name is not needed, miss->target already provides - * it, let's resolve the conflict */ - rmpkg = miss->depend.name; - } - if(rmpkg) { - pmsyncpkg_t *rsync = _alpm_sync_find(trans->packages, rmpkg); - void *vpkg; - _alpm_log(PM_LOG_DEBUG, "removing '%s' from target list", - rsync->pkg->name); - trans->packages = alpm_list_remove(trans->packages, rsync, - syncpkg_cmp, &vpkg); - _alpm_sync_free(vpkg); - continue; - } - } + for(i = deps; i; i = i->next) { + pmdepmissing_t *miss = i->data; + pmsyncpkg_t *rsync; + + /* have we already removed one of the conflicting targets? */ + if(!_alpm_sync_find(trans->packages, miss->target) || + !(rsync = _alpm_sync_find(trans->packages, miss->depend.name))) { + continue; + } + + _alpm_log(PM_LOG_DEBUG, "conflicting packages in the sync list: '%s' <-> '%s'", + miss->target, miss->depend.name); + /* we remove miss->depend.name, TODO: let user choose */ + _alpm_log(PM_LOG_DEBUG, "removing '%s' from target list", rsync->pkg->name); + pmsyncpkg_t *vpkg; + trans->packages = alpm_list_remove(trans->packages, rsync, + syncpkg_cmp, &vpkg);

gcc spits a warning here, because &vpkg isn't of type (void *).

+ list = alpm_list_remove(list, vpkg->pkg, _alpm_pkg_cmp, NULL); + _alpm_sync_free(vpkg); + continue; + } + + FREELIST(deps); + deps = NULL; + + /* 2. we check for target vs db conflicts (and resolve)*/ + alpm_list_t *dblist = alpm_list_diff(_alpm_db_get_pkgcache(db_local), list, _alpm_pkg_cmp); + _alpm_log(PM_LOG_DEBUG, "check targets vs db"); + check_conflict(list, dblist, &deps, 1); + _alpm_log(PM_LOG_DEBUG, "check db vs targets"); + check_conflict(dblist, list, &deps, -1); + alpm_list_free(dblist); + + for(i = deps; i; i = i->next) { + pmdepmissing_t *miss = i->data; + + /* if miss->depend.name (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) { + found = _alpm_pkg_find(miss->depend.name, sync->data);

and here too, because found is an int instead of pmpkg_t *.

} - /* It's a conflict -- see if they want to remove it */ - _alpm_log(PM_LOG_DEBUG, "resolving package '%s' conflict", - miss->target); - if(local) { - int doremove = 0; - if(!alpm_list_find_str(asked, miss->depend.name)) { - QUESTION(trans, PM_TRANS_CONV_CONFLICT_PKG, miss->target, + } + if(found) { + continue; + } + + _alpm_log(PM_LOG_DEBUG, "package '%s' conflicts with '%s'", + miss->target, miss->depend.name); + + pmsyncpkg_t *sync = _alpm_sync_find(trans->packages, miss->target); + pmpkg_t *local = _alpm_db_get_pkgfromcache(db_local, miss->depend.name); + int doremove = 0; + QUESTION(trans, PM_TRANS_CONV_CONFLICT_PKG, miss->target, miss->depend.name, NULL, &doremove); - asked = alpm_list_add(asked, strdup(miss->depend.name)); - if(doremove) { - pmpkg_t *q = _alpm_pkg_dup(local); - q->requiredby = alpm_list_strdup(alpm_pkg_get_requiredby(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", - miss->depend.name); - 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, - miss->depend.name); - if(rsync) { - /* remove it from the target list */ - void *vpkg; - _alpm_log(PM_LOG_DEBUG, "removing '%s' from target list", - miss->depend.name); - 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")); - errorout = 1; - if(data) { - if((miss = malloc(sizeof(pmdepmissing_t))) == NULL) { - _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %d bytes"), sizeof(pmdepmissing_t)); - FREELIST(*data); - pm_errno = PM_ERR_MEMORY; - ret = -1; - goto cleanup; - } - *miss = *(pmdepmissing_t *)i->data; - *data = alpm_list_add(*data, miss); - } - } - } - } else { - _alpm_log(PM_LOG_ERROR, _("unresolvable package conflicts detected")); - errorout = 1; - if(data) { - if((miss = malloc(sizeof(pmdepmissing_t))) == NULL) { - _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %d bytes"), sizeof(pmdepmissing_t)); - FREELIST(*data); - pm_errno = PM_ERR_MEMORY; - ret = -1; - goto cleanup; - } - *miss = *(pmdepmissing_t *)i->data; - *data = alpm_list_add(*data, miss);

+ if(doremove) { + pmpkg_t *q = _alpm_pkg_dup(local); + q->requiredby = alpm_list_strdup(alpm_pkg_get_requiredby(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", miss->depend.name); + sync->data = alpm_list_add(sync->data, q); + } else { /* abort */ + _alpm_log(PM_LOG_ERROR, _("unresolvable package conflicts detected")); + if(data) { + pmdepmissing_t *retmiss; + if((retmiss = malloc(sizeof(pmdepmissing_t))) == NULL) { + _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %d bytes"), sizeof(pmdepmissing_t)); + FREELIST(*data); + pm_errno = PM_ERR_MEMORY; + ret = -1; + goto cleanup; } + *retmiss = *miss; + *data = alpm_list_add(*data, retmiss); } - }

Could this part be factorized, for helping fixing the TODO ?

- if(errorout) { pm_errno = PM_ERR_CONFLICTING_DEPS; ret = -1; + FREELIST(deps); goto cleanup; } - FREELIST(deps); - FREELIST(asked); } 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) {

Also, maybe that whole resolve conflict part of sync_prepare could be moved to a function in conflict.c , like resolveconflict ? I'm not sure how difficult that would be.