improve dir->file transition conflict resolution

Packages removed due to conflicts are always removed at the beginning of
the transaction and as such can be included in the check for whether all
owners of a directory will be removed in a transaction.  Installed
versions of packages being upgraded, other than the one with the
conflict, cannot be used because our transaction ordering is not
intelligent enough to ensure that they are removed prior to the
installation of the conflicted package.

Also, return false from dir_belongsto_pkgs on errors.  Previously, we
simply continued which could return true even if we were unable to
actually establish that the package owned the entire tree.

Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com>
Signed-off-by: Allan McRae <allan@archlinux.org>

2 files changed, 82 insertions, 68 deletions

diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
index ab9546cd..a00efe5c 100644
--- a/lib/libalpm/conflict.c
+++ b/lib/libalpm/conflict.c
@@ -299,94 +299,73 @@ void _alpm_fileconflict_free(alpm_fileconflict_t *conflict)
 }
 
 /**
- * @brief Recursively checks if a package owns all subdirectories and files in
- * a directory.
+ * @brief Recursively checks if a set of packages own all subdirectories and
+ * files in a directory.
  *
  * @param handle the context handle
  * @param dirpath path of the directory to check
- * @param pkg package being checked against
+ * @param pkgs packages being checked against
  *
- * @return 1 if a package owns all subdirectories and files or a directory
- * cannot be opened, 0 otherwise
+ * @return 1 if a package owns all subdirectories and files, 0 otherwise
  */
-static int dir_belongsto_pkg(alpm_handle_t *handle, const char *dirpath,
-		alpm_pkg_t *pkg)
+static int dir_belongsto_pkgs(alpm_handle_t *handle, const char *dirpath,
+		alpm_list_t *pkgs)
 {
-	alpm_list_t *i;
-	struct stat sbuf;
-	char path[PATH_MAX];
-	char abspath[PATH_MAX];
+	char path[PATH_MAX], full_path[PATH_MAX];
 	DIR *dir;
 	struct dirent *ent = NULL;
-	const char *root = handle->root;
-
-	/* check directory is actually in package - used for subdirectory checks */
-	if(!alpm_filelist_contains(alpm_pkg_get_files(pkg), dirpath)) {
-		_alpm_log(handle, ALPM_LOG_DEBUG,
-				"directory %s not in package %s\n", dirpath, pkg->name);
-		return 0;
-	}
-
-	/* TODO: this is an overly strict check but currently pacman will not
-	 * overwrite a directory with a file (case 10/11 in add.c). Adjusting that
-	 * is not simple as even if the directory is being unowned by a conflicting
-	 * package, pacman does not sort this to ensure all required directory
-	 * "removals" happen before installation of file/symlink */
 
-	/* check that no other _installed_ package owns the directory */
-	for(i = _alpm_db_get_pkgcache(handle->db_local); i; i = i->next) {
-		if(pkg == i->data) {
-			continue;
-		}
-
-		if(alpm_filelist_contains(alpm_pkg_get_files(i->data), dirpath)) {
-			_alpm_log(handle, ALPM_LOG_DEBUG,
-					"file %s also in package %s\n", dirpath,
-					((alpm_pkg_t*)i->data)->name);
-			return 0;
-		}
-	}
-
-	/* check all files in directory are owned by the package */
-	snprintf(abspath, PATH_MAX, "%s%s", root, dirpath);
-	dir = opendir(abspath);
+	snprintf(full_path, PATH_MAX, "%s%s", handle->root, dirpath);
+	dir = opendir(full_path);
 	if(dir == NULL) {
-		return 1;
+		return 0;
 	}
 
 	while((ent = readdir(dir)) != NULL) {
 		const char *name = ent->d_name;
+		int owned = 0;
+		alpm_list_t *i;
+		struct stat sbuf;
 
 		if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) {
 			continue;
 		}
+
 		snprintf(path, PATH_MAX, "%s%s", dirpath, name);
-		snprintf(abspath, PATH_MAX, "%s%s", root, path);
-		if(stat(abspath, &sbuf) != 0) {
-			continue;
-		}
-		if(S_ISDIR(sbuf.st_mode)) {
-			if(dir_belongsto_pkg(handle, path, pkg)) {
-				continue;
-			} else {
-				closedir(dir);
-				return 0;
-			}
-		} else {
-			if(alpm_filelist_contains(alpm_pkg_get_files(pkg), path)) {
-				continue;
-			} else {
-				closedir(dir);
-				_alpm_log(handle, ALPM_LOG_DEBUG,
-						"unowned file %s found in directory\n", path);
-				return 0;
+		snprintf(full_path, PATH_MAX, "%s%s", handle->root, path);
+
+		for(i = pkgs; i && !owned; i = i->next) {
+			if(alpm_filelist_contains(alpm_pkg_get_files(i->data), path)) {
+				owned = 1;
 			}
 		}
+
+		if(owned && stat(full_path, &sbuf) != 0 && S_ISDIR(sbuf.st_mode)) {
+			owned = dir_belongsto_pkgs(handle, path, pkgs);
+		}
+
+		if(!owned) {
+			closedir(dir);
+			_alpm_log(handle, ALPM_LOG_DEBUG,
+					"unowned file %s found in directory\n", path);
+			return 0;
+		}
 	}
 	closedir(dir);
 	return 1;
 }
 
+static alpm_list_t *alpm_db_find_file_owners(alpm_db_t* db, const char *path)
+{
+	alpm_list_t *i, *owners = NULL;
+	for(i = alpm_db_get_pkgcache(db); i; i = i->next) {
+		if(alpm_filelist_contains(alpm_pkg_get_files(i->data), path)) {
+			owners = alpm_list_add(owners, i->data);
+		}
+	}
+	return owners;
+}
+
 /**
  * @brief Find file conflicts that may occur during the transaction.
  *
@@ -578,14 +557,32 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 			}
 
 			/* check if all files of the dir belong to the installed pkg */
-			if(!resolved_conflict && S_ISDIR(lsbuf.st_mode) && dbpkg) {
+			if(!resolved_conflict && S_ISDIR(lsbuf.st_mode)) {
+				alpm_list_t *owners;
 				char *dir = malloc(strlen(relative_path) + 2);
 				sprintf(dir, "%s/", relative_path);
-				if(alpm_filelist_contains(alpm_pkg_get_files(dbpkg), dir)) {
-					_alpm_log(handle, ALPM_LOG_DEBUG,
-							"checking if all files in %s belong to %s\n",
-							dir, dbpkg->name);
-					resolved_conflict = dir_belongsto_pkg(handle, dir, dbpkg);
+
+				owners = alpm_db_find_file_owners(handle->db_local, dir);
+				if(owners) {
+					alpm_list_t *pkgs = NULL, *diff;
+
+					if(dbpkg) {
+						pkgs = alpm_list_add(pkgs, dbpkg);
+					}
+					pkgs = alpm_list_join(pkgs, alpm_list_copy(rem));
+					if((diff = alpm_list_diff(owners, pkgs, _alpm_pkg_cmp))) {
+						/* dir is owned by files we aren't removing */
+						/* TODO: with better commit ordering, we may be able to check
+						 * against upgrades as well */
+						alpm_list_free(diff);
+					} else {
+						_alpm_log(handle, ALPM_LOG_DEBUG,
+								"checking if all files in %s belong to removed packages\n",
+								dir);
+						resolved_conflict = dir_belongsto_pkgs(handle, dir, owners);
+					}
+					alpm_list_free(pkgs);
+					alpm_list_free(owners);
 				}
 				free(dir);
 			}
diff --git a/test/pacman/tests/fileconflict030.py b/test/pacman/tests/fileconflict030.py
new file mode 100644
index 00000000..1de77813
--- /dev/null
+++ b/test/pacman/tests/fileconflict030.py
@@ -0,0 +1,17 @@
+self.description = "Dir->file transition filesystem conflict resolved by removal"
+
+lp1 = pmpkg("foo")
+lp1.files = ["foo/"]
+self.addpkg2db("local", lp1)
+
+sp1 = pmpkg("bar")
+sp1.conflicts = ["foo"]
+sp1.files = ["foo"]
+self.addpkg2db("sync", sp1)
+
+self.args = "-S %s --ask=4" % sp1.name
+
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("PKG_EXIST=bar")
+self.addrule("!PKG_EXIST=foo")
+self.addrule("FILE_EXIST=foo")