Problem with non-conflicting merges of moves

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Problem with non-conflicting merges of moves

Stefan Sperling
During a recent discussion in #svn-dev IRC, Johan and I identified a
problem connected to merges of non-conflicting moves.
(http://colabti.org/irclogger/irclogger_log/svn-dev?date=2017-08-22#l81)

The new resolver in 1.10 makes sure that copyfrom info of a moved file
or directory points back into the natural history of the same branch.
It essentially makes the move appear as if the user had used 'svn move'
locally before running the merge which introduced an incoming move.
This is important because the resolver's move detection heuristic relies on it.
If copyfrom points to a different branch, rather than to the path which
represents the delete-half of the move, an incoming move won't be detected.

SVN 1.10 will still produce copyfrom pointing to another branch in
(at least) the following cases:

  1) The user runs a manual merge of the added half of a move to a branch

  2) A move is merged in a way that does not produce a tree conflict.

There is not much we can do about 1), and I don't think it's an
important case to address. We're not trying to be perfect, we are
trying to make the most common tree conflicts work well. The common
case I am concered about is where merges happen at 'branch roots'.

But scenario number 2) is worrying. It means that move detection will
fail if the merged move produces a tree conflict if it is merged again
to another branch.

For example, consider a moved directory on trunk:

r3 | stsp | 2017-08-24 13:10:55 +0200 (Thu, 24 Aug 2017) | 1 line
Changed paths:
   D /trunk/epsilon
   A /trunk/epsilon2 (from /trunk/epsilon:2)

Which is merged to ^/branch without a conflict (because ^/branch
has never changed 'epsilon'):

r6 | stsp | 2017-08-24 13:11:43 +0200 (Thu, 24 Aug 2017) | 1 line
Changed paths:
   M /branch
   D /branch/epsilon
   A /branch/epsilon2 (from /trunk/epsilon2:5)

If this merged move is now merged to ^/branch2, which is a branch of
^/branch and contains a text edit in file epsilon/zeta, the resolver
incorrectly sees an incoming deletion rather than an incoming move:

$ svn merge ^/branch
--- Merging r3 through r6 into '.':
   C epsilon
A    epsilon2
A    epsilon2/zeta
 U   .
--- Recording mergeinfo for merge of r3 through r6 into '.':
 G   .
Summary of conflicts:
  Tree conflicts: 1
Searching tree conflict details for 'epsilon' in repository:
Checking r6... done
Tree conflict on 'epsilon':
Directory merged from
'^/branch/epsilon@2'
to
'^/branch/epsilon@6'
was deleted by stsp in r6.
A directory which differs from the corresponding directory on the merge source branch was found in the working copy.
Select: (p) Postpone, (r) Mark as resolved, (i) Ignore incoming deletion,
        (a) Accept incoming deletion, (h) Help, (q) Quit resolution:


Note that the ' (m) Move and merge' option, which we would want here,
is not available because the move is not detected.

The problem from the resolver's point of view is that r6 looks like this:

   D /branch/epsilon
   A /branch/epsilon2 (from /trunk/epsilon2:5)

instead of this:

   D /branch/epsilon
   A /branch/epsilon2 (from /branch/epsilon2:5)

I see the following options going forward:

 1) Postpone the above problem and declare it as an unsupported resolver
    use case for 1.10. The resolver won't handle it in 1.10, perhaps later.
   
 2) Try to make the resolver handle the case where copyfrom does not point
    back into the same branch. (How?)

 3) Always flag a tree conflict for every incoming deletion during a merge.
    Make the resolver fix up copyfrom for such moves, and make it auto-resolve
    any trivial/actual deletions. This has performance implications because
    merges will now run the resolver even if there was no actual conflict.

 4) Teach 'svn merge' to fix up copyfrom after a merge without help from the
    resolver. This can be done but has the same performance problem.
    It also becomes tricky if moves are nested (cases like 'svn mv A/ B/'
    followed by 'svn mv C/foo.txt B/'). The resolver uses nested tree-conflicts
    to resolve such situations step by step, but the expectation here would be
    that no conflicts are raised at all.
    I expect that this would have to share some code with the resolver anyway.
    I have started exploring this idea on the addremove branch because the
    'addremove' feature will have to solve a similar problem of matching up
    deletions and adds/copies in order to record local moves.
   
 5) Make the server send copyfrom args again (as it did in 1.5 times for
    files during 'svn update'), but only during merges/(diffs?) and for
    files and directories, with a heuristic that sends the correct copyfrom
    source to the client. Allows 'svn merge' to record "correct" copyfrom
    in the WC without overhead and conflicts, but implies additional
    server-side processing. Also requires RA protocol changes.
    My impression is that some of the code which was reverted in r1597989
    could be used for making this work.
    This is probably a lot of effort but could be worth it.

Any other ideas?

What do you think we should do?
Reply | Threaded
Open this post in threaded view
|

Re: Problem with non-conflicting merges of moves

Daniel Shahaf-2
Stefan Sperling wrote on Thu, 24 Aug 2017 13:44 +0200:
>  5) Make the server send copyfrom args again (as it did in 1.5 times for
>     files during 'svn update'), but only during merges/(diffs?) and for
>     files and directories, with a heuristic that sends the correct copyfrom
>     source to the client. Allows 'svn merge' to record "correct" copyfrom
>     in the WC without overhead and conflicts, but implies additional
>     server-side processing. Also requires RA protocol changes.
>     My impression is that some of the code which was reverted in r1597989
>     could be used for making this work.
>     This is probably a lot of effort but could be worth it.

Doesn't directly answer your question, but: If we do end up making the
server send heuristic guesses to the client, could we please have them
*explicitly* marked as such, so API consumers don't mistaken the
heuristic guess for an accurate answer.

Cheers,

Daniel
Reply | Threaded
Open this post in threaded view
|

Re: Problem with non-conflicting merges of moves

Stefan Sperling
In reply to this post by Stefan Sperling
On Thu, Aug 24, 2017 at 01:44:48PM +0200, Stefan Sperling wrote:
> I see the following options going forward:

>  2) Try to make the resolver handle the case where copyfrom does not point
>     back into the same branch. (How?)

I have implemented option 2) in r1806831.

It seems to work, but additional testing would be appreciated.

While doing random manual tests I saw some cases where the resolver
correctly identifies the move but then offered no useful resolution option.
I have not yet investigated why this is happening. I suspect we are
simply lacking conflict option implementations for these cases.
Reply | Threaded
Open this post in threaded view
|

Re: Problem with non-conflicting merges of moves

Johan Corveleyn-3
On Thu, Aug 31, 2017 at 7:47 PM, Stefan Sperling <[hidden email]> wrote:
> On Thu, Aug 24, 2017 at 01:44:48PM +0200, Stefan Sperling wrote:
>> I see the following options going forward:
>
>>  2) Try to make the resolver handle the case where copyfrom does not point
>>     back into the same branch. (How?)
>
> I have implemented option 2) in r1806831.

Awesome!

--
Johan