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

RFC: new conflict callback API

From: Stefan Sperling <stsp_at_elego.de>
Date: Wed, 16 May 2012 13:25:12 +0200

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.

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 13:25:55 CEST

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.