[pacman-dev] [PATCH 1/2] File conflict code cleanups
While researching the root cause of FS#24904, I couldn't help but clean up some of the cruft in here. A few whitespace/line-wrapping issues, but also fix shadowed variables and add some const where applicable. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/conflict.c | 22 +++++++++++----------- 1 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 6faced1..b1df108 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -296,14 +296,14 @@ static alpm_list_t *chk_filedifference(alpm_list_t *filesA, alpm_list_t *filesB) return(ret); } -/* Adds pmfileconflict_t to a conflicts list. Pass the conflicts list, type (either - * PM_FILECONFLICT_TARGET or PM_FILECONFLICT_FILESYSTEM), a file string, and either - * two package names or one package name and NULL. This is a wrapper for former - * functionality that was done inline. +/* Adds pmfileconflict_t to a conflicts list. Pass the conflicts list, type + * (either PM_FILECONFLICT_TARGET or PM_FILECONFLICT_FILESYSTEM), a file + * string, and either two package names or one package name and NULL. This is + * a wrapper for former functionality that was done inline. */ static alpm_list_t *add_fileconflict(alpm_list_t *conflicts, pmfileconflicttype_t type, const char *filestr, - const char* name1, const char* name2) + const char *name1, const char *name2) { pmfileconflict_t *conflict; MALLOC(conflict, sizeof(pmfileconflict_t), RET_ERR(PM_ERR_MEMORY, NULL)); @@ -334,7 +334,7 @@ void _alpm_fileconflict_free(pmfileconflict_t *conflict) FREE(conflict); } -static int dir_belongsto_pkg(char *dirpath, pmpkg_t *pkg) +static int dir_belongsto_pkg(const char *dirpath, pmpkg_t *pkg) { struct dirent *ent = NULL; struct stat sbuf; @@ -462,7 +462,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans, } stat(path, &sbuf); - if(path[strlen(path)-1] == '/') { + if(path[strlen(path) - 1] == '/') { if(S_ISDIR(lsbuf.st_mode)) { _alpm_log(PM_LOG_DEBUG, "%s is a directory, not a conflict\n", path); continue; @@ -518,19 +518,19 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans, if(!resolved_conflict && dbpkg) { char *rpath = calloc(PATH_MAX+1, sizeof(char)); + const char *relative_rpath; if(!realpath(path, rpath)) { - FREE(rpath); + free(rpath); continue; } - char *filestr = rpath + strlen(handle->root); - if(alpm_list_find_str(alpm_pkg_get_files(dbpkg),filestr)) { + relative_rpath = rpath + strlen(handle->root); + if(alpm_list_find_str(alpm_pkg_get_files(dbpkg), relative_rpath)) { resolved_conflict = 1; } free(rpath); } if(!resolved_conflict) { - _alpm_log(PM_LOG_DEBUG, "file found in conflict: %s\n", path); conflicts = add_fileconflict(conflicts, PM_FILECONFLICT_FILESYSTEM, path, p1->name, NULL); } -- 1.7.5.2
This addresses FS#24904. In a normal upgrade case, this replacement seems to work just fine. However, when doing a sync "replace" type upgrade, we weren't properly handling this edge case due to path comparison not ignoring trailing slashes. Fix this by pruning any trailing slashes past a certain point of file conflict resolution where we no longer need them, which allows us to safely detect cases such as now tested in the new pactest. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/conflict.c | 26 ++++++++++++++++++-------- test/pacman/tests/fileconflict008.py | 21 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 test/pacman/tests/fileconflict008.py diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index b1df108..db10d32 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -433,7 +433,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans, /* declarations for second check */ struct stat lsbuf, sbuf; - char *filestr = NULL; /* CHECK 2: check every target against the filesystem */ _alpm_log(PM_LOG_DEBUG, "searching for filesystem conflicts: %s\n", p1->name); @@ -452,7 +451,9 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans, } for(j = tmpfiles; j; j = j->next) { - filestr = j->data; + const char *filestr = j->data, *relative_path; + /* have we acted on this conflict? */ + int resolved_conflict = 0; snprintf(path, PATH_MAX, "%s%s", handle->root, filestr); @@ -471,16 +472,22 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans, "%s is a symlink to a dir, hopefully not a conflict\n", path); continue; } + /* if we made it to here, we want all subsequent path comparisons to + * not include the trailing slash. This allows things like file -> + * directory replacements. */ + path[strlen(path) - 1] = '\0'; } - _alpm_log(PM_LOG_DEBUG, "checking possible conflict: %s\n", path); - int resolved_conflict = 0; /* have we acted on this conflict? */ + _alpm_log(PM_LOG_DEBUG, "checking possible conflict: %s\n", path); + relative_path = path + strlen(handle->root); /* Check remove list (will we remove the conflicting local file?) */ for(k = remove; k && !resolved_conflict; k = k->next) { pmpkg_t *rempkg = k->data; - if(rempkg && alpm_list_find_str(alpm_pkg_get_files(rempkg), filestr)) { - _alpm_log(PM_LOG_DEBUG, "local file will be removed, not a conflict: %s\n", filestr); + if(alpm_list_find_str(alpm_pkg_get_files(rempkg), relative_path)) { + _alpm_log(PM_LOG_DEBUG, + "local file will be removed, not a conflict: %s\n", + relative_path); resolved_conflict = 1; } } @@ -498,8 +505,11 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans, /* skip removal of file, but not add. this will prevent a second * package from removing the file when it was already installed * by its new owner (whether the file is in backup array or not */ - trans->skip_remove = alpm_list_add(trans->skip_remove, strdup(filestr)); - _alpm_log(PM_LOG_DEBUG, "file changed packages, adding to remove skiplist: %s\n", filestr); + trans->skip_remove = alpm_list_add(trans->skip_remove, + strdup(filestr)); + _alpm_log(PM_LOG_DEBUG, + "file changed packages, adding to remove skiplist: %s\n", + filestr); resolved_conflict = 1; } } diff --git a/test/pacman/tests/fileconflict008.py b/test/pacman/tests/fileconflict008.py new file mode 100644 index 0000000..24ea885 --- /dev/null +++ b/test/pacman/tests/fileconflict008.py @@ -0,0 +1,21 @@ +self.description = "Fileconflict file -> dir on package replacement (FS#24904)" + +lp = pmpkg("dummy") +lp.files = ["dir/filepath", + "dir/file"] +self.addpkg2db("local", lp) + +p1 = pmpkg("replace") +p1.provides = ["dummy"] +p1.replaces = ["dummy"] +p1.files = ["dir/filepath/", + "dir/filepath/file", + "dir/file", + "dir/file2"] +self.addpkg2db("sync", p1) + +self.args = "-Su" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PKG_EXIST=dummy") +self.addrule("PKG_EXIST=replace") -- 1.7.5.2
participants (1)
-
Dan McGee