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

Junio C Hamano [off-list ref] wrote:

Eric Wong [off-list ref] writes:

quoted

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.

I left it as the default, too.  I figured that it's best not
to change the default (and most likely faster) behavior of
something people rely on.

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

Good point, I'll change it.

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

Nope.  perl -e 'print reverse <>' is equivalent and we already use
plenty of perl.

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

Oops, I'll change that, too.

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.

quoted

+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

Changing the 'git-merge $strategy_args "rebase-merge: $cmt" HEAD "$cmt"'
line in call_merge() to this seems to have broken more tests.

I'm not an expert at merging strategies by any measure, I've just
trusted merge-recursive to Do The Right Thing(TM) more often than not,
and use rerere to avoid repeating work.

I'm not entirely sure I want (or fully understand how) to support
cherry-picking/revert with this, as I've already dropped --skip when
--merge was in use.

I'll think about this some more when I'm less distracted.

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

Cool, that's good.

-- 
Eric Wong