Re: [PATCH v2 06/20] merge-recursive: don't force external callers to do our logging
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 26, 2019, 19:57 UTC
- Message-ID
- <xmqq36iso9gb.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190726155258.28561-7-newren@gmail.com>
Elijah Newren <newren@gmail.com> writes:
Show 8 quoted lines
> 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.
Interesting. It seems that b520abf1c8f did a suboptimal jobs but this step cleans it up quite nicely.
Are there callers of merge_trees() in other codepaths, and do they want this change?
There is only one, builtin/checkout.c::merge_working_tree(), which is used when switching to a different commit. I think it would not hurt to give the same warning (but I also think it would not hurt to simply disable rename detection in that context); we should record the whatever decision we make in the log message.
Show 43 quoted lines
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> merge-recursive.c | 7 +++----
> sequencer.c | 1 -
> 2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 4a481c3929..308e350ff1 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -3583,9 +3583,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;
> }
>
> @@ -3607,7 +3604,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
> index c4ed30f1b4..094a4dd03d 100644
> --- 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;