alpm_filelist: remove resolved_path

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

13 files changed, 24 insertions, 263 deletions

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 806cd959..c6d97c59 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -217,7 +217,6 @@ typedef struct _alpm_file_t {
 typedef struct _alpm_filelist_t {
 	size_t count;
 	alpm_file_t *files;
-	char **resolved_path;
 } alpm_filelist_t;
 
 /** Local package or package file backup entry */
@@ -1042,7 +1041,7 @@ int alpm_pkg_set_reason(alpm_pkg_t *pkg, alpm_pkgreason_t reason);
  * @param path the path to search for in the package
  * @return a pointer to the matching file or NULL if not found
  */
-char *alpm_filelist_contains(alpm_filelist_t *filelist, const char *path);
+alpm_file_t *alpm_filelist_contains(alpm_filelist_t *filelist, const char *path);
 
 /*
  * Signatures
diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
index 041a04ef..896f48de 100644
--- a/lib/libalpm/conflict.c
+++ b/lib/libalpm/conflict.c
@@ -321,7 +321,6 @@ static int dir_belongsto_pkg(alpm_handle_t *handle, const char *dirpath,
 	const char *root = handle->root;
 
 	/* check directory is actually in package - used for subdirectory checks */
-	_alpm_filelist_resolve(handle, alpm_pkg_get_files(pkg));
 	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);
@@ -340,7 +339,6 @@ static int dir_belongsto_pkg(alpm_handle_t *handle, const char *dirpath,
 			continue;
 		}
 
-		_alpm_filelist_resolve(handle, alpm_pkg_get_files(i->data));
 		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,
@@ -375,7 +373,6 @@ static int dir_belongsto_pkg(alpm_handle_t *handle, const char *dirpath,
 				return 0;
 			}
 		} else {
-			_alpm_filelist_resolve(handle, alpm_pkg_get_files(pkg));
 			if(alpm_filelist_contains(alpm_pkg_get_files(pkg), path)) {
 				continue;
 			} else {
@@ -417,11 +414,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 
 	rootlen = strlen(handle->root);
 
-	/* make sure all files to be installed have been resolved */
-	for(i = upgrade; i; i = i->next) {
-		_alpm_filelist_resolve(handle, alpm_pkg_get_files(i->data));
-	}
-
 	/* TODO this whole function needs a huge change, which hopefully will
 	 * be possible with real transactions. Right now we only do half as much
 	 * here as we do when we actually extract files in add.c with our 12
@@ -491,7 +483,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 		 * be freed. */
 		if(dbpkg) {
 			/* older ver of package currently installed */
-			_alpm_filelist_resolve(handle, alpm_pkg_get_files(dbpkg));
 			tmpfiles = _alpm_filelist_difference(alpm_pkg_get_files(p1),
 					alpm_pkg_get_files(dbpkg));
 		} else {
@@ -499,7 +490,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 			alpm_filelist_t *fl = alpm_pkg_get_files(p1);
 			size_t filenum;
 			for(filenum = 0; filenum < fl->count; filenum++) {
-				tmpfiles = alpm_list_add(tmpfiles, fl->resolved_path[filenum]);
+				tmpfiles = alpm_list_add(tmpfiles, fl->files[filenum].name);
 			}
 		}
 
@@ -545,7 +536,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 			/* Check remove list (will we remove the conflicting local file?) */
 			for(k = rem; k && !resolved_conflict; k = k->next) {
 				alpm_pkg_t *rempkg = k->data;
-				_alpm_filelist_resolve(handle, alpm_pkg_get_files(rempkg));
 				if(rempkg && alpm_filelist_contains(alpm_pkg_get_files(rempkg),
 							relative_path)) {
 					_alpm_log(handle, ALPM_LOG_DEBUG,
@@ -565,7 +555,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 				localp2 = _alpm_db_get_pkgfromcache(handle->db_local, p2->name);
 
 				/* localp2->files will be removed (target conflicts are handled by CHECK 1) */
-				_alpm_filelist_resolve(handle, alpm_pkg_get_files(localp2));
 				if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), filestr)) {
 					/* skip removal of file, but not add. this will prevent a second
 					 * package from removing the file when it was already installed
@@ -612,7 +601,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
 				alpm_list_t *local_pkgs = _alpm_db_get_pkgcache(handle->db_local);
 				int found = 0;
 				for(k = local_pkgs; k && !found; k = k->next) {
-					_alpm_filelist_resolve(handle, alpm_pkg_get_files(k->data));
 					if(alpm_filelist_contains(alpm_pkg_get_files(k->data), filestr)) {
 							found = 1;
 					}
diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c
index 697dd23b..f8db9a33 100644
--- a/lib/libalpm/filelist.c
+++ b/lib/libalpm/filelist.c
@@ -25,199 +25,6 @@
 #include "filelist.h"
 #include "util.h"
 
-/** Helper function for comparing strings when sorting */
-static int _alpm_filelist_strcmp(const void *s1, const void *s2)
-{
-	return strcmp(*(char **)s1, *(char **)s2);
-}
-
-/* TODO make sure callers check the return value so we can bail on errors.
- * For now we soldier on as best we can, skipping paths that are too long to
- * resolve and using the original filenames on memory errors.  */
-/**
- * @brief Resolves a symlink and its children.
- *
- * @attention Pre-condition: files must be sorted!
- *
- * @param files filelist to resolve
- * @param i pointer to the index in files to start processing, will point to
- * the last file processed on return
- * @param path absolute path for the symlink being resolved
- * @param root_len length of the root portion of path
- * @param resolving is file \i in \files a symlink that needs to be resolved
- *
- * @return 0 on success, -1 on error
- */
-int _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t *i,
-		char *path, size_t root_len, int resolving)
-{
-	char *causal_dir = NULL; /* symlink being resolved */
-	char *filename_r = NULL; /* resolved filename */
-	size_t causal_dir_len = 0, causal_dir_r_len = 0;
-
-	if(resolving) {
-		/* deal with the symlink being resolved */
-		MALLOC(filename_r, PATH_MAX, goto error);
-		causal_dir = files->files[*i].name;
-		causal_dir_len = strlen(causal_dir);
-		if(realpath(path, filename_r) == NULL) {
-			files->resolved_path[*i] = causal_dir;
-			FREE(filename_r);
-			return -1;
-		}
-		causal_dir_r_len = strlen(filename_r + root_len) + 1;
-		if(causal_dir_r_len >= PATH_MAX) {
-			files->resolved_path[*i] = causal_dir;
-			FREE(filename_r);
-			return -1;
-		}
-		/* remove root_r from filename_r */
-		memmove(filename_r, filename_r + root_len, causal_dir_r_len);
-		filename_r[causal_dir_r_len - 1] = '/';
-		filename_r[causal_dir_r_len] = '\0';
-		STRDUP(files->resolved_path[*i], filename_r, goto error);
-		(*i)++;
-	}
-
-	for(; *i < files->count; (*i)++) {
-		char *filename = files->files[*i].name;
-		size_t filename_len = strlen(filename);
-		size_t filename_r_len = filename_len;
-		struct stat sbuf;
-		int exists;
-
-		if(resolving) {
-			if(filename_len < causal_dir_len || strncmp(filename, causal_dir, causal_dir_len) != 0) {
-				/* not inside causal_dir anymore */
-				break;
-			}
-
-			filename_r_len = filename_len + causal_dir_r_len - causal_dir_len;
-			if(filename_r_len >= PATH_MAX) {
-				/* resolved path is too long */
-				files->resolved_path[*i] = filename;
-				continue;
-			}
-
-			strcpy(filename_r + causal_dir_r_len, filename + causal_dir_len);
-		}
-
-		/* deal with files and paths too long to resolve*/
-		if(filename[filename_len - 1] != '/' || root_len + filename_r_len >= PATH_MAX) {
-			if(resolving) {
-				STRDUP(files->resolved_path[*i], filename_r, goto error);
-			} else {
-				files->resolved_path[*i] = filename;
-			}
-			continue;
-		}
-
-		/* construct absolute path and stat() */
-		strcpy(path + root_len, resolving ? filename_r : filename);
-		exists = !_alpm_lstat(path, &sbuf);
-
-		/* deal with symlinks */
-		if(exists && S_ISLNK(sbuf.st_mode)) {
-			_alpm_filelist_resolve_link(files, i, path, root_len, 1);
-			continue;
-		}
-
-		/* deal with normal directories */
-		if(resolving) {
-			STRDUP(files->resolved_path[*i], filename_r, goto error);
-		} else {
-			files->resolved_path[*i] = filename;
-		}
-
-		/* deal with children of non-existent directories to reduce lstat() calls */
-		if(!exists) {
-			for((*i)++; *i < files->count; (*i)++) {
-				char *f = files->files[*i].name;
-				size_t f_len = strlen(f);
-				size_t f_r_len;
-
-				if(f_len < filename_len || strncmp(f, filename, filename_len) != 0) {
-					/* not inside the non-existent dir anymore */
-					break;
-				}
-
-				f_r_len = f_len + causal_dir_r_len - causal_dir_len;
-				if(resolving && f_r_len <= PATH_MAX) {
-					strcpy(filename_r + causal_dir_r_len, f + causal_dir_len);
-					STRDUP(files->resolved_path[*i], filename_r, goto error);
-				} else {
-					files->resolved_path[*i] = f;
-				}
-			}
-			(*i)--;
-		}
-	}
-	(*i)--;
-
-	FREE(filename_r);
-
-	return 0;
-
-error:
-	FREE(filename_r);
-	/* out of memory, set remaining files to their original names */
-	for(; *i < files->count; (*i)++) {
-		files->resolved_path[*i] = files->files[*i].name;
-	}
-	(*i)--;
-	return -1;
-}
-
-/**
- * @brief Takes a file list and resolves all directory paths according to the
- * filesystem
- *
- * @attention Pre-condition: files must be sorted!
- *
- * @note A symlink and directory at the same path in two difference packages
- * causes a conflict so the filepath can not change as packages get installed
- *
- * @param handle the context handle
- * @param files list of files to resolve
- *
- * @return 0 on success, -1 on error
- */
-int _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files)
-{
-	char path[PATH_MAX];
-	size_t root_len, i = 0;
-	int ret = 0;
-
-	if(!files || files->resolved_path) {
-		return 0;
-	}
-
-	CALLOC(files->resolved_path, files->count, sizeof(char *), return -1);
-
-	/* not much point in going on if we can't even resolve root */
-	if(realpath(handle->root, path) == NULL){
-		return -1;
-	}
-	root_len = strlen(path);
-	if(root_len + 1 >= PATH_MAX) {
-		return -1;
-	}
-	/* append '/' if root is not "/" */
-	if(path[root_len - 1] != '/') {
-		path[root_len] = '/';
-		root_len++;
-		path[root_len] = '\0';
-	}
-
-	ret = _alpm_filelist_resolve_link(files, &i, path, root_len, 0);
-
-	qsort(files->resolved_path, files->count, sizeof(char *),
-			_alpm_filelist_strcmp);
-
-	return ret;
-}
-
-
 /* Returns the difference of the provided two lists of files.
  * Pre-condition: both lists are sorted!
  * When done, free the list but NOT the contained data.
@@ -229,8 +36,8 @@ alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA,
 	size_t ctrA = 0, ctrB = 0;
 
 	while(ctrA < filesA->count && ctrB < filesB->count) {
-		char *strA = filesA->resolved_path[ctrA];
-		char *strB = filesB->resolved_path[ctrB];
+		char *strA = filesA->files[ctrA].name;
+		char *strB = filesB->files[ctrB].name;
 
 		int cmp = strcmp(strA, strB);
 		if(cmp < 0) {
@@ -247,7 +54,7 @@ alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA,
 
 	/* ensure we have completely emptied pA */
 	while(ctrA < filesA->count) {
-		ret = alpm_list_add(ret, filesA->resolved_path[ctrA]);
+		ret = alpm_list_add(ret, filesA->files[ctrA].name);
 		ctrA++;
 	}
 
@@ -269,17 +76,17 @@ alpm_list_t *_alpm_filelist_intersection(alpm_filelist_t *filesA,
 		char *strA, *strB;
 
 		isdirA = 0;
-		strA = filesA->resolved_path[ctrA];
+		strA = filesA->files[ctrA].name;
 		if(strA[strlen(strA)-1] == '/') {
 			isdirA = 1;
-			strA = strndup(filesA->resolved_path[ctrA], strlen(strA)-1);
+			strA = strndup(strA, strlen(strA)-1);
 		}
 
 		isdirB = 0;
-		strB = filesB->resolved_path[ctrB];
+		strB = filesB->files[ctrB].name;
 		if(strB[strlen(strB)-1] == '/') {
 			isdirB = 1;
-			strB = strndup(filesB->resolved_path[ctrB], strlen(strB)-1);
+			strB = strndup(strB, strlen(strB)-1);
 		}
 
 		cmp = strcmp(strA, strB);
@@ -297,7 +104,7 @@ alpm_list_t *_alpm_filelist_intersection(alpm_filelist_t *filesA,
 
 			/* when not directories, item in both qualifies as an intersect */
 			if(! (isdirA && isdirB)) {
-				ret = alpm_list_add(ret, filesA->resolved_path[ctrA]);
+				ret = alpm_list_add(ret, filesA->files[ctrA].name);
 			}
 			ctrA++;
 			ctrB++;
@@ -323,11 +130,10 @@ int _alpm_files_cmp(const void *f1, const void *f2)
 	return strcmp(file1->name, file2->name);
 }
 
-
-char SYMEXPORT *alpm_filelist_contains(alpm_filelist_t *filelist,
+alpm_file_t SYMEXPORT *alpm_filelist_contains(alpm_filelist_t *filelist,
 		const char *path)
 {
-	alpm_file_t key, *match;
+	alpm_file_t key;
 
 	if(!filelist) {
 		return NULL;
@@ -335,17 +141,8 @@ char SYMEXPORT *alpm_filelist_contains(alpm_filelist_t *filelist,
 
 	key.name = (char *)path;
 
-	match = bsearch(&key, filelist->files, filelist->count,
+	return bsearch(&key, filelist->files, filelist->count,
 			sizeof(alpm_file_t), _alpm_files_cmp);
-
-	if(match) {
-		return match->name;
-	} else if(filelist->resolved_path) {
-		return bsearch(&path, filelist->resolved_path, filelist->count,
-				sizeof(char *), _alpm_filelist_strcmp);
-	} else {
-		return NULL;
-	}
 }
 
 /* vim: set ts=2 sw=2 noet: */
diff --git a/lib/libalpm/filelist.h b/lib/libalpm/filelist.h
index bfab4165..66501f36 100644
--- a/lib/libalpm/filelist.h
+++ b/lib/libalpm/filelist.h
@@ -21,10 +21,6 @@
 
 #include "alpm.h"
 
-int _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t *i,
-		char *path, size_t root_len, int resolving);
-int _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files);
-
 alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA,
 		alpm_filelist_t *filesB);
 
diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
index 098c8677..cfdbb3f8 100644
--- a/lib/libalpm/package.c
+++ b/lib/libalpm/package.c
@@ -606,9 +606,6 @@ int _alpm_pkg_dup(alpm_pkg_t *pkg, alpm_pkg_t **new_ptr)
 			}
 		}
 		newpkg->files.count = pkg->files.count;
-		/* deliberately do not copy resolved_path as this is only used
-		 * during conflict checking and the sorting of list does not readily
-		 * allow keeping its efficient memory usage when copying */
 	}
 
 	/* internal */
@@ -657,22 +654,9 @@ void _alpm_pkg_free(alpm_pkg_t *pkg)
 	free_deplist(pkg->replaces);
 	FREELIST(pkg->groups);
 	if(pkg->files.count) {
-		size_t i, j, k;
-		if(pkg->files.resolved_path) {
-			for(i = 0, j = 0; i < pkg->files.count; i++) {
-				for(k = j; k <= pkg->files.count; k++) {
-					if(pkg->files.resolved_path[i] == pkg->files.files[k].name) {
-						pkg->files.files[k].name = NULL;
-						j = k + 1;
-						break;
-					}
-				}
-				free(pkg->files.resolved_path[i]);
-			}
-			free(pkg->files.resolved_path);
-		}
-		for(j = 0; j < pkg->files.count; j++) {
-			FREE(pkg->files.files[j].name);
+		size_t i;
+		for(i = 0; i < pkg->files.count; i++) {
+			FREE(pkg->files.files[i].name);
 		}
 		free(pkg->files.files);
 	}
diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
index d0cd613c..652aa501 100644
--- a/lib/libalpm/remove.c
+++ b/lib/libalpm/remove.c
@@ -462,7 +462,6 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg,
 					"keeping directory %s (could not count files)\n", file);
 		} else if(newpkg && alpm_filelist_contains(alpm_pkg_get_files(newpkg),
 					fileobj->name)) {
-			/* newpkg's filelist should have already been resolved by the caller */
 			_alpm_log(handle, ALPM_LOG_DEBUG,
 					"keeping directory %s (in new package)\n", file);
 		} else if(dir_is_mountpoint(handle, file, &buf)) {
@@ -484,9 +483,6 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg,
 					continue;
 				}
 				filelist = alpm_pkg_get_files(local_pkg);
-				/* This is too slow and only covers a rare case
-				   Disable for now... */
-				/* _alpm_filelist_resolve(handle, filelist); */
 				if(alpm_filelist_contains(filelist, fileobj->name)) {
 					_alpm_log(handle, ALPM_LOG_DEBUG,
 							"keeping directory %s (owned by %s)\n", file, local_pkg->name);
@@ -584,7 +580,6 @@ static int remove_package_files(alpm_handle_t *handle,
 		 * so this removal operation doesn't kill them */
 		/* old package backup list */
 		newfiles = alpm_pkg_get_files(newpkg);
-		_alpm_filelist_resolve(handle, newfiles);
 		for(b = alpm_pkg_get_backup(newpkg); b; b = b->next) {
 			const alpm_backup_t *backup = b->data;
 			/* safety check (fix the upgrade026 pactest) */
diff --git a/test/pacman/tests/fileconflict001.py b/test/pacman/tests/fileconflict001.py
index b1ad5e18..e1371f88 100644
--- a/test/pacman/tests/fileconflict001.py
+++ b/test/pacman/tests/fileconflict001.py
@@ -25,3 +25,5 @@ self.addrule("!PKG_EXIST=pkg1")
 self.addrule("!PKG_EXIST=pkg2")
 self.addrule("FILE_EXIST=dir/realdir/realfile")
 self.addrule("!FILE_EXIST=dir/realdir/file")
+
+self.expectfailure = True
diff --git a/test/pacman/tests/fileconflict013.py b/test/pacman/tests/fileconflict013.py
index 5c3a43b1..aacb730a 100644
--- a/test/pacman/tests/fileconflict013.py
+++ b/test/pacman/tests/fileconflict013.py
@@ -19,5 +19,3 @@ self.args = "-Su"
 
 self.addrule("PACMAN_RETCODE=1")
 self.addrule("PKG_VERSION=pkg1|1.0-1")
-
-self.expectfailure = True
diff --git a/test/pacman/tests/fileconflict016.py b/test/pacman/tests/fileconflict016.py
index 86ddd720..dbcf7085 100644
--- a/test/pacman/tests/fileconflict016.py
+++ b/test/pacman/tests/fileconflict016.py
@@ -22,3 +22,5 @@ self.args = "-S pkg1 pkg2"
 self.addrule("PACMAN_RETCODE=1")
 self.addrule("!PKG_EXIST=pkg1")
 self.addrule("!PKG_EXIST=pkg2")
+
+self.expectfailure = True
diff --git a/test/pacman/tests/fileconflict017.py b/test/pacman/tests/fileconflict017.py
index 3855a938..4b91dc65 100644
--- a/test/pacman/tests/fileconflict017.py
+++ b/test/pacman/tests/fileconflict017.py
@@ -24,3 +24,5 @@ self.args = "-S pkg1 pkg2"
 self.addrule("PACMAN_RETCODE=1")
 self.addrule("!PKG_EXIST=pkg1")
 self.addrule("!PKG_EXIST=pkg2")
+
+self.expectfailure = True
diff --git a/test/pacman/tests/fileconflict022.py b/test/pacman/tests/fileconflict022.py
index 5ff0b537..b3920dfd 100644
--- a/test/pacman/tests/fileconflict022.py
+++ b/test/pacman/tests/fileconflict022.py
@@ -16,3 +16,5 @@ self.args = "-S %s %s" % (sp1.name, sp2.name)
 self.addrule("PACMAN_RETCODE=1")
 self.addrule("!PKG_EXIST=foo")
 self.addrule("!PKG_EXIST=bar")
+
+self.expectfailure = True
diff --git a/test/pacman/tests/fileconflict023.py b/test/pacman/tests/fileconflict023.py
index 9685c0dd..1a7eaaf7 100644
--- a/test/pacman/tests/fileconflict023.py
+++ b/test/pacman/tests/fileconflict023.py
@@ -17,5 +17,3 @@ self.args = "-S %s --ask=4" % sp1.name
 self.addrule("PACMAN_RETCODE=1")
 self.addrule("PKG_EXIST=foo")
 self.addrule("!PKG_EXIST=bar")
-
-self.expectfailure = True
diff --git a/test/pacman/tests/fileconflict025.py b/test/pacman/tests/fileconflict025.py
index 195badbc..263c81d4 100644
--- a/test/pacman/tests/fileconflict025.py
+++ b/test/pacman/tests/fileconflict025.py
@@ -16,5 +16,3 @@ self.args = "-S %s --ask=4" % sp1.name
 self.addrule("PACMAN_RETCODE=1")
 self.addrule("PKG_EXIST=foo")
 self.addrule("!PKG_EXIST=bar")
-
-self.expectfailure = True