Fix issues with replacing unowned symlinks

There aretwo seperate issues in the same block of file conflict
checking code here:
1) If realpath errored, such as when a symlink was broken, we would call
   'continue' rather than simply exit this particular method of
   resolution. This was likely just a copy-paste mistake as the previous
   resolving steps all use loops where continue makes sense. Refactor
   the check so we only proceed if realpath is successful, and continue
   with the rest of the checks either way.
2) The real problem this code was trying to solve was canonicalizing
   path component (e.g., directory) symlinks. The final component, if
   not a directory, should not be handled at all in this loop. Add a
   !S_ISLNK() condition to the loop so we only call this for real files.

There are few other small cleanups to the debug messages that I made
while debugging this problem- we don't need to keep printing the file
name, and ensure every block that sets resolved_conflict to true prints
a debug message so we know how it was resolved.

This fixes the expected failures from symlink010.py and symlink011.py,
while still ensuring the fix for fileconflict007.py works.

Signed-off-by: Dan McGee <dan@archlinux.org>

3 files changed, 19 insertions, 18 deletions

diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
index 14c23f45..32f6f303 100644
--- a/lib/libalpm/conflict.c
+++ b/lib/libalpm/conflict.c
@@ -469,16 +469,18 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 				continue;
 			}
 
+			_alpm_log(handle, ALPM_LOG_DEBUG, "checking possible conflict: %s\n", path);
+
 			if(S_ISDIR(file->mode)) {
 				struct stat sbuf;
 				if(S_ISDIR(lsbuf.st_mode)) {
-					_alpm_log(handle, ALPM_LOG_DEBUG, "%s is a directory, not a conflict\n", path);
+					_alpm_log(handle, ALPM_LOG_DEBUG, "file is a directory, not a conflict\n");
 					continue;
 				}
 				stat(path, &sbuf);
 				if(S_ISLNK(lsbuf.st_mode) && S_ISDIR(sbuf.st_mode)) {
 					_alpm_log(handle, ALPM_LOG_DEBUG,
-							"%s is a symlink to a dir, hopefully not a conflict\n", path);
+							"file is a symlink to a dir, hopefully not a conflict\n");
 					continue;
 				}
 				/* if we made it to here, we want all subsequent path comparisons to
@@ -487,7 +489,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 				path[strlen(path) - 1] = '\0';
 			}
 
-			_alpm_log(handle, ALPM_LOG_DEBUG, "checking possible conflict: %s\n", path);
 			relative_path = path + strlen(handle->root);
 
 			/* Check remove list (will we remove the conflicting local file?) */
@@ -496,7 +497,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 				if(rempkg && _alpm_filelist_contains(alpm_pkg_get_files(rempkg),
 							relative_path)) {
 					_alpm_log(handle, ALPM_LOG_DEBUG,
-							"local file will be removed, not a conflict: %s\n", path);
+							"local file will be removed, not a conflict\n");
 					resolved_conflict = 1;
 				}
 			}
@@ -517,7 +518,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 					handle->trans->skip_remove =
 						alpm_list_add(handle->trans->skip_remove, strdup(filestr));
 					_alpm_log(handle, ALPM_LOG_DEBUG,
-							"file changed packages, adding to remove skiplist: %s\n", path);
+							"file changed packages, adding to remove skiplist\n");
 					resolved_conflict = 1;
 				}
 			}
@@ -535,16 +536,20 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 				free(dir);
 			}
 
-			if(!resolved_conflict && dbpkg) {
+			/* check if a component of the filepath was a link. canonicalize the path
+			 * and look for it in the old package. note that the actual file under
+			 * consideration cannot itself be a link, as it might be unowned- path
+			 * components can be safely checked as all directories are "unowned". */
+			if(!resolved_conflict && dbpkg && !S_ISLNK(lsbuf.st_mode)) {
 				char *rpath = calloc(PATH_MAX, sizeof(char));
 				const char *relative_rpath;
-				if(!realpath(path, rpath)) {
-					free(rpath);
-					continue;
-				}
-				relative_rpath = rpath + strlen(handle->root);
-				if(_alpm_filelist_contains(alpm_pkg_get_files(dbpkg), relative_rpath)) {
-					resolved_conflict = 1;
+				if(realpath(path, rpath)) {
+					relative_rpath = rpath + strlen(handle->root);
+					if(_alpm_filelist_contains(alpm_pkg_get_files(dbpkg), relative_rpath)) {
+						_alpm_log(handle, ALPM_LOG_DEBUG,
+								"package contained the resolved realpath\n");
+						resolved_conflict = 1;
+					}
 				}
 				free(rpath);
 			}
@@ -560,7 +565,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 				}
 				if(!found) {
 					_alpm_log(handle, ALPM_LOG_DEBUG,
-							"file was unowned but in new backup list: %s\n", path);
+							"file was unowned but in new backup list\n");
 					resolved_conflict = 1;
 				}
 			}
diff --git a/test/pacman/tests/symlink010.py b/test/pacman/tests/symlink010.py
index 8c80dbc9..a1e562e1 100644
--- a/test/pacman/tests/symlink010.py
+++ b/test/pacman/tests/symlink010.py
@@ -22,5 +22,3 @@ self.addrule("FILE_EXIST=usr/bin/otherprog")
 self.addrule("FILE_TYPE=usr/bin/myprog|file")
 self.addrule("FILE_TYPE=usr/bin/myprogsuffix|link")
 self.addrule("FILE_TYPE=usr/bin/otherprog|file")
-
-self.expectfailure = True
diff --git a/test/pacman/tests/symlink011.py b/test/pacman/tests/symlink011.py
index 85101725..93cd9ddf 100644
--- a/test/pacman/tests/symlink011.py
+++ b/test/pacman/tests/symlink011.py
@@ -22,5 +22,3 @@ self.addrule("FILE_EXIST=usr/bin/otherprog")
 self.addrule("FILE_TYPE=usr/bin/myprog|file")
 self.addrule("FILE_TYPE=usr/bin/myprogsuffix|link")
 self.addrule("FILE_TYPE=usr/bin/otherprog|file")
-
-self.expectfailure = True