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

Miklos Vajna <vmiklos@frugalware.org> writes:

I think you have.

Let's look at the call chain from cmd_merge_recursive() and think again.

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

cmd_merge_recursive() takes subtree option and set of object names (two commits and zero or more base commits), massages them and calls merge_recursive(). merge_recursive_setup() and merge_recursive_generic() are involved in this massaging process.

merge_recursive() computes the bases itself when given no base, and in a multi-base situation, does its thing recursively to come up with a consolidated base, using virtual commits. After coming up with the three (virtual or real) commits to use, it gives them to merge_trees(), which operate solely on tree objects.

In addition, merge_recursive() currently *requires* the caller to wrap bare tree objects in virtual commits, if the caller wants to do a simple three-way merge of trees (in which case because there is no ancestry information available you would naturally not do any recursive behaviour). This "input must be commit" requirement is why you think you need to have get_ref() that uses make_virtual_commit() in the caller.

But it does not have to be that way. It is merely an artifact of the current refactoring that kept the interface into merge_recursive() based on commit objects. You could further refine the refactoring so that:

 - merge_trees(), in addition to the three tree objects, takes options
   such as use of the subtree behaviour, descriptive names for heads to be
   used for conflict markers, verbosity level, and other future options
   (such as "use this lower rename detaction threshold").  Introduce
   "struct merge_options" for that and pass it around.  These show() and
   output() calls could even become callbacks, but I didn't look very
   carefully.

 - merge_recursive(), in addition to that "merge_options" structure, will
   take heads, and list of common ancestors.

 - merge_recursive_generic() can be a layer on top of merge_recursive() to
   allow the caller to feed tree objects.  Use of "const unsigned char *"
   to give raw object names (or even "const char *" to feed texual object
   names) would be easier for the callers.  Wrapping a tree into virtual
   commit can and should be done at this layer, hidden away inside
   merge-recursive.c from the callers.

   Alternatively, you can do away without such preparation step, and move
   the "wrap a tree into a virtual commit" inside merge_recursive()
   itself.  If you take that route, merge_recursive() will take heads and
   list of common ancestors all in "const unsigned char *" object names,
   in addition to the "merge_options" structure.

When you rewrite cmd_merge() to make direct call to bypass a subprocess, your callchain would look like:

    cmd_merge()
    -> merge_recursive()
       -> merge_recursive()
       -> merge_trees()

cmd_merge() needs to do the same arrangement for "subtree" and any possible future options, and feed the same set of object names to merge_recursive(). You cannot give a bare tree to "git merge", so you do not have to worry about having to wrap it in a virtual commit.

So my gut feeling is that the interface may look something like:

struct merge_options {
        const char *branch1_label;
        const char *branch2_label;
        unsigned subtree_merge : 1;
	int verbosity;
        /* other options here ... */
};

/* rename-detecting three-way merge, no recursion */
int merge_trees(struct merge_options *,
                struct tree *head,
                struct tree *merge,
                struct tree *common,
                struct tree **result);

/* merge_trees() but with recursive ancestor consolidation */
int merge_recursive(struct merge_options *,
                    struct commit *h1,
                    struct commit *h2,
                    struct commit_list *ca,
                    struct commit **result);

/*
 * "git-merge-recursive" can be fed trees; wrap them into
 * virtual commits and call merge_recursive() proper.
 */
int merge_recursive_generic(struct merge_options *,
                            const unsigned char *head,
                            const unsigned char *merge,
                            int num_ca,
                            const unsigned char **ca,
                            struct commit **result);

and the call chain would become:

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

    cmd_merge()
    -> merge_recursive()
       -> merge_recursive()
       -> merge_trees()

    cmd_revert(), cmd_am(), cmd_checkout(), cmd_stash(), ...
    -> merge_trees()