Re: [PATCH 3/8] sequencer: fast-forward merge commits, if possible

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 23, 2018, 19:12 UTC
Message-ID: <xmqqr2qggzie.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <7e6906c9-d642-ee8d-82fd-29ee6c60e308@talktalk.net>

Phillip Wood <phillip.wood@talktalk.net> writes:

Show 10 quoted lines

> On 18/01/18 15:35, Johannes Schindelin wrote:
>> 
>> Just like with regular `pick` commands, if we are trying to recreate a
>> merge commit, we now test whether the parents of said commit match HEAD
>> and the commits to be merged, and fast-forward if possible.
>> 
>> This is not only faster, but also avoids unnecessary proliferation of
>> new objects.
>
> I might have missed something but shouldn't this be checking opts->allow_ff?

Because the whole point of this mechanism is to recreate the topology faithfully to the original, even if the original was a redundant merge (which has a side parent that is an ancestor or a descendant of the first parent), we should just point at the original merge when the condition allows it, regardless of opts->allow_ff.

I think it is a different matter if an insn to create a new merge (i.e. "merge - <parent> <message>", not "merge <commit> <parent>") should honor opts->allow_ff; because it is not about recreating an existing history but is a way to create what did not exist before, I think it is sensible if allow_ff option is honored.