Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)

[Junio C Hamano <gitster@pobox.com>]

Miklos Vajna <vmiklos@frugalware.org> writes:

> On Tue, Aug 19, 2008 at 02:05:42AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
>> [On Hold]
>> (...)
>> * mv/merge-recursive (Tue Aug 12 22:14:00 2008 +0200) 3 commits
>>  - Make builtin-revert.c use merge_recursive_generic()
>>  - merge-recursive.c: Add more generic merge_recursive_generic()
>>  - Split out merge_recursive() to merge-recursive.c
>> 
>> I do not think builtlin-revert should use "recursive", but these patches
>> give a good starting point to separate the bulk of the "rename-aware
>> three-way merge" into library form.
>
> I wanted to send a patch that makes builtin-merge use the new
> merge_recursive_setup(), but then I was not able to decide to use
> merge_recursive_generic() or not.

I think git-merge and git-merge-recursive should be the only two that
actually trigger the "recursive" behaviour.  Everybody else should be
using non-recursive one, and that non-recursive one can be shared by the
one that is recursive.

Here is how the callchain looks like with your variant.

 cmd_merge_recursive()
 -> merge_recursive_setup()
 -> merge_recursive_generic()
    -> merge_recursive()
       -> merge_recursive()
       -> merge_trees()

The merge_recursive() is the "recursive" one.  The workhorse that is not
recursive is merge_trees().

Since the latter is what everybody else ("checkout -m", "revert",
"cherry-pick", "am -3", "stash apply") should be using, I think it is
pretty much up to "git-merge" and "git-merge-recursive" implementations
how the caller of merge_recursive() function is structured.  I suspect
that you would not need two separate functions, _setup() and _generic(),
for these two codepaths, but I didn't look closely.

And make_virtual_commit() should become static inside merge_recursive.c;
use of these fake commits is strictly an internal implementation issue of
how merge_recursive() function works and does not concern the caller, does
it?

By the way, the calling convention of merge_recursive_generic() looks
confusing (even though by the above reasoning it does not matter very much
outside "git-merge" and "git-merge-recursive").  Why does it take textual
object names for bases but binary object names for head and next?