All of lore.kernel.org
 help / color / mirror / Atom feed
From: Derrick Stolee <stolee@gmail.com>
To: "git@vger.kernel.org" <git@vger.kernel.org>,
	Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
	Jakub Narebski <jnareb@gmail.com>,
	Jeff Hostetler <git@jeffhostetler.com>
Subject: commit-graph: change in "best" merge-base when ambiguous
Date: Mon, 21 May 2018 14:10:54 -0400	[thread overview]
Message-ID: <e78a115a-a5ea-3c0a-5437-51ba0bcc56e1@gmail.com> (raw)

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.

In the Discussion section of the `git merge-base` docs [1], we have the 
following:

     When the history involves criss-cross merges, there can be more 
than one best common ancestor for two commits. For example, with this 
topology:

     ---1---o---A
         \ /
          X
         / \
     ---2---o---o---B

     both 1 and 2 are merge-bases of A and B. Neither one is better than 
the other (both are best merge bases). When the --all option is not 
given,     it is unspecified which best one is output.

This means our official documentation mentions that we do not have a 
concrete way to differentiate between these choices. This makes me think 
that this change in behavior is not a bug, but it _is_ a change in 
behavior. It's worth mentioning, but I don't think there is any value in 
making sure `git merge-base` returns the same output.

Does anyone disagree? Is this something we should solidify so we always 
have a "definitive" merge-base?

The biggest reason I think we should avoid sticking to the existing 
behavior is that the current behavior depends on the walk order. That 
means we would not be able to concretely define a tie-breaker without 
changing the existing behavior anyway.

Thanks,
-Stolee

[1] https://git-scm.com/docs/git-merge-base#_discussion


             reply	other threads:[~2018-05-21 18:10 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-21 18:10 Derrick Stolee [this message]
2018-05-21 18:33 ` commit-graph: change in "best" merge-base when ambiguous Elijah Newren
2018-05-21 21:50   ` Jeff King
2018-05-21 22:28     ` Stefan Beller
2018-05-21 21:54 ` Jeff King
2018-05-21 22:25   ` Jacob Keller
2018-05-22  5:39 ` Michael Haggerty
2018-05-22 12:48   ` Derrick Stolee
2018-05-24 22:08     ` Jakub Narebski
2018-05-25  6:03       ` Michael Haggerty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e78a115a-a5ea-3c0a-5437-51ba0bcc56e1@gmail.com \
    --to=stolee@gmail.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.