Re: [PATCH 2/3] merge-base: return fork-point outside reflog

From: Michael J Gruber <git@grubix.eu>
Date: Sep 22, 2017, 08:34 UTC
Message-ID: <4a14c35c-88b0-7bc3-0a6e-c69ed9ec7ee7@grubix.eu>
In-Reply-To: <xmqqzi9nlcyw.fsf@gitster.mtv.corp.google.com>

Junio C Hamano venit, vidit, dixit 22.09.2017 03:49:

Show 22 quoted lines

> Michael J Gruber <git@grubix.eu> writes:
> 
>> Also, I'm undecided about about your reflog argument above - if we leave
>> "--fork-point" to be the current behaviour including Jeff's fix then the
>> documentation would need an even bigger overhaul, because it's neither
>> "reflog also" (as claimed in the doc) nor "reflog only" (as in the
>> original implementation) but "historical tips as inferred from the
>> current value and the reflog".
> 
> Even though things like "reflog only", "reflog also", may be
> something implementors may care about, they are irrelevant
> implementation details to the intended audience.  "The bases that
> are not in reflogs are ignored" _does_ matter, as it affects the
> outcome, and that may be a bit too strict a filter (which this
> series takes us in a good direction to fix) but what the readers
> need to know is the real-world implications of the choices made at
> the implementation detail level, and more importantly, what the
> implementation is trying to compute.
> 
> It is a documentation bug (with or without these patches) if the
> current text gives an impression that the code is trying to do
> anything but "guessing the fork point using historical tips".

I'm still trying to understand what the original intent was: If we
abstract from the implementation (as we should, as you rightly
emphasize) and talk about historical tips then we have to ask ourselves:
- What is "historical"?
- What is tip?
- Tip of what, i.e. what is a "branch"?

If by "branch" we mean the moving branch ref that it is in git then by all means, historical tips are the values that that ref ever had, and all that we can say is that this includes the current value and the current contents of the branch refs's reflog (which may or may not be "complete").

Note that this notion of "branch" is completely independent of the DAG, whereas by definition a "merge-base" is a concept that relies on an ancestry graph.

If by "branch" we mean everything that is "on" (or in, think "--contains") that branch - and I assume that is how most users think about a branch - then it is not clear at all why we should focus on "historical values that that refname had", which is an implemenation detail in itself (branch refs is how we implement the branch concept).

Especially, it's not clear why we should exclude for example a commit that is in between two commits that are in the reflog ("historical tips") of a branch that has been fast forwarded or reset (-A, then fast forward to -A-B-C; this excludes B from the list of merge-base candidates).

Show 6 quoted lines

>> In any case, for two modes we need two names for the options. Maybe
>> --fork-point and --fork-base because in the loose mode, you may get a
>> "base of a strict fork point"?
> 
> Perhaps.
>