On Wed, Jul 23, 2008 at 4:14 AM, Dan McGee <dan@archlinux.org> wrote:
This patch makes both fileconflict003 and fileconflict004 pactests successfully pass. It is a rehash of the check was removed in commit 584ffa6aef13d0933ad4930ab9cb70d3af2977ff, but done slightly differently. I'm still not very fond of this code, and it would be much better if we knew more about the filelist than a listing of paths- we have no real concept of whether a to-be-installed file is a symlink, etc.
Issue: http://archlinux.org/pipermail/pacman-dev/2008-July/012465.html
Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/conflict.c | 30 ++++++++++++++++++++++-------- 1 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index a8bcdd5..f0e1116 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -434,14 +434,28 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans, char *roo } stat(path, &sbuf);
- 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); - skip_conflict = 1; - } else if(S_ISLNK(lsbuf.st_mode) && S_ISDIR(sbuf.st_mode)) { - _alpm_log(PM_LOG_DEBUG, - "%s is a symlink to a dir, hopefully not a conflict\n", path); - skip_conflict = 1; + if(path[strlen(path)-1] == '/' && S_ISDIR(lsbuf.st_mode)) { + _alpm_log(PM_LOG_DEBUG, "%s is a directory, not a conflict\n", path); + skip_conflict = 1; + } else if(S_ISLNK(lsbuf.st_mode) && S_ISDIR(sbuf.st_mode)) { + _alpm_log(PM_LOG_DEBUG, + "%s is a symlink to a dir, hopefully not a conflict\n", path); + skip_conflict = 1; + } else if(S_ISDIR(sbuf.st_mode)) { + if(dbpkg) { + /* Make sure the possible conflict is not a symlink that points + * to a path in the old package. */ + struct stat pkgbuf; + char str[PATH_MAX+1]; + for(k = alpm_pkg_get_files(dbpkg); k; k = k->next) { + snprintf(str, PATH_MAX, "%s%s", root, (char*)k->data); + if(!_alpm_lstat(str, &pkgbuf) && lsbuf.st_ino == pkgbuf.st_ino) { + skip_conflict = 1; + _alpm_log(PM_LOG_DEBUG, + "%s is a symlink to a dir in previous package\n", path); + break; + } + } } } if(!skip_conflict) { -- 1.5.6.4
Well, I am unable to understand that code, but maybe Nagy did, since he was able to provide a (fileconflict005 ?) pactest which fails after this patch : http://archlinux.org/pipermail/pacman-dev/2008-July/012522.html That pactest also triggers this "%s is a symlink to a dir in previous package\n" message which looks totally bogus. "/path/to/pacman/root/test is a symlink to a dir in previous package" I don't see how this reflects the situation of "fileconflict005" at all. In the new package, test is a symlink to a new directory test2. and test2 did not exist in any previous package. However, test existed in previous packages, as a directory. Anyway, here are the different ways I propose to deal with this issue : 1) choose that fileconflict003 is more important than 004, and only supports that situation (current 3.2 code) 2) choose that 004 is more important than 003, so slightly edit the code to get back to this old behavior (of 3.0?) 3) we decide that we need to support both, in that case we go the way that Nagy suggested in the beginning : http://archlinux.org/pipermail/pacman-dev/2008-July/012470.html In the first 2 cases, the code will remain (relatively) simple and understandable. In the third one, it will get more complex, but it will still be logical and handle all the cases correctly.