[PATCH] backout: handle file moves correctly (issue1932)

[Mateusz Kwapich]

On 1/8/15, 3:06 PM, "Matt Mackall" <mpm at selenic.com> wrote:

>On Thu, 2015-01-08 at 13:39 -0800, Mateusz Kwapich wrote:
>> # HG changeset patch
>> # User Mateusz Kwapich <mitrandir at fb.com>
>> # Date 1420752870 28800
>> #      Thu Jan 08 13:34:30 2015 -0800
>> # Node ID b140af105987b9db7a296d24584a557bdc67fc6e
>> # Parent  7ad155e13f0f51df8e986a0ec4e58ac9a0ccedbb
>> backout: handle file moves correctly (issue1932)
>
>> The merge logic (ab)used by backout
>
>It's actually not abuse. It's very important core functionality.
>
>>  wasn't detecting file moves when the
>> parameter which should be common ancestor of merge heads was in fact
>>child of
>> one of them.
>
>In the abstract, merge is simply the operation that given three state
>vectors A, X, Y, creates a new state M = A + (X - A) + (Y - A) (where +
>is "patch" and - is "diff"). It doesn't matter what the relationships
>between the three inputs are. Don't get too distracted that we call 'A'
>the 'ancestor' in the code, it's simply more convenient to reason about
>in terms of a normal merge.
>
>However, various parts of the merge and copy code were indeed originally
>written with the assumption that a would be an ancestor of x and y and
>made various assumptions about revlog ordering, which
>has implications for every single thing that calls merge in a
>non-vanilla fashion: backout, rebase, graft, evolve, histedit, etc..
>
>Most of that is fixed. The primary offender at this point is
>mergecopies(). I started down the path to replacing that by writing
>pathcopies() a few years back, which doesn't care about direction in
>history and deals with most of our non-merge copy-detection needs.
>
>But finishing the job requires rewriting mergecopies() based on
>pathcopies() or otherwise making it direction-agnostic... which is what
>I've told everyone who's asked about merge+copy bugs for the past few
>years.
>
>Since you only mention "backout" in your subject and only hit a very
>specific special case, I suspect you're missing most of the related
>problems here and are just making things that much more confusing for
>whoever comes along to do the larger fix.
>
>-- 
>Mathematics is the supreme nostalgia of our time.

Hello Matt,

Thanks for detailed info about merge logic. I see that mergecopies() needs
a direction agnostic rewrite. This patch came from the need for working
backouting moves and copies. It¹s definitely more hotfix than a permanent
solution and yes: it¹s not making the mergecopies() more readable. But I
think I works good in this particular case and it¹s fixing a backout
behavior to match users expectations. So I think before somebody rewrites
mergecopies() in near future (maybe me? :-) ) it will be good to have
backout working correctly.

What do you think?

Best,
Mateusz