Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.

[Marc Branchaud <marcnarc@xiplink.com>]

Jonathan Nieder wrote:
> Hi Marc,
> 
> Marc Branchaud wrote:
> 
>> This option tells rebase--interactive to cherry-pick all the commits in the
>> rebased branch, instead of fast-forwarding over any unchanged commits.
>>
>> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
> [...]
>> This offers an alterntive way to deal with reverted merges (the
>> revert-a-faulty-merge.txt howto recommends reverting the initial reversion
>> before re-merging a modified topic branch).
>>
>> With this change, you can instead use the --no-ff option to recreate the
>> branch with entirely new commits (they're new because at the very least the
>> committer time is different).  This obviates the need to revert the
>> reversion, as you can re-merge the new topic branch directly.
> 
> I think this rationale should go in the commit message, for the sake of
> future readers studying the code you’ve changed.

Easy to do, but how necessary?  The rationale is already in the description
of --no-ff in rebase's man page.

There's an extra aspect to my motivation that I failed to mention, which is
that I would like the topic branch to stay rooted at its original commit.
This is because in our workflow that original commit is a synchronization
point for several maintenance and "next"-style branches, and keeping the
topic rooted there makes it easy to merge into different branches.

> But let me make sure I understand it.
> 
> If I am understanding properly, your idea is that this would be used on
> a branch after “unmerging” it from master:
> 
>     B --- C --- D [topic]
>   /              \
>  A ---  ...   --- M ... --- U [master]
> 
> Here M is a merge commit and U a commit reverting the change from M^
> to M.
> 
> As howto/revert-a-faulty-merge explains, this is an accident waiting to
> happen: if you merge master into topic, then it will pull in the revert
> along with the rest of the changes.  Similarly, if you merge topic into
> master, it will be a no-op (“already up to date”).
> 
> Your solution, to rewrite the branch, should address both of these
> problems.  The new history looks like this:
> 
>     B' --- C' --- D' [topic]
>   /
>  |  B --- C --- D
>  |/              \
>  A ---   ...  --- M ... --- U [master]
> 
> What happens now if you merge topic into master (or master into topic)?
> 
> Git will try to reconcile the changes from the two branches since when
> they diverged.  master gained the changes from the old topic branch and
> then reverted them since it diverged from topic; since merge algorithm
> is a simple three-way merge, all that history is ignored and Git will
> just combine any new changes from master with the changes from topic.
> Success.

Yup, you got it!

> Side note:
> 
> After this maneuver, I would want to merge ‘master’ into ‘topic’ as
> soon as possible.

Well, I wouldn't want to do that because it would make it impossible to merge
the topic into some other branch without also bringing in all of master.

> Why?  Because there is still a failure mode possible
> after this: if you merge any commits from the old topic into the new
> one before then, you’re back in trouble.  This could happen if some
> topic2 that topic ends up needing is based on a point in master after M
> and before U.

If the re-cast topic1 doesn't rewrite those commits, then the merge will
simply succeed because the code is already identical in both branches.

But if topic1 does rewrite those commits then there'll be a conflict.  IMO
that's correct, because with the merging of topic2 the code in master really
did diverge in a relevant way from what's in topic1, so that conflict should
get resolved in the normal way.

> After merging master into topic, the _meaning_ of the history is much
> clearer: by making U an ancestor, we are saying that as far as this
> branch is concerned, the state at the tip of the topic branch is to be
> preferred over the reverted state.

Yes, but it also means that all the history of master since commit A is now
part of the topic, which isn't acceptable to me.

> This suggests an alternative method to achieve your goal:
> 
>  git checkout master
>  git revert -m 1 M;	# (1)
>  git checkout topic
>  git merge master^;	# (2)
>  git merge -s ours master;	# (3)
> 
> 1. Add a commit U removing the changes introduced by topic from master.
> 
> 2. Merge all changes from master before the revert into topic.
> 
> 3. Declare the changes from topic to supersede the master branch.
>    This way, if topic is ever merged back to master, the changes will be
>    reintroduced again.
> 
> Resulting history:
> 
>     B --- C --- D --- ... --- T^ --- T [topic]
>   /              \            /     /
>  A ---   ...  --- M ... --- U^ --- U [master]

I understand how this achieves your goal but, aside from dragging all of
master into the topic, this is way more complicated than recreating the topic
or even doing the double-revert.  By creating T with "-s ours" U becomes like
a surrogate ancestor of the topic -- it's there, but it doesn't share any
genetic material with the topic.  I think that's a recipe for confusion to
future explorers of the repo's history.

Besides, I do expect at least one of the topic's original commits to actually
be different in the recast topic -- the original merge was reverted because
the topic was faulty in some way, so there needs to be a real change made
somewhere.  If the topic could simply be fixed with an extra commit or two
after D then re-merged, there wouldn't be a need to revert the original merge
in the first place.

> You can avoid the perhaps pointless commit T^ by using --no-commit in
> the command (2).  If not all changes from M..master are suitable for
> merging into topic, you can use the same trick on a temporary side
> branch:
> 
>  git checkout -b revert-topic M
>  git revert -m 1 M
>  git checkout topic
>  git merge --no-commit revert-topic^
>  git merge -s ours revert-topic
>  git checkout master
>  git merge revert-topic
> 
> If not all changes from topic..M are suitable for merging into topic,
> then things are harder.  I’d suggest making a special unrevert-topic
> branch as above to keep in the wings until its time.

This still brings all of the master's A..M^ commits into the topic.

>> (Honestly, I wouldn't say that this approach is vastly superior to
>> reverting the reversion.  I just find it a little less messy and a little
>> more intuitive.  It's also a bit easier to explain to people to "use --no-ff
>> after reverting a merge" instead of making sure they get the double-
>> reversion right.)
> 
> Hope this helps clarify things.
> 
> Your patch itself might still be a good idea.  I think plain
> ‘git rebase’ already has something like it, in the form of the
> --force-rebase option.  I have used it before, though I don’t remember
> why.

No, the man page says --force-rebase does something else.

Thanks for the feedback!

		M.