2007/6/18, Dan McGee :

On 6/18/07, ngaba@petra.hos.u-szeged.hu wrote:

...

Hi!

I attached the pactest file and my patch. I also made some modifications in alpm_removedeps' args, because the old one was a bit messy for me. And I think that my version is faster.

can_remove_package is a bit stupid (compared to alpm_checkdeps), but I leave it untouched, because speed is more important here than "perfection".

IMO, doing things right is what we want to do in 99% of the cases this speed vs. perfection issue comes up, so I'm going to disagree with you here.

It looks like checkdeps could indeed be used here. There is also the install reason which comes into play here though. This problem was mentioned here : http://bugs.archlinux.org/task/3241 Aaron didn't see any reason for checking that the packages removed by -Rs are only dependencies, but I believe there is one, as I tried to explain in my comment.

In addition, to assume anything only makes an ass out of u (you) and me. I don't want to introduce any functions that depend on a particular quirk of our list implementation, especially since this is one of the things we have considered changing.

What about just renaming alpm_list_add to alpm_list_add_last ? That way, there isn't any assumption, it's explicit :)

I feel that if we look at the bigger picture and stop thinking of add, remove, and sync as isolated actions, then we could get some HUGE refactoring and code reduction going on with cases like this. However, this will be one large undertaking.

There are often several ways to do things right, and we probably don't all see the same, so it would help if it was a bit more concrete :) You are probably the only one who can get things exactly like you want, unless maybe you explain it in details.

To Nagy : I don't even understand why this assumption is needed : * assumptions: alpm_list_add adds new member to the end of the list, so we can reuse the list pointer

As far as I can tell, this isn't needed, so please explain more in detail :)

OK, you are right. To be perfect, this is not needed, but this cause a big performance boost: If this assumption is true, you can simply leave the "ready" variable and the "while(!ready)" stuff, because every dependencies will be found in the first loop (ready is just a safety variable now...). This wonderful fact proves the speed-up IMHO. However, if we want to keep the nice feature that this function keeps topo sort, this is needed. However, the algorithm (expected to) works fine when input list is not topo sorted (I put it in the comment just as a suggestion, because topo sort is slow, and small list's topo-sort is faster). Anyway, this assumption is _used_ in alpm_resolvedeps, so I put that comment to the patch as a reminder. Bye, ngaba PS: It would be nice if someone created "-Rs" pactests to test this patch. PS2: Xavier, sry for replying to your mail address earlier ---------------------------------------------------------------- This message was sent using IMP, the Internet Messaging Program.

I don't want to introduce any functions that depend on a particular quirk of our list implementation, especially since this is one of the things we have considered changing.

Give a look at alpm_resolvedeps;-) PS: Dan, sry for reply to your mail address earlier. ---------------------------------------------------------------- This message was sent using IMP, the Internet Messaging Program.

IMO, doing things right is what we want to do in 99% of the cases this speed vs. perfection issue comes up, so I'm going to disagree with you here. I have the same opinion usually (see my earlier sortbydeps complaints .-). But now we just search for removable dependencies, if we miss some pathological cases this is not a big problem, the database won't be corrupted, just we don't remove a dependency which we could etc. Let me explain: I meant in my previous mail that we say that we cannot remove a package bacause it is needed by an other one without checking that it cannot be "replaced" by an other installed one (see alpm_checkdeps && multiple provision). But this "fault" is _very_ rare and this extra check would cause _notable_ slowdown, because can_remove_package is (/was?) called "often". Bye, ngaba

---------------------------------------------------------------- This message was sent using IMP, the Internet Messaging Program.