All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: "Eckhard Maaß" <eckhard.s.maass@googlemail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Ben Peart <peartben@gmail.com>,
	Ben Peart <Ben.Peart@microsoft.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	"peff@peff.net" <peff@peff.net>,
	"pclouds@gmail.com" <pclouds@gmail.com>,
	"vmiklos@frugalware.org" <vmiklos@frugalware.org>,
	Kevin Willford <kewillf@microsoft.com>,
	"Johannes.Schindelin@gmx.de" <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v3 2/3] merge: Add merge.renames config setting
Date: Fri, 27 Apr 2018 13:23:20 -0700	[thread overview]
Message-ID: <CABPp-BHwM1jx2+VTxt7hga7v-E6gvHuxVNPqm-MPRXYe5CDVtA@mail.gmail.com> (raw)
In-Reply-To: <20180427183752.GA2799@esm>

On Fri, Apr 27, 2018 at 11:37 AM, Eckhard Maaß
<eckhard.s.maass@googlemail.com> wrote:
> On Fri, Apr 27, 2018 at 11:23:56AM +0900, Junio C Hamano wrote:
>> I think demoting from copy to rename-only is a good idea, at least
>> for now, because I do not believe we have figured out what we want
>> to happen when we detect copied files are involved in a merge.
>
> Does anyone know some threads concerning that topic? I tried to search
> for it, but somehow "merge copy detection" did not find me useful
> threads so far. I would be interested in that topic anyway, so I would
> like to know what the ideas are that floated so far.
>

I doubt it has ever been discussed before this thread.  But, if you're
curious, I'll try to dump a few thoughts.  Let's say we have branches
A and B, and:
   A: modifies file z
   B: copies z to y

Should the modifications to z done in A propagate to both z and y?  If
not, what good is copy detection?  If so, then there are several
ramifications...

- If B not only copied z but also first modified it, then do we have
  potential conflicts with both z and y -- possibly the exact same
  conflicts, making the user resolve them repeatedly?

- What if A copied z to x?  Do changes to z propagate to all three of
  z and x and y?  Do changes to either x or y affect z?  Do they
  affect each other?

- If A deleted z, does that give us a copy/delete conflict for y?  Do
  we also have to worry about copy/add conflicts?  copy/add/delete?
  rename/copy (multiple variants)?  copy/copy?

- Extra degrees of freedom may mean new conflict types:

  - The extra degrees of freedom from renames introduced multiple new
    conflict types (e.g. rename/add, rename/rename(1to2),
    rename/rename(2to1)).

  - Directory rename detection added more (rename/rename/rename(1to3),
    rename/rename(Nto1), add/add/add, directory/file/file, n-fold
    transitive rename, etc.), -- and forced us to specify various
    rules to limit the possibility space so that conflicts could be
    representable in the index and understandable by the user.

  - I suspect adding copies would add new types of conflicts we
    haven't thought of yet.

The more I think about it, the more I think that attempting to detect
copies in a merge algorithm just doesn't make sense.  Anything I can
think of that someone might attempt to use detected copies for would
just surprise users in a bad way...and it'd open up a big can of edge
and corner case problems as well.

  reply	other threads:[~2018-04-27 20:23 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-20 13:36 [PATCH v1 0/2] add additional config settings for merge Ben Peart
2018-04-20 13:36 ` [PATCH v1 1/2] merge: Add merge.renames config setting Ben Peart
2018-04-20 17:02   ` Elijah Newren
2018-04-20 17:26     ` Elijah Newren
2018-04-23 12:57       ` Ben Peart
2018-04-20 17:59     ` Ben Peart
2018-04-20 18:34       ` Elijah Newren
2018-04-21  4:23         ` Junio C Hamano
2018-04-23 16:00           ` Ben Peart
2018-04-23 23:23             ` Junio C Hamano
2018-04-24 11:58               ` Johannes Schindelin
2018-04-24 17:47                 ` Elijah Newren
2018-04-25  8:20                   ` Johannes Schindelin
2018-04-22 12:07         ` Eckhard Maaß
2018-04-23 13:15           ` Ben Peart
2018-04-23 21:32             ` Eckhard Maaß
2018-04-24 16:53               ` Ben Peart
2018-04-23 13:22         ` Ben Peart
2018-04-20 13:36 ` [PATCH v1 2/2] merge: Add merge.aggressive " Ben Peart
2018-04-20 17:22   ` Elijah Newren
2018-04-24 16:45     ` Ben Peart
2018-04-24 17:36       ` Elijah Newren
2018-04-24 23:57       ` Junio C Hamano
2018-04-25 14:47         ` Ben Peart
2018-04-20 17:34 ` [PATCH v1 0/2] add additional config settings for merge Elijah Newren
2018-04-20 18:19   ` Ben Peart
2018-04-24 17:11 ` [PATCH v2 " Ben Peart
2018-04-24 17:11   ` [PATCH v2 1/2] merge: Add merge.renames config setting Ben Peart
2018-04-24 18:11     ` Elijah Newren
2018-04-24 18:59     ` Elijah Newren
2018-04-24 20:31       ` Ben Peart
2018-04-25 16:01         ` Elijah Newren
2018-04-24 17:11   ` [PATCH v2 2/2] merge: Add merge.aggressive " Ben Peart
2018-04-25  0:13   ` [PATCH v2 0/2] add additional config settings for merge Junio C Hamano
2018-04-25 15:22     ` Ben Peart
2018-04-26  1:48       ` Junio C Hamano
2018-04-26 20:52 ` [PATCH v3 0/3] add merge.renames config setting Ben Peart
2018-04-26 20:52   ` [PATCH v3 1/3] merge: update documentation for {merge,diff}.renameLimit Ben Peart
2018-04-26 23:11     ` Elijah Newren
2018-04-26 23:23       ` Jonathan Tan
2018-04-26 20:52   ` [PATCH v3 2/3] merge: Add merge.renames config setting Ben Peart
2018-04-26 22:52     ` Elijah Newren
2018-04-27  0:54       ` Ben Peart
2018-04-27  2:23         ` Junio C Hamano
2018-04-27  3:28           ` Elijah Newren
2018-04-27  7:23             ` Johannes Schindelin
2018-04-27 14:32               ` Elijah Newren
2018-04-27 18:37           ` Eckhard Maaß
2018-04-27 20:23             ` Elijah Newren [this message]
2018-04-30  8:03               ` Eckhard Maaß
2018-04-30 16:54                 ` Elijah Newren
2018-04-27  4:17         ` Elijah Newren
2018-04-27 18:19         ` Elijah Newren
2018-04-30 13:11           ` Ben Peart
2018-04-30 16:12             ` Re: Elijah Newren
2018-05-02 14:33               ` Re: Ben Peart
2018-04-26 20:52   ` [PATCH v3 3/3] merge: pass aggressive when rename detection is turned off Ben Peart
2018-04-26 23:00     ` Elijah Newren
2018-04-26 22:08   ` [PATCH v3 0/3] add merge.renames config setting Elijah Newren
2018-05-02 16:01 ` [PATCH v4 0/3] add additional config settings for merge Ben Peart
2018-05-02 16:01   ` [PATCH v4 1/3] merge: update documentation for {merge,diff}.renameLimit Ben Peart
2018-05-02 16:01   ` [PATCH v4 2/3] merge: Add merge.renames config setting Ben Peart
2018-05-04  3:07     ` Junio C Hamano
2018-05-02 16:01   ` [PATCH v4 3/3] merge: pass aggressive when rename detection is turned off Ben Peart
2018-05-02 17:20   ` [PATCH v4 0/3] add additional config settings for merge Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABPp-BHwM1jx2+VTxt7hga7v-E6gvHuxVNPqm-MPRXYe5CDVtA@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=Ben.Peart@microsoft.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=eckhard.s.maass@googlemail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kewillf@microsoft.com \
    --cc=pclouds@gmail.com \
    --cc=peartben@gmail.com \
    --cc=peff@peff.net \
    --cc=vmiklos@frugalware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.