> -----Original Message-----
> From: Stefan Sperling [mailto:stsp_at_elego.de]
> Sent: woensdag 16 mei 2012 13:25
> To: dev_at_subversion.apache.org
> Subject: RFC: new conflict callback API
>
> The current conflict callback API passes a svn_wc_conflict_description2_t
> struct to the callback, and allows the callback to freely return any
> possible value of svn_wc_conflict_choice_t as a result.
>
> This works fine for text and prop conflicts, which is what this API was
> designed for. However, if we want to allow interactive resolution of
> tree conflicts, the above approach is too simplistic.
>
> With tree conflicts, some choices might not make any sense, or we might
> not be able to satisfy a choice even if the choice is a reasonable one.
> For example, if the resolution choice requires a WC->WC merge, as
described
> in the example at
> https://svn.apache.org/repos/asf/subversion/branches/moves-scan-
> log/BRANCH-README,
> then we currently have no way of moving the working copy into the
> desired target state. Until we can do that, the conflict callback
> should not offer this option to the user.
>
> Also, we might need additional, and more complex, options that aren't
> represented by svn_wc_conflict_choice_t. Some contrived examples:
> - "specify a local new name for an incoming file"
> - "apply incoming edits to file <foo>"
> - "use the incoming name for the local move and apply edits"
> - "use the incoming name for the local move but do not apply edits"
> - "move all children of subtree X into subtree Y"
> I'm not saying that we'll have any of these options at some point.
> I just want to illustrate that the current svn_wc_conflict_choice_t
> concept is insufficient for dealing with tree conflicts.
>
> What I'd like to do is create a new conflict callback API and deprecate
> the existing one. This new API would differ in some significant ways.
>
> Most importantly, I'd like to move any resolution logic out of the
callback
> and into the client library. The new conflict callback API would be a set
> of callbacks which concern itself primarily with presenting possible
choices
> or other information to the user, passing user responses back into the
> client library.
Note that the simple view of this model doesn't match GUI clients
expectations.
GUI's can stay inside a callback and keep the gui functional for other
operations, so callbacks can only be handled by application-modal dialogs
(You can't do something else).
That is why most GUI's use svn status to determine the commit targets and
then pass a list of targets to the API, to specify what to commit.
Please make sure the new api also handles these scenarios, instead of just
optimizing for the callback case.
The recent changes would apply 3 separate resolve operations for a node when
you call 'svn resolved X' on a tree+text+property conflicted node, while the
wc_db api can resolve them in a single db transaction. (This is done in the
step where the accept argument is verified).
I wouldn't have a problem if at the libsvn_wc level we create a simple
callback that just says something like: 'This local_abspath is conflicted.
Resolve it now if you like'. The callback can then use the normal apis to
examine and resolve the conflict using the same svn_wc_context_t (and
svn_client_ctx_t).
The api can then also show the property and text conflicts on a node at the
same time, which in the current resolver require two operations.
On the libsvn_client layer we can then wrap this api with something more
friendly to 'svn' and/or GUI tools.
Bert
>
> Implementations of this set of callbacks would need to concern themselves
> only with presentation issues, not with any actual conflict resolution
logic.
> The callbacks would not receive a conflict description struct (i.e. we're
not
> exposing the internal representation of conflicts anymore). They would
receive
> the set of possible options for each conflict instead.
>
> Something like:
>
> /* The set of all conflict choices for all kinds of conflicts. */
> enum svn_client_resolution_choice_t {
> svn_client_resolution_choice_mark_resolved,
> svn_client_resolution_choice_theirs_full,
> svn_client_resolution_choice_mine_full,
> svn_client_resolution_choice_theirs_conflict,
> svn_client_resolution_choice_mine_conflict,
> [...]
> svn_client_resolution_choice_diff_full,
> svn_client_resolution_choice_edit_file,
> [...]
> svn_client_resolution_choice_scan_log_for_moves,
> svn_client_resolution_choice_specify_local_target_for_incoming_move,
> svn_client_resolution_choice_merge_edits_into_local_file,
> [...]
> };
>
> /* An option the user may pick in response to a conflict prompt. */
> struct svn_client_resolution_option_t {
> /* The choice the user is making by picking this option. */
> svn_client_resolution_choice_t choice;
>
> /* Brief and human-readable option description the UI should display.
*/
> const char *option_desc;
> };
>
> /* A conflict description the UI should present and get an answer to. */
> struct svn_client_conflict_description_t {
> /* The conflict victim file or directory. */
> const char *victim_abspath;
>
> /* Is this a text, prop, or tree conflict? */
> svn_wc_conflict_kind_t conflict_kind;
>
> /* Brief human-readable conflict description the UI should display. */
> const char *conflict_desc;
>
> /* An array of svn_client_resolution_option_t objects, one of
> which the user can pick to continue the resolution process.
> This array is sorted by the numeric value of
conflict_option->choice
> so that all Subversion clients list these options in the same
order. */
> apr_array_header_t *conflict_options;
> };
>
> The client library would then implement a conflict-resolution state
machine
> that is driven by conflict callbacks. The callbacks are invoked to
transition
> between states. They can both get user input or present information
requested
> by the user. E.g. the user might request to see a diff or edit a file,
> if that is one of the possible answers presented at the conflict prompt.
>
> Here is a rough sketch of the callbacks based on what I think we might
need.
> This would obviously be refined during further design and implementation.
>
> struct svn_client_conflict_resolution_callbacks_t {
> /* Invoked when a new conflict resolution process is started.
> The NUM_* parameters indicate the number of unresolved conflicts
> in the working copy, per conflict type. This callback can be used
> by implementors to show a conflict summary to the user, and to
> initialise any required state for a new resolution process. */
> svn_error_t *(*start_resolution)(void *baton,
> int num_text_conflicts,
> int num_prop_conflicts,
> int num_tree_conflicts,
> apr_pool_t *scratch_pool);
>
> /* Present one conflict and possible options to the user.
> Return the user's choice in *CHOICE. */
> svn_error_t *(*get_choice)(void *baton,
> svn_client_resolution_choice_t *choice,
> svn_client_conflict_description_t desc,
> apr_pool_t *scratch_pool);
>
> /* Generic prompt function that used to obtain a string of input from
> the user. Implementors should show the prompt to the user and
return
> the answer in *ANSWER, allocated in RESULT_POOL. */
> svn_error_t *(*get_input)(void *baton,
> const char **answer,
> const char *prompt,
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool);
>
> /* Generic prompt function that used to obtain a local filename from
> the user. Implementors should show the prompt to the user and
return
> the answer in *ANSWER, allocated in RESULT_POOL. The UI might use
> a file selection dialog, or tab-completion, to enhance convinence.
*/
> svn_error_t *(*get_local_filename)(void *baton,
> const char **filename,
> const char *prompt,
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool);
>
> /* Show a diff between LOCAL_ABSPATH1 and LOCAL_ABSPATH2 to the user.
> The diff can either be read directly from DIFF_OUTPUT, or the
> callback may choose to create the diff itself (e.g. by invoking a
> third-party diff tool). */
> svn_error_t *(*show_diff)(void *baton,
> const char *local_abspath1,
> const char *local_abspath2,
> svn_stream_t *diff_output,
> apr_pool_t *scratch_pool);
>
> /* Same as above, but for a 3-way diff. */
> svn_error_t *(*show_diff3)(void *baton,
> const char *local_abspath1,
> const char *local_abspath2,
> const char *local_abspath3,
> svn_stream_t *diff_output,
> apr_pool_t *scratch_pool);
>
> /* Allow the user to make arbitrary edits to the file at
LOCAL_ABSPATH.
> This may or may not be a temporary file. The callback should not
> delete the file. */
> svn_error_t *(*edit_file)(void *baton,
> const char *local_abspath,
> apr_pool_t *scratch_pool);
>
> /* The resolution process has ended, zero or more conflicts were
> resolved, as indicuted by NUM_RESOLVED_* parameters.
> Implementors can dispose of resources in this function. */
> svn_error_t *(*end_resolution)(void *baton,
> int num_resolved_text_conflicts,
> int num_resolved_prop_conflicts,
> int num_resolved_tree_conflicts,
> apr_pool_t *scratch_pool);
> };
>
> This callback table would probably live in the svn_client_ctx_t structure.
>
> Note that this approach will unify conflict resolution behaviour across
all
> Subversion clients. Currently, each client implements a custom set of
> possible conflict resolution choices and also some resolution logic.
> In particular, there is very little cohesion between the UIs clients
> present during tree conflict resolution. For example, TSVN sometimes
> offers to merge changes into a different file, while the 'svn' client does
> no such thing. Subclipse has its own set of conflict dialogs as well.
>
> Also, the API allows us to incrementally improve the set of options we
show
> to users, without requiring clients to change their callbacks. This is
> quite important for incremental improvements in tree conflict resolution
> over the next few Subversion releases.
>
> The API leaves room for improving interactive resolution for text and
> property conflicts as well. For example, the client library could call
> get_choice() or edit_file() for each conflicted hunk in a file, allowing
> user the pick "mine" or "theirs" or edit individual conflicted parts of
> a file, rather than always applying such choices to the entire file.
> (Of course, this could be done today in the conflict callback of the 'svn'
> clients, but not in a client-independent manner.)
>
> I'd like to know if there any suggestions or concerns regarding this
approach.
> I'd be particularly interested in input from GUI client developers.
> Thanks!
Received on 2012-05-16 14:00:28 CEST