[pacman-dev] [patch] resolveconflict clean-up

Xavier shiningxc at gmail.com
Sun Aug 12 18:41:31 EDT 2007


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.




More information about the pacman-dev mailing list