Re: Unexpected or wrong ff, no-ff and ff-only behaviour

[Sergey Organov <sorganov@gmail.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> [...]
>>
>>> The "the tip being merged into the mainline must always be
>>> fast-forwardable",
>>
>> It's rather "the tip being merged into the mainline must be fast-forwardable the
>> first time it is merged".
>>
>>> however, is not consistent with the topic branch workflow, and I do
>>> not mean this in the sense that you should never rebase just before
>>> submitting (which is a bad practice). For an initial merge of the
>>> topic to the mainline, the project can keep rebasing to the
>>> then-current tip of the mainline, and as long as they can afford the
>>> cycle to test the result, "record the range of the topic branch by
>>> making a redundant merge" would work.
>>
>> Yes, that's exactly it, and, as the rule holds for the first topic merge
>> only, the rest of workflow is as usual, no drastic changes.
>>
>> Overall, it only ensures the first merge of the topic is semantically
>> simpler, nothing more.
>
> But then you have to know to say or omit --ff-only if you are
> mergint the topic for the first time.  The second and subsequent
> merges, your merges won't be "semantically simple" at all.

Yes, the second and the rest (if any) won't differ, but having the first
one, that supposedly introduces most of changes, "semantically simple"
is still an advantage.

>
> And your first "semantically simple" merge is likely to be a merge
> between the then-current tip, and a topic that has been prepared
> over a few hours (or longer) and has sufficiently been tested, but
> has gotten rebased immediately before being merged because of this
> "must fast-forwardable to maintain semantic simplicity" policy to
> catch up with the ever-moving-forward tip of the mainline.  The end
> result of such a "semantically simpler" merge is as good as the
> "freshly rebased, never had sufficient time to verify the result"
> iteration of the topic branch.

Yes, this makes sense only in otherwise rebase-based workflows, when you
do rebase anyway.

> How would such a state that was freshly rebased without chance of
> enough validation be better than merging the "sufficiently been
> tested in isolation" state without doing such a rebase?  If your
> answer is "but the contributor would test after rebasing and before
> doing the --ff-only-plus-no-ff merge", then perhaps the contributor
> can also test the result of a (trial) merge before it actually gets
> merged, no?

Yes, I mean a workflow where testing of rebased (or merged, doesn't
matter, as they should actually be the same thing) version is possible
or even required, and I believe that in this particular case testing
rebased version and merging the result makes more sense, see below.

>
> If we have a project like this:
>
>         A               topic that is slightly stale
>        /
>   o---F---o---o---X     mainline
>
> M, A', and N should end up with identical trees:
>
>
>         A-----------M   topic that is slightly stale, merged into mainline
>        /           /
>   o---F---o---o---X---N mainline with A' merged
>                    \ /
>                     A'  mainline with A rebased on top as A'
>
> And by forcing to rebase A to A' before merging into the mainline as
> N, compared to advancing mainline from X to M, one major difference
> the workflow is making is to _lose_ the information that the topic
> was cooked in the context of an older mainline and did not take what
> happened since F until X into account.  Rebasing it to A' without
> giving the result sufficient vetting means you are forcing the
> updated topic to pretend as if it also took F..X into account,
> making it harder to diagnose subtle interactions with A and F..X
> later that you did not spot when you made A'; merging to create M
> does not have such a problem.  As A' and M would have the identical
> trees, the same bug resulting from such a subtle interactions would
> be there if A' has such an issue, but at least M is more honest and
> lets us know that A itself predated what happened between F..X.
>
> So...

So it has a disadvantage for the workflow you have in mind, and you
won't use the feature should you decide the disadvantage outweighs the
aforementioned advantage, as usual.

However, committing untested M still doesn't taste as the best possible
way of handling things in general. It'd be best to actually test M or N
before publishing. In a workflow where there are measures to do it,
testing A' (= N) and publishing N both makes more sense and results in
simpler history than merging A and then testing and publishing M.

That said, even if we rather all do agree rebase workflow is always
inferior to merge one, is it satisfactory excuse to actively resist
otherwise logical behavior of 'git merge' that is even documented? I
don't think so.

Thus, one way or another, I'd still vote for keeping options description
intact, and rather fix the implementation to match the manual, i.e.,
make the --[no]-ff actually orthogonal to --ff-only.

-- Sergey