[pacman-dev] Duplicated requiredby conclusion
Hi! Yes, we fixed this problem: We didn't check the existency of the entry before inserting it to the requiredby field. But how could duplicated entries appear? Update is a remove-then-install process, so we simply remove the package first (and removing it from requiredby fields), then install it (which is not installed now, so that is not listed in any requiredby fields). The answer: That remove process is not a real remove transaction (a pseudo transaction is created); look into add.c(389): ==== _alpm_trans_init(tr, PM_TRANS_TYPE_UPGRADE, ... ==== Wow, the transaction type is UPGRADE (this is mainly for logging imho). But look into trans.c/_alpm_trans_update_depends(342): ==== if(trans->type == PM_TRANS_TYPE_REMOVE) { void *data = NULL; rqdby = alpm_list_remove(rqdby, pkgname, _alpm_str_cmp, &data); FREE(data); deppkg->requiredby = rqdby; } else { if(!alpm_list_find_str(rqdby, pkgname)) { rqdby = alpm_list_add(rqdby, strdup(pkgname)); deppkg->requiredby = rqdby; } } ==== So, we want to add it, instead of removing. After adding "if(!alpm_list_find_str(rqdby, pkgname))", this is not so dangerous (but needless), but this a _very odd_ method. However, this is absolutely not safe yet: If the remove was successful, but we cannot install the package, we get false requiredby entries. Bye, Nagy Gabor
_alpm_trans_init(tr, PM_TRANS_TYPE_UPGRADE, ... I think that as a hotfix, a new transaction type would solve this problem: PM_TRANS_TYPE_UPGRADERM or similar. And if you want to speed-up pacman by some nanosec, you can remove "if(!alpm_list_find_str(rqdby, pkgname))" because that is not needed now :-) Bye, Nagy Gabor
On 4/3/07, Nagy Gabor
_alpm_trans_init(tr, PM_TRANS_TYPE_UPGRADE, ... I think that as a hotfix, a new transaction type would solve this problem: PM_TRANS_TYPE_UPGRADERM or similar. And if you want to speed-up pacman by some nanosec, you can remove "if(!alpm_list_find_str(rqdby, pkgname))" because that is not needed now :-) Bye, Nagy Gabor
Please provide a patch.
Hi! Here is my patch, plz test it. A note: now ldconfig doesn't run in case of upgrade (why?). This is not safe imho, so you I fixed this in my patch too. ---------------------- } /* run ldconfig if it exists */ - if((trans->type != PM_TRANS_TYPE_UPGRADE) && (handle->trans->state != STATE_INTERRUPTED)) { + if((trans->type != PM_TRANS_TYPE_UPGRADERM) && (handle->trans->state != STATE_INTERRUPTED)) { _alpm_log(PM_LOG_DEBUG, _("running \"ldconfig -r %s\""), handle->root); _alpm_ldconfig(handle->root); } diff -Naur pacman-lib/lib/libalpm/trans.c pacman-lib.new/lib/libalpm/trans.c --- pacman-lib/lib/libalpm/trans.c 2007-04-04 06:43:24.000000000 +0200 +++ pacman-lib.new/lib/libalpm/trans.c 2007-04-10 21:21:07.000000000 +0200 @@ -148,6 +148,7 @@ } break; case PM_TRANS_TYPE_REMOVE: + case PM_TRANS_TYPE_UPGRADERM: if(_alpm_remove_loadtarget(trans, handle->db_local, target) == -1) { /* pm_errno is set by remove_loadtarget() */ return(-1); @@ -189,6 +190,7 @@ } break; case PM_TRANS_TYPE_REMOVE: + case PM_TRANS_TYPE_UPGRADERM: if(_alpm_remove_prepare(trans, handle->db_local, data) == -1) { /* pm_errno is set by _alpm_remove_prepare() */ return(-1); @@ -233,6 +235,7 @@ } break; case PM_TRANS_TYPE_REMOVE: + case PM_TRANS_TYPE_UPGRADERM: if(_alpm_remove_commit(trans, handle->db_local) == -1) { /* pm_errno is set by _alpm_remove_prepare() */ return(-1); @@ -310,16 +313,16 @@ _alpm_log(PM_LOG_DEBUG, _("updating 'requiredby' field for package '%s'"), alpm_pkg_get_name(deppkg)); - if(trans->type == PM_TRANS_TYPE_REMOVE) { + if(trans->type == PM_TRANS_TYPE_REMOVE || trans->type == PM_TRANS_TYPE_UPGRADERM) { void *data = NULL; rqdby = alpm_list_remove(rqdby, pkgname, _alpm_str_cmp, &data); FREE(data); deppkg->requiredby = rqdby; } else { - if(!alpm_list_find_str(rqdby, pkgname)) { +// if(!alpm_list_find_str(rqdby, pkgname)) { rqdby = alpm_list_add(rqdby, strdup(pkgname)); deppkg->requiredby = rqdby; - } +// } } if(_alpm_db_write(localdb, deppkg, INFRQ_DEPENDS)) { @@ -340,16 +343,16 @@ _alpm_log(PM_LOG_DEBUG, _("updating 'requiredby' field for package '%s'"), alpm_pkg_get_name(deppkg)); - if(trans->type == PM_TRANS_TYPE_REMOVE) { + if(trans->type == PM_TRANS_TYPE_REMOVE || trans->type == PM_TRANS_TYPE_UPGRADERM) { void *data = NULL; rqdby = alpm_list_remove(rqdby, pkgname, _alpm_str_cmp, &data); FREE(data); deppkg->requiredby = rqdby; } else { - if(!alpm_list_find_str(rqdby, pkgname)) { +// if(!alpm_list_find_str(rqdby, pkgname)) { rqdby = alpm_list_add(rqdby, strdup(pkgname)); deppkg->requiredby = rqdby; - } +// } } if(_alpm_db_write(localdb, deppkg, INFRQ_DEPENDS)) { ----------------------------- Bye, ngaba
Na Tue, Apr 10, 2007 at 09:36:33PM +0200, Nagy Gabor
Here is my patch, plz test it.
just to clear the situation, this is not a patch ;) you should rtfm of diff thanks, VMiklos -- developer of Frugalware Linux - http://frugalware.org
just to clear the situation, this is not a patch ;)
Oh, thx, sry. ------------- diff -Naur pacman-lib/lib/libalpm/add.c pacman-lib.new/lib/libalpm/add.c --- pacman-lib/lib/libalpm/add.c 2007-03-22 19:33:27.000000000 +0100 +++ pacman-lib.new/lib/libalpm/add.c 2007-04-10 21:21:46.000000000 +0200 @@ -378,7 +378,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. TODO: kill this weird behavior. */ + * with the type PM_TRANS_TYPE_UPGRADERM. 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); @@ -386,7 +386,7 @@ RET_ERR(PM_ERR_TRANS_ABORT, -1); } - if(_alpm_trans_init(tr, PM_TRANS_TYPE_UPGRADE, trans->flags, NULL, NULL, NULL) == -1) { + if(_alpm_trans_init(tr, PM_TRANS_TYPE_UPGRADERM, trans->flags, NULL, NULL, NULL) == -1) { FREETRANS(tr); RET_ERR(PM_ERR_TRANS_ABORT, -1); } @@ -837,7 +837,7 @@ } /* run ldconfig if it exists */ - if((trans->type != PM_TRANS_TYPE_UPGRADE) && (handle->trans->state != STATE_INTERRUPTED)) { + if(handle->trans->state != STATE_INTERRUPTED) { _alpm_log(PM_LOG_DEBUG, _("running \"ldconfig -r %s\""), handle->root); _alpm_ldconfig(handle->root); } diff -Naur pacman-lib/lib/libalpm/alpm.h pacman-lib.new/lib/libalpm/alpm.h --- pacman-lib/lib/libalpm/alpm.h 2007-03-19 05:23:45.000000000 +0100 +++ pacman-lib.new/lib/libalpm/alpm.h 2007-04-10 20:41:45.000000000 +0200 @@ -268,7 +268,8 @@ PM_TRANS_TYPE_ADD = 1, PM_TRANS_TYPE_REMOVE, PM_TRANS_TYPE_UPGRADE, - PM_TRANS_TYPE_SYNC + PM_TRANS_TYPE_SYNC, + PM_TRANS_TYPE_UPGRADERM } pmtranstype_t; /* Flags */ diff -Naur pacman-lib/lib/libalpm/remove.c pacman-lib.new/lib/libalpm/remove.c --- pacman-lib/lib/libalpm/remove.c 2007-04-05 18:37:23.000000000 +0200 +++ pacman-lib.new/lib/libalpm/remove.c 2007-04-10 21:03:04.000000000 +0200 @@ -105,7 +105,7 @@ ASSERT(db != NULL, RET_ERR(PM_ERR_DB_NULL, -1)); ASSERT(trans != NULL, RET_ERR(PM_ERR_TRANS_NULL, -1)); - if(!(trans->flags & (PM_TRANS_FLAG_NODEPS)) && (trans->type != PM_TRANS_TYPE_UPGRADE)) { + if(!(trans->flags & (PM_TRANS_FLAG_NODEPS)) && (trans->type != PM_TRANS_TYPE_UPGRADERM)) { EVENT(trans, PM_TRANS_EVT_CHECKDEPS_START, NULL, NULL); _alpm_log(PM_LOG_DEBUG, _("looking for unsatisfied dependencies")); @@ -207,7 +207,7 @@ FREE(hash); } - if(trans->type == PM_TRANS_TYPE_UPGRADE) { + if(trans->type == PM_TRANS_TYPE_UPGRADERM) { /* check noupgrade */ if(alpm_list_find_str(handle->noupgrade, lp->data)) { _alpm_log(PM_LOG_DEBUG, _("Skipping removal of '%s' due to NoUpgrade"), file); @@ -237,7 +237,7 @@ return; } else if(needbackup) { /* if the file is flagged, back it up to .pacsave */ - if(!(trans->type == PM_TRANS_TYPE_UPGRADE)) { + if(!(trans->type == PM_TRANS_TYPE_UPGRADERM)) { /* if it was an upgrade, the file would be left alone because * pacman_add() would handle it */ if(!(trans->flags & PM_TRANS_FLAG_NOSAVE)) { @@ -289,7 +289,7 @@ snprintf(scriptlet, PATH_MAX, "%s%s-%s/install", db->path, pkgname, alpm_pkg_get_version(info)); - if(trans->type != PM_TRANS_TYPE_UPGRADE) { + if(trans->type != PM_TRANS_TYPE_UPGRADERM) { EVENT(trans, PM_TRANS_EVT_REMOVE_START, info, NULL); _alpm_log(PM_LOG_DEBUG, _("removing package %s-%s"), pkgname, alpm_pkg_get_version(info)); @@ -321,7 +321,7 @@ } } - if(trans->type != PM_TRANS_TYPE_UPGRADE) { + if(trans->type != PM_TRANS_TYPE_UPGRADERM) { /* run the post-remove script if it exists */ if(alpm_pkg_has_scriptlet(info) && !(trans->flags & PM_TRANS_FLAG_NOSCRIPTLET)) { _alpm_runscriptlet(handle->root, scriptlet, "post_remove", @@ -353,13 +353,13 @@ PROGRESS(trans, PM_TRANS_PROGRESS_REMOVE_START, pkgname, 100, alpm_list_count(trans->packages), (alpm_list_count(trans->packages) - alpm_list_count(targ) +1)); - if(trans->type != PM_TRANS_TYPE_UPGRADE) { + if(trans->type != PM_TRANS_TYPE_UPGRADERM) { EVENT(trans, PM_TRANS_EVT_REMOVE_DONE, info, NULL); } } /* run ldconfig if it exists */ - if((trans->type != PM_TRANS_TYPE_UPGRADE) && (handle->trans->state != STATE_INTERRUPTED)) { + if((trans->type != PM_TRANS_TYPE_UPGRADERM) && (handle->trans->state != STATE_INTERRUPTED)) { _alpm_log(PM_LOG_DEBUG, _("running \"ldconfig -r %s\""), handle->root); _alpm_ldconfig(handle->root); } diff -Naur pacman-lib/lib/libalpm/trans.c pacman-lib.new/lib/libalpm/trans.c --- pacman-lib/lib/libalpm/trans.c 2007-04-04 06:43:24.000000000 +0200 +++ pacman-lib.new/lib/libalpm/trans.c 2007-04-10 21:21:07.000000000 +0200 @@ -148,6 +148,7 @@ } break; case PM_TRANS_TYPE_REMOVE: + case PM_TRANS_TYPE_UPGRADERM: if(_alpm_remove_loadtarget(trans, handle->db_local, target) == -1) { /* pm_errno is set by remove_loadtarget() */ return(-1); @@ -189,6 +190,7 @@ } break; case PM_TRANS_TYPE_REMOVE: + case PM_TRANS_TYPE_UPGRADERM: if(_alpm_remove_prepare(trans, handle->db_local, data) == -1) { /* pm_errno is set by _alpm_remove_prepare() */ return(-1); @@ -233,6 +235,7 @@ } break; case PM_TRANS_TYPE_REMOVE: + case PM_TRANS_TYPE_UPGRADERM: if(_alpm_remove_commit(trans, handle->db_local) == -1) { /* pm_errno is set by _alpm_remove_prepare() */ return(-1); @@ -310,16 +313,16 @@ _alpm_log(PM_LOG_DEBUG, _("updating 'requiredby' field for package '%s'"), alpm_pkg_get_name(deppkg)); - if(trans->type == PM_TRANS_TYPE_REMOVE) { + if(trans->type == PM_TRANS_TYPE_REMOVE || trans->type == PM_TRANS_TYPE_UPGRADERM) { void *data = NULL; rqdby = alpm_list_remove(rqdby, pkgname, _alpm_str_cmp, &data); FREE(data); deppkg->requiredby = rqdby; } else { - if(!alpm_list_find_str(rqdby, pkgname)) { +// if(!alpm_list_find_str(rqdby, pkgname)) { rqdby = alpm_list_add(rqdby, strdup(pkgname)); deppkg->requiredby = rqdby; - } +// } } if(_alpm_db_write(localdb, deppkg, INFRQ_DEPENDS)) { @@ -340,16 +343,16 @@ _alpm_log(PM_LOG_DEBUG, _("updating 'requiredby' field for package '%s'"), alpm_pkg_get_name(deppkg)); - if(trans->type == PM_TRANS_TYPE_REMOVE) { + if(trans->type == PM_TRANS_TYPE_REMOVE || trans->type == PM_TRANS_TYPE_UPGRADERM) { void *data = NULL; rqdby = alpm_list_remove(rqdby, pkgname, _alpm_str_cmp, &data); FREE(data); deppkg->requiredby = rqdby; } else { - if(!alpm_list_find_str(rqdby, pkgname)) { +// if(!alpm_list_find_str(rqdby, pkgname)) { rqdby = alpm_list_add(rqdby, strdup(pkgname)); deppkg->requiredby = rqdby; - } +// } } if(_alpm_db_write(localdb, deppkg, INFRQ_DEPENDS)) {
participants (3)
-
Aaron Griffin
-
Nagy Gabor
-
VMiklos