[pacman-dev] [patch] alpm_splitdep overwrites input string
Hi! I told everything in the subject, patch attached. This also fixes sync300.py 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:
Hi! I told everything in the subject, patch attached. This also fixes sync300.py Bye, ngaba
Pushed to my working GIT tree (with a few naming changes to avoid having to change alpm.h since it is a public function). Thanks. http://code.toofishes.net/gitweb.cgi?p=pacman.git;a=commit;h=9aa885b18422661... -Dan
Nice catch, thanks a lot. But while sync300 now pass, what the code does looks still odd. It first calls resolvedeps, which calls checkdeps. This returns the missing dependency pkg2=1.1 . But for resolving it, resolvedeps pull pkg2 1.0 , without checking the version. However, checkdeps is called again after resolvedeps, which now finds the problem that pkg1 depends on pkg2=1.1 which isn't available. Before you fixed that bug in splitdep, it changed the pkg2=1.1 dependency into pkg2 the first time checkdeps was called. So it didn't find the problem the second time. As you said the first time you described the problem, resolvedeps should probably checks version too, when it tries to resolve the missing dependencies.
As you said the first time you described the problem, resolvedeps should probably checks version too, when it tries to resolve the missing dependencies. Yes, alpm_resolvedeps is still negligent, but sync.c is quite a black box for me;-) It creates some pseudo transactions/stuffs, probably one of them prevent pacman from fault ;-). I don't create a patch for resolvedeps, because in my "dream" alpm_checkdeps can add missing dependencies from sync repos if requested with much smaller overhead... (And maybe it should be able to do topo sort too, because it computes the dependency edges...) You may say, that we shouldn't create such "super-function", but we cannot store and reuse some crutial computation results now (because of current data structures...) Bye, ngaba
---------------------------------------------------------------- This message was sent using IMP, the Internet Messaging Program.
2007/6/19, ngaba@petra.hos.u-szeged.hu <ngaba@petra.hos.u-szeged.hu>:
Yes, alpm_resolvedeps is still negligent, but sync.c is quite a black box for me;-) It creates some pseudo transactions/stuffs, probably one of them prevent pacman from fault ;-). I don't create a patch for resolvedeps, because in my "dream" alpm_checkdeps can add missing dependencies from sync repos if requested with much smaller overhead... (And maybe it should be able to do topo sort too, because it computes the dependency edges...) You may say, that we shouldn't create such "super-function", but we cannot store and reuse some crutial computation results now (because of current data structures...)
Indeed, there is a lot of duplicated job here, I think we all agree on that. But having everything in one function is indeed as worse :d There is probably a way to avoid this. If the only way is using other data structure (you mean graph here?), maybe it could be discussed here. When is pacman rewritten using the kernel list implementation, and on top of that, other structures like graph more adapted to packages, and doing what Dan said there : http://www.archlinux.org/pipermail/pacman-dev/2007-June/008544.html ;-)
On 6/18/07, Xavier <shiningxc@gmail.com> wrote:
Nice catch, thanks a lot.
But while sync300 now pass, what the code does looks still odd. It first calls resolvedeps, which calls checkdeps. This returns the missing dependency pkg2=1.1 . But for resolving it, resolvedeps pull pkg2 1.0 , without checking the version.
However, checkdeps is called again after resolvedeps, which now finds the problem that pkg1 depends on pkg2=1.1 which isn't available. Before you fixed that bug in splitdep, it changed the pkg2=1.1 dependency into pkg2 the first time checkdeps was called. So it didn't find the problem the second time.
As you said the first time you described the problem, resolvedeps should probably checks version too, when it tries to resolve the missing dependencies.
I'm going to push this patch to my local tree. Do we need to do changes elsewhere that relied on this previous broken behavior? -Dan
Since afaik this change affects only the data returned by alpm_pkg_get_depends, and that this data seems to be used consistently, by calling splitdeps on it, and then depcmp, it looks like it's ok. Also, it didn't break other pactests, so that's another good indication. None of these prove anything though. I would need to understand the whole code to answer confidently, and I only understand a tiny part of it :(
participants (3)
-
Dan McGee
-
ngaba＠petra.hos.u-szeged.hu
-
Xavier