On Nov 19, 2007 5:11 AM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Yes, I saw that and I answered : http://www.archlinux.org/pipermail/pacman-dev/2007-November/010033.html
For a more realistic usage like -Qt, I couldn't measure any differences. It'll probably be easier to see a difference with smoke001, but is that really problematic?
I didn't like that const strings were modified in _alpm_provision_cmp, so I duplicated the strings. And since the code im depcmp was similar, I did the same there, to keep things consistent.
And I put a debug back because Aaron asked for it, and I also think it can be very handy sometimes, to debug quicker.
IMHO pacman debugging tends to be unusable. The debug output of smoke001.py is now ~100MB(!!); before alpm_depcmp logging it was "only" ~30MB. alpm_depcmp debugging added 1.5 million (!) lines. So even the compressed output is 5 MB for me (user cannot attach this easily with an e-mail etc. etc.). And don't forget that alpm_depcmp is used everywhere, so this is not an extreme example: compute_requiredby, depcheck etc. I simply cannot understand how this can help us. The debug.log is totally unreadable now (at least to me), you have to know the debug format and search for the interesting part (by doing this you may miss something...). Logging atomic functions is simply needless imho (the same for "checking target1 target2 conflict"-like messages); the caller function usually informs us about the result in a debug message (foo satisfies dependency bar, pulling into the target list; foo conflicts with installed package bar, bar selected for removal ...). Bye, ngaba
Well its better that you brought it up on the list first, so I do appreciate that. And if you submit a patch to do *just this*, it is much more likely to get considered than if you try to lump this change together in another patch. I do agree that debugging is less useful when the function is stateless, so that would be one reason to get rid of this debug logging in depcmp. Next, I will say that depcmp is a fairly stable function (although we did just change it)- I have faith that it will work. Finally, the debug text does carry some extra CPU cycles with the alpm_dep_get_string() call, so that would be justification to remove as well. Of course, you made it sound like such a big hassle to look through debug logs. Wouldn't something along the lines of "sed -i '/depcmp/d' <logfile>" work just fine? So in short- submit a patch for *one* feature at a time, and justfiy it in the commit message. Don't try to sneak things by. I do think this may be a valid complaint though. -Dan