Re: Improving merge failure message

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Nanako Shiraishi <nanako3@xxxxxxxxxxx> 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;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]