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

[Junio C Hamano <gitster@pobox.com>]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 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.

> 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

>     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.