Re: [PATCH v6] Implement --first-parent for git rev-list --bisect

[Duy Nguyen <pclouds@gmail.com>]

On Tue, Aug 28, 2018 at 6:45 PM Junio C Hamano <gitster@pobox.com> wrote:
> > @@ -146,10 +147,14 @@ static void show_list(const char *debug, int counted, int nr,
>
> An unrelated tangent, but I think I just spotted a bug in the
> existing code on the line immediately before this hunk, which reads
>
>                 if (commit->util)
>                         fprintf(stderr, "%3d", weight(p));
>
> I think this was a bug introduced at bb408ac9 ("bisect.c: use
> commit-slab for commit weight instead of commit->util", 2018-05-19)
> where the internal implementation of weight() was changed not to
> touch commit->util but instead to use a separate commit-slab storage
>
> Looking at the code before that conversion, it seems that we were
> using ->util to store a pointer to an integer, so we had the ability
> to differenciate non-negative weight (i.e. weight already computed
> for the commit), negative weight (i.e. not computed yet, but will
> be), and commits to which the concept of weight is not applicable.
> When we went to the commit-slab with the change, we lost the ability
> to represent the third case.  I am offhand not sure what the best
> remedy would be.  Perhaps stuff a so-far unused value like -3 to the
> weight() and use weight(p) == -3 instead of the old !commit->util or
> something like that?

Hmm.. no? the commit-slab stores the pointer to the weight, not the
weight itself, so we still have the ability to check the third case, I
think.

> (Duy CC'ed to help checking my sanity on this point).
>
> In any case, this is an existing bug in a debug helper, and the
> focus of this patch is not about fixing that bug, so you can and
> should leave it as-is, until this patch successfully adds the
> "bisection following only the first parent" feature.

Yes. I'll post a patch soon to fix this "commit->util" leftover.
-- 
Duy