Hi Elijah,
On Sat, 22 Jan 2022, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@xxxxxxxxx>
>
> When running `git merge-tree --write-tree`, we previously would only
> return an exit status reflecting the cleanness of a merge, and print out
> the toplevel tree of the resulting merge. Merges also have
> informational messages, ("Auto-merging <PATH>", "CONFLICT (content):
> ...", "CONFLICT (file/directory)", etc.) In fact, when non-content
> conflicts occur (such as file/directory, modify/delete, add/add with
> differing modes, rename/rename (1to2), etc.), these informational
> messages are often the only notification since these conflicts are not
> representable in the contents of the file.
>
> Add a --[no-]messages option so that callers can request these messages
> be included at the end of the output. Include such messages by default
> when there are conflicts, and omit them by default when the merge is
> clean.
Makes sense.
> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> index 0c19639594d..560640ad911 100644
> --- a/builtin/merge-tree.c
> +++ b/builtin/merge-tree.c
> @@ -440,22 +441,30 @@ static int real_merge(struct merge_tree_options *o,
> commit_list_insert(j->item, &merge_bases);
>
> merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);
> - printf("%s\n", oid_to_hex(&result.tree->object.oid));
> +
> if (result.clean < 0)
> die(_("failure to merge"));
> - else if (!result.clean)
> - printf(_("Conflicts!\n"));
> +
> + if (o->show_messages == -1)
> + o->show_messages = !result.clean;
> +
> + printf("%s\n", oid_to_hex(&result.tree->object.oid));
> + if (o->show_messages) {
> + printf("\n");
> + merge_display_update_messages(&opt, &result, stdout);
> + }
Excellent.
> merge_finalize(&opt, &result);
> return !result.clean; /* result.clean < 0 handled above */
> }
>
> int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> {
> - struct merge_tree_options o = { 0 };
> + struct merge_tree_options o = { .show_messages = -1 };
> int expected_remaining_argc;
> + int original_argc;
>
> const char * const merge_tree_usage[] = {
> - N_("git merge-tree [--write-tree] <branch1> <branch2>"),
> + N_("git merge-tree [--write-tree] [<options>] <branch1> <branch2>"),
> N_("git merge-tree [--trivial-merge] <base-tree> <branch1> <branch2>"),
> NULL
> };
> @@ -464,6 +473,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> N_("do a real merge instead of a trivial merge")),
> OPT_BOOL(0, "trivial-merge", &o.trivial,
> N_("do a trivial merge only")),
> + OPT_BOOL(0, "messages", &o.show_messages,
> + N_("also show informational/conflict messages")),
> OPT_END()
> };
>
> @@ -472,10 +483,13 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> usage_with_options(merge_tree_usage, mt_options);
>
> /* Parse arguments */
> + original_argc = argc;
> argc = parse_options(argc, argv, prefix, mt_options,
> merge_tree_usage, 0);
> if (o.real && o.trivial)
> die(_("--write-tree and --trivial-merge are incompatible"));
> + if (!o.real && original_argc < argc)
> + die(_("--write-tree must be specified if any other options are"));
Hmm. Well. Hmm.
I'd rather keep `--write-tree` neat and optional. What's wrong with
allowing
git merge-tree --no-messages HEAD MERGE_HEAD
?
To be clear, I think we need this instead:
if (o.trivial && o.show_messages >= 0)
die(_("--trivial-merge is incompatible with additional options"));
I like the rest of the patch very much!
Thank you,
Dscho
> if (o.real || o.trivial) {
> expected_remaining_argc = (o.real ? 2 : 3);
> if (argc != expected_remaining_argc)
> diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh
> index e03688515c5..c34f8e6c1ed 100755
> --- a/t/t4301-merge-tree-real.sh
> +++ b/t/t4301-merge-tree-real.sh
> @@ -84,4 +84,25 @@ test_expect_success 'Barf on too many arguments' '
> grep "^usage: git merge-tree" expect
> '
>
> +test_expect_success 'test conflict notices and such' '
> + test_expect_code 1 git merge-tree --write-tree side1 side2 >out &&
> + sed -e "s/[0-9a-f]\{40,\}/HASH/g" out >actual &&
> +
> + # Expected results:
> + # "greeting" should merge with conflicts
> + # "numbers" should merge cleanly
> + # "whatever" has *both* a modify/delete and a file/directory conflict
> + cat <<-EOF >expect &&
> + HASH
> +
> + Auto-merging greeting
> + CONFLICT (content): Merge conflict in greeting
> + Auto-merging numbers
> + CONFLICT (file/directory): directory in the way of whatever from side1; moving it to whatever~side1 instead.
> + CONFLICT (modify/delete): whatever~side1 deleted in side2 and modified in side1. Version side1 of whatever~side1 left in tree.
> + EOF
> +
> + test_cmp expect actual
> +'
> +
> test_done
> --
> gitgitgadget
>
>
