On 4/27/2018 2:19 PM, Elijah Newren wrote:
From: Elijah Newren <newren@xxxxxxxxx>
On Thu, Apr 26, 2018 at 5:54 PM, Ben Peart <peartben@xxxxxxxxx> wrote:
Can you write the documentation that clearly explains the exact behavior you
want? That would kill two birds with one stone... :)
Sure, something like the following is what I envision, and I've tried to
include the suggestion from Junio to document the copy behavior in the
merge-recursive documentation.
-- 8< --
Subject: [PATCH] fixup! merge: Add merge.renames config setting
---
Documentation/merge-config.txt | 3 +--
Documentation/merge-strategies.txt | 5 +++--
merge-recursive.c | 8 ++++++++
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index 59848e5634..662c2713ca 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -41,8 +41,7 @@ merge.renameLimit::
merge.renames::
Whether and how Git detects renames. If set to "false",
rename detection is disabled. If set to "true", basic rename
- detection is enabled. If set to "copies" or "copy", Git will
- detect copies, as well. Defaults to the value of diff.renames.
+ detection is enabled. Defaults to the value of diff.renames.
merge.renormalize::
Tell Git that canonical representation of files in the
diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 1e0728aa12..aa66cbe41e 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -23,8 +23,9 @@ recursive::
causing mismerges by tests done on actual merge commits
taken from Linux 2.6 kernel development history.
Additionally this can detect and handle merges involving
- renames. This is the default merge strategy when
- pulling or merging one branch.
+ renames, but currently cannot make use of detected
+ copies. This is the default merge strategy when pulling
+ or merging one branch.
+
The 'recursive' strategy can take the following options:
diff --git a/merge-recursive.c b/merge-recursive.c
index 6cc4404144..b618f134d2 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -564,6 +564,14 @@ static struct string_list *get_renames(struct merge_options *o,
opts.flags.recursive = 1;
opts.flags.rename_empty = 0;
opts.detect_rename = merge_detect_rename(o);
+ /*
+ * We do not have logic to handle the detection of copies. In
+ * fact, it may not even make sense to add such logic: would we
+ * really want a change to a base file to be propagated through
+ * multiple other files by a merge?
+ */
+ if (opts.detect_rename > DIFF_DETECT_RENAME)
+ opts.detect_rename = DIFF_DETECT_RENAME;
opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :
o->diff_rename_limit >= 0 ? o->diff_rename_limit :
1000;
Thanks Elijah. I've applied this patch and reviewed and tested it. It
works and addresses the concerns around the settings inheritance from
diff.renames. I still _prefer_ the simpler model that doesn't do the
partial inheritance but I can use this model as well.
I'm unsure on the protocol here. Should I incorporate this patch and
submit a reroll or can it just be applied as is?
