merge-recursive: don't force external callers to do our logging

newren · gitster · commit 345480d1ed46 · 2019-08-19T10:08:03.000-07:00
Alternatively, you can view this as "make the merge functions behave
more similarly."  merge-recursive has three different entry points:
merge_trees(), merge_recursive(), and merge_recursive_generic().  Two of
these would call diff_warn_rename_limit(), but merge_trees() didn't.
This lead to callers of merge_trees() needing to manually call
diff_warn_rename_limit() themselves.  Move this to the new
merge_finalize() function to make sure that all three entry points run
this function.

Note that there are two external callers of merge_trees(), one in
sequencer.c and one in builtin/checkout.c.  The one in sequencer.c is
cleaned up by this patch and just transfers where the call to
diff_warn_rename_limit() is made; the one in builtin/checkout.c is for
switching to a different commit and in the very rare case where the
warning might be triggered, it would probably be helpful to include
(e.g. if someone is modifying a file that has been renamed in moving to
the other commit, but there are so many renames between the commits that
the limit kicks in and none are detected, it may help to have an
explanation about why they got a delete/modify conflict instead of a
proper content merge in a renamed file).

Signed-off-by: Elijah Newren &lt;newren@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/merge-recursive.c b/merge-recursive.c
@@ -3595,9 +3595,6 @@ static int merge_recursive_internal(struct merge_options *opt,
 	flush_output(opt);
 	if (!opt->call_depth && opt->buffer_output < 2)
 		strbuf_release(&opt->obuf);
-	if (show(opt, 2))
-		diff_warn_rename_limit("merge.renamelimit",
-				       opt->needed_rename_limit, 0);
 	return clean;
 }
 
@@ -3617,7 +3614,9 @@ static int merge_start(struct merge_options *opt, struct tree *head)
 
 static void merge_finalize(struct merge_options *opt)
 {
-	/* Common code for wrapping up merges will be added here later */
+	if (show(opt, 2))
+		diff_warn_rename_limit("merge.renamelimit",
+				       opt->needed_rename_limit, 0);
 }
 
 int merge_trees(struct merge_options *opt,
diff --git a/sequencer.c b/sequencer.c
@@ -617,7 +617,6 @@ static int do_recursive_merge(struct repository *r,
 	if (is_rebase_i(opts) && clean <= 0)
 		fputs(o.obuf.buf, stdout);
 	strbuf_release(&o.obuf);
-	diff_warn_rename_limit("merge.renamelimit", o.needed_rename_limit, 0);
 	if (clean < 0) {
 		rollback_lock_file(&index_lock);
 		return clean;

Original file line number	Diff line number	Diff line change
`@@ -3595,9 +3595,6 @@ static int merge_recursive_internal(struct merge_options *opt,`
`3595`	`3595`	`flush_output(opt);`
`3596`	`3596`	`if (!opt->call_depth && opt->buffer_output < 2)`
`3597`	`3597`	`strbuf_release(&opt->obuf);`
`3598`		`- if (show(opt, 2))`
`3599`		`- diff_warn_rename_limit("merge.renamelimit",`
`3600`		`- opt->needed_rename_limit, 0);`
`3601`	`3598`	`return clean;`
`3602`	`3599`	`}`
`3603`	`3600`
`@@ -3617,7 +3614,9 @@ static int merge_start(struct merge_options opt, struct tree head)`
`3617`	`3614`
`3618`	`3615`	`static void merge_finalize(struct merge_options *opt)`
`3619`	`3616`	`{`
`3620`		`- /* Common code for wrapping up merges will be added here later */`
	`3617`	`+ if (show(opt, 2))`
	`3618`	`+ diff_warn_rename_limit("merge.renamelimit",`
	`3619`	`+ opt->needed_rename_limit, 0);`
`3621`	`3620`	`}`
`3622`	`3621`
`3623`	`3622`	`int merge_trees(struct merge_options *opt,`