[pacman-dev] do we need debug in alpm_depcmp?

[Dan McGee]

On Nov 19, 2007 5:11 AM, Nagy Gabor <ngaba at 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