Re: [PATCH] rebase: Allow merge strategies to be used when rebasing

From: Junio C Hamano <junkio@cox.net>
Date: Jun 18, 2006, 09:08 UTC
Message-ID: <7vd5d63k7f.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <1150599735483-git-send-email-normalperson@yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

> This solves the problem of rebasing local commits against an
> upstream that has renamed files.

I think leveraging the merge strategy to perform rebase is sound, but the selection of merge base for this purpose is quite different from the regular merge, and I think unfortunately this patch is probably wrong in letting git-merge choose the merge base.

But let's mention other things as well.

 - You kept the original "format-patch piped to am" workflow
   optionally working.

 - You check if merge or patch was used for failed rebase and
   follow the appropriate codepath while resuming, which is
   good.

 - The list of commits you generate with tac seem to include
   merge commit -- you may want to give --no-merges to
   rev-list.

 - I do not think we use "tac" elsewhere -- is it portable
   enough?

 - Exiting with success unconditionally after "git am" feels
   wrong.  I would do "exit $?" instead of "exit 0" there.

Suppose you have this commit ancestry graph:

----------------------------------------------------------------
Example:       git-rebase --onto master A topic

        A---B---C topic                       B'--C' topic
       /                   -->               /
  D---E---F---G master          D---E---F---G master
----------------------------------------------------------------

This is slightly different from the one at the beginning of the script. The idea is A turned out to be not so cool, and we would want to drop it.

Show 6 quoted lines

> +call_merge () {
> +	cmt="$(cat $dotest/`printf %0${prec}d $1`)"
> +	echo "$cmt" > "$dotest/current"
> +	git-merge $strategy_args "rebase-merge: $cmt" HEAD "$cmt" \
> +			|| die "$MRESOLVEMSG"
> +}

call_merge is first called with B in cmt, and HEAD is pointing at G. But the merge in this function makes a merge between B and G, taking the effect of E->A.

I think the three-way merge you would want here is not between B and G using E as the pivot, but between B and G using A as the pivot. That's how cherry-pick and revert works. I would leverage the interface that is one level lower for this -- the strategy modules themselves.

	git-merge-$strategy $cmt^ -- HEAD $cmt

The strategy modules take merge base(s), double-dash as the separator, our head and the other head. They do not make commit themselves (instead they leave working tree and index in committable state) and signal the results with their exit status:

	0 -- success
        1 -- conflicts
        2 -- did not handle the merge at all