On Sat, Jan 22, 2022 at 10:56 PM Elijah Newren via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Elijah Newren <newren@xxxxxxxxx>
>
> This adds the ability to perform real merges rather than just trivial
> merges (meaning handling three way content merges, recursive ancestor
> consolidation, renames, proper directory/file conflict handling, and so
> forth). However, unlike `git merge`, the working tree and index are
> left alone and no branch is updated.
>
> The only output is:
> - the toplevel resulting tree printed on stdout
> - exit status of 0 (clean) or 1 (conflicts present)
The exit status can now actually be something other than 0 and 1
according to the doc and code below.
> +Performs a merge, but does not make any new commits and does not read
> +from or write to either the working tree or index.
> +
> +The first form will merge the two branches, doing a full recursive
> +merge with rename detection.
Maybe this could already tell that the first form will also write a
tree with the result of the merge (even in case of conflict) as this
could help understand the reason why the associated option is called
'--write-tree'. It could also help to say that we call such a merge a
'real' merge.
> The rest of this manual (other than the
> +next paragraph) describes the first form in more detail -- including
> +options, output format, exit status, and usage notes.
> static int real_merge(struct merge_tree_options *o,
> const char *branch1, const char *branch2)
> {
> - die(_("real merges are not yet implemented"));
> + struct commit *parent1, *parent2;
> + struct commit_list *common;
> + struct commit_list *merge_bases = NULL;
> + struct commit_list *j;
> + struct merge_options opt;
> + struct merge_result result = { 0 };
> +
> + parent1 = get_merge_parent(branch1);
> + if (!parent1)
> + help_unknown_ref(branch1, "merge",
> + _("not something we can merge"));
The second argument is supposed to be the command (it's called "cmd"),
so maybe "merge-tree" instead of "merge".
> + parent2 = get_merge_parent(branch2);
> + if (!parent2)
> + help_unknown_ref(branch2, "merge",
> + _("not something we can merge"));
idem
> + opt.show_rename_progress = 0;
> +
> + opt.branch1 = merge_remote_util(parent1)->name; /* or just branch1? */
> + opt.branch2 = merge_remote_util(parent2)->name; /* or just branch2? */
I think just:
opt.branch1 = branch1
opt.branch2 = branch2
might be better for users as it should show the name as it was passed
to the command.
> + merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);
> + printf("%s\n", oid_to_hex(&result.tree->object.oid));
I wonder if we can actually always output a valid tree when
result.clean < 0. In case we might not, the printing should go a few
lines below.
> + if (result.clean < 0)
> + die(_("failure to merge"));
> + else if (!result.clean)
The "else" is not necessary above.
> + printf(_("Conflicts!\n"));
> + merge_finalize(&opt, &result);
> + return !result.clean; /* result.clean < 0 handled above */
> }
> diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh
> new file mode 100755
> index 00000000000..e03688515c5
> --- /dev/null
> +++ b/t/t4301-merge-tree-real.sh
I wonder if it would be better named 't4301-merge-tree-write-tree.sh'...
> @@ -0,0 +1,87 @@
> +#!/bin/sh
> +
> +test_description='git merge-tree --write-tree'
... especially given this description.
