Thread (45 messages) 45 messages, 6 authors, 2018-05-02

Re:

From: Ben Peart <hidden>
Date: 2018-04-30 13:11:44


On 4/27/2018 2:19 PM, Elijah Newren wrote:
quoted hunk ↗ jump to hunk
From: Elijah Newren <redacted>

On Thu, Apr 26, 2018 at 5:54 PM, Ben Peart [off-list ref] wrote:
quoted
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?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help