On Thu, Jan 24, 2013 at 5:16 PM, Julian Foad <[email protected]> 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@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
r2
r3
r4
>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':
svn:mergeinfo
/iota:3
### 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':
svn:mergeinfo
/iota:2-6
### 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
