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 <[email protected]>
> ---

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
> 


Reply via email to