Use sync.c for upgrade transaction prepare and commit

This patch utilizes the power of sync.c to fix FS#3492 and FS#5798.
Now an upgrade transaction is just a sync transaction internally (in alpm),
so all sync features are available with -U as well:
* conflict resolving
* sync dependencies from sync repos
* remove unresolvable targets

See http://www.archlinux.org/pipermail/pacman-dev/2009-June/008725.html
for the concept.

We use "mixed" target list, where PKG_FROM_FILE origin indicates local
package file, PKG_FROM_CACHE indicates sync package. The front-end can add
only one type of packages (depending on transaction type) atm, but if alpm
resolves dependencies for -U, we may get a real mixed trans->packages list.

_alpm_pkg_free_trans() was modified so that it can handle both target types
_alpm_add_prepare() was removed, we use _alpm_sync_prepare() instead
_alpm_add_commit() was renamed to _alpm_upgrade_targets()

sync.c (and deps.c) was modified slightly to handle mixed target lists,
the modifications are straightforward. There is one notable change here: We
don't create new upgrade trans in sync.c, we replace the pkgcache entries
with the loaded package files in the target list (this is a bit hackish) and
call _alpm_upgrade_targets(). This implies a TODO (pkg->origin_data.db is
not accessible anymore), but it doesn't hurt anything with pacman front-end,
so it will be fixed later (otherwise this patch would be huge).

I updated the documentation of -U and I added a new pactest, upgrade090.py,
to test the syncdeps feature of -U.

Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu>
Signed-off-by: Dan McGee <dan@archlinux.org>

11 files changed, 133 insertions, 174 deletions

diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt
index 559b45d8..7f92ec71 100644
--- a/doc/pacman.8.txt
+++ b/doc/pacman.8.txt
@@ -82,7 +82,8 @@ to determine which packages need upgrading. This behavior operates as follows:
 	"bash>=3.2"`.
 
 *-U, \--upgrade*::
-	Upgrade or add package(s) to the system. Either a URL or file path can be
+	Upgrade or add package(s) to the system and install the required
+	dependencies from sync repos. Either a URL or file path can be
 	specified. This is a ``remove-then-add'' process. See <<HCF,Handling Config
 	Files>> for an explanation on how pacman takes care of config files.
 
diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index 1b228b2d..86355007 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -99,91 +99,6 @@ error:
 	return(-1);
 }
 
-int _alpm_add_prepare(pmtrans_t *trans, pmdb_t *db, alpm_list_t **data)
-{
-	alpm_list_t *lp = NULL;
-
-	ALPM_LOG_FUNC;
-
-	ASSERT(trans != NULL, RET_ERR(PM_ERR_TRANS_NULL, -1));
-	ASSERT(db != NULL, RET_ERR(PM_ERR_DB_NULL, -1));
-
-	/* Check dependencies
-	 */
-	if(!(trans->flags & PM_TRANS_FLAG_NODEPS)) {
-		EVENT(trans, PM_TRANS_EVT_CHECKDEPS_START, NULL, NULL);
-
-		/* look for unsatisfied dependencies */
-		_alpm_log(PM_LOG_DEBUG, "looking for unsatisfied dependencies\n");
-		lp = alpm_checkdeps(_alpm_db_get_pkgcache(db), 1, NULL, trans->packages);
-		if(lp != NULL) {
-			if(data) {
-				*data = lp;
-			} else {
-				alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free);
-				alpm_list_free(lp);
-			}
-			RET_ERR(PM_ERR_UNSATISFIED_DEPS, -1);
-		}
-
-		/* no unsatisfied deps, so look for conflicts */
-		_alpm_log(PM_LOG_DEBUG, "looking for conflicts\n");
-		alpm_list_t *inner = _alpm_innerconflicts(trans->packages);
-		alpm_list_t *outer = _alpm_outerconflicts(db, trans->packages);
-		lp = alpm_list_join(inner, outer);
-
-		/* TODO : factorize the conflict resolving code from sync.c to use it here (FS#3492) */
-
-		if(lp != NULL) {
-			if(data) {
-				*data = lp;
-			} else {
-				alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_conflict_free);
-				alpm_list_free(lp);
-			}
-			if(inner) {
-				_alpm_log(PM_LOG_ERROR, _("conflicting packages were found in the target list\n"));
-				_alpm_log(PM_LOG_ERROR, _("you cannot install two conflicting packages at the same time\n"));
-			}
-			if(outer) {
-				_alpm_log(PM_LOG_ERROR, _("replacing packages with -U is not supported yet\n"));
-				_alpm_log(PM_LOG_ERROR, _("you can replace packages manually using -Rd and -U\n"));
-			}
-			RET_ERR(PM_ERR_CONFLICTING_DEPS, -1);
-		}
-
-		/* re-order w.r.t. dependencies */
-		_alpm_log(PM_LOG_DEBUG, "sorting by dependencies\n");
-		lp = _alpm_sortbydeps(trans->packages, 0);
-		/* free the old alltargs */
-		alpm_list_free(trans->packages);
-		trans->packages = lp;
-
-		EVENT(trans, PM_TRANS_EVT_CHECKDEPS_DONE, NULL, NULL);
-	}
-
-	/* Check for file conflicts */
-	if(!(trans->flags & PM_TRANS_FLAG_FORCE)) {
-		EVENT(trans, PM_TRANS_EVT_FILECONFLICTS_START, NULL, NULL);
-
-		_alpm_log(PM_LOG_DEBUG, "looking for file conflicts\n");
-		lp = _alpm_db_find_fileconflicts(db, trans, trans->packages, NULL);
-		if(lp != NULL) {
-			if(data) {
-				*data = lp;
-			} else {
-				alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_fileconflict_free);
-				alpm_list_free(lp);
-			}
-			RET_ERR(PM_ERR_FILE_CONFLICTS, -1);
-		}
-
-		EVENT(trans, PM_TRANS_EVT_FILECONFLICTS_DONE, NULL, NULL);
-	}
-
-	return(0);
-}
-
 static int upgrade_remove(pmpkg_t *oldpkg, pmpkg_t *newpkg, pmtrans_t *trans, pmdb_t *db) {
 	/* this is kinda odd.  If the old package exists, at this point we make a
 	 * NEW transaction, unrelated to handle->trans, and instantiate a "remove"
@@ -858,7 +773,7 @@ cleanup:
 	return(ret);
 }
 
-int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db)
+int _alpm_upgrade_packages(pmtrans_t *trans, pmdb_t *db)
 {
 	int pkg_count, pkg_current;
 	alpm_list_t *targ;
diff --git a/lib/libalpm/add.h b/lib/libalpm/add.h
index 6983de9e..1a3b799d 100644
--- a/lib/libalpm/add.h
+++ b/lib/libalpm/add.h
@@ -25,8 +25,7 @@
 #include "trans.h"
 
 int _alpm_add_loadtarget(pmtrans_t *trans, pmdb_t *db, char *name);
-int _alpm_add_prepare(pmtrans_t *trans, pmdb_t *db, alpm_list_t **data);
-int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db);
+int _alpm_upgrade_packages(pmtrans_t *trans, pmdb_t *db);
 
 #endif /* _ALPM_ADD_H */
 
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c
index 02a4c7b6..46699ac6 100644
--- a/lib/libalpm/deps.c
+++ b/lib/libalpm/deps.c
@@ -605,7 +605,7 @@ int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *pkg,
 
 	ALPM_LOG_FUNC;
 
-	if(local == NULL || dbs_sync == NULL) {
+	if(local == NULL) {
 		return(-1);
 	}
 
diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
index 002b1c89..2e0d0dfd 100644
--- a/lib/libalpm/package.c
+++ b/lib/libalpm/package.c
@@ -858,7 +858,11 @@ void _alpm_pkg_free(pmpkg_t *pkg)
 	FREE(pkg);
 }
 
-/* Free transaction specific fields */
+/* This function should be used when removing a target from upgrade/sync target list
+ * Case 1: If pkg is a loaded package file (PKG_FROM_FILE), it will be freed.
+ * Case 2: If pkg is a pkgcache entry (PKG_FROM_CACHE), it won't be freed,
+ *         only the transaction specific fields of pkg will be freed.
+ */
 void _alpm_pkg_free_trans(pmpkg_t *pkg)
 {
 	ALPM_LOG_FUNC;
@@ -867,6 +871,11 @@ void _alpm_pkg_free_trans(pmpkg_t *pkg)
 		return;
 	}
 
+	if(pkg->origin == PKG_FROM_FILE) {
+		_alpm_pkg_free(pkg);
+		return;
+	}
+
 	alpm_list_free(pkg->removes);
 	pkg->removes = NULL;
 }
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index dda4953d..a738d1b8 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -43,6 +43,7 @@
 #include "deps.h"
 #include "conflict.h"
 #include "trans.h"
+#include "add.h"
 #include "util.h"
 #include "handle.h"
 #include "alpm.h"
@@ -282,6 +283,11 @@ static int compute_download_size(pmpkg_t *newpkg)
 	char *fpath;
 	off_t size = 0;
 
+	if(newpkg->origin == PKG_FROM_FILE) {
+		newpkg->download_size = 0;
+		return(0);
+	}
+
 	fname = alpm_pkg_get_filename(newpkg);
 	ASSERT(fname != NULL, RET_ERR(PM_ERR_PKG_INVALID_NAME, -1));
 	fpath = _alpm_filecache_find(fname);
@@ -392,10 +398,6 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
 			}
 		}
 
-		/* Unresolvable packages will be removed from the target list, so
-		   we free the transaction specific fields */
-		alpm_list_free_inner(unresolvable, (alpm_list_fn_free)_alpm_pkg_free_trans);
-
 		/* Set DEPEND reason for pulled packages */
 		for(i = resolved; i; i = i->next) {
 			pmpkg_t *pkg = i->data;
@@ -404,6 +406,10 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
 			}
 		}
 
+		/* Unresolvable packages will be removed from the target list, so
+		   we free the transaction specific fields */
+		alpm_list_free_inner(unresolvable, (alpm_list_fn_free)_alpm_pkg_free_trans);
+
 		/* re-order w.r.t. dependencies */
 		alpm_list_free(trans->packages);
 		trans->packages = _alpm_sortbydeps(resolved, 0);
@@ -468,8 +474,8 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
 			_alpm_log(PM_LOG_WARNING,
 					_("removing '%s' from target list because it conflicts with '%s'\n"),
 					rsync->name, sync->name);
-			_alpm_pkg_free_trans(rsync); /* rsync is not transaction target anymore */
 			trans->packages = alpm_list_remove(trans->packages, rsync, _alpm_pkg_cmp, NULL);
+			_alpm_pkg_free_trans(rsync); /* rsync is not transaction target anymore */
 			continue;
 		}
 
@@ -711,7 +717,7 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 {
 	alpm_list_t *i, *j, *files = NULL;
 	alpm_list_t *deltas = NULL;
-	pmtrans_t *tr_remove = NULL, *tr_upgrade = NULL;
+	pmtrans_t *tr_remove = NULL;
 	int replaces = 0;
 	int errors = 0;
 	const char *cachedir = NULL;
@@ -743,9 +749,8 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 
 		for(j = trans->packages; j; j = j->next) {
 			pmpkg_t *spkg = j->data;
-			pmdb_t *dbs = spkg->origin_data.db;
 
-			if(current == dbs) {
+			if(spkg->origin == PKG_FROM_CACHE && current == spkg->origin_data.db) {
 				const char *fname = NULL;
 
 				fname = alpm_pkg_get_filename(spkg);
@@ -837,13 +842,35 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 	errors = 0;
 	for(i = trans->packages; i; i = i->next) {
 		pmpkg_t *spkg = i->data;
+		if(spkg->origin == PKG_FROM_FILE) {
+			continue; /* pkg_load() has been already called, this package is valid */
+		}
+
 		const char *filename = alpm_pkg_get_filename(spkg);
 		const char *md5sum = alpm_pkg_get_md5sum(spkg);
 
 		if(test_md5sum(trans, filename, md5sum) != 0) {
 			errors++;
 			*data = alpm_list_add(*data, strdup(filename));
+			continue;
+		}
+		/* load the package file and replace pkgcache entry with it in the target list */
+		/* TODO: alpm_pkg_get_db() will not work on this target anymore */
+		_alpm_log(PM_LOG_DEBUG, "replacing pkgcache entry with package file for target %s\n", spkg->name);
+		char *filepath = _alpm_filecache_find(filename);
+		pmpkg_t *pkgfile;
+		if(alpm_pkg_load(filepath, 1, &pkgfile) != 0) {
+			_alpm_pkg_free(pkgfile);
+			errors++;
+			*data = alpm_list_add(*data, strdup(filename));
+			FREE(filepath);
+			continue;
 		}
+		FREE(filepath);
+		pkgfile->reason = spkg->reason; /* copy over install reason */
+		pkgfile->removes = alpm_list_copy(spkg->removes); /* copy over removes list */
+		i->data = pkgfile;
+		_alpm_pkg_free_trans(spkg); /* spkg has been removed from the target list */
 	}
 	if(errors) {
 		pm_errno = PM_ERR_PKG_INVALID;
@@ -856,32 +883,22 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 
 	trans->state = STATE_COMMITING;
 
-	/* Create remove and upgrade transactions */
+	/* Create remove transaction */
 	tr_remove = _alpm_trans_new();
 	if(tr_remove == NULL) {
 		_alpm_log(PM_LOG_ERROR, _("could not create removal transaction\n"));
 		goto error;
 	}
-	tr_upgrade = _alpm_trans_new();
-	if(tr_upgrade == NULL) {
-		_alpm_log(PM_LOG_ERROR, _("could not create transaction\n"));
-		goto error;
-	}
 
 	if(_alpm_trans_init(tr_remove, PM_TRANS_TYPE_REMOVE, PM_TRANS_FLAG_NODEPS, NULL, NULL, NULL) == -1) {
 		_alpm_log(PM_LOG_ERROR, _("could not initialize the removal transaction\n"));
 		goto error;
 	}
-	if(_alpm_trans_init(tr_upgrade, PM_TRANS_TYPE_UPGRADE, trans->flags, trans->cb_event, trans->cb_conv, trans->cb_progress) == -1) {
-		_alpm_log(PM_LOG_ERROR, _("could not initialize transaction\n"));
-		goto error;
-	}
 
-	/* adding targets */
+	/* adding targets to the remove transaction */
 	for(i = trans->packages; i; i = i->next) {
 		pmpkg_t *spkg = i->data;
 		alpm_list_t *j;
-		/* remove transaction */
 		for(j = spkg->removes; j; j = j->next) {
 			pmpkg_t *pkg = j->data;
 			if(!_alpm_pkg_find(tr_remove->packages, pkg->name)) {
@@ -891,27 +908,6 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 				replaces++;
 			}
 		}
-		/* upgrade transaction */
-		const char *fname;
-		char *fpath;
-
-		fname = alpm_pkg_get_filename(spkg);
-		if(fname == NULL) {
-			goto error;
-		}
-		/* Loop through the cache dirs until we find a matching file */
-		fpath = _alpm_filecache_find(fname);
-
-		if(_alpm_trans_addtarget(tr_upgrade, fpath) == -1) {
-			FREE(fpath);
-			goto error;
-		}
-		FREE(fpath);
-
-		/* using alpm_list_last() is ok because addtarget() adds the new target at the
-		 * end of the tr->packages list */
-		pmpkg_t *ipkg = alpm_list_last(tr_upgrade->packages)->data;
-		ipkg->reason = spkg->reason;
 	}
 
 	/* fileconflict check */
@@ -919,8 +915,8 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 		EVENT(trans, PM_TRANS_EVT_FILECONFLICTS_START, NULL, NULL);
 
 		_alpm_log(PM_LOG_DEBUG, "looking for file conflicts\n");
-		alpm_list_t *conflict = _alpm_db_find_fileconflicts(db_local, tr_upgrade,
-								    tr_upgrade->packages, tr_remove->packages);
+		alpm_list_t *conflict = _alpm_db_find_fileconflicts(db_local, trans,
+								    trans->packages, tr_remove->packages);
 		if(conflict) {
 			pm_errno = PM_ERR_FILE_CONFLICTS;
 			if(data) {
@@ -953,8 +949,7 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 
 	/* install targets */
 	_alpm_log(PM_LOG_DEBUG, "installing packages\n");
-	/* add_prepare is not needed */
-	if(_alpm_trans_commit(tr_upgrade, NULL) == -1) {
+	if(_alpm_upgrade_packages(trans, handle->db_local) == -1) {
 		_alpm_log(PM_LOG_ERROR, _("could not commit transaction\n"));
 		goto error;
 	}
@@ -964,7 +959,6 @@ error:
 	FREELIST(files);
 	alpm_list_free(deltas);
 	_alpm_trans_free(tr_remove);
-	_alpm_trans_free(tr_upgrade);
 	return(ret);
 }
 
diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c
index 240bf816..f913ba9e 100644
--- a/lib/libalpm/trans.c
+++ b/lib/libalpm/trans.c
@@ -247,7 +247,7 @@ void _alpm_trans_free(pmtrans_t *trans)
 		return;
 	}
 
-	if(trans->type == PM_TRANS_TYPE_SYNC) {
+	if(trans->type == PM_TRANS_TYPE_SYNC || trans->type == PM_TRANS_TYPE_UPGRADE) {
 		alpm_list_free_inner(trans->packages, (alpm_list_fn_free)_alpm_pkg_free_trans);
 	} else {
 		alpm_list_free_inner(trans->packages, (alpm_list_fn_free)_alpm_pkg_free);
@@ -367,12 +367,6 @@ int _alpm_trans_prepare(pmtrans_t *trans, alpm_list_t **data)
 	}
 
 	switch(trans->type) {
-		case PM_TRANS_TYPE_UPGRADE:
-			if(_alpm_add_prepare(trans, handle->db_local, data) == -1) {
-				/* pm_errno is set by _alpm_add_prepare() */
-				return(-1);
-			}
-		break;
 		case PM_TRANS_TYPE_REMOVE:
 		case PM_TRANS_TYPE_REMOVEUPGRADE:
 			if(_alpm_remove_prepare(trans, handle->db_local, data) == -1) {
@@ -380,6 +374,7 @@ int _alpm_trans_prepare(pmtrans_t *trans, alpm_list_t **data)
 				return(-1);
 			}
 		break;
+		case PM_TRANS_TYPE_UPGRADE:
 		case PM_TRANS_TYPE_SYNC:
 			if(_alpm_sync_prepare(trans, handle->db_local, handle->dbs_sync, data) == -1) {
 				/* pm_errno is set by _alpm_sync_prepare() */
@@ -411,12 +406,6 @@ int _alpm_trans_commit(pmtrans_t *trans, alpm_list_t **data)
 	trans->state = STATE_COMMITING;
 
 	switch(trans->type) {
-		case PM_TRANS_TYPE_UPGRADE:
-			if(_alpm_add_commit(trans, handle->db_local) == -1) {
-				/* pm_errno is set by _alpm_add_commit() */
-				return(-1);
-			}
-		break;
 		case PM_TRANS_TYPE_REMOVE:
 		case PM_TRANS_TYPE_REMOVEUPGRADE:
 			if(_alpm_remove_commit(trans, handle->db_local) == -1) {
@@ -424,6 +413,7 @@ int _alpm_trans_commit(pmtrans_t *trans, alpm_list_t **data)
 				return(-1);
 			}
 		break;
+		case PM_TRANS_TYPE_UPGRADE:
 		case PM_TRANS_TYPE_SYNC:
 			if(_alpm_sync_commit(trans, handle->db_local, data) == -1) {
 				/* pm_errno is set by _alpm_sync_commit() */
diff --git a/pactest/tests/depconflict110.py b/pactest/tests/depconflict110.py
index fd56fa89..82e17d0b 100644
--- a/pactest/tests/depconflict110.py
+++ b/pactest/tests/depconflict110.py
@@ -7,7 +7,7 @@ self.addpkg(p);
 lp = pmpkg("pkg2", "1.0-1")
 self.addpkg2db("local", lp)
 
-self.args = "-U %s" % p.filename()
-self.addrule("PACMAN_RETCODE=1")
-self.addrule("!PKG_EXIST=pkg1")
-self.addrule("PKG_EXIST=pkg2")
+self.args = "-U %s --ask=4" % p.filename()
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("PKG_EXIST=pkg1")
+self.addrule("!PKG_EXIST=pkg2")
diff --git a/pactest/tests/upgrade051.py b/pactest/tests/upgrade051.py
index 4346c24d..1d173218 100644
--- a/pactest/tests/upgrade051.py
+++ b/pactest/tests/upgrade051.py
@@ -8,10 +8,8 @@ p.conflicts = ["pkg1"]
 p.provides = ["pkg1"]
 self.addpkg(p)
 
-self.args = "-U %s" % p.filename()
+self.args = "-U %s --ask=4" % p.filename()
 
 self.addrule("PACMAN_RETCODE=0")
 self.addrule("!PKG_EXIST=pkg1")
 self.addrule("PKG_EXIST=pkg2")
-
-self.expectfailure = True
diff --git a/pactest/tests/upgrade090.py b/pactest/tests/upgrade090.py
new file mode 100644
index 00000000..26e531c9
--- /dev/null
+++ b/pactest/tests/upgrade090.py
@@ -0,0 +1,28 @@
+self.description = "-U syncdeps test"
+
+p1 = pmpkg("pkg1", "1.0-2")
+p1.files = ["bin/pkg1"]
+
+p2 = pmpkg("pkg2", "1.0-2")
+p2.depends = ["dep"]
+
+p3 = pmpkg("pkg3", "1.0-2")
+p3.depends = ["unres"]
+
+for p in p1, p2, p3:
+	self.addpkg(p)
+
+sp = pmpkg("dep")
+sp.files = ["bin/dep"]
+self.addpkg2db("sync", sp)
+
+self.args = "-U %s --ask=32" % " ".join([p.filename() for p in p1, p2, p3])
+
+self.addrule("PACMAN_RETCODE=0")
+for p in p1, p2, sp:
+	self.addrule("PKG_EXIST=%s" % p.name)
+	for f in p.files:
+		self.addrule("FILE_EXIST=%s" % f)
+self.addrule("PKG_VERSION=pkg1|1.0-2")
+self.addrule("PKG_VERSION=pkg2|1.0-2")
+self.addrule("!PKG_EXIST=pkg3")
diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c
index 91888fec..dc82e4de 100644
--- a/src/pacman/upgrade.c
+++ b/src/pacman/upgrade.c
@@ -121,38 +121,63 @@ int pacman_upgrade(alpm_list_t *targets)
 					}
 				}
 				break;
+			default:
+				break;
+		}
+		trans_release();
+		FREELIST(data);
+		return(1);
+	}
+
+	/* Step 3: perform the installation */
+	/* print targets and ask user confirmation */
+	alpm_list_t *packages = alpm_trans_get_pkgs();
+	if(packages == NULL) { /* we are done */
+		trans_release();
+		return(retval);
+	}
+	display_synctargets(packages);
+	printf("\n");
+	int confirm = yesno(_("Proceed with installation?"));
+	if(!confirm) {
+		trans_release();
+		return(retval);
+	}
+
+	if(alpm_trans_commit(&data) == -1) {
+		pm_fprintf(stderr, PM_LOG_ERROR, _("failed to commit transaction (%s)\n"),
+				alpm_strerrorlast());
+		switch(pm_errno) {
+			alpm_list_t *i;
 			case PM_ERR_FILE_CONFLICTS:
 				for(i = data; i; i = alpm_list_next(i)) {
 					pmfileconflict_t *conflict = alpm_list_getdata(i);
 					switch(alpm_fileconflict_get_type(conflict)) {
 						case PM_FILECONFLICT_TARGET:
 							printf(_("%s exists in both '%s' and '%s'\n"),
-							        alpm_fileconflict_get_file(conflict),
-							        alpm_fileconflict_get_target(conflict),
-							        alpm_fileconflict_get_ctarget(conflict));
-						break;
+									alpm_fileconflict_get_file(conflict),
+									alpm_fileconflict_get_target(conflict),
+									alpm_fileconflict_get_ctarget(conflict));
+							break;
 						case PM_FILECONFLICT_FILESYSTEM:
 							printf(_("%s: %s exists in filesystem\n"),
-							        alpm_fileconflict_get_target(conflict),
-							        alpm_fileconflict_get_file(conflict));
-						break;
+									alpm_fileconflict_get_target(conflict),
+									alpm_fileconflict_get_file(conflict));
+							break;
 					}
 				}
-				printf(_("\nerrors occurred, no packages were upgraded.\n"));
+				break;
+			case PM_ERR_PKG_INVALID:
+			case PM_ERR_DLT_INVALID:
+				for(i = data; i; i = alpm_list_next(i)) {
+					char *filename = alpm_list_getdata(i);
+					printf(_("%s is invalid or corrupted\n"), filename);
+				}
 				break;
 			default:
 				break;
 		}
-		trans_release();
 		FREELIST(data);
-		return(1);
-	}
-	alpm_list_free(data);
-
-	/* Step 3: perform the installation */
-	if(alpm_trans_commit(NULL) == -1) {
-		pm_fprintf(stderr, PM_LOG_ERROR, _("failed to commit transaction (%s)\n"),
-			alpm_strerrorlast());
 		trans_release();
 		return(1);
 	}