[PATCH] Fix issue #4316 - Merge errors out after resolving conflicts
From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Fri, 15 Mar 2013 22:03:51 +0000 (GMT)
The attached patch fixes issue #4316 'Merge errors out after resolving conflicts'.
I have a particular question about the notifications printed after applying this patch -- see the end of this mail.
The patch makes the libsvn_client merge function call the resolver for all conflicts raised while merging one revision range, and then continue with the merge of the next revision range if all the conflicts were resolved.
In a previous attempt I started to make 'svn' repeat the merge after resolving any conflicts, if the merge had bailed out early because of conflicts. Brane suggested that made for a poor merge API if the caller has to be prepared to figure out how much of the merging was done and how much more needs to be done and to call the merge again itself. Better for the merge API to do everything itself.
Of course the caller still has the possibility of *not* resolving conflicts, and in that case the merge is still going to bail out. But at least we have a good solution for the case where the user *does* resolve all the conflicts.
NOTIFICATIONS CHANGED
As I mentioned in my "Conflict resolver callback design" email, doing this does mean that the notification receiver will get a 'C' (conflict) notification for every conflict even if that conflict is going to be resolved to a pre-determined choice. In terms of the 'svn' client and the 'svn merge' command, this means that 'svn merge --accept=[mine-full, etc.]' will, if we don't take further action, print something like in this example:
[[[
--- Merging r3 through r4 into 'merge_tests-135/A2/mu':
C merge_tests-135/A2/mu
--- Recording mergeinfo for merge of r3 through r4 into 'merge_tests-135/A2/mu':
G merge_tests-135/A2/mu
Resolved conflicted state of 'merge_tests-135/A2/mu'
--- Merging r6 through r8 into 'merge_tests-135/A2/mu':
U merge_tests-135/A2/mu
--- Merging r10 through r11 into 'merge_tests-135/A2/mu':
U merge_tests-135/A2/mu
--- Recording mergeinfo for merge of r5 through r11 into 'merge_tests-135/A2/mu':
G merge_tests-135/A2/mu
Summary of conflicts:
Property conflicts: 1
]]]
I think if this was changed to print a slightly different summary, something like...
[[[
Summary of conflicts:
Property conflicts: 0 (and 1 already resolved)
]]]
... then it would be fine. I don't see that the interleaved 'Resolved ...' line is a problem.
Do others agree?
If so, I'll make it so. I would make the logic something like, for each kind of conflicts: if (number-remaining > 0 || number-already-resolved > 0) { print 'Property conflics: <number-remaining>'; if (already-resolved > 0) print ' (and <number-already-resolved> already resolved)'; }.
I notice that if 'svn update' or 'switch' is invoked with
'--accept=[mine-conflict, etc.]' then currently it produces first the
general update notifications including any 'C' status for conflicts,
then a 'Summary of conflicts', and after that it proceeds to resolve the conflicts and prints 'Resolved ...' for each one. I think (again as mentioned in that other email) it might make sense to move the calling of the resolver into the libsvn_client functions.
Thoughts? Any review of the patch is welcome too.
- Julian
--
Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
|
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.