[pacman-dev] [patch] sync_prepare clean-up I: 2in1 -Se bugfix (fwd)

[Xavier]

On Thu, Jul 19, 2007 at 08:31:11PM +0200, Nagy Gabor wrote:
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 81c287c..c182d36 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -414,7 +414,8 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
>  		for(i = list; i; i = i->next) {
>  			/* add the dependencies found by resolvedeps to the transaction set */
>  			pmpkg_t *spkg = i->data;
> -			if(!_alpm_sync_find(trans->packages, alpm_pkg_get_name(spkg))) {
> +			pmsyncpkg_t *tmpsync = _alpm_sync_find(trans->packages, alpm_pkg_get_name(spkg));
> +			if(!tmpsync) {
>  				pmsyncpkg_t *sync = _alpm_sync_new(PM_SYNC_TYPE_DEPEND, spkg, NULL);
>  				if(sync == NULL) {
>  					ret = -1;
> @@ -427,18 +428,26 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
>  				/* remove the original targets from the list if requested */
>  				if((trans->flags & PM_TRANS_FLAG_DEPENDSONLY)) {
>  					void *vpkg;
> -					pmsyncpkg_t *sync;
>  
>  					_alpm_log(PM_LOG_DEBUG, "removing package %s-%s from the transaction targets",
>  										alpm_pkg_get_name(spkg), alpm_pkg_get_version(spkg));
>  
> -					sync = _alpm_sync_find(trans->packages, alpm_pkg_get_name(spkg));
> -					trans->packages = alpm_list_remove(trans->packages, sync, syncpkg_cmp, &vpkg);
> +					trans->packages = alpm_list_remove(trans->packages, tmpsync, syncpkg_cmp, &vpkg);
>  					_alpm_sync_free(vpkg);
>  				}
>  			}
>  		}
>  
> +		/* if we removed some targets, we must resolve dependencies again */
> +		/* since PM_TRANS_FLAG_DEPENDSONLY is used nowhere else, we can do
> +		 * a recursive call, type of trans->packages is PM_SYNC_TYPE_DEPEND*/
> +		if((trans->flags & PM_TRANS_FLAG_DEPENDSONLY)) {
> +			trans->flags ^= PM_TRANS_FLAG_DEPENDSONLY;
> +			alpm_list_free(list);
> +			EVENT(trans, PM_TRANS_EVT_RESOLVEDEPS_DONE, NULL, NULL);			
> +			return (_alpm_sync_prepare(trans, db_local, dbs_sync, data));
> +		}	
> +
>  		/* re-order w.r.t. dependencies */
>  		alpm_list_t *sortlist = NULL;
>  		alpm_list_t *newpkgs = NULL;


I don't really like this way of handling it, it looks rather odd and not
natural.
So I searched for another way of fixing it, here is my attempt (hopefully it
is better and not worse, please let me know ;) ).


>From ec6d57e3d368ccb838132af63ec96574b5f3f7b9 Mon Sep 17 00:00:00 2001
From: Chantry Xavier <shiningxc at gmail.com>
Date: Fri, 20 Jul 2007 10:22:36 +0200
Subject: [PATCH] libalpm/sync.c : fix DEPENDSONLY flag handling.

I didn't like the patch proposed by Nagy for the sync1002 pactest here:
http://www.archlinux.org/pipermail/pacman-dev/2007-July/008971.html

So here is another attempt of fixing it.
In case of the DEPENDSONLY flag :
1) pass an empty list to resolvedeps instead of the list of targets
2) empty the trans->packages targets list before adding the resolved deps.

Signed-off-by: Chantry Xavier <shiningxc at gmail.com>
---
 lib/libalpm/sync.c |   25 +++++++++----------------
 1 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index c1d2f9e..7e83120 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -392,9 +392,11 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
 		*data = NULL;
 	}
 
-	for(i = trans->packages; i; i = i->next) {
-		pmsyncpkg_t *sync = i->data;
-		list = alpm_list_add(list, sync->pkg);
+	if(!(trans->flags & PM_TRANS_FLAG_DEPENDSONLY)) {
+		for(i = trans->packages; i; i = i->next) {
+			pmsyncpkg_t *sync = i->data;
+			list = alpm_list_add(list, sync->pkg);
+		}
 	}
 
 	if(!(trans->flags & PM_TRANS_FLAG_NODEPS)) {
@@ -411,6 +413,10 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
 			}
 		}
 
+		if((trans->flags & PM_TRANS_FLAG_DEPENDSONLY)) {
+			FREELIST(trans->packages);
+		}
+
 		for(i = list; i; i = i->next) {
 			/* add the dependencies found by resolvedeps to the transaction set */
 			pmpkg_t *spkg = i->data;
@@ -423,19 +429,6 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
 				trans->packages = alpm_list_add(trans->packages, sync);
 				_alpm_log(PM_LOG_DEBUG, "adding package %s-%s to the transaction targets",
 									alpm_pkg_get_name(spkg), alpm_pkg_get_version(spkg));
-			} else {
-				/* remove the original targets from the list if requested */
-				if((trans->flags & PM_TRANS_FLAG_DEPENDSONLY)) {
-					void *vpkg;
-					pmsyncpkg_t *sync;
-
-					_alpm_log(PM_LOG_DEBUG, "removing package %s-%s from the transaction targets",
-										alpm_pkg_get_name(spkg), alpm_pkg_get_version(spkg));
-
-					sync = _alpm_sync_find(trans->packages, alpm_pkg_get_name(spkg));
-					trans->packages = alpm_list_remove(trans->packages, sync, syncpkg_cmp, &vpkg);
-					_alpm_sync_free(vpkg);
-				}
 			}
 		}
 
-- 
1.5.2.4