[pacman-dev] [PATCH 5/5] use resolved_path for filelist_contains
Andrew Gregory
andrew.gregory.8 at gmail.com
Sat Feb 16 20:08:05 EST 2013
On Sun, 17 Feb 2013 10:32:48 +1000
Allan McRae <allan at archlinux.org> wrote:
> 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 <andrew.gregory.8 at gmail.com>
> > ---
>
> 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...
>
I considered doing that. The reason I decided not to was that
alpm_filelist_contains wouldn't be able to easily notify the caller if
there's an error resolving the files. We aren't checking for those
errors at the moment, but it would be good to add that at some point.
> > 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
> >
>
>
More information about the pacman-dev
mailing list