* 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.

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);
+

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?

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,6 +365,12 @@ 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));
+		} else {
+			/* no version of package currently installed */
+			tmpfiles = alpm_list_strdup(p1->files);
+		}
+
+		/* 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);
@@ -378,13 +383,26 @@ alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root,
 				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)) {
+					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)) {
-								*skip_list = alpm_list_add(*skip_list, strdup(filestr));
-								_alpm_log(PM_LOG_DEBUG, "adding to skiplist: %s", 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);
@@ -393,23 +411,8 @@ alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root,
 				}
 			}
 		}
-			}
 		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);
-
-				/* 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);
-				}
-			}
-		}
 	}
 
 	return(conflicts);

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 */
 

lib/libalpm/remove.c

@@ -155,27 +155,24 @@ 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 */
+	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 */
 	if(access(file, W_OK) == -1) {
 		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);
 				}

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);
 }

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;

src/pacman/package.c

@@ -152,7 +152,9 @@ void dump_pkg_backups(pmpkg_t *pkg)
 {
 	alpm_list_t *i;
 	const char *root = alpm_option_get_root();
-	printf(_("Backup Files :\n"));
+	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];
@@ -196,6 +198,10 @@ void dump_pkg_backups(pmpkg_t *pkg)
 			}
 			FREE(str);
 		}
+	} else {
+		/* package had no backup files */
+		printf(_("(none)\n"));
+	}
 }
 	
 /* List all files contained in a package