git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Duy Nguyen <pclouds@gmail.com>
Date
Sep 2, 2018, 07:34 UTC
Message-ID
<CACsJy8AFo+mb8R-O-JKRPZk__csq6mbVXbnZhSd-nZ08zWfSeg@mail.gmail.com>
In-Reply-To
<xmqqlg8qh22q.fsf@gitster-ct.c.googlers.com>
On Tue, Aug 28, 2018 at 6:45 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 23 quoted lines
> > @@ -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.

Show 6 quoted lines
> (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
Previous: Junio C HamanoNext: Nguyễn Thái Ngọc Duy
Message 7 of 12 in “Implement --first-parent for git rev-list --bisect”
  1. Implement --first-parent for git rev-list --bisectTiago Botelho, Aug 28, 2018
  2. Johannes SchindelinAug 28, 2018
  3. Junio C HamanoAug 28, 2018
  4. Junio C HamanoAug 28, 2018
  5. Junio C HamanoAug 28, 2018
  6. Junio C HamanoAug 28, 2018
  7. Duy NguyenSep 2, 2018
  8. bisect.c: make show_list() build againNguyễn Thái Ngọc Duy, Sep 2, 2018
  9. Christian CouderSep 2, 2018
  10. Duy NguyenSep 3, 2018
  11. Christian CouderSep 4, 2018
  12. Junio C HamanoSep 4, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.