* Updated conflict checking one last time. You can finally have a file move

  from one package to another seemlessly (knock on wood). This is implemented
  through the use of two skip lists in the trans struct- skip_add and
  skip_remove, which replace the former trans->skiplist.
* Removed an unnecessary function parameter, added a necessary one.
* If a package has no backup files, print '(none)' under the heading so it is
  more obvious.
* Updated my TODO list.

8 files changed, 123 insertions, 114 deletions

diff --git a/TODO.dan b/TODO.dan
index f6ba6a99..357e22e6 100644
--- a/TODO.dan
+++ b/TODO.dan
@@ -117,3 +117,9 @@ Fix inconsistency of args- _alpm_db_read, _alpm_db_write
 Resurrect test scripts, and add ones as needed. Testing by scripts > testing by
 hand.
 
+Build a replacement for this, or at least standardize its use. We shouldn't
+always need to pass handle->root around, it is constant. Something like char*
+buildpath(file).
+	/* build the new entryname relative to handle->root */
+	snprintf(filename, PATH_MAX, "%s%s", handle->root, entryname);
+
diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index b503323b..28e66354 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -316,25 +316,19 @@ int _alpm_add_prepare(pmtrans_t *trans, pmdb_t *db, alpm_list_t **data)
 	/* Check for file conflicts
 	 */
 	if(!(trans->flags & PM_TRANS_FLAG_FORCE)) {
-		alpm_list_t *skiplist = NULL;
-
 		EVENT(trans, PM_TRANS_EVT_FILECONFLICTS_START, NULL, NULL);
 
 		_alpm_log(PM_LOG_DEBUG, _("looking for file conflicts"));
-		lp = _alpm_db_find_conflicts(db, trans, handle->root, &skiplist);
+		lp = _alpm_db_find_conflicts(db, trans, handle->root);
 		if(lp != NULL) {
 			if(data) {
 				*data = lp;
 			} else {
 				FREELIST(lp);
 			}
-			FREELIST(skiplist);
 			RET_ERR(PM_ERR_FILE_CONFLICTS, -1);
 		}
 
-		/* copy the file skiplist into the transaction */
-		trans->skiplist = skiplist;
-
 		EVENT(trans, PM_TRANS_EVT_FILECONFLICTS_DONE, NULL, NULL);
 	}
 
@@ -423,7 +417,7 @@ int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db)
 		if(oldpkg) {
 			/* 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"
-			 * with the type PM_TRANS_TYPE_UPGRADE */
+			 * with the type PM_TRANS_TYPE_UPGRADE. TODO: kill this weird behavior. */
 			pmtrans_t *tr = _alpm_trans_new();
 			_alpm_log(PM_LOG_DEBUG, _("removing old package first (%s-%s)"), oldpkg->name, oldpkg->version);
 
@@ -441,8 +435,8 @@ int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db)
 				RET_ERR(PM_ERR_TRANS_ABORT, -1);
 			}
 
-			/* copy the skiplist over */
-			tr->skiplist = alpm_list_strdup(trans->skiplist);
+			/* copy the remove skiplist over */
+			tr->skip_remove = alpm_list_strdup(trans->skip_remove);
 			alpm_list_t *b;
 
 			/* Add files in the NEW package's backup array to the noupgrade array
@@ -538,11 +532,19 @@ int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db)
 
 				/* if a file is in NoExtract then we never extract it */
 				if(alpm_list_find_str(handle->noextract, entryname)) {
+					_alpm_log(PM_LOG_DEBUG, _("%s is in NoExtract, skipping extraction"), entryname);
 					alpm_logaction(_("notice: %s is in NoExtract -- skipping extraction"), entryname);
 					archive_read_data_skip(archive);
 					continue;
 				}
 
+				/* if a file is in the add skiplist we never extract it */
+				if(alpm_list_find_str(trans->skip_add, filename)) {
+					_alpm_log(PM_LOG_DEBUG, _("%s is in trans->skip_add, skipping extraction"), entryname);
+					archive_read_data_skip(archive);
+					continue;
+				}
+
 				/* check is file already exists */
 				if(stat(filename, &buf) == 0 && !S_ISDIR(buf.st_mode)) {
 					/* it does, is it a backup=() file?
diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
index 66e0b08d..a5f9590f 100644
--- a/lib/libalpm/conflict.c
+++ b/lib/libalpm/conflict.c
@@ -313,8 +313,7 @@ static alpm_list_t *add_fileconflict(alpm_list_t *conflicts,
 	return(conflicts);
 }
 
-alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root,
-                                     alpm_list_t **skip_list)
+alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root)
 {
 	alpm_list_t *i, *j, *k;
 	alpm_list_t *conflicts = NULL;
@@ -366,50 +365,54 @@ alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root,
 		if(dbpkg) {
 			/* older ver of package currently installed */
 			tmpfiles = chk_filedifference(p1->files, alpm_pkg_get_files(dbpkg));
-			for(j = tmpfiles; j; j = j->next) {
-				filestr = j->data;
-				_alpm_log(PM_LOG_DEBUG, "checking possible conflict: %s", filestr);
-
-				snprintf(path, PATH_MAX, "%s%s", root, filestr);
-
-				/* stat the file - if it exists and is not a dir, do some checks */
-				if(lstat(path, &buf) == 0 && !S_ISDIR(buf.st_mode)) {
-					/* Look at all the targets to see if file has changed hands */
-					for(k = targets; k; k = k->next) {
-						p2 = (pmpkg_t *)k->data;
-						/* Ensure we aren't looking at current package */
-						if(strcmp(p2->name, p1->name)) {
-							pmpkg_t *localp2 = _alpm_db_get_pkgfromcache(db, p2->name);
-							/* Check if it used to exist in a package, but doesn't anymore */
-							if(localp2 && !alpm_list_find_str(alpm_pkg_get_files(p2), filestr)
-									&& alpm_list_find_str(alpm_pkg_get_files(localp2), filestr)) {
-								*skip_list = alpm_list_add(*skip_list, strdup(filestr));
-								_alpm_log(PM_LOG_DEBUG, "adding to skiplist: %s", filestr);
-							} else {
-								conflicts = add_fileconflict(conflicts, PM_CONFLICT_TYPE_FILE,
-								                             filestr, p1->name, NULL);
-								break;
-							}
-						}
-					}
-				}
-			}
-			alpm_list_free_inner(tmpfiles, &free);
-			alpm_list_free(tmpfiles);
 		} else {
 			/* no version of package currently installed */
-			for(j = p1->files; j; j = j->next) {
-				filestr = j->data;
-
-				snprintf(path, PATH_MAX, "%s%s", root, filestr);
+			tmpfiles = alpm_list_strdup(p1->files);
+		}
 
-				/* stat the file - if it exists and is not a dir, report a conflict */
-				if(lstat(path, &buf) == 0 && !S_ISDIR(buf.st_mode)) {
-					conflicts = add_fileconflict(conflicts, PM_CONFLICT_TYPE_FILE,
-					                             filestr, p1->name, NULL);
+		/* loop over each file to be installed */
+		for(j = tmpfiles; j; j = j->next) {
+			filestr = j->data;
+			_alpm_log(PM_LOG_DEBUG, "checking possible conflict: %s", filestr);
+
+			snprintf(path, PATH_MAX, "%s%s", root, filestr);
+
+			/* stat the file - if it exists and is not a dir, do some checks */
+			if(lstat(path, &buf) == 0 && !S_ISDIR(buf.st_mode)) {
+				/* Look at all the targets to see if file has changed hands */
+				for(k = targets; k; k = k->next) {
+					p2 = (pmpkg_t *)k->data;
+					/* Ensure we aren't looking at current package */
+					if(p2 == p1) {
+						continue;
+					}
+					pmpkg_t *localp2 = _alpm_db_get_pkgfromcache(db, p2->name);
+					/* Check if it used to exist in a package, but doesn't anymore */
+					if(localp2 && !alpm_list_find_str(alpm_pkg_get_files(p2), filestr)
+							&& alpm_list_find_str(alpm_pkg_get_files(localp2), filestr)) {
+						/* check if the file is now in the backup array */
+						if(alpm_list_find_str(alpm_pkg_get_backup(p1), filestr)) {
+							/* keep file intact if it is in backup array */
+							trans->skip_add = alpm_list_add(trans->skip_add, strdup(path));
+							trans->skip_remove = alpm_list_add(trans->skip_remove, strdup(path));
+							_alpm_log(PM_LOG_DEBUG, "file in backup array, adding to add and remove skiplist: %s", filestr);
+						} else {
+							/* skip removal of file, but not add. this will prevent a second
+							 * package from removing the file when it was already installed
+							 * by its new owner */
+							trans->skip_remove = alpm_list_add(trans->skip_remove, strdup(path));
+							_alpm_log(PM_LOG_DEBUG, "file changed packages, adding to remove skiplist: %s", filestr);
+						}
+					} else {
+						conflicts = add_fileconflict(conflicts, PM_CONFLICT_TYPE_FILE,
+																				 filestr, p1->name, NULL);
+						break;
+					}
 				}
 			}
 		}
+		alpm_list_free_inner(tmpfiles, &free);
+		alpm_list_free(tmpfiles);
 	}
 
 	return(conflicts);
diff --git a/lib/libalpm/conflict.h b/lib/libalpm/conflict.h
index 69ce727c..44507f72 100644
--- a/lib/libalpm/conflict.h
+++ b/lib/libalpm/conflict.h
@@ -34,7 +34,7 @@ struct __pmconflict_t {
 };
 
 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, alpm_list_t **skip_list);
+alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root);
 
 #endif /* _ALPM_CONFLICT_H */
 
diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
index faa58f9c..4b2350f8 100644
--- a/lib/libalpm/remove.c
+++ b/lib/libalpm/remove.c
@@ -155,19 +155,15 @@ int _alpm_remove_prepare(pmtrans_t *trans, pmdb_t *db, alpm_list_t **data)
 	return(0);
 }
 
-static int can_remove_file(const char *path)
+static int can_remove_file(pmtrans_t *trans, const char *path)
 {
-	alpm_list_t *i;
 	char file[PATH_MAX+1];
 
 	snprintf(file, PATH_MAX, "%s%s", handle->root, path);
 
-	for(i = handle->trans->skiplist; i; i = i->next) {
-		if(strcmp(file, i->data) == 0) {
-			/* skipping this file, return success because "removing" this
-			 * file does nothing */
-			return(1);
-		}
+	if(alpm_list_find_str(trans->skip_remove, file)) {
+		/* return success because we will never actually remove this file */
+		return(1);
 	}
 	/* If we fail write permissions due to a read-only filesystem, abort.
 	 * Assume all other possible failures are covered somewhere else */
@@ -175,7 +171,8 @@ static int can_remove_file(const char *path)
 		if(access(file, F_OK) == 0) {
 			/* only return failure if the file ACTUALLY exists and we don't have
 			 * permissions */
-			_alpm_log(PM_LOG_ERROR, _("cannot remove file '%s': %s"), file, strerror(errno));
+			_alpm_log(PM_LOG_ERROR, _("cannot remove file '%s': %s"),
+			          file, strerror(errno));
 			return(0);
 		}
 	}
@@ -230,19 +227,11 @@ static void unlink_file(pmpkg_t *info, alpm_list_t *lp, alpm_list_t *targ,
 			_alpm_log(PM_LOG_DEBUG, _("removing directory %s"), file);
 		}
 	} else {
-		/* check the "skip list" before removing the file.
+		/* check the remove skip list before removing the file.
 		 * see the big comment block in db_find_conflicts() for an
 		 * explanation. */
-		int skipit = 0;
-		alpm_list_t *j;
-		for(j = trans->skiplist; j; j = j->next) {
-			if(!strcmp(lp->data, (char*)j->data)) {
-				skipit = 1;
-			}
-		}
-		if(skipit) {
-			_alpm_log(PM_LOG_WARNING, _("%s has changed ownership, skipping removal"),
-								file);
+		if(alpm_list_find_str(trans->skip_remove, file)) {
+			_alpm_log(PM_LOG_DEBUG, _("%s is in trans->skip_remove, skipping removal"), file);
 		} else if(needbackup) {
 			/* if the file is flagged, back it up to .pacsave */
 			if(!(trans->type == PM_TRANS_TYPE_UPGRADE)) {
@@ -301,7 +290,7 @@ int _alpm_remove_commit(pmtrans_t *trans, pmdb_t *db)
 
 		if(!(trans->flags & PM_TRANS_FLAG_DBONLY)) {
 			for(lp = info->files; lp; lp = lp->next) {
-				if(!can_remove_file(lp->data)) {
+				if(!can_remove_file(trans, lp->data)) {
 					_alpm_log(PM_LOG_DEBUG, _("not removing package '%s', can't remove all files"), info->name);
 					RET_ERR(PM_ERR_PKG_CANT_REMOVE, -1);
 				}
diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c
index 4969214f..64cb47c5 100644
--- a/lib/libalpm/trans.c
+++ b/lib/libalpm/trans.c
@@ -55,7 +55,8 @@ pmtrans_t *_alpm_trans_new()
 
 	trans->targets = NULL;
 	trans->packages = NULL;
-	trans->skiplist = NULL;
+	trans->skip_add = NULL;
+	trans->skip_remove = NULL;
 	trans->type = 0;
 	trans->flags = 0;
 	trans->cb_event = NULL;
@@ -87,7 +88,8 @@ void _alpm_trans_free(void *data)
 		FREELISTPKGS(trans->packages);
 	}
 
-	FREELIST(trans->skiplist);
+	FREELIST(trans->skip_add);
+	FREELIST(trans->skip_remove);
 
 	FREE(trans);
 }
diff --git a/lib/libalpm/trans.h b/lib/libalpm/trans.h
index f0fe1e57..83b99abb 100644
--- a/lib/libalpm/trans.h
+++ b/lib/libalpm/trans.h
@@ -41,9 +41,10 @@ struct __pmtrans_t {
 	pmtranstype_t type;
 	unsigned int flags;
 	pmtransstate_t state;
-	alpm_list_t *targets;     /* alpm_list_t of (char *) */
-	alpm_list_t *packages;    /* alpm_list_t of (pmpkg_t *) or (pmsyncpkg_t *) */
-	alpm_list_t *skiplist;    /* alpm_list_t of (char *) */
+	alpm_list_t *targets;       /* list of (char *) */
+	alpm_list_t *packages;      /* list of (pmpkg_t *) or (pmsyncpkg_t *) */
+	alpm_list_t *skip_add;      /* list of (char *) */
+	alpm_list_t *skip_remove;   /* list of (char *) */
 	alpm_trans_cb_event cb_event;
 	alpm_trans_cb_conv cb_conv;
 	alpm_trans_cb_progress cb_progress;
diff --git a/src/pacman/package.c b/src/pacman/package.c
index 437e4420..ad629f73 100644
--- a/src/pacman/package.c
+++ b/src/pacman/package.c
@@ -152,49 +152,55 @@ void dump_pkg_backups(pmpkg_t *pkg)
 {
 	alpm_list_t *i;
 	const char *root = alpm_option_get_root();
-	printf(_("Backup Files :\n"));
-	for(i = alpm_pkg_get_backup(pkg); i; i = alpm_list_next(i)) {
-		struct stat buf;
-		char path[PATH_MAX];
-		char *str = strdup(alpm_list_getdata(i));
-		char *ptr = index(str, '\t');
-		if(ptr == NULL) {
-			FREE(str);
-			continue;
-		}
-		*ptr = '\0';
-		ptr++;
-		snprintf(path, PATH_MAX-1, "%s%s", root, str);
-		/* if we find the file, calculate checksums, otherwise it is missing */
-		if(!stat(path, &buf)) {
-			char *sum;
-			char *md5sum = alpm_get_md5sum(path);
-			char *sha1sum = alpm_get_sha1sum(path);
-
-			if(md5sum == NULL || sha1sum == NULL) {
-				ERR(NL, _("error calculating checksums for %s\n"), path);
+	printf(_("Backup Files:\n"));
+	if(alpm_pkg_get_backup(pkg)) {
+		/* package has backup files, so print them */
+		for(i = alpm_pkg_get_backup(pkg); i; i = alpm_list_next(i)) {
+			struct stat buf;
+			char path[PATH_MAX];
+			char *str = strdup(alpm_list_getdata(i));
+			char *ptr = index(str, '\t');
+			if(ptr == NULL) {
 				FREE(str);
 				continue;
 			}
-			/* TODO Is this a good way to check type of backup stored?
-			 * We aren't storing it anywhere in the database. */
-			if (strlen(ptr) == 32) {
-				sum = md5sum;
-			} else { /*if (strlen(ptr) == 40) */
-				sum = sha1sum;
-			}
-			/* if checksums don't match, file has been modified */
-			if (strcmp(sum, ptr)) {
-				printf(_("MODIFIED\t%s\n"), path);
+			*ptr = '\0';
+			ptr++;
+			snprintf(path, PATH_MAX-1, "%s%s", root, str);
+			/* if we find the file, calculate checksums, otherwise it is missing */
+			if(!stat(path, &buf)) {
+				char *sum;
+				char *md5sum = alpm_get_md5sum(path);
+				char *sha1sum = alpm_get_sha1sum(path);
+
+				if(md5sum == NULL || sha1sum == NULL) {
+					ERR(NL, _("error calculating checksums for %s\n"), path);
+					FREE(str);
+					continue;
+				}
+				/* TODO Is this a good way to check type of backup stored?
+				 * We aren't storing it anywhere in the database. */
+				if (strlen(ptr) == 32) {
+					sum = md5sum;
+				} else { /*if (strlen(ptr) == 40) */
+					sum = sha1sum;
+				}
+				/* if checksums don't match, file has been modified */
+				if (strcmp(sum, ptr)) {
+					printf(_("MODIFIED\t%s\n"), path);
+				} else {
+					printf(_("Not Modified\t%s\n"), path);
+				}
+				FREE(md5sum);
+				FREE(sha1sum);
 			} else {
-				printf(_("Not Modified\t%s\n"), path);
+				printf(_("MISSING\t\t%s\n"), path);
 			}
-			FREE(md5sum);
-			FREE(sha1sum);
-		} else {
-			printf(_("MISSING\t\t%s\n"), path);
+			FREE(str);
 		}
-		FREE(str);
+	} else {
+		/* package had no backup files */
+		printf(_("(none)\n"));
 	}
 }