Date: Monday, February 19, 2007 @ 21:14:28 Author: dan Path: /home/cvs-pacman/pacman-lib Modified: TODO.dan (1.11 -> 1.12) lib/libalpm/add.c (1.116 -> 1.117) lib/libalpm/conflict.c (1.38 -> 1.39) lib/libalpm/conflict.h (1.11 -> 1.12) lib/libalpm/remove.c (1.67 -> 1.68) lib/libalpm/trans.c (1.35 -> 1.36) lib/libalpm/trans.h (1.23 -> 1.24) src/pacman/package.c (1.30 -> 1.31) * Updated conflict checking one last time. You can finally have a file move from one package to another seemlessly (knock on wood). This is implemented through the use of two skip lists in the trans struct- skip_add and skip_remove, which replace the former trans->skiplist. * Removed an unnecessary function parameter, added a necessary one. * If a package has no backup files, print '(none)' under the heading so it is more obvious. * Updated my TODO list. ------------------------+ TODO.dan | 6 +++ lib/libalpm/add.c | 22 ++++++------ lib/libalpm/conflict.c | 83 ++++++++++++++++++++++++----------------------- lib/libalpm/conflict.h | 2 - lib/libalpm/remove.c | 31 +++++------------ lib/libalpm/trans.c | 6 ++- lib/libalpm/trans.h | 7 ++- src/pacman/package.c | 80 ++++++++++++++++++++++++--------------------- 8 files changed, 123 insertions(+), 114 deletions(-) Index: pacman-lib/TODO.dan diff -u pacman-lib/TODO.dan:1.11 pacman-lib/TODO.dan:1.12 --- pacman-lib/TODO.dan:1.11 Sat Feb 17 00:06:13 2007 +++ pacman-lib/TODO.dan Mon Feb 19 21:14:27 2007 @@ -117,3 +117,9 @@ Resurrect test scripts, and add ones as needed. Testing by scripts > testing by hand. +Build a replacement for this, or at least standardize its use. We shouldn't +always need to pass handle->root around, it is constant. Something like char* +buildpath(file). + /* build the new entryname relative to handle->root */ + snprintf(filename, PATH_MAX, "%s%s", handle->root, entryname); + Index: pacman-lib/lib/libalpm/add.c diff -u pacman-lib/lib/libalpm/add.c:1.116 pacman-lib/lib/libalpm/add.c:1.117 --- pacman-lib/lib/libalpm/add.c:1.116 Sun Feb 18 17:07:11 2007 +++ pacman-lib/lib/libalpm/add.c Mon Feb 19 21:14:27 2007 @@ -316,25 +316,19 @@ /* Check for file conflicts */ if(!(trans->flags & PM_TRANS_FLAG_FORCE)) { - alpm_list_t *skiplist = NULL; - EVENT(trans, PM_TRANS_EVT_FILECONFLICTS_START, NULL, NULL); _alpm_log(PM_LOG_DEBUG, _("looking for file conflicts")); - lp = _alpm_db_find_conflicts(db, trans, handle->root, &skiplist); + lp = _alpm_db_find_conflicts(db, trans, handle->root); if(lp != NULL) { if(data) { *data = lp; } else { FREELIST(lp); } - FREELIST(skiplist); RET_ERR(PM_ERR_FILE_CONFLICTS, -1); } - /* copy the file skiplist into the transaction */ - trans->skiplist = skiplist; - EVENT(trans, PM_TRANS_EVT_FILECONFLICTS_DONE, NULL, NULL); } @@ -423,7 +417,7 @@ if(oldpkg) { /* this is kinda odd. If the old package exists, at this point we make a * NEW transaction, unrelated to handle->trans, and instantiate a "remove" - * with the type PM_TRANS_TYPE_UPGRADE */ + * with the type PM_TRANS_TYPE_UPGRADE. TODO: kill this weird behavior. */ pmtrans_t *tr = _alpm_trans_new(); _alpm_log(PM_LOG_DEBUG, _("removing old package first (%s-%s)"), oldpkg->name, oldpkg->version); @@ -441,8 +435,8 @@ RET_ERR(PM_ERR_TRANS_ABORT, -1); } - /* copy the skiplist over */ - tr->skiplist = alpm_list_strdup(trans->skiplist); + /* copy the remove skiplist over */ + tr->skip_remove = alpm_list_strdup(trans->skip_remove); alpm_list_t *b; /* Add files in the NEW package's backup array to the noupgrade array @@ -538,11 +532,19 @@ /* if a file is in NoExtract then we never extract it */ if(alpm_list_find_str(handle->noextract, entryname)) { + _alpm_log(PM_LOG_DEBUG, _("%s is in NoExtract, skipping extraction"), entryname); alpm_logaction(_("notice: %s is in NoExtract -- skipping extraction"), entryname); archive_read_data_skip(archive); continue; } + /* if a file is in the add skiplist we never extract it */ + if(alpm_list_find_str(trans->skip_add, filename)) { + _alpm_log(PM_LOG_DEBUG, _("%s is in trans->skip_add, skipping extraction"), entryname); + archive_read_data_skip(archive); + continue; + } + /* check is file already exists */ if(stat(filename, &buf) == 0 && !S_ISDIR(buf.st_mode)) { /* it does, is it a backup=() file? Index: pacman-lib/lib/libalpm/conflict.c diff -u pacman-lib/lib/libalpm/conflict.c:1.38 pacman-lib/lib/libalpm/conflict.c:1.39 --- pacman-lib/lib/libalpm/conflict.c:1.38 Sun Feb 18 22:18:59 2007 +++ pacman-lib/lib/libalpm/conflict.c Mon Feb 19 21:14:27 2007 @@ -313,8 +313,7 @@ return(conflicts); } -alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root, - alpm_list_t **skip_list) +alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root) { alpm_list_t *i, *j, *k; alpm_list_t *conflicts = NULL; @@ -366,50 +365,54 @@ if(dbpkg) { /* older ver of package currently installed */ tmpfiles = chk_filedifference(p1->files, alpm_pkg_get_files(dbpkg)); - for(j = tmpfiles; j; j = j->next) { - filestr = j->data; - _alpm_log(PM_LOG_DEBUG, "checking possible conflict: %s", filestr); - - snprintf(path, PATH_MAX, "%s%s", root, filestr); - - /* stat the file - if it exists and is not a dir, do some checks */ - if(lstat(path, &buf) == 0 && !S_ISDIR(buf.st_mode)) { - /* Look at all the targets to see if file has changed hands */ - for(k = targets; k; k = k->next) { - p2 = (pmpkg_t *)k->data; - /* Ensure we aren't looking at current package */ - if(strcmp(p2->name, p1->name)) { - pmpkg_t *localp2 = _alpm_db_get_pkgfromcache(db, p2->name); - /* Check if it used to exist in a package, but doesn't anymore */ - if(localp2 && !alpm_list_find_str(alpm_pkg_get_files(p2), filestr) - && alpm_list_find_str(alpm_pkg_get_files(localp2), filestr)) { - *skip_list = alpm_list_add(*skip_list, strdup(filestr)); - _alpm_log(PM_LOG_DEBUG, "adding to skiplist: %s", filestr); - } else { - conflicts = add_fileconflict(conflicts, PM_CONFLICT_TYPE_FILE, - filestr, p1->name, NULL); - break; - } - } - } - } - } - alpm_list_free_inner(tmpfiles, &free); - alpm_list_free(tmpfiles); } else { /* no version of package currently installed */ - for(j = p1->files; j; j = j->next) { - filestr = j->data; - - snprintf(path, PATH_MAX, "%s%s", root, filestr); + tmpfiles = alpm_list_strdup(p1->files); + } - /* stat the file - if it exists and is not a dir, report a conflict */ - if(lstat(path, &buf) == 0 && !S_ISDIR(buf.st_mode)) { - conflicts = add_fileconflict(conflicts, PM_CONFLICT_TYPE_FILE, - filestr, p1->name, NULL); + /* loop over each file to be installed */ + for(j = tmpfiles; j; j = j->next) { + filestr = j->data; + _alpm_log(PM_LOG_DEBUG, "checking possible conflict: %s", filestr); + + snprintf(path, PATH_MAX, "%s%s", root, filestr); + + /* stat the file - if it exists and is not a dir, do some checks */ + if(lstat(path, &buf) == 0 && !S_ISDIR(buf.st_mode)) { + /* Look at all the targets to see if file has changed hands */ + for(k = targets; k; k = k->next) { + p2 = (pmpkg_t *)k->data; + /* Ensure we aren't looking at current package */ + if(p2 == p1) { + continue; + } + pmpkg_t *localp2 = _alpm_db_get_pkgfromcache(db, p2->name); + /* Check if it used to exist in a package, but doesn't anymore */ + if(localp2 && !alpm_list_find_str(alpm_pkg_get_files(p2), filestr) + && alpm_list_find_str(alpm_pkg_get_files(localp2), filestr)) { + /* check if the file is now in the backup array */ + if(alpm_list_find_str(alpm_pkg_get_backup(p1), filestr)) { + /* keep file intact if it is in backup array */ + trans->skip_add = alpm_list_add(trans->skip_add, strdup(path)); + trans->skip_remove = alpm_list_add(trans->skip_remove, strdup(path)); + _alpm_log(PM_LOG_DEBUG, "file in backup array, adding to add and remove skiplist: %s", filestr); + } else { + /* 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 */ + trans->skip_remove = alpm_list_add(trans->skip_remove, strdup(path)); + _alpm_log(PM_LOG_DEBUG, "file changed packages, adding to remove skiplist: %s", filestr); + } + } else { + conflicts = add_fileconflict(conflicts, PM_CONFLICT_TYPE_FILE, + filestr, p1->name, NULL); + break; + } } } } + alpm_list_free_inner(tmpfiles, &free); + alpm_list_free(tmpfiles); } return(conflicts); Index: pacman-lib/lib/libalpm/conflict.h diff -u pacman-lib/lib/libalpm/conflict.h:1.11 pacman-lib/lib/libalpm/conflict.h:1.12 --- pacman-lib/lib/libalpm/conflict.h:1.11 Tue Jan 23 22:02:54 2007 +++ pacman-lib/lib/libalpm/conflict.h Mon Feb 19 21:14:27 2007 @@ -34,7 +34,7 @@ }; alpm_list_t *_alpm_checkconflicts(pmdb_t *db, alpm_list_t *packages); -alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root, alpm_list_t **skip_list); +alpm_list_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root); #endif /* _ALPM_CONFLICT_H */ Index: pacman-lib/lib/libalpm/remove.c diff -u pacman-lib/lib/libalpm/remove.c:1.67 pacman-lib/lib/libalpm/remove.c:1.68 --- pacman-lib/lib/libalpm/remove.c:1.67 Sun Feb 18 23:13:13 2007 +++ pacman-lib/lib/libalpm/remove.c Mon Feb 19 21:14:27 2007 @@ -155,19 +155,15 @@ return(0); } -static int can_remove_file(const char *path) +static int can_remove_file(pmtrans_t *trans, const char *path) { - alpm_list_t *i; char file[PATH_MAX+1]; snprintf(file, PATH_MAX, "%s%s", handle->root, path); - for(i = handle->trans->skiplist; i; i = i->next) { - if(strcmp(file, i->data) == 0) { - /* skipping this file, return success because "removing" this - * file does nothing */ - return(1); - } + if(alpm_list_find_str(trans->skip_remove, file)) { + /* return success because we will never actually remove this file */ + return(1); } /* If we fail write permissions due to a read-only filesystem, abort. * Assume all other possible failures are covered somewhere else */ @@ -175,7 +171,8 @@ if(access(file, F_OK) == 0) { /* only return failure if the file ACTUALLY exists and we don't have * permissions */ - _alpm_log(PM_LOG_ERROR, _("cannot remove file '%s': %s"), file, strerror(errno)); + _alpm_log(PM_LOG_ERROR, _("cannot remove file '%s': %s"), + file, strerror(errno)); return(0); } } @@ -230,19 +227,11 @@ _alpm_log(PM_LOG_DEBUG, _("removing directory %s"), file); } } else { - /* check the "skip list" before removing the file. + /* check the remove skip list before removing the file. * see the big comment block in db_find_conflicts() for an * explanation. */ - int skipit = 0; - alpm_list_t *j; - for(j = trans->skiplist; j; j = j->next) { - if(!strcmp(lp->data, (char*)j->data)) { - skipit = 1; - } - } - if(skipit) { - _alpm_log(PM_LOG_WARNING, _("%s has changed ownership, skipping removal"), - file); + if(alpm_list_find_str(trans->skip_remove, file)) { + _alpm_log(PM_LOG_DEBUG, _("%s is in trans->skip_remove, skipping removal"), file); } else if(needbackup) { /* if the file is flagged, back it up to .pacsave */ if(!(trans->type == PM_TRANS_TYPE_UPGRADE)) { @@ -301,7 +290,7 @@ if(!(trans->flags & PM_TRANS_FLAG_DBONLY)) { for(lp = info->files; lp; lp = lp->next) { - if(!can_remove_file(lp->data)) { + if(!can_remove_file(trans, lp->data)) { _alpm_log(PM_LOG_DEBUG, _("not removing package '%s', can't remove all files"), info->name); RET_ERR(PM_ERR_PKG_CANT_REMOVE, -1); } Index: pacman-lib/lib/libalpm/trans.c diff -u pacman-lib/lib/libalpm/trans.c:1.35 pacman-lib/lib/libalpm/trans.c:1.36 --- pacman-lib/lib/libalpm/trans.c:1.35 Sun Feb 18 17:07:11 2007 +++ pacman-lib/lib/libalpm/trans.c Mon Feb 19 21:14:27 2007 @@ -55,7 +55,8 @@ trans->targets = NULL; trans->packages = NULL; - trans->skiplist = NULL; + trans->skip_add = NULL; + trans->skip_remove = NULL; trans->type = 0; trans->flags = 0; trans->cb_event = NULL; @@ -87,7 +88,8 @@ FREELISTPKGS(trans->packages); } - FREELIST(trans->skiplist); + FREELIST(trans->skip_add); + FREELIST(trans->skip_remove); FREE(trans); } Index: pacman-lib/lib/libalpm/trans.h diff -u pacman-lib/lib/libalpm/trans.h:1.23 pacman-lib/lib/libalpm/trans.h:1.24 --- pacman-lib/lib/libalpm/trans.h:1.23 Sun Feb 18 17:07:11 2007 +++ pacman-lib/lib/libalpm/trans.h Mon Feb 19 21:14:27 2007 @@ -41,9 +41,10 @@ pmtranstype_t type; unsigned int flags; pmtransstate_t state; - alpm_list_t *targets; /* alpm_list_t of (char *) */ - alpm_list_t *packages; /* alpm_list_t of (pmpkg_t *) or (pmsyncpkg_t *) */ - alpm_list_t *skiplist; /* alpm_list_t of (char *) */ + alpm_list_t *targets; /* list of (char *) */ + alpm_list_t *packages; /* list of (pmpkg_t *) or (pmsyncpkg_t *) */ + alpm_list_t *skip_add; /* list of (char *) */ + alpm_list_t *skip_remove; /* list of (char *) */ alpm_trans_cb_event cb_event; alpm_trans_cb_conv cb_conv; alpm_trans_cb_progress cb_progress; Index: pacman-lib/src/pacman/package.c diff -u pacman-lib/src/pacman/package.c:1.30 pacman-lib/src/pacman/package.c:1.31 --- pacman-lib/src/pacman/package.c:1.30 Wed Feb 14 10:54:35 2007 +++ pacman-lib/src/pacman/package.c Mon Feb 19 21:14:28 2007 @@ -152,49 +152,55 @@ { alpm_list_t *i; const char *root = alpm_option_get_root(); - printf(_("Backup Files :\n")); - for(i = alpm_pkg_get_backup(pkg); i; i = alpm_list_next(i)) { - struct stat buf; - char path[PATH_MAX]; - char *str = strdup(alpm_list_getdata(i)); - char *ptr = index(str, '\t'); - if(ptr == NULL) { - FREE(str); - continue; - } - *ptr = '\0'; - ptr++; - snprintf(path, PATH_MAX-1, "%s%s", root, str); - /* if we find the file, calculate checksums, otherwise it is missing */ - if(!stat(path, &buf)) { - char *sum; - char *md5sum = alpm_get_md5sum(path); - char *sha1sum = alpm_get_sha1sum(path); - - if(md5sum == NULL || sha1sum == NULL) { - ERR(NL, _("error calculating checksums for %s\n"), path); + printf(_("Backup Files:\n")); + if(alpm_pkg_get_backup(pkg)) { + /* package has backup files, so print them */ + for(i = alpm_pkg_get_backup(pkg); i; i = alpm_list_next(i)) { + struct stat buf; + char path[PATH_MAX]; + char *str = strdup(alpm_list_getdata(i)); + char *ptr = index(str, '\t'); + if(ptr == NULL) { FREE(str); continue; } - /* TODO Is this a good way to check type of backup stored? - * We aren't storing it anywhere in the database. */ - if (strlen(ptr) == 32) { - sum = md5sum; - } else { /*if (strlen(ptr) == 40) */ - sum = sha1sum; - } - /* if checksums don't match, file has been modified */ - if (strcmp(sum, ptr)) { - printf(_("MODIFIED\t%s\n"), path); + *ptr = '\0'; + ptr++; + snprintf(path, PATH_MAX-1, "%s%s", root, str); + /* if we find the file, calculate checksums, otherwise it is missing */ + if(!stat(path, &buf)) { + char *sum; + char *md5sum = alpm_get_md5sum(path); + char *sha1sum = alpm_get_sha1sum(path); + + if(md5sum == NULL || sha1sum == NULL) { + ERR(NL, _("error calculating checksums for %s\n"), path); + FREE(str); + continue; + } + /* TODO Is this a good way to check type of backup stored? + * We aren't storing it anywhere in the database. */ + if (strlen(ptr) == 32) { + sum = md5sum; + } else { /*if (strlen(ptr) == 40) */ + sum = sha1sum; + } + /* if checksums don't match, file has been modified */ + if (strcmp(sum, ptr)) { + printf(_("MODIFIED\t%s\n"), path); + } else { + printf(_("Not Modified\t%s\n"), path); + } + FREE(md5sum); + FREE(sha1sum); } else { - printf(_("Not Modified\t%s\n"), path); + printf(_("MISSING\t\t%s\n"), path); } - FREE(md5sum); - FREE(sha1sum); - } else { - printf(_("MISSING\t\t%s\n"), path); + FREE(str); } - FREE(str); + } else { + /* package had no backup files */ + printf(_("(none)\n")); } }