Re: [pacman-dev] [PATCH 5/5] use resolved_path for filelist_contains

On 16/02/13 12:02, Andrew Gregory wrote:

alpm_filelist_contains was being used to search for resolved paths, but searching in the unresolved paths, causing it to miss matches.

Signed-off-by: Andrew Gregory ---

Instead of requiring _alpm_filelist_resolve() to be called before calling alpm_filelist_contains(), just add the call in alpm_filelist_contains() itself. If the filelist has already been resolved, it returns immediately anyway. As an aside - this whole situation really sucks! We now are resolving every path...

lib/libalpm/alpm.h | 2 +- lib/libalpm/conflict.c | 5 +++++ lib/libalpm/filelist.c | 6 +++--- lib/libalpm/remove.c | 3 +++ test/pacman/tests/fileconflict023.py | 2 -- 5 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 5037859..afa5cd7 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1023,7 +1023,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 */ -alpm_file_t *alpm_filelist_contains(alpm_filelist_t *filelist, const char *path); +char *alpm_filelist_contains(alpm_filelist_t *filelist, const char *path);

/* * Signatures diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index ba642dd..afed895 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -321,6 +321,7 @@ 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); @@ -339,6 +340,7 @@ 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, @@ -373,6 +375,7 @@ 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 { @@ -544,6 +547,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, alpm_pkg_t *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 @@ -590,6 +594,7 @@ 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 b0dcee4..f9092c1 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -324,7 +324,7 @@ int _alpm_files_cmp(const void *f1, const void *f2) }

-alpm_file_t *alpm_filelist_contains(alpm_filelist_t *filelist, +char *alpm_filelist_contains(alpm_filelist_t *filelist, const char *path) { alpm_file_t key; @@ -335,8 +335,8 @@ alpm_file_t *alpm_filelist_contains(alpm_filelist_t *filelist,

key.name = (char *)path;

- return bsearch(&key, filelist->files, filelist->count, - sizeof(alpm_file_t), _alpm_files_cmp); + return bsearch(&key, filelist->resolved_path, filelist->count, + sizeof(char *), _alpm_filelist_strcmp); }

/* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 64888c5..60ea8de 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -462,6 +462,7 @@ 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)) { @@ -483,6 +484,7 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, continue; } filelist = alpm_pkg_get_files(local_pkg); + _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); @@ -580,6 +582,7 @@ 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/fileconflict023.py b/test/pacman/tests/fileconflict023.py index 1310b68..ce72087 100644 --- a/test/pacman/tests/fileconflict023.py +++ b/test/pacman/tests/fileconflict023.py @@ -16,5 +16,3 @@ self.addrule("PACMAN_RETCODE=0") self.addrule("!PKG_EXIST=foo") self.addrule("PKG_EXIST=bar") - -self.expectfailure = True