git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Michael J Gruber <git@grubix.eu>
Cc: Ekelhart Jakob <jakob.ekelhart@fsw.at>,
	"git\@vger.kernel.org" <git@vger.kernel.org>,
	Jeff King <peff@peff.net>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Matthias Lischka <matthias.lischka@fsw.at>
Subject: Re: [PATCH 2/3] merge-base: return fork-point outside reflog
Date: Thu, 09 Nov 2017 11:49:45 +0900	[thread overview]
Message-ID: <xmqq375okvxy.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1a375938-a3dc-d670-dd41-de524ac0f93d@grubix.eu> (Michael J. Gruber's message of "Wed, 8 Nov 2017 10:33:41 +0100")

Michael J Gruber <git@grubix.eu> writes:

> It seems the consensus was that current functionality is as designed but
> not necessarily as expected, and another mode "--fork-base" (that does
> what I suggested as "fix") would meet these expectations. I would reuse
> the documentation of the current mode as a description of the new mode
> and add documentation for the existing mode ;)

If we are going to have two modes, it would be absolutely necessary
to explain in the documentation what they compute exactly in terms
that the end users understand, so that the users can choose which
one to use.

The current documentation is not that great because it can get away
with "we automatically by magic find the commit on top of which you
forked your branch", but when we have two different kinds of magic,
that would no longer be a useful description to help users to
choose.  We probably need to enhance the description in "Discussion
on fork-point mode" as a preliminary clean-up.  The attached is my
attempt.

I'd expect `--fork-base` to be explained in a similar way and to the
level of detail to help users pick which one of the three options
(i.e. without any option, with fork-point and with the new one) is
appropriate for their situation.  I've been thinking about what your
patch does, and I cannot come up with a reasonable way to explain
what it computes in the terms the users can get their heads around,
with history illustrations.

--- >8 ---
Subject: merge-base --fork-point doc: clarify the example and failure modes

The illustrated history used to explain the `--fork-point` mode
named three keypoint commits B3, B2 and B1 from the oldest to the
newest, which was hard to read.  Relabel them to B0, B1, B2.  Also
illustrate the history after the rebase using the `--fork-point`
facility was made.

The text already mentions use of reflog, but the description is not
clear what benefit we are trying to gain by using reflog.  Clarify
that it is to find the commits that were known to be at the tip of
the remote-tracking branch.  This in turn necessitates users to know
the ramifications of the underlying assumptions, namely, expiry of
reflog entries will make it impossible to determine which commits
were at the tip of the remote-tracking branches and we fail when in
doubt (instead of giving a random and incorrect result without even
warning).  Another limitation is that it won't be useful if you did
not fork from the tip of a remote-tracking branch but from in the
middle.

Describe them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-merge-base.txt | 64 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 56 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-merge-base.txt b/Documentation/git-merge-base.txt
index b968b64c38..a4859c8597 100644
--- a/Documentation/git-merge-base.txt
+++ b/Documentation/git-merge-base.txt
@@ -154,23 +154,71 @@ topic origin/master`, the history of remote-tracking branch
 `origin/master` may have been rewound and rebuilt, leading to a
 history of this shape:
 
-	                 o---B1
+	                 o---B2
 	                /
-	---o---o---B2--o---o---o---B (origin/master)
+	---o---o---B1--o---o---o---B (origin/master)
 	        \
-	         B3
+	         B0
 	          \
-	           Derived (topic)
+	           D0---D1---D (topic)
 
-where `origin/master` used to point at commits B3, B2, B1 and now it
+where `origin/master` used to point at commits B0, B1, B2 and now it
 points at B, and your `topic` branch was started on top of it back
-when `origin/master` was at B3. This mode uses the reflog of
-`origin/master` to find B3 as the fork point, so that the `topic`
-can be rebased on top of the updated `origin/master` by:
+when `origin/master` was at B0, and you built three commits, D0, D1,
+and D, on top of it.  Imagine that you now want to rebase the work
+you did on the topic on top of the updated origin/master.
+
+In such a case, `git merge-base origin/master topic` would return the
+parent of B0 in the above picture, but B0^..D is *not* the range of
+commits you would want to replay on top of B (it includes B0, which
+is not what you wrote; it is a commit the other side discarded when
+it moved its tip from B0 to B1).  
+
+`git merge-base --fork-point origin/master topic` is designed to
+help in such a case.  It takes not only B but also B0, B1, and B2
+(i.e. old tips of the remote-tracking branches your repository's
+reflog knows about) into account to see on which commit your topic
+branch was built and finds B0, allowing you to replay only the
+commits on your topic, excluding the commits the other side later
+discarded.
+
+Hence
 
     $ fork_point=$(git merge-base --fork-point origin/master topic)
+
+will find B0, and
+
     $ git rebase --onto origin/master $fork_point topic
 
+will replay D0, D1 and D on top of B to create a new history of this
+shape:
+
+                         o---B2
+                        /
+        ---o---o---B1--o---o---o---B (origin/master)
+                \                   \
+                 B0                  D0'--D1'--D' (topic - updated)
+                  \
+                   D0---D1---D (topic - old)
+
+A caveat is that older reflog entries in your repository may be
+expired by `git gc`.  If B0 no longer appears in the reflog of the
+remote-tracking branch `origin/master`, the `--fork-point` mode
+obviously cannot find it and fails, avoiding to give a random and
+useless result (such as the parent of B0, like the same command
+without the `--fork-point` option gives).  
+
+Also, the remote-tracking branch you use the `--fork-point` mode
+with must be the one your topic forked from its tip.  If you forked
+from an older commit than the tip, this mode would not find the fork
+point (imagine in the above sample history B0 did not exist,
+origin/master started at B1, moved to B2 and then B, and you forked
+your topic at origin/master^ when origin/master was B1; the shape of
+the history would be the same as above, without B0, and the parent
+of B1 is what `git merge-base origin/master topic` correctly finds,
+but the `--fork-point` mode will not, because it is not one of the
+commits that used to be at the tip of origin/master).
+
 
 See also
 --------

  reply	other threads:[~2017-11-09  2:49 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <c76e76a4ef11480da9995b0bec5a70e1@SFSWW2K12EX02.intern.fsw.at>
2017-09-13 15:07 ` merge-base not working as expected when base is ahead Ekelhart Jakob
2017-09-14  8:09   ` Michael J Gruber
2017-09-14 13:15     ` [PATCH 0/3] merge-base --fork-point fixes Michael J Gruber
2017-09-14 13:15       ` [PATCH 1/3] t6010: test actual test output Michael J Gruber
2017-09-14 14:34         ` Jeff King
2017-09-15 10:01           ` Michael J Gruber
2017-09-15 11:20             ` Jeff King
2017-09-14 13:15       ` [PATCH 2/3] merge-base: return fork-point outside reflog Michael J Gruber
2017-09-15  2:48         ` Junio C Hamano
2017-09-15  9:59           ` Phillip Wood
2017-09-15 10:23           ` Michael J Gruber
2017-09-15 18:24             ` Junio C Hamano
2017-09-21  6:27               ` Junio C Hamano
2017-09-21  9:39                 ` Michael J Gruber
2017-09-22  1:49                   ` Junio C Hamano
2017-09-22  8:34                     ` Michael J Gruber
2017-09-22  9:14                       ` Junio C Hamano
2017-10-03  6:05                         ` Junio C Hamano
2017-11-08  8:52                           ` Ekelhart Jakob
2017-11-08  9:33                             ` Michael J Gruber
2017-11-09  2:49                               ` Junio C Hamano [this message]
2017-09-14 13:15       ` [PATCH 3/3] merge-base: find fork-point outside partial reflog Michael J Gruber
2017-09-14 14:37         ` Jeff King
2017-09-14 13:49       ` [PATCH 0/3] merge-base --fork-point fixes Johannes Schindelin
2017-09-14 14:38       ` Jeff King

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqq375okvxy.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@grubix.eu \
    --cc=git@vger.kernel.org \
    --cc=jakob.ekelhart@fsw.at \
    --cc=johannes.schindelin@gmx.de \
    --cc=matthias.lischka@fsw.at \
    --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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).