[pacman-dev] [patch] I. deps.c/checkdeps bugfix + upgrade059.py
Hi! I started to work on deps.c. I send smaller patches instead of a big one. At first I rewrite alpm_*deps functions (implement topological sort...); I merge them later (which would be the most optimal imho) after testing. I found a bug: ---upgrade059.py--- self.description = "Try to upgrade packages which would break a multiple-depend" lp1 = pmpkg("pkg1") lp1.depends = ["imaginary"] self.addpkg2db("local", lp1) lp2 = pmpkg("pkg2", "1.0-1") lp2.provides = ["imaginary"] lp2.requiredby = [ "pkg1" ] self.addpkg2db("local", lp2) lp3 = pmpkg("pkg3", "1.0-1") lp3.provides = ["imaginary"] lp3.requiredby = [ "pkg1" ] self.addpkg2db("local", lp3) p2 = pmpkg("pkg2", "1.0-2") self.addpkg(p2) p3 = pmpkg("pkg3", "1.0-2") self.addpkg(p3) self.args = "-U %s" % " ".join([p.filename() for p in p2, p3]) self.addrule("PACMAN_RETCODE=1") self.addrule("PKG_EXIST=pkg1") self.addrule("PKG_VERSION=pkg2|1.0-1") self.addrule("PKG_VERSION=pkg3|1.0-1") ------------------- Here is my patch (WARNING: with large targetlist this may cause notable slowdown). ------------------- diff -Naur pacman-lib/lib/libalpm/deps.c pacman-lib.new/lib/libalpm/deps.c --- pacman-lib/lib/libalpm/deps.c 2007-03-19 05:23:45.000000000 +0100 +++ pacman-lib.new/lib/libalpm/deps.c 2007-04-21 16:42:29.000000000 +0200 @@ -271,8 +271,9 @@ for(l = _alpm_db_get_pkgcache(db); l; l = l->next) { pmpkg_t *pkg = l->data; - if(strcmp(alpm_pkg_get_name(pkg), alpm_pkg_get_name(oldpkg)) == 0) { - /* well, we know this one succeeds, but we're removing it... skip it */ + if(_alpm_pkg_find(alpm_pkg_get_name(pkg), packages)) { + /* we ignore packages that will be updated because we know that + updated ones don't satisfy depend.*/ continue; } ------------------ Bye, ngaba
Hi! My patch won't fix the problem, just cuts off a needless part from the source (because depcmp takes care of provides too). This bug is similar to the previous one. But I think this is a serious one, because this occurs in case of -Su too (as bug I.), so can be very common (whenever depends changed in sync repo). ----upgrade060.py---- self.description = "Try to upgrade 2 packages which would break deps" lp1 = pmpkg("pkg1") lp1.depends = ["pkg2=1.0-1"] self.addpkg2db("local", lp1) lp2 = pmpkg("pkg2", "1.0-1") lp2.requiredby = [ "pkg1" ] self.addpkg2db("local", lp2) p1 = pmpkg("pkg1", "1.0-2") p1.depends = ["pkg2=1.0-1"] self.addpkg(p1) p2 = pmpkg("pkg2", "1.0-2") self.addpkg(p2) self.args = "-U %s" % " ".join([p.filename() for p in p1, p2]) self.addrule("PACMAN_RETCODE=1") self.addrule("PKG_VERSION=pkg1|1.0-1") self.addrule("PKG_VERSION=pkg2|1.0-1") ------------------- You find the problem here around lines 323, we don't ensure that we won't overwrite the local "satisfyer" package with a "non-satisfyer" one. This could be fixed similarly to my previous patch, but imho that would be too slow. An idea: we should temporarily attach to the old packages in the pkgcache a "pointer" which points to the "new" target (we found all old packages earlier); and this could be deleted at the end of transaction (anyway, if the transaction is successful, we always must modify that entries). The implementation is waiting for you ;-) -----patch---------- diff -Naur pacman-lib/lib/libalpm/deps.c pacman-lib.new/lib/libalpm/deps.c --- pacman-lib/lib/libalpm/deps.c 2007-03-19 05:23:45.000000000 +0100 +++ pacman-lib.new/lib/libalpm/deps.c 2007-04-21 16:42:29.000000000 +0200 @@ -319,36 +320,12 @@ } found = 0; - /* check database for literal packages */ + /* check database for satisfying packages */ for(k = _alpm_db_get_pkgcache(db); k && !found; k = k->next) { pmpkg_t *p = (pmpkg_t *)k->data; found = alpm_depcmp(p, depend); } - /* check database for provides matches */ - if(!found) { - alpm_list_t *m; - for(m = _alpm_db_whatprovides(db, depend->name); m && !found; m = m->next) { - /* look for a match that isn't one of the packages we're trying - * to install. this way, if we match against a to-be-installed - * package, we'll defer to the NEW one, not the one already - * installed. */ - pmpkg_t *p = m->data; - alpm_list_t *n; - int skip = 0; - for(n = packages; n && !skip; n = n->next) { - pmpkg_t *ptp = n->data; - if(strcmp(alpm_pkg_get_name(ptp), alpm_pkg_get_name(p)) == 0) { - skip = 1; - } - } - if(skip) { - continue; - } - found = alpm_depcmp(p, depend); - } - FREELISTPTR(k); - } /* check other targets */ for(k = packages; k && !found; k = k->next) { pmpkg_t *p = k->data; -------------------- Bye, ngaba
Gave this a try (my previous clean-up is merged too): ----------------------- diff -Naur pacman-lib/lib/libalpm/deps.c pacman-lib.new/lib/libalpm/deps.c --- pacman-lib/lib/libalpm/deps.c 2007-03-19 05:23:45.000000000 +0100 +++ pacman-lib.new/lib/libalpm/deps.c 2007-04-21 20:23:10.000000000 +0200 @@ -319,41 +319,19 @@ } found = 0; - /* check database for literal packages */ - for(k = _alpm_db_get_pkgcache(db); k && !found; k = k->next) { - pmpkg_t *p = (pmpkg_t *)k->data; - found = alpm_depcmp(p, depend); - } - /* check database for provides matches */ - if(!found) { - alpm_list_t *m; - for(m = _alpm_db_whatprovides(db, depend->name); m && !found; m = m->next) { - /* look for a match that isn't one of the packages we're trying - * to install. this way, if we match against a to-be-installed - * package, we'll defer to the NEW one, not the one already - * installed. */ - pmpkg_t *p = m->data; - alpm_list_t *n; - int skip = 0; - for(n = packages; n && !skip; n = n->next) { - pmpkg_t *ptp = n->data; - if(strcmp(alpm_pkg_get_name(ptp), alpm_pkg_get_name(p)) == 0) { - skip = 1; - } - } - if(skip) { - continue; - } - - found = alpm_depcmp(p, depend); - } - FREELISTPTR(k); - } /* check other targets */ for(k = packages; k && !found; k = k->next) { pmpkg_t *p = k->data; found = alpm_depcmp(p, depend); } + + /* check database for satisfying packages */ + /* we can ignore packages which will be updated, they were checked above*/ + for(k = _alpm_db_get_pkgcache(db); k && !found; k = k->next) { + pmpkg_t *p = (pmpkg_t *)k->data; + found = alpm_depcmp(p, depend) && !_alpm_pkg_find(alpm_pkg_get_name(p), packages); + } + /* else if still not found... */ if(!found) { _alpm_log(PM_LOG_DEBUG, _("missing dependency '%s' for package '%s'"), ---------------------- enjoy, ngaba
That looks fine too : http://chantry.homelinux.org/~xav/git/gitweb.cgi?p=pacman.git;a=commit;h=a8f...
Hi! My patch creates remove041.py remove050.py and patches remove040.py. I removed a messy part of the source and patched sync.c instead; and implemented sophisticated (but slower) remove-checkdep (derived from the upgrade-checkdep). I also add a small speed-tuning to upgrade-checkdep ;-) Notes: sync895.py reports false error imho, we should implement --debug to pactest. (sync.c lines 680- is very messy... ) Enjoy, ngaba -------------------- diff -Naur pacman-lib/lib/libalpm/deps.c pacman-lib.new/lib/libalpm/deps.c --- pacman-lib/lib/libalpm/deps.c 2007-03-19 05:23:45.000000000 +0100 +++ pacman-lib.new/lib/libalpm/deps.c 2007-04-21 22:45:25.000000000 +0200 @@ -234,12 +234,12 @@ for(j = alpm_pkg_get_requiredby(oldpkg); j; j = j->next) { pmpkg_t *p; found = 0; - if((p = _alpm_db_get_pkgfromcache(db, j->data)) == NULL) { - /* hmmm... package isn't installed.. */ + if(_alpm_pkg_find(j->data, packages)) { + /* this package also in the upgrade list, so don't worry about it */ continue; } - if(_alpm_pkg_find(alpm_pkg_get_name(p), packages)) { - /* this package also in the upgrade list, so don't worry about it */ + if((p = _alpm_db_get_pkgfromcache(db, j->data)) == NULL) { + /* hmmm... package isn't installed.. */ continue; } for(k = alpm_pkg_get_depends(p); k; k = k->next) { @@ -370,52 +370,55 @@ } } } else if(op == PM_TRANS_TYPE_REMOVE) { - /* check requiredby fields */ for(i = packages; i; i = i->next) { - pmpkg_t *tp = i->data; - if(tp == NULL) { + pmpkg_t *rempkg = i->data; + + if(rempkg == NULL) { + _alpm_log(PM_LOG_DEBUG, _("null package found in package list")); continue; } - found=0; - for(j = alpm_pkg_get_requiredby(tp); j; j = j->next) { - /* Search for 'reqname' in packages for removal */ - char *reqname = j->data; - alpm_list_t *x = NULL; - for(x = packages; x; x = x->next) { - pmpkg_t *xp = x->data; - if(strcmp(reqname, alpm_pkg_get_name(xp)) == 0) { - found = 1; - break; - } + for(j = alpm_pkg_get_requiredby(rempkg); j; j = j->next) { + pmpkg_t *p; + found = 0; + if(_alpm_pkg_find(j->data, packages)) { + /* this package also in the remove list, so don't worry about it */ + continue; } - if(!found) { - /* check if a package in trans->packages provides this package */ - for(k = trans->packages; !found && k; k=k->next) { - pmpkg_t *spkg = NULL; - if(trans->type == PM_TRANS_TYPE_SYNC) { - pmsyncpkg_t *sync = k->data; - spkg = sync->pkg; - } else { - spkg = k->data; - } - if(spkg) { - if(alpm_list_find_str(alpm_pkg_get_provides(spkg), tp->name)) { - found = 1; + if((p = _alpm_db_get_pkgfromcache(db, j->data)) == NULL) { + /* hmmm... package isn't installed.. */ + continue; + } + for(k = alpm_pkg_get_depends(p); k; k = k->next) { + pmdepend_t *depend = alpm_splitdep(k->data); + if(depend == NULL) { + continue; + } + /* if rempkg satisfied this dep, we try to find an other satisfyer (which won't be removed)*/ + if(alpm_depcmp(rempkg, depend)) { + int satisfied = 0; + for(l = _alpm_db_get_pkgcache(db); l; l = l->next) { + pmpkg_t *pkg = l->data; + if(alpm_depcmp(pkg, depend) && !_alpm_pkg_find(alpm_pkg_get_name(pkg), packages)) { + _alpm_log(PM_LOG_DEBUG, _("checkdeps: dependency '%s' satisfied by installed package '%s'"), + depend->name, alpm_pkg_get_name(pkg)); + satisfied = 1; + break; } } - } - if(!found) { - _alpm_log(PM_LOG_DEBUG, _("checkdeps: found %s as required by %s"), - reqname, alpm_pkg_get_name(tp)); - miss = _alpm_depmiss_new(alpm_pkg_get_name(tp), PM_DEP_TYPE_DEPEND, - PM_DEP_MOD_ANY, j->data, NULL); - if(!_alpm_depmiss_isin(miss, baddeps)) { - baddeps = alpm_list_add(baddeps, miss); - } else { - FREE(miss); + if(!satisfied) { + _alpm_log(PM_LOG_DEBUG, _("checkdeps: found %s as required by %s"), + alpm_pkg_get_name(rempkg), alpm_pkg_get_name(p)); + miss = _alpm_depmiss_new(p->name, PM_DEP_TYPE_DEPEND, depend->mod, + depend->name, depend->version); + if(!_alpm_depmiss_isin(miss, baddeps)) { + baddeps = alpm_list_add(baddeps, miss); + } else { + FREE(miss); + } } } + free(depend); } } } diff -Naur pacman-lib/pactest/tests/remove040.py pacman-lib.new/pactest/tests/remove040.py --- pacman-lib/pactest/tests/remove040.py 2007-03-04 01:14:26.000000000 +0100 +++ pacman-lib.new/pactest/tests/remove040.py 2007-04-21 21:00:03.000000000 +0200 @@ -10,9 +10,9 @@ self.addpkg2db("local", lp2) -self.args = "-R %s" % lp1.name +self.args = "-R %s" % lp2.name -self.addrule("PACMAN_RETCODE=0") -self.addrule("!PKG_EXIST=pkg1") +self.addrule("!PACMAN_RETCODE=0") +self.addrule("PKG_EXIST=pkg1") self.addrule("PKG_EXIST=pkg2") -self.addrule("!PKG_REQUIREDBY=pkg2|pkg1") +self.addrule("PKG_REQUIREDBY=pkg2|pkg1") diff -Naur pacman-lib/pactest/tests/remove041.py pacman-lib.new/pactest/tests/remove041.py --- pacman-lib/pactest/tests/remove041.py 1970-01-01 01:00:00.000000000 +0100 +++ pacman-lib.new/pactest/tests/remove041.py 2007-04-21 22:56:16.000000000 +0200 @@ -0,0 +1,21 @@ +self.description = "Remove a package required by another package, but no longer needed (multiple provision)" + +lp1 = pmpkg("pkg1") +lp1.provides = ["imaginary"] +lp1.requiredby = ["pkg3"] +self.addpkg2db("local", lp1) + +lp2 = pmpkg("pkg2") +lp2.provides = ["imaginary"] +lp2.requiredby = ["pkg3"] +self.addpkg2db("local", lp2) + +lp3 = pmpkg("pkg3") +lp3.depends = ["imaginary"] +self.addpkg2db("local", lp3) + +self.args = "-R %s" % lp1.name + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PKG_EXIST=pkg1") +self.addrule("PKG_EXIST=pkg2")
Notes: sync895.py reports false error imho, we should implement --debug to pactest. No, this patch really fails on sync895.py :-( (Yesterday I tested with a wrong pacman binary by hand ;-). So I must look into sync.c again... Bye
Hi! Here a small fix to patchIII (apply it first plz). I noticed that you use depmiss here in an unusual way (why?) [Anyway, in the non-patched pacman the debug-log message found in this patch is "swapped"]. -------patchIII/A------ diff -Naur pacman-lib/lib/libalpm/deps.c pacman-lib.new/lib/libalpm/deps.c --- pacman-lib/lib/libalpm/deps.c 2007-04-21 22:45:25.000000000 +0200 +++ pacman-lib.new/lib/libalpm/deps.c 2007-04-22 13:26:20.000000000 +0200 @@ -409,8 +409,8 @@ if(!satisfied) { _alpm_log(PM_LOG_DEBUG, _("checkdeps: found %s as required by %s"), alpm_pkg_get_name(rempkg), alpm_pkg_get_name(p)); - miss = _alpm_depmiss_new(p->name, PM_DEP_TYPE_DEPEND, depend->mod, - depend->name, depend->version); + miss = _alpm_depmiss_new(alpm_pkg_get_name(rempkg), PM_DEP_TYPE_DEPEND, PM_DEP_MOD_ANY, + alpm_pkg_get_name(p), NULL); if(!_alpm_depmiss_isin(miss, baddeps)) { baddeps = alpm_list_add(baddeps, miss); } else { ------------------------ Some other notes for dependency error messages (these can be found in pacman/add.c remove.c ...): See upgrade052.py: User says: pacman -U pkg2 pacman says: pkg1 requires imaginary Hm. This is not very informative to the user. "After the transaction the following dependencies would become broken: ... " (just smaller and not Hunglish ;-) would be better imho. Bye, ngaba PS: sync895.py: As I see, the glitch is in sync.c; as I debugged it doesn't correctly "copies" the package to the removal list (probably forgets about provides for example). The modified deps.c works correctly if I want to remove the replaced package by hand. But this fix is up to you ;-P
Hi! 1. As I see, can_remove_package should be replaced with direct call of _alpm_checkdeps with TRANS_TYPE_REMOVE + check the reason flag by hand, because the dependency check is not really sophisticated in this function /and reimplemented/ . 2. _alpm_checkdeps doesn't really need trans param (neither in my patched version nor the original imho <- its usage in cvs version is messy for me) 3. Some words about speed again: We must make the "usual" decision again: a.) We prefer speed: we don't care about the fact multiple provisions can exist on the system and we don't let pacman remove the provision even if other package provides it too (current state). b.) We prefer "perfection": this is notably slower of course (if only if the current version reports: package cannot be removed) 4. I repeat myself: all dependency check stuff from sync.c should be moved (and rewritten) to deps.c. 5. That is very common in libalpm, that we search for a depend-satisfyer, in most cases we forget check version number or provides etc., so I think a "search_for_satisfyer" function would vanish these bugs and reduce redundancies in the source (it's just a for "pkg in pkgcache" alpm_depcmp(pkg,depend)). To show this, here is a sync300.py file (see _alpm_resolvedeps for the "negligent" search_for_satisfyer): ---------- Bye, ngaba ---------- self.description = "Corrupt database (broken deps)" sp1 = pmpkg("pkg1") sp1.depends = ["pkg2=1.0-2"] self.addpkg2db("sync", sp1) sp2 = pmpkg("pkg2", "1.0-1") self.addpkg2db("sync", sp2) self.args = "-S %s" % sp1.name self.addrule("PACMAN_RETCODE=1") self.addrule("!PKG_EXIST=pkg1") self.addrule("!PKG_EXIST=pkg2")
sp1 = pmpkg("pkg1") sp1.depends = ["pkg2=1.0-2"] self.addpkg2db("sync", sp1)
sp2 = pmpkg("pkg2", "1.0-1") self.addpkg2db("sync", sp2)
self.args = "-S %s" % sp1.name
self.addrule("PACMAN_RETCODE=1") self.addrule("!PKG_EXIST=pkg1") self.addrule("!PKG_EXIST=pkg2") sp1.depends = ["pkg2=1.0-2"] <- this fails too, see my previous mail: http://archlinux.org/pipermail/pacman-dev/2007-April/008037.html
Bye, ngaba
sp1.depends = ["pkg2=1.0-2"]. Oh, I wanted to say: self.addrule("!PKG_REQUIREDBY=pkg2|pkg1")
2007/4/24, Nagy Gabor <ngaba@petra.hos.u-szeged.hu>:
Hi! 1. As I see, can_remove_package should be replaced with direct call of _alpm_checkdeps with TRANS_TYPE_REMOVE + check the reason flag by hand, because the dependency check is not really sophisticated in this function /and reimplemented/ . 2. _alpm_checkdeps doesn't really need trans param (neither in my patched version nor the original imho <- its usage in cvs version is messy for me) 3. Some words about speed again: We must make the "usual" decision again: a.) We prefer speed: we don't care about the fact multiple provisions can exist on the system and we don't let pacman remove the provision even if other package provides it too (current state). b.) We prefer "perfection": this is notably slower of course (if only if the current version reports: package cannot be removed) 4. I repeat myself: all dependency check stuff from sync.c should be moved (and rewritten) to deps.c. 5. That is very common in libalpm, that we search for a depend-satisfyer, in most cases we forget check version number or provides etc., so I think a "search_for_satisfyer" function would vanish these bugs and reduce redundancies in the source (it's just a for "pkg in pkgcache" alpm_depcmp(pkg,depend)). To show this, here is a sync300.py file (see _alpm_resolvedeps for the "negligent" search_for_satisfyer): ---------- Bye, ngaba ---------- self.description = "Corrupt database (broken deps)"
sp1 = pmpkg("pkg1") sp1.depends = ["pkg2=1.0-2"] self.addpkg2db("sync", sp1)
sp2 = pmpkg("pkg2", "1.0-1") self.addpkg2db("sync", sp2)
self.args = "-S %s" % sp1.name
self.addrule("PACMAN_RETCODE=1") self.addrule("!PKG_EXIST=pkg1") self.addrule("!PKG_EXIST=pkg2")
I would love to see patches for all of these points too :) (at least, if you see a clean/correct way to do it) Maybe at least for 5. , since you provided a testcase, and that you say the code could be cleaned up while fixing it. Thanks
2. _alpm_checkdeps doesn't really need trans param (neither in my patched version nor the original imho <- its usage in cvs version is messy for me) I would love to see patches for all of these points too :) (at least, if you see a clean/correct way to do it) Maybe at least for 5. , since you provided a testcase, and that you say the code could be cleaned up while fixing it. Hi! Here is a simple patch for 2. alpm_splitdep (submitted today) patch fixes 5. Bye, ngaba
---------------------------------------------------------------- This message was sent using IMP, the Internet Messaging Program.
On 6/18/07, ngaba@petra.hos.u-szeged.hu <ngaba@petra.hos.u-szeged.hu> wrote:
2. _alpm_checkdeps doesn't really need trans param (neither in my patched version nor the original imho <- its usage in cvs version is messy for me) I would love to see patches for all of these points too :) (at least, if you see a clean/correct way to do it) Maybe at least for 5. , since you provided a testcase, and that you say the code could be cleaned up while fixing it. Hi! Here is a simple patch for 2. alpm_splitdep (submitted today) patch fixes 5. Bye, ngaba
Also in my working branch, thanks. -Dan
I indeed had a problem with sync895.py as well. Look at my commit here : http://chantry.homelinux.org/~xav/git/gitweb.cgi?p=pacman.git;a=commit;h=08d... I made this change : - pmpkg_t *q = _alpm_pkg_new(miss->depend.name, NULL); + pmpkg_t *q = _alpm_pkg_dup(local); Does this look fine? (it works correctly at least..) I agree about depmiss used in an usual way, I hopefully fixed it in this commit : http://chantry.homelinux.org/~xav/git/gitweb.cgi?p=pacman.git;a=commit;h=351...
I made this change : - pmpkg_t *q = _alpm_pkg_new(miss->depend.name, NULL); + pmpkg_t *q = _alpm_pkg_dup(local);
Does this look fine? (it works correctly at least..)
Yes, at least this is much better than the old one :-P
Na Sat, Apr 21, 2007 at 11:57:13PM +0200, Nagy Gabor <ngaba@petra.hos.u-szeged.hu> pisal(a):
Notes: sync895.py reports false error imho, we should implement --debug to pactest. (sync.c lines 680- is very messy... )
it is implemented. read the fine source VMiklos -- developer of Frugalware Linux - http://frugalware.org
Hi! I think at least the test files should be added mentioned in these patches... In my opinion this is a serious bug: pacman fails in case of multiple package update when an old satisfied depend "disappear" as shown in my .py file. You may think, that is rare, but I show here an other example as a motivation ;-): ==upgrade061.py== self.description = "Try to upgrade 2 packages which would break deps" lp1 = pmpkg("pkg1", "1.0-1") lp1.depends = ["imaginary"] self.addpkg2db("local", lp1) lp2 = pmpkg("pkg2", "1.0-1") lp2.requiredby = [ "pkg1" ] lp2.provides = ["imaginary"] self.addpkg2db("local", lp2) p1 = pmpkg("pkg1", "1.0-2") p1.depends = ["imaginary"] self.addpkg(p1) p2 = pmpkg("pkg2", "1.0-2") self.addpkg(p2) self.args = "-U %s" % " ".join([p.filename() for p in p1, p2]) self.addrule("PACMAN_RETCODE=1") self.addrule("PKG_VERSION=pkg1|1.0-1") self.addrule("PKG_VERSION=pkg2|1.0-1")
Oh, this is much more plausible ;-): -------- diff -Naur pacman-lib/lib/libalpm/deps.c pacman-lib.new/lib/libalpm/deps.c --- pacman-lib/lib/libalpm/deps.c 2007-03-19 05:23:45.000000000 +0100 +++ pacman-lib.new/lib/libalpm/deps.c 2007-04-21 20:02:07.000000000 +0200 @@ -271,12 +271,8 @@ for(l = _alpm_db_get_pkgcache(db); l; l = l->next) { pmpkg_t *pkg = l->data; - if(strcmp(alpm_pkg_get_name(pkg), alpm_pkg_get_name(oldpkg)) == 0) { - /* well, we know this one succeeds, but we're removing it... skip it */ - continue; - } - - if(alpm_depcmp(pkg, depend)) { + if(alpm_depcmp(pkg, depend) && !_alpm_pkg_find(alpm_pkg_get_name(pkg), packages)) { + /* we ignore packages that will be updated because we know that updated ones don't satisfy depend.*/ _alpm_log(PM_LOG_DEBUG, _("checkdeps: dependency '%s' satisfied by installed package '%s'"), depend->name, alpm_pkg_get_name(pkg)); satisfied = 1; --------------------- Enjoy, ngaba
Looks fine to me, I applied it in my local tree here : http://chantry.homelinux.org/~xav/git/gitweb.cgi?p=pacman.git;a=commit;h=7b9...
Hi! I implemented a standard topological sort algorithm. I'm not satisfied yet, because I must compute the edges in O(n^2) time (checkdeps also does this (and more), so its result should be used somehow). If my algorithm is OK, then extra contains a dep circle ;-) This is not radically faster anyway, but at least (hopefully) produces good result. Here is the patch: ------------------- diff -Naur pacman-lib/lib/libalpm/deps.c pacman-lib.new/lib/libalpm/deps.c --- pacman-lib/lib/libalpm/deps.c 2007-04-22 14:54:33.000000000 +0200 +++ pacman-lib.new/lib/libalpm/deps.c 2007-04-22 18:34:36.000000000 +0200 @@ -30,7 +30,6 @@ #include <strings.h> #endif #include <libintl.h> -#include <math.h> /* libalpm */ #include "deps.h" @@ -47,6 +46,38 @@ extern pmhandle_t *handle; +pmgraph_t *_alpm_graph_new(void) +{ + pmgraph_t *graph = NULL; + + graph = (pmgraph_t *)malloc(sizeof(pmgraph_t)); + if(graph) { + graph->state = 0; + graph->data = NULL; + graph->father = NULL; + graph->sons = NULL; + graph->sonptr = NULL; + } + return(graph); +} + +void _alpm_graph_free(pmgraph_t *graph) +{ + FREELISTPTR(graph->sons); + free(graph); +} + +pmgraph_t *_alpm_graph_finduntouched(alpm_list_t *vertices) +{ + alpm_list_t *i; + pmgraph_t *found = NULL; + for (i = vertices; i && !found; i = i->next) { + pmgraph_t *vertex = i->data; + if (vertex->state == 0) found = vertex; + } + return(found); +} + pmdepmissing_t *_alpm_depmiss_new(const char *target, pmdeptype_t type, pmdepmod_t depmod, const char *depname, const char *depversion) @@ -109,11 +140,9 @@ alpm_list_t *_alpm_sortbydeps(alpm_list_t *targets, pmtranstype_t mode) { alpm_list_t *newtargs = NULL; - alpm_list_t *i, *j, *k, *l; - int change = 1; - int numscans = 0; - int numtargs = 0; - int maxscans; + alpm_list_t *i, *j, *k; + alpm_list_t *vertices = NULL; + pmgraph_t *vertex; ALPM_LOG_FUNC; @@ -121,65 +150,64 @@ return(NULL); } - for(i = targets; i; i = i->next) { - newtargs = alpm_list_add(newtargs, i->data); - numtargs++; - } - - maxscans = (int)sqrt(numtargs); - _alpm_log(PM_LOG_DEBUG, _("started sorting dependencies")); - while(change) { - alpm_list_t *tmptargs = NULL; - change = 0; - if(numscans > maxscans) { - _alpm_log(PM_LOG_DEBUG, _("possible dependency cycle detected")); - continue; - } - numscans++; - /* run thru targets, moving up packages as necessary */ - for(i = newtargs; i; i = i->next) { - pmpkg_t *p = i->data; - _alpm_log(PM_LOG_DEBUG, " sorting %s", alpm_pkg_get_name(p)); - for(j = alpm_pkg_get_depends(p); j; j = j->next) { - pmdepend_t *depend = alpm_splitdep(j->data); - pmpkg_t *q = NULL; - if(depend == NULL) { - continue; - } - /* look for depend->name -- if it's farther down in the list, then - * move it up above p - */ - for(k = i->next; k; k = k->next) { - q = k->data; - const char *qname = alpm_pkg_get_name(q); - if(!strcmp(depend->name, qname)) { - if(!_alpm_pkg_find(qname, tmptargs)) { - change = 1; - tmptargs = alpm_list_add(tmptargs, q); - } - break; - } - for(l = alpm_pkg_get_provides(q); l; l = l->next) { - const char *provname = l->data; - if(!strcmp(depend->name, provname)) { - if(!_alpm_pkg_find(provname, tmptargs)) { - change = 1; - tmptargs = alpm_list_add(tmptargs, q); - } - break; - } - } - } + + /* We create the vertices */ + for(i = targets; i; i = i->next) { + pmgraph_t *vertex = _alpm_graph_new(); + vertex->data = (void *)i->data; + vertices = alpm_list_add(vertices, vertex); + } + + /* We compute the edges */ + for(i = vertices; i; i = i->next) { + pmgraph_t *vertex_i = i->data; + pmpkg_t *p_i = vertex_i->data; + //TODO: this should be somehow combined with _alpm_checkdeps + for(j = vertices; j; j = j->next) { + pmgraph_t *vertex_j = j->data; + pmpkg_t *p_j = vertex_j->data; + int son=0; + for(k = alpm_pkg_get_depends(p_i); k && !son; k = k->next) { + pmdepend_t *depend = alpm_splitdep(k->data); + son = alpm_depcmp(p_j, depend); free(depend); } - if(!_alpm_pkg_find(alpm_pkg_get_name(p), tmptargs)) { - tmptargs = alpm_list_add(tmptargs, p); + if(son) vertex_i->sons=alpm_list_add(vertex_i->sons, vertex_j); + } + vertex_i->sonptr=vertex_i->sons; + } + + vertex = vertices->data; + while(vertex) { + vertex->state = -1; // we touched this vertex + int found = 0; + while(vertex->sonptr && !found) { + pmgraph_t *nextson = (vertex->sonptr)->data; + vertex->sonptr = (vertex->sonptr)->next; + if (nextson->state == 0) { + found = 1; + nextson->father = vertex; + vertex = nextson; } + else if(nextson->state == -1) { + _alpm_log(PM_LOG_WARNING, _("dependency cycle detected\n")); + /* We add remaining targets */ + for(i = targets; i; i = i->next) + if(!_alpm_pkg_find(alpm_pkg_get_name(i->data), newtargs)) + newtargs = alpm_list_add(newtargs, i->data); + goto cleanup; + } + } + if(!found) { + newtargs = alpm_list_add(newtargs, vertex->data); + vertex->state = 1; //we leave this vertex + vertex = vertex->father; + if(!vertex) vertex=_alpm_graph_finduntouched(vertices); } - FREELISTPTR(newtargs); - newtargs = tmptargs; } +cleanup: + _alpm_log(PM_LOG_DEBUG, _("sorting dependencies finished")); if(mode == PM_TRANS_TYPE_REMOVE) { @@ -190,6 +218,8 @@ newtargs = tmptargs; } + _FREELIST(vertices, _alpm_graph_free); + return(newtargs); } @@ -234,12 +264,12 @@ for(j = alpm_pkg_get_requiredby(oldpkg); j; j = j->next) { pmpkg_t *p; found = 0; - if((p = _alpm_db_get_pkgfromcache(db, j->data)) == NULL) { - /* hmmm... package isn't installed.. */ + if(_alpm_pkg_find(j->data, packages)) { + /* this package also in the upgrade list, so don't worry about it */ continue; } - if(_alpm_pkg_find(alpm_pkg_get_name(p), packages)) { - /* this package also in the upgrade list, so don't worry about it */ + if((p = _alpm_db_get_pkgfromcache(db, j->data)) == NULL) { + /* hmmm... package isn't installed.. */ continue; } for(k = alpm_pkg_get_depends(p); k; k = k->next) { @@ -271,12 +301,8 @@ for(l = _alpm_db_get_pkgcache(db); l; l = l->next) { pmpkg_t *pkg = l->data; - if(strcmp(alpm_pkg_get_name(pkg), alpm_pkg_get_name(oldpkg)) == 0) { - /* well, we know this one succeeds, but we're removing it... skip it */ - continue; - } - - if(alpm_depcmp(pkg, depend)) { + if(alpm_depcmp(pkg, depend) && !_alpm_pkg_find(alpm_pkg_get_name(pkg), packages)) { + /* we ignore packages that will be updated because we know that updated ones don't satisfy depend.*/ _alpm_log(PM_LOG_DEBUG, _("checkdeps: dependency '%s' satisfied by installed package '%s'"), depend->name, alpm_pkg_get_name(pkg)); satisfied = 1; @@ -319,36 +345,12 @@ } found = 0; - /* check database for literal packages */ + /* check database for satisfying packages */ for(k = _alpm_db_get_pkgcache(db); k && !found; k = k->next) { pmpkg_t *p = (pmpkg_t *)k->data; found = alpm_depcmp(p, depend); } - /* check database for provides matches */ - if(!found) { - alpm_list_t *m; - for(m = _alpm_db_whatprovides(db, depend->name); m && !found; m = m->next) { - /* look for a match that isn't one of the packages we're trying - * to install. this way, if we match against a to-be-installed - * package, we'll defer to the NEW one, not the one already - * installed. */ - pmpkg_t *p = m->data; - alpm_list_t *n; - int skip = 0; - for(n = packages; n && !skip; n = n->next) { - pmpkg_t *ptp = n->data; - if(strcmp(alpm_pkg_get_name(ptp), alpm_pkg_get_name(p)) == 0) { - skip = 1; - } - } - if(skip) { - continue; - } - found = alpm_depcmp(p, depend); - } - FREELISTPTR(k); - } /* check other targets */ for(k = packages; k && !found; k = k->next) { pmpkg_t *p = k->data; @@ -370,52 +372,55 @@ } } } else if(op == PM_TRANS_TYPE_REMOVE) { - /* check requiredby fields */ for(i = packages; i; i = i->next) { - pmpkg_t *tp = i->data; - if(tp == NULL) { + pmpkg_t *rempkg = i->data; + + if(rempkg == NULL) { + _alpm_log(PM_LOG_DEBUG, _("null package found in package list")); continue; } - found=0; - for(j = alpm_pkg_get_requiredby(tp); j; j = j->next) { - /* Search for 'reqname' in packages for removal */ - char *reqname = j->data; - alpm_list_t *x = NULL; - for(x = packages; x; x = x->next) { - pmpkg_t *xp = x->data; - if(strcmp(reqname, alpm_pkg_get_name(xp)) == 0) { - found = 1; - break; - } + for(j = alpm_pkg_get_requiredby(rempkg); j; j = j->next) { + pmpkg_t *p; + found = 0; + if(_alpm_pkg_find(j->data, packages)) { + /* this package also in the remove list, so don't worry about it */ + continue; } - if(!found) { - /* check if a package in trans->packages provides this package */ - for(k = trans->packages; !found && k; k=k->next) { - pmpkg_t *spkg = NULL; - if(trans->type == PM_TRANS_TYPE_SYNC) { - pmsyncpkg_t *sync = k->data; - spkg = sync->pkg; - } else { - spkg = k->data; - } - if(spkg) { - if(alpm_list_find_str(alpm_pkg_get_provides(spkg), tp->name)) { - found = 1; + if((p = _alpm_db_get_pkgfromcache(db, j->data)) == NULL) { + /* hmmm... package isn't installed.. */ + continue; + } + for(k = alpm_pkg_get_depends(p); k; k = k->next) { + pmdepend_t *depend = alpm_splitdep(k->data); + if(depend == NULL) { + continue; + } + /* if rempkg satisfied this dep, we try to find an other satisfyer (which won't be removed)*/ + if(alpm_depcmp(rempkg, depend)) { + int satisfied = 0; + for(l = _alpm_db_get_pkgcache(db); l; l = l->next) { + pmpkg_t *pkg = l->data; + if(alpm_depcmp(pkg, depend) && !_alpm_pkg_find(alpm_pkg_get_name(pkg), packages)) { + _alpm_log(PM_LOG_DEBUG, _("checkdeps: dependency '%s' satisfied by installed package '%s'"), + depend->name, alpm_pkg_get_name(pkg)); + satisfied = 1; + break; } } - } - if(!found) { - _alpm_log(PM_LOG_DEBUG, _("checkdeps: found %s as required by %s"), - reqname, alpm_pkg_get_name(tp)); - miss = _alpm_depmiss_new(alpm_pkg_get_name(tp), PM_DEP_TYPE_DEPEND, - PM_DEP_MOD_ANY, j->data, NULL); - if(!_alpm_depmiss_isin(miss, baddeps)) { - baddeps = alpm_list_add(baddeps, miss); - } else { - FREE(miss); + if(!satisfied) { + _alpm_log(PM_LOG_DEBUG, _("checkdeps: found %s as required by %s"), + alpm_pkg_get_name(rempkg), alpm_pkg_get_name(p)); + miss = _alpm_depmiss_new(p->name, PM_DEP_TYPE_DEPEND, depend->mod, + depend->name, depend->version); + if(!_alpm_depmiss_isin(miss, baddeps)) { + baddeps = alpm_list_add(baddeps, miss); + } else { + FREE(miss); + } } } + free(depend); } } } diff -Naur pacman-lib/lib/libalpm/deps.h pacman-lib.new/lib/libalpm/deps.h --- pacman-lib/lib/libalpm/deps.h 2007-04-22 14:54:33.000000000 +0200 +++ pacman-lib.new/lib/libalpm/deps.h 2007-04-22 16:59:20.000000000 +0200 @@ -42,6 +42,19 @@ pmdepend_t depend; }; +/* Graphs */ +struct __pmgraph_t { + int state; //0: untouched, -1: entered, other: leaving time + void *data; + struct __pmgraph_t *father; //where did we come from? + alpm_list_t *sons; + alpm_list_t *sonptr; // points to a son in the sons list +}; + +pmgraph_t *_alpm_graph_new(void); +void _alpm_graph_free(pmgraph_t *graph); +pmgraph_t *_alpm_graph_finduntouched(alpm_list_t *vertices); + pmdepmissing_t *_alpm_depmiss_new(const char *target, pmdeptype_t type, pmdepmod_t depmod, const char *depname, const char *depversion); @@ -53,7 +66,6 @@ int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *syncpkg, alpm_list_t *list, alpm_list_t *trail, pmtrans_t *trans, alpm_list_t **data); - #endif /* _ALPM_DEPS_H */ /* vim: set ts=2 sw=2 noet: */ diff -Naur pacman-lib/lib/libalpm/Makefile.am pacman-lib.new/lib/libalpm/Makefile.am --- pacman-lib/lib/libalpm/Makefile.am 2007-04-22 14:54:33.000000000 +0200 +++ pacman-lib.new/lib/libalpm/Makefile.am 2007-04-22 18:06:04.000000000 +0200 @@ -39,7 +39,7 @@ versioncmp.h versioncmp.c libalpm_la_LDFLAGS = -no-undefined -version-info $(LIB_VERSION_INFO) -libalpm_la_LIBADD = -larchive -ldownload -lm +libalpm_la_LIBADD = -larchive -ldownload if HAS_DOXYGEN all: doxygen.in diff -Naur pacman-lib/src/pacman/Makefile.am pacman-lib.new/src/pacman/Makefile.am --- pacman-lib/src/pacman/Makefile.am 2007-04-22 14:54:33.000000000 +0200 +++ pacman-lib.new/src/pacman/Makefile.am 2007-04-22 18:11:40.000000000 +0200 @@ -28,7 +28,7 @@ util.h util.c pacman_LDADD = $(top_builddir)/lib/libalpm/.libs/libalpm.la \ - -ldownload + -ldownload -lm pacman_static_SOURCES = $(pacman_SOURCES) pacman_static_LDFLAGS = $(LDFLAGS) -all-static ----------------------- bye, ngaba
Hi! First, hopefully this is my last for this weekend ;-) And subject was a little cheating because the O(n^2) prepare part still exists, but the core part now O(n+e) where e is the number of depends which is usually O(n), so the core algorithm is about O(n) now. Here is the little patch: ------------------------- diff -Naur pacman-lib/lib/libalpm/deps.c pacman-lib.new/lib/libalpm/deps.c --- pacman-lib/lib/libalpm/deps.c 2007-04-22 19:46:40.000000000 +0200 +++ pacman-lib.new/lib/libalpm/deps.c 2007-04-22 19:46:09.000000000 +0200 @@ -69,9 +69,10 @@ pmgraph_t *_alpm_graph_finduntouched(alpm_list_t *vertices) { - alpm_list_t *i; + static alpm_list_t *i = NULL; + if (!i) i = vertices; pmgraph_t *found = NULL; - for (i = vertices; i && !found; i = i->next) { + for (; i && !found; i = i->next) { pmgraph_t *vertex = i->data; if (vertex->state == 0) found = vertex; } -------------------------- Enjoy, ngaba
Hi! Sorry for flooding the channel; I hope you can use something from my patches. This is not a cosmetic patch only, because static variable is not good if sortbydeps called multiple times, so I removed that ugly part from IV/A. And this version doesn't stop sorting in case of dep circle just notices us (it continues sorting to get the "best" possible result (but stops always)). You can ignore IV/A, but apply IV first. ---patch-IV/B----------- diff -Naur pacman-lib/lib/libalpm/deps.c pacman-lib.new/lib/libalpm/deps.c --- pacman-lib/lib/libalpm/deps.c 2007-04-22 19:46:40.000000000 +0200 +++ pacman-lib.new/lib/libalpm/deps.c 2007-04-22 21:29:05.000000000 +0200 @@ -67,17 +67,6 @@ free(graph); } -pmgraph_t *_alpm_graph_finduntouched(alpm_list_t *vertices) -{ - alpm_list_t *i; - pmgraph_t *found = NULL; - for (i = vertices; i && !found; i = i->next) { - pmgraph_t *vertex = i->data; - if (vertex->state == 0) found = vertex; - } - return(found); -} - pmdepmissing_t *_alpm_depmiss_new(const char *target, pmdeptype_t type, pmdepmod_t depmod, const char *depname, const char *depversion) @@ -142,6 +131,7 @@ alpm_list_t *newtargs = NULL; alpm_list_t *i, *j, *k; alpm_list_t *vertices = NULL; + alpm_list_t *vptr; pmgraph_t *vertex; ALPM_LOG_FUNC; @@ -158,6 +148,7 @@ vertex->data = (void *)i->data; vertices = alpm_list_add(vertices, vertex); } + vptr = vertices; /* We compute the edges */ for(i = vertices; i; i = i->next) { @@ -179,7 +170,7 @@ } vertex = vertices->data; - while(vertex) { + while(vptr) { vertex->state = -1; // we touched this vertex int found = 0; while(vertex->sonptr && !found) { @@ -192,21 +183,21 @@ } else if(nextson->state == -1) { _alpm_log(PM_LOG_WARNING, _("dependency cycle detected\n")); - /* We add remaining targets */ - for(i = targets; i; i = i->next) - if(!_alpm_pkg_find(alpm_pkg_get_name(i->data), newtargs)) - newtargs = alpm_list_add(newtargs, i->data); - goto cleanup; } } if(!found) { newtargs = alpm_list_add(newtargs, vertex->data); vertex->state = 1; //we leave this vertex vertex = vertex->father; - if(!vertex) vertex=_alpm_graph_finduntouched(vertices); + if(!vertex) { + while(vptr) { + vertex = vptr->data; + vptr = vptr->next; + if (vertex->state == 0) break; + } + } } } -cleanup: _alpm_log(PM_LOG_DEBUG, _("sorting dependencies finished")); diff -Naur pacman-lib/lib/libalpm/deps.h pacman-lib.new/lib/libalpm/deps.h --- pacman-lib/lib/libalpm/deps.h 2007-04-22 19:21:24.000000000 +0200 +++ pacman-lib.new/lib/libalpm/deps.h 2007-04-22 21:27:15.000000000 +0200 @@ -53,7 +53,6 @@ pmgraph_t *_alpm_graph_new(void); void _alpm_graph_free(pmgraph_t *graph); -pmgraph_t *_alpm_graph_finduntouched(alpm_list_t *vertices); pmdepmissing_t *_alpm_depmiss_new(const char *target, pmdeptype_t type, pmdepmod_t depmod, const char *depname, -------------------------- bye, ngaba
Hi! This should be OK, now:-( Plz ignore IV/A, IV/B, apply IV first. ----patch IV/C----- 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-22 15:28:16.000000000 +0200 @@ -58,6 +58,7 @@ typedef struct __pmdepend_t pmdepend_t; typedef struct __pmdepmissing_t pmdepmissing_t; typedef struct __pmconflict_t pmconflict_t; +typedef struct __pmgraph_t pmgraph_t; /* * Library diff -Naur pacman-lib/lib/libalpm/deps.c pacman-lib.new/lib/libalpm/deps.c --- pacman-lib/lib/libalpm/deps.c 2007-04-22 22:09:23.000000000 +0200 +++ pacman-lib.new/lib/libalpm/deps.c 2007-04-22 22:06:11.000000000 +0200 @@ -67,17 +67,6 @@ free(graph); } -pmgraph_t *_alpm_graph_finduntouched(alpm_list_t *vertices) -{ - alpm_list_t *i; - pmgraph_t *found = NULL; - for (i = vertices; i && !found; i = i->next) { - pmgraph_t *vertex = i->data; - if (vertex->state == 0) found = vertex; - } - return(found); -} - pmdepmissing_t *_alpm_depmiss_new(const char *target, pmdeptype_t type, pmdepmod_t depmod, const char *depname, const char *depversion) @@ -142,6 +131,7 @@ alpm_list_t *newtargs = NULL; alpm_list_t *i, *j, *k; alpm_list_t *vertices = NULL; + alpm_list_t *vptr; pmgraph_t *vertex; ALPM_LOG_FUNC; @@ -178,8 +168,10 @@ vertex_i->sonptr=vertex_i->sons; } + vptr = vertices; vertex = vertices->data; - while(vertex) { + + while(vptr) { vertex->state = -1; // we touched this vertex int found = 0; while(vertex->sonptr && !found) { @@ -191,22 +183,21 @@ vertex = nextson; } else if(nextson->state == -1) { - _alpm_log(PM_LOG_WARNING, _("dependency cycle detected")); - /* We add remaining targets */ - for(i = targets; i; i = i->next) - if(!_alpm_pkg_find(alpm_pkg_get_name(i->data), newtargs)) - newtargs = alpm_list_add(newtargs, i->data); - goto cleanup; + _alpm_log(PM_LOG_WARNING, _("dependency cycle detected\n")); } } if(!found) { newtargs = alpm_list_add(newtargs, vertex->data); vertex->state = 1; //we leave this vertex vertex = vertex->father; - if(!vertex) vertex=_alpm_graph_finduntouched(vertices); + if(!vertex) { + while(vptr = vptr->next) { + vertex = vptr->data; + if (vertex->state == 0) break; + } + } } } -cleanup: _alpm_log(PM_LOG_DEBUG, _("sorting dependencies finished")); diff -Naur pacman-lib/lib/libalpm/deps.h pacman-lib.new/lib/libalpm/deps.h --- pacman-lib/lib/libalpm/deps.h 2007-04-22 22:09:23.000000000 +0200 +++ pacman-lib.new/lib/libalpm/deps.h 2007-04-22 21:27:15.000000000 +0200 @@ -53,7 +53,6 @@ pmgraph_t *_alpm_graph_new(void); void _alpm_graph_free(pmgraph_t *graph); -pmgraph_t *_alpm_graph_finduntouched(alpm_list_t *vertices); pmdepmissing_t *_alpm_depmiss_new(const char *target, pmdeptype_t type, pmdepmod_t depmod, const char *depname, ----------------------------------- bye, ngaba
Na Sun, Apr 22, 2007 at 10:17:58PM +0200, Nagy Gabor <ngaba@petra.hos.u-szeged.hu> pisal(a):
Hi! This should be OK, now:-( Plz ignore IV/A, IV/B, apply IV first.
lol :) so, first if you send a series of patches, then use a subject like [patch 1/2] then [patch 2/2], no IV and C and whatever else. this is not a mathematician course ;)
pmdepmod_t depmod, const char *depname, ----------------------------------- bye, ngaba
second, please avoid such footers, it makes your mail unapplyable as a patch. really annoying thanks, VMiklos -- developer of Frugalware Linux - http://frugalware.org
On 4/22/07, VMiklos <vmiklos@frugalware.org> wrote:
Na Sun, Apr 22, 2007 at 10:17:58PM +0200, Nagy Gabor <ngaba@petra.hos.u-szeged.hu> pisal(a):
Hi! This should be OK, now:-( Plz ignore IV/A, IV/B, apply IV first.
lol :)
so, first if you send a series of patches, then use a subject like [patch 1/2] then [patch 2/2], no IV and C and whatever else. this is not a mathematician course ;)
I agree with VMiklos here. Could you please send the entire thing as one patch - there's little sense in having to apply broken patches before I apply the final working patch.
Hi! As a proof of concept, run (with and without the patch) smoke001.py, and see the pactest.log. This example shows that the old version is really broken (and calculates 1000^(1/2) ;-). In case of "-A/U foo1 foo2 foo3..." where foo1->foo2->... (which is quite common imho) the old sortbydeps fails. Bye, ngaba PS: You can do a time test too.
Na Sun, Apr 22, 2007 at 07:51:18PM +0200, Nagy Gabor <ngaba@petra.hos.u-szeged.hu> pisal(a):
+ static alpm_list_t *i = NULL;
static variables in a library? VMiklos -- developer of Frugalware Linux - http://frugalware.org
Na Sat, Apr 21, 2007 at 05:07:55PM +0200, Nagy Gabor <ngaba@petra.hos.u-szeged.hu> pisal(a):
Here is my patch (WARNING: with large targetlist this may cause notable slowdown).
are you sure anyone is interested in your "-slowdown" patchset? VMiklos -- developer of Frugalware Linux - http://frugalware.org
> are you sure anyone is interested in your "-slowdown" patchset? Hi! 1. See patchI; speed improved-version ;-) 2. This is a bugfix. 3. You needn't apply it [but it's hard to find much faster solution imho (design limitation)], but at least the pactest files may be helpful. 4. If you don't appreciate my waste of time when I try to improve pacman, please simply ignore me instead of laugh at/brief me, thx. [This was an answer to all your e-mail today, however I will take your advices] Bye, ngaba
Hi! A few words would be nice ;-): -rejected, but working on the bugs -rejected, ignoring bugs -reviewing patches ... Bye, ngaba PS: I think at least the *.py files were useful...
On 5/8/07, Nagy Gabor <ngaba@petra.hos.u-szeged.hu> wrote:
Hi!
A few words would be nice ;-): -rejected, but working on the bugs -rejected, ignoring bugs -reviewing patches
Hi Nagy, Without trying to sound rude, here is why I haven't taken a closer look, although you definitely seem to make valid points: 1. Email titles like "IV/B (apply this on IV/A) blah..." are a turn-off. We all shouldn't have to follow 10 steps for what could be one single patch. The patches would be easier to review if they were a 1-step apply. 2. This is likely due to the language barrier, but I have a hard time understanding your explanations at times. I will go back and look at your emails soon and email you back for clarification on points I didn't understand. You may want to seperate some of your long descriptions into seperate paragraphs as well. 3. A lot of your emails were "this is a problem that needs to be fixed". We have a bugtracker for those type of reports, things that come to my email inbox without patches tend to get lost quickly. We do appreciate your help and investigation into the code, but realize that it doesn't help anyone if we spend time trying to figure out the problem you are addressing if you already have- we just need a better explanation. I've been pretty busy with school lately too so tinkering on my own time has been limited to my own things. Once June comes around I anticipate having quite a bit more time to work on stuff. Thanks! -Dan
On 5/8/07, Nagy Gabor <ngaba@petra.hos.u-szeged.hu> wrote:
Hi!
A few words would be nice ;-): -rejected, but working on the bugs -rejected, ignoring bugs -reviewing patches
Hi Nagy, Without trying to sound rude, here is why I haven't taken a closer look, although you definitely seem to make valid points: 1. Email titles like "IV/B (apply this on IV/A) blah..." are a turn-off. We all shouldn't have to follow 10 steps for what could be one single patch. The patches would be easier to review if they were a 1-step apply. 2. This is likely due to the language barrier, but I have a hard time understanding your explanations at times. I will go back and look at your emails soon and email you back for clarification on points I didn't understand. You may want to seperate some of your long descriptions into seperate paragraphs as well. 3. A lot of your emails were "this is a problem that needs to be fixed". We have a bugtracker for those type of reports, things that come to my email inbox without patches tend to get lost quickly. We do appreciate your help and investigation into the code, but realize that it doesn't help anyone if we spend time trying to figure out the problem you are addressing if you already have- we just need a better explanation. I've been pretty busy with school lately too so tinkering on my own time has been limited to my own things. Once June comes around I anticipate having quite a bit more time to work on stuff. Thanks! -Dan
On 5/8/07, Dan McGee <dpmcgee@gmail.com> wrote:
On 5/8/07, Nagy Gabor <ngaba@petra.hos.u-szeged.hu> wrote:
Hi!
A few words would be nice ;-): -rejected, but working on the bugs -rejected, ignoring bugs -reviewing patches
Hi Nagy,
Without trying to sound rude, here is why I haven't taken a closer look, although you definitely seem to make valid points: 1. Email titles like "IV/B (apply this on IV/A) blah..." are a turn-off. We all shouldn't have to follow 10 steps for what could be one single patch. The patches would be easier to review if they were a 1-step apply. 2. This is likely due to the language barrier, but I have a hard time understanding your explanations at times. I will go back and look at your emails soon and email you back for clarification on points I didn't understand. You may want to seperate some of your long descriptions into seperate paragraphs as well. 3. A lot of your emails were "this is a problem that needs to be fixed". We have a bugtracker for those type of reports, things that come to my email inbox without patches tend to get lost quickly.
We do appreciate your help and investigation into the code, but realize that it doesn't help anyone if we spend time trying to figure out the problem you are addressing if you already have- we just need a better explanation.
Sorry about that Nagy. I am definitely interested in your contributions, and have been attempting to review some of the patches when I get time. As Dan had mentioned before, there is a slight language barrier (I mean no offense at all) and sometimes it is difficult to understand what you're trying to fix, or why it is needed. If you could resubmit the patch that was in 3 parts (A, B, and C), that would be great too - it's generally better to resubmit a patch than to patch the patch.
participants (6)
-
Aaron Griffin
-
Dan McGee
-
Nagy Gabor
-
ngaba＠petra.hos.u-szeged.hu
-
VMiklos
-
Xavier