Re:
- From
Ben Peart <peartben@gmail.com>
- Date
- Apr 30, 2018, 13:11 UTC
- Message-ID
- <e753d8fd-5329-b819-0076-0ff4659dabf1@gmail.com>
- In-Reply-To
- <20180427181937.7607-1-newren@palantir.com>
On 4/27/2018 2:19 PM, Elijah Newren wrote:
Show 70 quoted lines
> From: Elijah Newren <newren@gmail.com> > > On Thu, Apr 26, 2018 at 5:54 PM, Ben Peart <peartben@gmail.com> 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?