Re: commit-graph: change in "best" merge-base when ambiguous

From: Elijah Newren <newren@gmail.com>
Date: May 21, 2018, 18:33 UTC
Message-ID: <CABPp-BFEd+fK_i3qoYWudYS5mhWE1jsXR_xcSCZoJ=4Vd61LAQ@mail.gmail.com>
In-Reply-To: <e78a115a-a5ea-3c0a-5437-51ba0bcc56e1@gmail.com>

Hi,

On Mon, May 21, 2018 at 11:10 AM, Derrick Stolee <stolee@gmail.com> wrote:

Show 25 quoted lines

> Hello all,
>
> While working on the commit-graph feature, I made a test commit that sets
> core.commitGraph and gc.commitGraph to true by default AND runs 'git
> commit-graph write --reachable' after each 'git commit' command. This helped
> me find instances in the test suite where the commit-graph feature changes
> existing functionality. Most of these were in regards to grafts,
> replace-objects, and shallow-clones (as expected) or when trying to find a
> corrupt or hidden commit (the commit-graph hides this corrupt/missing data).
> However, there was one interesting case that I'd like to mention on-list.
>
> In t6024-recursive-merge.sh, we have the following commit structure:
>
>     # 1 - A - D - F
>     #   \   X   /
>     #     B   X
>     #       X   \
>     # 2 - C - E - G
>
> When merging F to G, there are two "best" merge-bases, A and C. With
> core.commitGraph=false, 'git merge-base F G' returns A, while it returns C
> when core.commitGraph=true. This is due to the new walk order when using
> generation numbers, although I have not dug deep into the code to point out
> exactly where the choice between A and C is made. Likely it's just whatever
> order they are inserted into a list.

Ooh, interesting.

Just a guess, but could it be related to relative ordering of committer timestamps? Ordering of committer timestamps apparently affects order of merge-bases returned to merge-recursive, and although that shouldn't have mattered, a few bugs meant that it did and the order ended up determining what contents a successful merge would have. See this recent post:

https://public-inbox.org/git/CABPp-BFc1OLYKzS5rauOehvEugPc0oGMJp-NMEAmVMW7QR=4Eg@mail.gmail.com/

The fact that the merge was successful for both orderings of merge bases was the real bug, though; it should have detected and reported a conflict both ways.

I'm not sure where else we have an accidental and incorrect dependence on merge-base tie-breaker or ordering logic, but if it's like this one, changing the tie-breaker should be okay.