A long email, for folks interested in conflict resolution.
I have been evaluating our conflict resolution callback support, partly in the
course of studying how to provide better conflict resolution, and partly in the
course of issue #4316 -- fixing 'merge' so that it continues to the next
revision range if you interactively resolve all conflicts.
In this email I analyze the current state of support for the conflict
resolution callback design and suggest ways in which it could be improved. I
haven't mentioned everything here -- for example the several known problems
with the current "conflict description" data type.
At the end I talk about how this impacts a fix for issue #4316.
Don't panic -- I'm not suggesting we should hold up 1.8 for any of this, and I
haven't written a concrete proposal for such changes at this point. With that
in mind, comments welcome.
INTRODUCTION
We have a conflict resolver callback function ('CB') defined as:
/** A callback used in merge, update and switch for resolving conflicts
* during the application of a tree delta to a working copy. [...] */
typedef svn_error_t *(*svn_wc_conflict_resolver_func2_t)(
svn_wc_conflict_result_t **result,
const svn_wc_conflict_description2_t *description,
baton, result_pool, scratch_pool);
The rough idea is that the client's callback is called every time a conflict is
raised, and can decide whether to postpone it (record it in the WC and move
on), or resolve the conflict according to some pre-determined fixed choice or
set of rules, or let the user interactively resolve it.
=== Pre-conflict and post-conflict operating modes.
The problem is basically that we have confused two different operating modes --
with different purposes and requirements -- for such a callback:
(1) Sometimes we call the CB before raising a conflict, in order to decide
whether we need to record a conflict or can immediately resolve it.
(2) Other times we record a conflict in the WC and then, later, we call the
CB as a way of presenting the stored conflict to the client. In this case we
want the user to be offered a range of interactive options including examining
the conflict and the WC and making changes to the WC.
If we like the "hook" terminology, we could call mode 1 a "pre-conflict hook"
and mode 2 a "post-conflict hook". Both of these modes have their uses.
Mode 1 is viable for a text or prop conflict, since the result can reasonably
be assumed to be a single file or property value that is to be installed at the
current path. With a tree conflict, however, the result is expected to be at
least a tree change at the current path, and possibly (especially when moves
are involved) a tree change at another path (or paths). A tree change
generally requires using the WC APIs to build the result incrementally, not
just returning a single object and asking the WC to install it.
In mode 1, the CB needs to return quickly because we are in the middle of an
update, switch or merge, with a network connection open, and we haven't yet
recorded the conflict or completed the update (or switch or merge) so the new
WC state is not complete (for example the incoming change might be half of a
move and we might not have received the other half of the move yet). For both
of these reasons, it is not appropriate to offer interactive resolution. It
may not be feasible to offer all forms of tree conflict resolution, especially
where a move is involved.
Advantages of mode 1 include: it avoids sending 'C' notifications; it may
result in less CPU and network load. The avoidance of 'C' notifications may be
important for improving signal to noise ratio, in large scale usage. It is not
necessarily a significant advantage, since the notification receiver should be
able to process them and filter them out if required, but that may add unwanted
complexity to the receiver.
In mode 2, the update or switch can be complete before we start resolving, or
in a merge of multiple revision ranges, at least one revision range can be
completely merged.
Mode 2 doesn't need a callback function to implement it: we could instead
provide a "pull" interface for the client to request information about the
conflicts and
Advantages of mode 2 include: plenty of time for interactive resolution; all
the WC state is available to be examined; client can implement whatever logic
it likes to resolve a conflict rather than having to provide one of a limited
number of answers; can operate on more than one node and so is suitable for
tree conflicts and conflicts involving moves.
When the callback is called, these conditions should be well defined ...
- The conflict description provided.
- Whether the WC write lock is held.
- Whether the conflict is already recorded in the WC.
- The WC state of the victim path.
- The WC state of other related paths (such as parent, children, move-to).
- What the callback is allowed to do to the WC (queries, changes) in order to
resolve the conflict.
... but currently are not.
ANALYSIS
=== Where is it called?
CB is called during up/sw/merge/resolve.
Passed through 'ctx' to these libsvn_client (public/inter-library) functions:
svn_client_update
svn_client_switch
svn_client_merge
svn_client_resolve
Passed by parameters to these libsvn_wc(public/inter-library) functions:
svn_wc_merge5
svn_wc_merge_props3
svn_wc__get_update_editor
svn_wc__get_switch_editor
svn_wc__get_file_external_editor
svn_wc__resolve_conflicts
up/sw/merge: Library functions call CB only for text and prop conflicts.
resolve: Library functions call CB for all conflicts.
up/sw/merge: Library functions call CB *before* installing a conflict
description in the WC DB (mode 1) -- at least for text & prop conflicts; not
sure about local-move tree conflicts.
resolve: Library functions call CB *after* installing a conflict description in
the WC DB (mode 2).
'svn update' (and 'svn switch') currently uses mode 2. When it calls the
libsvn_client update function, it passes in a (mode 1) CB that simply records
the paths and postpones the conflict resolution. After the update is complete,
it then calls 'resolve' on each recorded conflicted path (mode 2).
'svn merge' currently uses a mixture of mode 1 and mode 2. When interactive
resolution is requested, it works the same way 'update' does; otherwise it uses
mode 1 for text and prop conflicts and (AFAICS) mode 2 for tree conflicts.
=== Is the conflict description consistent?
up/sw/merge: CB is passed a conflict description.
resolve: CB is passed a *different* description for the same conflict, in some
cases (notably, property conflicts).
I modified 'svn' so that it ran a merge using a mode 1 callback (for text and
prop conflicts) that records each conflict description, and then ran 'resolve'
with a callback that compares each conflict description with the corresponding
one that was recorded earlier. There are big differences in the property
conflict descriptions. This is a symptom of using different code paths to
generate the descriptions.
=== The WC write lock
up/sw/merge: CB is called while there is a WC write lock on the 'victim' path.
resolve: CB is called while there is a WC write lock covering 'all possible
paths affected by resolving' the conflicts in the given tree.
=== What may the CB do with the WC?
It's not clear what the CB is allowed to do to the WC.
=== The 'adds_as_modification' flag
Some existing APIs have an 'adds_as_modification' flag, which says don't raise
a tree conflict for add-onto-add (if the node kind is the same), but instead
resolve the potential conflict by collapsing the two adds and treating the
local add as the desired new value, so the result looks like a modification --
or non-modification -- of the incoming added node.
This flag is an example of a "mode 1" pre-determination for certain kinds of
conflict. This kind of functionality should be provided by a mode-1 conflict
resolution callback.
It is present in: svn_client_update4(), svn_client__update_internal(),
svn_wc__get_update_editor().
MERGE WITH INTERACTIVE RESOLUTION
=== Issue #4316 'Merge errors out after resolving conflicts'
The interactive resolution for 'svn merge' is broken: it doesn't go on to merge
the remaining revision ranges even if you resolve all the conflicts -- issue
#4316.
In order to fix that, I have a patch which makes svn_client_merge*() call the
CB for all conflicts raised, after merging each revision range, and then go on
to the next revision range. Merge will no longer ever make "mode 1" calls to
the CB, always mode 2.
For consistency I would consider doing the same kind of thing to 'update' and
'switch' APIs -- that is, make them call the CB at the end of the operation,
for each conflict that was raised, instead the 'svn' client running the update
with a 'postpone' CB installed and then running 'resolve' afterwards. I
haven't yet planned to do this but it would seem to be the sensible thing.
=== Notifications
This changes the notifications that are produced by a merge when using a fixed
conflict resolution option such as '--accept=mine-full'. Now, even these
conflicts will result in a 'C' notification being sent to the notification
receiver, and later a 'Resolved conflicts on xxx' notification if the CB
resolves the conflict. I will attempt to patch the notification receiver in
'svn' to notice if some or all of the reported conflicts were resolved, and at
least make the summary of conflicts say something intelligent about that.
NEXT STEPS
At some point after fixing #4316, and subject to feedback, I intend to follow
up with a more concrete proposal for how we can revise our API support for the
conflict resolution callback, to recognize the difference between mode 1 and
mode 2. We should provide two very different callbacks or mechanisms,
something very minimal or nothing at all for mode 1, and something rather more
flexible than we have for mode 2.
I'm not suggesting any of this should happen before 1.8, but now is the time to
say if you think it should.
Thoughts?
- Julian
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
