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

Re: [PATCH] commit-reach: fix in_merge_bases_many bug

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 2, 2020, 15:59 UTC
Message-ID
<xmqq8scobovn.fsf@gitster.c.googlers.com>
In-Reply-To
<pull.739.git.1601650736489.gitgitgadget@gmail.com>
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 7 quoted lines
> From: Derrick Stolee <dstolee@microsoft.com>
>
> Way back in f9b8908b (commit.c: use generation numbers for
> in_merge_bases(), 2018-05-01), a heuristic was used to short-circuit
> the in_merge_bases() walk. This works just fine as long as the
> caller is checking only two commits, but when there are multiple,
> there is a possibility that this heuristic is _very wrong_.
Thanks.  That bug defeats the point of _many() part of the function.
Show 14 quoted lines
> Some code moves since then has changed this method to
> repo_in_merge_bases_many() inside commit-reach.c. The heuristic
> computes the minimum generation number of the "reference" list, then
> compares this number to the generation number of the "commit".
>
> In a recent topic, a test was added that used in_merge_bases_many()
> to test if a commit was reachable from a number of commits pulled
> from a reflog. However, this highlighted the problem: if any of the
> reference commits have a smaller generation number than the given
> commit, then the walk is skipped _even if there exist some with
> higher generation number_.
>
> This heuristic is wrong! It must check the MAXIMUM generation number
> of the reference commits, not the MINIMUM.
Correct
Show 10 quoted lines
>     Fix in_merge_bases_many() with commit-graphs
>     
>     Johannes alerted me to the difficulties Srinidhi was having with 
>     in_merge_bases_many() and commit-graphs. Sorry that I hadn't seen that
>     thread and the issues therein.
>     
>     After working with Johannes to investigate what was happening, we found
>     a 2-year-old bug in the generation number checks!
>     
>     Thanks, -Stolee
Good.  Thanks.
Previous: Derrick Stolee via GitGitGadgetNext: Junio C Hamano
Message 2 of 7 in “commit-reach: fix in_merge_bases_many bug”
  1. commit-reach: fix in_merge_bases_many bugDerrick Stolee via GitGitGadget, Oct 2, 2020
  2. Junio C HamanoOct 2, 2020
  3. Junio C HamanoOct 2, 2020
  4. Derrick StoleeOct 2, 2020
  5. Junio C HamanoOct 2, 2020
  6. Derrick StoleeOct 2, 2020
  7. Srinidhi KaushikOct 2, 2020

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.