From cbbd3781c249c8318934fe0810a09546c2760f53 Mon Sep 17 00:00:00 2001 From: Andrew Gregory Date: Fri, 12 Apr 2013 22:26:14 -0400 Subject: improve dir->file transition conflict resolution Packages removed due to conflicts are always removed at the beginning of the transaction and as such can be included in the check for whether all owners of a directory will be removed in a transaction. Installed versions of packages being upgraded, other than the one with the conflict, cannot be used because our transaction ordering is not intelligent enough to ensure that they are removed prior to the installation of the conflicted package. Also, return false from dir_belongsto_pkgs on errors. Previously, we simply continued which could return true even if we were unable to actually establish that the package owned the entire tree. Signed-off-by: Andrew Gregory Signed-off-by: Allan McRae --- lib/libalpm/conflict.c | 133 +++++++++++++++++------------------ test/pacman/tests/fileconflict030.py | 17 +++++ 2 files changed, 82 insertions(+), 68 deletions(-) create mode 100644 test/pacman/tests/fileconflict030.py diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index ab9546cd..a00efe5c 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -299,94 +299,73 @@ void _alpm_fileconflict_free(alpm_fileconflict_t *conflict) } /** - * @brief Recursively checks if a package owns all subdirectories and files in - * a directory. + * @brief Recursively checks if a set of packages own all subdirectories and + * files in a directory. * * @param handle the context handle * @param dirpath path of the directory to check - * @param pkg package being checked against + * @param pkgs packages being checked against * - * @return 1 if a package owns all subdirectories and files or a directory - * cannot be opened, 0 otherwise + * @return 1 if a package owns all subdirectories and files, 0 otherwise */ -static int dir_belongsto_pkg(alpm_handle_t *handle, const char *dirpath, - alpm_pkg_t *pkg) +static int dir_belongsto_pkgs(alpm_handle_t *handle, const char *dirpath, + alpm_list_t *pkgs) { - alpm_list_t *i; - struct stat sbuf; - char path[PATH_MAX]; - char abspath[PATH_MAX]; + char path[PATH_MAX], full_path[PATH_MAX]; DIR *dir; struct dirent *ent = NULL; - const char *root = handle->root; - - /* check directory is actually in package - used for subdirectory checks */ - 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); - return 0; - } - - /* TODO: this is an overly strict check but currently pacman will not - * overwrite a directory with a file (case 10/11 in add.c). Adjusting that - * is not simple as even if the directory is being unowned by a conflicting - * package, pacman does not sort this to ensure all required directory - * "removals" happen before installation of file/symlink */ - /* check that no other _installed_ package owns the directory */ - for(i = _alpm_db_get_pkgcache(handle->db_local); i; i = i->next) { - if(pkg == i->data) { - continue; - } - - 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, - ((alpm_pkg_t*)i->data)->name); - return 0; - } - } - - /* check all files in directory are owned by the package */ - snprintf(abspath, PATH_MAX, "%s%s", root, dirpath); - dir = opendir(abspath); + snprintf(full_path, PATH_MAX, "%s%s", handle->root, dirpath); + dir = opendir(full_path); if(dir == NULL) { - return 1; + return 0; } while((ent = readdir(dir)) != NULL) { const char *name = ent->d_name; + int owned = 0; + alpm_list_t *i; + struct stat sbuf; if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) { continue; } + snprintf(path, PATH_MAX, "%s%s", dirpath, name); - snprintf(abspath, PATH_MAX, "%s%s", root, path); - if(stat(abspath, &sbuf) != 0) { - continue; - } - if(S_ISDIR(sbuf.st_mode)) { - if(dir_belongsto_pkg(handle, path, pkg)) { - continue; - } else { - closedir(dir); - return 0; - } - } else { - if(alpm_filelist_contains(alpm_pkg_get_files(pkg), path)) { - continue; - } else { - closedir(dir); - _alpm_log(handle, ALPM_LOG_DEBUG, - "unowned file %s found in directory\n", path); - return 0; + snprintf(full_path, PATH_MAX, "%s%s", handle->root, path); + + for(i = pkgs; i && !owned; i = i->next) { + if(alpm_filelist_contains(alpm_pkg_get_files(i->data), path)) { + owned = 1; } } + + if(owned && stat(full_path, &sbuf) != 0 && S_ISDIR(sbuf.st_mode)) { + owned = dir_belongsto_pkgs(handle, path, pkgs); + } + + if(!owned) { + closedir(dir); + _alpm_log(handle, ALPM_LOG_DEBUG, + "unowned file %s found in directory\n", path); + return 0; + } } closedir(dir); return 1; } +static alpm_list_t *alpm_db_find_file_owners(alpm_db_t* db, const char *path) +{ + alpm_list_t *i, *owners = NULL; + for(i = alpm_db_get_pkgcache(db); i; i = i->next) { + if(alpm_filelist_contains(alpm_pkg_get_files(i->data), path)) { + owners = alpm_list_add(owners, i->data); + } + } + return owners; +} + /** * @brief Find file conflicts that may occur during the transaction. * @@ -578,14 +557,32 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, } /* check if all files of the dir belong to the installed pkg */ - if(!resolved_conflict && S_ISDIR(lsbuf.st_mode) && dbpkg) { + if(!resolved_conflict && S_ISDIR(lsbuf.st_mode)) { + alpm_list_t *owners; char *dir = malloc(strlen(relative_path) + 2); sprintf(dir, "%s/", relative_path); - if(alpm_filelist_contains(alpm_pkg_get_files(dbpkg), dir)) { - _alpm_log(handle, ALPM_LOG_DEBUG, - "checking if all files in %s belong to %s\n", - dir, dbpkg->name); - resolved_conflict = dir_belongsto_pkg(handle, dir, dbpkg); + + owners = alpm_db_find_file_owners(handle->db_local, dir); + if(owners) { + alpm_list_t *pkgs = NULL, *diff; + + if(dbpkg) { + pkgs = alpm_list_add(pkgs, dbpkg); + } + pkgs = alpm_list_join(pkgs, alpm_list_copy(rem)); + if((diff = alpm_list_diff(owners, pkgs, _alpm_pkg_cmp))) { + /* dir is owned by files we aren't removing */ + /* TODO: with better commit ordering, we may be able to check + * against upgrades as well */ + alpm_list_free(diff); + } else { + _alpm_log(handle, ALPM_LOG_DEBUG, + "checking if all files in %s belong to removed packages\n", + dir); + resolved_conflict = dir_belongsto_pkgs(handle, dir, owners); + } + alpm_list_free(pkgs); + alpm_list_free(owners); } free(dir); } diff --git a/test/pacman/tests/fileconflict030.py b/test/pacman/tests/fileconflict030.py new file mode 100644 index 00000000..1de77813 --- /dev/null +++ b/test/pacman/tests/fileconflict030.py @@ -0,0 +1,17 @@ +self.description = "Dir->file transition filesystem conflict resolved by removal" + +lp1 = pmpkg("foo") +lp1.files = ["foo/"] +self.addpkg2db("local", lp1) + +sp1 = pmpkg("bar") +sp1.conflicts = ["foo"] +sp1.files = ["foo"] +self.addpkg2db("sync", sp1) + +self.args = "-S %s --ask=4" % sp1.name + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PKG_EXIST=bar") +self.addrule("!PKG_EXIST=foo") +self.addrule("FILE_EXIST=foo") -- cgit v1.2.3-70-g09d2