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 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.
