[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [RFC] Run the conflict resolver callback per file during merge - issue #4238

From: Paul Burba <ptburba_at_gmail.com>
Date: Fri, 25 Jan 2013 12:11:53 -0500

On Thu, Jan 24, 2013 at 5:16 PM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> Summary: I plan to make 'svn merge' act on --accept=theirs|mine|etc. per file, rather than (as it does now) postponing all conflicts until after the whole merge and then resolving them.
> I have been investigating the way "--accept=foo" resolves conflicts, for blocker issue #4238 "merge -cA,B with --accept option aborts if rA conflicts" [1]. Postponing all resolution until the end of the whole merge (as we do now) is not good enough in this scenario. A similar scenario can occur during an automatic merge when the merge code internally decides to do a multi-revision-range merge.

Minor nit: The internal split you speak of here isn't tied solely to
automatic merges. Cherry-picks can provoke that behavior just as
easily. For example:

svn merge SRC TGT -cY
svn merge SRC TGT -rX:Z

Where X < (Y-1) < Y < Z

The second merge will be split into two parts: X:(Y-1), Y:Z

I mention this only because the issue #4238 scenario of

  svn merge -cM,N

Could just as easily be,

  svn merge -rW:X,Y:Z

and either one of those ranges could be split into multiple merges.

> We need to be able to resolve conflicts at least after each phase (revision range) of merge.
> For the manual '-cA,B' example the client ('svn') could simply call svn_client_merge once for each specified revision or range, resolving any conflicts after each. That is not feasible for the ranges decided internally during an automatic merge, because the client doesn't have a way to discover those ranges in advance. (And the merge code architecture is such that it discovers those ranges incrementally as it goes.)

I just realized another problem for automatic (implied) or cherry-pick
(explicit) ranges in which a given range is split into multiple merges
under the covers: We record mergeinfo based on the requested merge
range, even if only part of the range is merged before we hit a
conflict and abort. This prevents the remainder of the range from
being merged if the conflict is resolved and the merge repeated.

A simple example (borrowing the WC from the issue #4238 test prior to
the merge):

### Copy a file:

>svn copy iota_at_1 iota-new
A iota-new

>svn ci -m "" -q

### Merge a rev from the middle of all eligible revs and commit:

>svn mergeinfo --show-revs eligible ^^/iota iota-new

>svn merge ^^/iota iota-new -c3 --accept theirs-conflict
--- Merging r3 into 'iota-new':
C iota-new
--- Recording mergeinfo for merge of r3 into 'iota-new':
 U iota-new
Summary of conflicts:
  Text conflicts: 1
Resolved conflicted state of 'iota-new'

>svn ci -m ""
Sending iota-new
Transmitting file data .
Committed revision 6.

>svn up -q

### The expected mergeinfo:

>svn pl -vR
Properties on 'iota-new':

### Do a merge which covers a range of revs which the previously merged
### rev is a proper subset of (nothing new here, this is just the "internally
### decided" range split Julian spoke of.

>svn merge ^^/iota iota-new
--- Merging r2 into 'iota-new':
C iota-new
--- Recording mergeinfo for merge of r2 through r6 into 'iota-new':
 U iota-new
Summary of conflicts:
  Text conflicts: 1
Conflict discovered in file 'iota-new'.
Select: (p) postpone, (df) diff-full, (e) edit, (m) merge,
        (mc) mine-conflict, (tc) theirs-conflict, (s) show all options: p
..\..\..\subversion\svn\util.c:553: (apr_err=155015)
..\..\..\subversion\svn\merge-cmd.c:138: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11611: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11582: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:9057: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:4715: (apr_err=155015)
svn: E155015: One or more conflicts were produced while merging r1:2 into
'\iota-new' --
resolve all conflicts and rerun the merge to apply the remaining
unmerged revisions

>svn st
CM iota-new
? iota-new.merge-left.r1
? iota-new.merge-right.r2
? iota-new.working
Summary of conflicts:
  Text conflicts: 1

### Obviously we only merged r2 before things blew up, but
### not according to the resulting mergeinfo :

>svn pl -vR
Properties on 'iota-new':

### Obviously this means if we resolve the conflict...

>svn resolved -R .
Resolved conflicted state of 'iota-new'

### ...and repeat the merge, then r4 stubbornly remains un-merged:

>svn merge ^^/iota iota-new

### Even if we explicitly ask for r4:

>svn merge ^^/iota iota-new -c4

### We have to disable merge tracking and use a cherry-pick to DTRT
### (i.e. get r4 applied) which is, to put it charitably, less than optimal:

>svn merge ^^/iota iota-new -c4 --ignore-ancestry
--- Merging r4 into 'iota-new':
C iota-new
Summary of conflicts:
  Text conflicts: 1
Conflict discovered in file 'iota-new'.
Select: (p) postpone, (df) diff-full, (e) edit, (m) merge,
        (mc) mine-conflict, (tc) theirs-conflict, (s) show all options: p


FWIW this isn't a regression from 1.7. I'll file an issue and add a
test shortly.

> I suppose the client could run the merge, let the merge abort if it finds any conflicts during a phase, resolve those conflicts, and then run the same merge again, repeating until the merge is complete. That may work if the merge code can be run again and again... but it isn't set up to do so in general. (For one thing, local mods aren't tolerated in all cases, and obviously there will be local mods after the first phase has run. I bet there would be other problems too.)

Look no further than the above example :-\

> I think a good enough solution for 1.8 is this:
> * With --accept=postpone, if any conflicts are raised during a merge phase (one revision range), then abort the merge after that phase. (The details are not relevant to this mail thread.)
> * Otherwise, the user requested a specific conflict resolution. At the moment, we run the merge with a per-node resolver callback installed that always chooses 'postpone' but also collects a list of the conflicted paths, and then we run 'resolve' on those paths afterwards. Instead, let
> the resolver callback be active during the merge, resolving each
> conflict on a node immediately after that conflict has been raised, according to '--accept'.
> * By resolving a conflict so soon, we can avoid notifying the merge result as 'C' for conflict and instead notify 'U' or 'G' for that node, and avoid "Resolved conflict on 'foo'" lines. I think that is better.
> * After the merge, there might still be some postponed conflicts, depending on whether the implementation of the chosen --accept was capable of handling every conflict that occurred. We might want to consider doing something like running the interactive resolver afterwards if this happens, but I think not.
> The main alternative I can think of to this plan would be to design a way to store multiple
> conflicts (of the same kind) per node and so be able to postpone all conflicts from multiple phases of merge. We have decided before that that
> is too difficult and I still think so.
> So, can you see any problems with the above approach?

Other than noted above no.

Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
> I think we were doing this at one stage during development.  I assume we stopped because (a) for interactive resolution, postponing that stage until after the merge avoids network timeouts due to waiting for the user; and (b) we wanted to test the ability to postpone all conflicts and/or there seemed to be no reason to make the '--accept=' case work differently from the interactive case.
> - Julian
> [1] <http://subversion.tigris.org/issues/show_bug.cgi?id=4238>
> --
> Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
Received on 2013-01-25 18:12:28 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.