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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23/01/18 19:12, Junio C Hamano wrote:
Phillip Wood <phillip.wood@xxxxxxxxxxxx> writes:

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 agree that the merge should be recreated, but I was thinking of 
something slightly different. Currently the sequencer uses 
opts->allow_ff to control whether a new commit with the same contents 
should be created even if the existing one could be reused. So I was 
querying whether we should recreate the commit when the user run 'git 
rebase --recreate-merges --no-ff' rather than just reusing it. As merges 
also have another meaning for fast-forward the terminology gets confusing.
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.
This is the merge sense of 'fast-forward' not the existing sequencer 
sense, without thinking about it more I'm not sure if one command line 
option for rebase is sufficient to cover both uses.
Best Wishes

Phillip




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux