Re: Improving merge failure message
Junio C Hamano <gitster@pobox.com> writes:
Show 23 quoted lines
> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
>> [2]% git merge feature
>> error: Entry 'cool' not uptodate. Cannot merge.
>> fatal: merging of trees 8ec1d96451ff05451720e4e8968812c46b35e5e4 and aad8d5cef3915ab78b3227abaaac99b62db9eb54 failed
>>
>> ... the messages look unnecessarily scary, with two
>> "error" and "fatal" comments, and long sha1 commit names.
>
> Just a technical nit. I think these are tree object names.
>
>> Those of us who used git for some time can tell what it wants to say.
>> The merge checked the files in the working tree before doing anything,
>> found that the user has uncommitted change to a file that is involved in
>> the merge, and it stopped. And it didn't change anything. It may be "fatal"
>> but the user has much less reason to be scared about this failure than
>> the conflicting case.
>>
>> It would be nice if the message in the latter case can be toned down.
>
> Yeah, it would be nice. This actually was something that bothered me as
> well while trying to explain the recovery procedure for these two cases.
> Give me half an hour or so to cook up something...
It turns out to be a lot simpler than I thought, because 8ccba00 (unpack-trees: allow Porcelain to give different error messages, 2008-05-17) already laid enough groundwork for doing this kind of thing easily.
Notable points are:
- End the messages with "Aborting."; they are given when the three-way
merge stops without harming the work tree;
- Do not give the extra message after unpack_trees() already errored out.
This "merging of trees failed" message was primarily for debugging
merge-recursive itself, and the end user cannot do much with the object
names given in the message anyway.
But do give it under higher verbosity level, or when it happens during
the inner merge (the "recursive" one), as unpack_trees() should not
fail for the inner merge under normal conditions.
We could later add instructions on how to recover (i.e. "stash changes away or commit on a side branch and retry") instead of the silent exit(128) I have down there, and then use Peff's advice.* mechanism to squelch it (e.g. "advice.mergeindirtytree"), but they are separate topics.
merge-recursive.c | 25 +++++++++++++++++++++----
1 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 10d7913..a237240 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -170,6 +170,18 @@ static int git_merge_trees(int index_only,
int rc;
struct tree_desc t[3];
struct unpack_trees_options opts;
+ static struct unpack_trees_error_msgs msgs = {
+ /* would_overwrite */
+ "Your local changes to '%s' will be clobbered by merge. Aborting.",
+ /* not_uptodate_file */
+ "Your local changes to '%s' will be clobbered by merge. Aborting.",
+ /* not_uptodate_dir */
+ "Updating '%s' would lose untracked files in it. Aborting.",
+ /* would_lose_untracked */
+ "Untracked working tree file '%s' would be %s by merge. Aborting",
+ /* bind_overlap -- will not happen here */
+ NULL,
+ };
memset(&opts, 0, sizeof(opts));
if (index_only)
@@ -181,6 +193,7 @@ static int git_merge_trees(int index_only,
opts.fn = threeway_merge;
opts.src_index = &the_index;
opts.dst_index = &the_index;
+ opts.msgs = msgs;
init_tree_desc_from_tree(t+0, common);
init_tree_desc_from_tree(t+1, head);
@@ -1188,10 +1201,14 @@ int merge_trees(struct merge_options *o,
code = git_merge_trees(o->call_depth, common, head, merge);
- if (code != 0)
- die("merging of trees %s and %s failed",
- sha1_to_hex(head->object.sha1),
- sha1_to_hex(merge->object.sha1));
+ if (code != 0) {
+ if (show(o, 4) || o->call_depth)
+ die("merging of trees %s and %s failed",
+ sha1_to_hex(head->object.sha1),
+ sha1_to_hex(merge->object.sha1));
+ else
+ exit(128);
+ }
if (unmerged_cache()) {
struct string_list *entries, *re_head, *re_merge;
Nanako Shiraishi
