You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2012/05/16 13:25:12 UTC
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.
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!
Re: new conflict callback API
Posted by Julian Foad <ju...@btopenworld.com>.
Hi Stefan.
I'm just about starting to get the idea that I think you're talking about, but I'm still having a hard time seeing how a state machine in the library would need more than one state transition for any chosen resolution, or why it would need to control information-display requests.
I think it would help if we could see how these ideas work, step by step, for some non-trivial use case.
Stefan Sperling wrote:
> On Thu, May 17, 2012 at 11:09:48AM +0100, Julian Foad wrote:
>> Let's take a closer look at what the user (through the client
>> application) needs to be able to do.
>
> Thanks for your comments. I think we're about to each consensus
> on this. We just have a slight misunderstanding to clear up, and
> need to agree on whether the main resolution logic should be
> implemented by clients themselves, or in the library.
>
>>> /* The set of all conflict choices for all kinds of conflicts. */
>>> enum svn_client_resolution_choice_t {
[...]
>>> svn_client_resolution_choice_theirs_conflict,
>>> svn_client_resolution_choice_mine_conflict,
>>
>> "theirs-conflict" and "mine-conflict" are good options for an
>> interactive 3-way merge tool to offer the user while showing the user
>> which sections of the file do have conflicts and which don't. It's not
>> an option I'd like to choose outside a 3-way merge tool. In terms of
>> tree conflicts, the same applies: I'd only be comfortable with this
>> option when it's presented in a 3-way directory-merge window that's
>> showing me which children are conflicting and which are not. After
>> pressing this button in the 3-way (file or dir) merge tool, I wouldn't
>> expect the tool to immediately close and Subversion library to
>> immediately finalize my choice, instead I'd expect to be able to review
>> and edit the result and then finally confirm.
>
> Agreed.
You agree that those options shouldn't be found outside a 3-way merge tool that's displaying the conflicts in question? So what are they doing in this list?
>>> [...]
>>> svn_client_resolution_choice_diff_full,
>>
>> "diff-full" isn't a resolution choice, it's the user asking to see
>> more information before choosing.
>
> There seems to be a slight misunderstanding here.
>
> I don't intend the values of this enum to represent "final"
> resolution choices.
OK, I'm glad to learn this was partly me misunderstanding your intentions.
> Rather, each choice allows the conflict
> resolver to make progress in the resolution process. Some
> options may result in a "final" resolution state where the
> conflict is resolved. Other options may simply allow the user
> to request information or carry out other actions that are
> in the set of permitted options of the current resolver state.
Perhaps you could give an example of one part of this state machine, that's sufficiently complex that a state machine is a useful way of achieving it. What's an example of when an information-display request such as "show me a diff between base and theirs" needs to be tied into the state machine?
> In technical terms, I think of the new resolver as a state machine,
> where each conflict choice triggers a state transition. The set of
> final states may either resolve the conflict or postpone resolution.
>
> Maybe we need a different name than svn_client_resolution_choice_t to
> avoid this misunderstanding? Note that it isn't called
> svn_client_final_resolution_choice_t :)
>
> Wherever you say "not a resolution" below, my above response applies.
>
>> We need to get away from the mode
>> of operation of the existing interactive resolver loop where the user
>> interacts my means of question-and-answer. That question-and-answer
>> loop should be in the presentation layer (svn), not in the library.
>
> Here I disagree. The loop we have today is a small state machine
> that runs within the 'svn' client. We need this loop in one form
> or another, either in the client or in the library. Today it is in
> the client, so each client has to reimplement this loop with a set of
> possible states and transitions. You seem to favour this model but I
> don't see any advantage in keeping it.
>
> I want to put as many aspects of the resolution process as possible
> into the library so that each client uses the same state machine to
> resolve conflicts. This makes it easier to work with several clients
> simultaneously because terminology and logic will be the same for all
> clients during the conflict resolution process. And it allows clients
> to implement conflict resolution dialogs with much less effort.
>
>>> svn_client_resolution_choice_edit_file,
>>
>> "edit-file" isn't a resolution choice that the library can then
>> "execute" in a black-box manner. Instead, the user wants the GUI to
>> help him/her prepare the final result with the aid of an editor, and
>> then the user may choose to confirm that resulting file as the final
>> resolved result. The resolution needs to be communicated to the Svn
>> lib in the form of a file, not "he chose 'edit' as the resolution".
>
> Yes, the library will receive a file.
>
> However, it will also have an internal resolver state that makes the
> acceptance of such a file a valid resolution choice. For example, it
> probably makes no sense to provide a file when resolving a directory
> vs. directory tree conflict. If the resolver knows the set of valid
> options for the conflict in question, it can reject such invalid input
> from clients. Your opinion seems to be that the decision about whether
> a choice is valid or not belongs into the client. I say that leads to
> unnecessary complexity and variance between client implementations.
>
>>> [...]
>>> svn_client_resolution_choice_scan_log_for_moves,
>>
>> Not a resolution. Instead, there should be a way for the GUI to get the
>> suggestion(s) of what moves might be applicable, and to offer those as
>> suggestions to the user, and then for the user to select maybe one of
>> the choices or maybe another resolution.
>
> Again, this is a good description of what I intend to do. It seems
> we're on the same page but you misunderstood my intentions.
>
>>> svn_client_resolution_choice_specify_local_target_for_incoming_move,
>>> svn_client_resolution_choice_merge_edits_into_local_file,
>>> [...]
>>> };
>>
>> A GUI wants the user to be in control. The first obvious thing is the
>> user should be able to select which file/dir to resolve next. If the
>> GUI wants to have a button that enters a loop that iterates over all
>> conflicts sequentially, that's it's business and we should make it
>> possible to do that, but we should not implement this loop in the Svn
>> lib.
>
> I realised after comments from Bert and Mark that we need to run the
> main loop of the resolution process within the client, not within the
> library. So I've ditched the callback-table idea in favour of a set of
> functions that receive a resolver state object which is opaque to the
> client (I'll elaborate on that in a different post). This way, the library
> can keep state and reject invalid or nonsense choices it receives
> from the client, and clients are in control of the overall flow.
>
>> The overall process the user goes through to resolve a particular
>> conflict needs to include a reviewing phase, an editing phase, and
>> finally a confirmation that the result is as the user wants it. How
>> does this fit with the idea of there being a point where the user
>> chooses a resolution such as "choose edit-file"?
>
> It fits because "choose edit-file" is just a state transition,
> rather than a final choice.
>
>> While in the reviewing phase of resolving any particular conflict, the
>> user should be able to bring up, on demand, a diff between 'mine' and
>> 'base' or 'mine:theirs' or 'base:theirs' or whatever he/she chooses,
>> jump to the location in their editor where the next text conflict
>> occurs, review the log messages on the source and/or target branch,
>> and so on. And all of this in parallel, in separate, non-modal
>> windows.
>
> Indeed, clients should be allowed to do all that. Do you think we even
> need to design the API such that clients can resolve multiple conflicts
> in parallel?
Yes.
> Or should we restrict clients to handle one conflict at
> a time?
No.
> This restriction might make it easier to implement the resolver.
> We could also make this restriction initially and lift it later.
You can only lift such a restriction if the initial architecture was designed for parallel resolving.
>> To enable this, the application needs access to the metadata
>> [...] like this:
>>
>> svn_node_kind_t node_kind;
>> svn_wc_conflict_kind_t kind;
>>
>> svn_wc_conflict_action_t action;
>> svn_wc_conflict_reason_t reason;
>> svn_wc_operation_t operation;
[...]
>> const svn_wc_conflict_version_t *src_left_version;
>> const svn_wc_conflict_version_t *src_right_version;
>>
>> ... which is an extract from svn_wc_conflict_description2_t.[...]
>
> This is the kind of data the resolver can use to determine a set of
> valid states and transitions for conflict resolution.
>
> The client will drive the state machine so just giving it an
> svn_wc_conflict_description2_t struct an insufficient client<->library
> interface. At the very least, we must also force clients to pass back
> in any state the resolver needs to keep.
>
> Does all that make sense?
Well, I'm not convinced yet.
- Julian
Re: new conflict callback API
Posted by Stefan Sperling <st...@elego.de>.
On Thu, May 17, 2012 at 11:09:48AM +0100, Julian Foad wrote:
> Let's take a closer look at what the user (through the client
> application) needs to be able to do.
Thanks for your comments. I think we're about to each consensus
on this. We just have a slight misunderstanding to clear up, and
need to agree on whether the main resolution logic should be implemented
by clients themselves, or in the library.
> > /* 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,
>
> "theirs-full" and "mine-full" are nice and simple: each choice leads to a
> single, definite result. If the user chooses one of these, the
> Subversion library can simply execute the user's decision with no
> further info or interaction needed.
Yes.
> > svn_client_resolution_choice_theirs_conflict,
> > svn_client_resolution_choice_mine_conflict,
>
> "theirs-conflict" and "mine-conflict" are good options for an
> interactive 3-way merge tool to offer the user while showing the user
> which sections of the file do have conflicts and which don't. It's not
> an option I'd like to choose outside a 3-way merge tool. In terms of
> tree conflicts, the same applies: I'd only be comfortable with this
> option when it's presented in a 3-way directory-merge window that's
> showing me which children are conflicting and which are not. After
> pressing this button in the 3-way (file or dir) merge tool, I wouldn't
> expect the tool to immediately close and Subversion library to
> immediately finalize my choice, instead I'd expect to be able to review
> and edit the result and then finally confirm.
Agreed.
> > [...]
> > svn_client_resolution_choice_diff_full,
>
> "diff-full" isn't a resolution choice, it's the user asking to see
> more information before choosing.
There seems to be a slight misunderstanding here.
I don't intend the values of this enum to represent "final" resolution
choices. Rather, each choice allows the conflict resolver to make
progress in the resolution process. Some options may result in a "final"
resolution state where the conflict is resolved. Other options may simply
allow the user to request information or carry out other actions that
are in the set of permitted options of the current resolver state.
In technical terms, I think of the new resolver as a state machine,
where each conflict choice triggers a state transition. The set of
final states may either resolve the conflict or postpone resolution.
Maybe we need a different name than svn_client_resolution_choice_t to
avoid this misunderstanding? Note that it isn't called
svn_client_final_resolution_choice_t :)
Wherever you say "not a resolution" below, my above response applies.
> We need to get away from the mode
> of operation of the existing interactive resolver loop where the user
> interacts my means of question-and-answer. That question-and-answer
> loop should be in the presentation layer (svn), not in the library.
Here I disagree. The loop we have today is a small state machine
that runs within the 'svn' client. We need this loop in one form
or another, either in the client or in the library. Today it is in
the client, so each client has to reimplement this loop with a set of
possible states and transitions. You seem to favour this model but I
don't see any advantage in keeping it.
I want to put as many aspects of the resolution process as possible into
the library so that each client uses the same state machine to resolve
conflicts. This makes it easier to work with several clients simultaneously
because terminology and logic will be the same for all clients during
the conflict resolution process. And it allows clients to implement
conflict resolution dialogs with much less effort.
> > svn_client_resolution_choice_edit_file,
>
> "edit-file" isn't a resolution choice that the library can then
> "execute" in a black-box manner. Instead, the user wants the GUI to
> help him/her prepare the final result with the aid of an editor, and
> then the user may choose to confirm that resulting file as the final
> resolved result. The resolution needs to be communicated to the Svn lib
> in the form of a file, not "he chose 'edit' as the resolution".
Yes, the library will receive a file.
However, it will also have an internal resolver state that makes the
acceptance of such a file a valid resolution choice. For example, it
probably makes no sense to provide a file when resolving a directory
vs. directory tree conflict. If the resolver knows the set of valid
options for the conflict in question, it can reject such invalid input
from clients. Your opinion seems to be that the decision about whether
a choice is valid or not belongs into the client. I say that leads to
unnecessary complexity and variance between client implementations.
> > [...]
> > svn_client_resolution_choice_scan_log_for_moves,
>
> Not a resolution. Instead, there should be a way for the GUI to get the
> suggestion(s) of what moves might be applicable, and to offer those as
> suggestions to the user, and then for the user to select maybe one of
> the choices or maybe another resolution.
Again, this is a good description of what I intend to do. It seems
we're on the same page but you misunderstood my intentions.
> > svn_client_resolution_choice_specify_local_target_for_incoming_move,
> > svn_client_resolution_choice_merge_edits_into_local_file,
> > [...]
> > };
>
> A GUI wants the user to be in control. The first obvious thing is the
> user should be able to select which file/dir to resolve next. If the
> GUI wants to have a button that enters a loop that iterates over all
> conflicts sequentially, that's it's business and we should make it
> possible to do that, but we should not implement this loop in the Svn
> lib.
I realised after comments from Bert and Mark that we need to run the
main loop of the resolution process within the client, not within the
library. So I've ditched the callback-table idea in favour of a set of
functions that receive a resolver state object which is opaque to the
client (I'll elaborate on that in a different post). This way, the library
can keep state and reject invalid or nonsense choices it receives
from the client, and clients are in control of the overall flow.
> The overall process the user goes through to resolve a particular
> conflict needs to include a reviewing phase, an editing phase, and
> finally a confirmation that the result is as the user wants it. How
> does this fit with the idea of there being a point where the user
> chooses a resolution such as "choose edit-file"?
It fits because "choose edit-file" is just a state transition,
rather than a final choice.
> While in the reviewing phase of resolving any particular conflict, the
> user should be able to bring up, on demand, a diff between 'mine' and
> 'base' or 'mine:theirs' or 'base:theirs' or whatever he/she chooses,
> jump to the location in their editor where the next text conflict
> occurs, review the log messages on the source and/or target branch,
> and so on. And all of this in parallel, in separate, non-modal
> windows.
Indeed, clients should be allowed to do all that. Do you think we even
need to design the API such that clients can resolve multiple conflicts
in parallel? Or should we restrict clients to handle one conflict at
a time? This restriction might make it easier to implement the resolver.
We could also make this restriction initially and lift it later.
> To enable this, the application needs access to the metadata about the conflict. This metadata will include at least the WC location or repository location (URL, rev, etc.) of each of the three files/dirs involved. It needs to be sufficient to be able to display any required diffs, displaying friendly file
> names (not the names of temp files) and labels (e.g. "Mine", "Theirs").
>
> In short, it needs information like this:
>
> svn_node_kind_t node_kind;
> svn_wc_conflict_kind_t kind;
>
> svn_wc_conflict_action_t action;
> svn_wc_conflict_reason_t reason;
> svn_wc_operation_t operation;
>
> const char *base_abspath;
> const char *their_abspath;
> const char *my_abspath;
> const char *merged_file;
>
> const svn_wc_conflict_version_t *src_left_version;
> const svn_wc_conflict_version_t *src_right_version;
>
> ... which is an extract from svn_wc_conflict_description2_t. Certainly we don't want the data to be presented in an "svn_wc_" struct, but I can't get away from the fact that that's the kind of metadata we need.
This is the kind of data the resolver can use to determine a set of
valid states and transitions for conflict resolution.
The client will drive the state machine so just giving it an
svn_wc_conflict_description2_t struct an insufficient client<->library
interface. At the very least, we must also force clients to pass back
in any state the resolver needs to keep.
Does all that make sense?
Re: new conflict callback API
Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Sperling wrote:
> On Wed, May 16, 2012 at 02:52:06PM +0100, Julian Foad wrote:
>> OK, I appreciate that issue. I see two ways the client lib can help:
>>
>> 1. providing a set of client-lib APIs to perform the most commonly
>> wanted resolutions;
>>
>> 2. providing a set of (APIs? comments?) that suggest what set of
>> possible resolutions to offer for a given conflict.
>>
>> Does that match what you're aiming for?
>
> Not quite. I am aiming for extensive coverage, not just the "most
> common" cases.
>
> The resolver should be good enough to handle any conflict people might
> run into, and offer all reasonable resolution strategies for a given
> conflict. We won't get there in one day, of course. It will evolve with
> time.
Let's take a closer look at what the user (through the client
application) needs to be able to do.
> /* 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,
"theirs-full" and "mine-full" are nice and simple: each choice leads to a
single, definite result. If the user chooses one of these, the
Subversion library can simply execute the user's decision with no
further info or interaction needed.
> svn_client_resolution_choice_theirs_conflict,
> svn_client_resolution_choice_mine_conflict,
"theirs-conflict" and "mine-conflict" are good options for an
interactive 3-way merge tool to offer the user while showing the user
which sections of the file do have conflicts and which don't. It's not
an option I'd like to choose outside a 3-way merge tool. In terms of
tree conflicts, the same applies: I'd only be comfortable with this
option when it's presented in a 3-way directory-merge window that's
showing me which children are conflicting and which are not. After
pressing this button in the 3-way (file or dir) merge tool, I wouldn't
expect the tool to immediately close and Subversion library to
immediately finalize my choice, instead I'd expect to be able to review
and edit the result and then finally confirm.
So, the resolution that the 3-way merge tool provides to the Subversion
library should not be a simple "he chose 'mine-conflict'", but rather
should be a complete file (or directory tree) that the user finally
chose as the result. As an implementation alternative, of course it could be in the form of instructions to
generate such a file (or dir).
> [...]
> svn_client_resolution_choice_diff_full,
"diff-full" isn't a resolution choice, it's the user asking to see more information before choosing. We need to get away from the mode
of operation of the existing interactive resolver loop where the user interacts my means of question-and-answer. That question-and-answer loop should be in the presentation layer (svn), not in the library.
> svn_client_resolution_choice_edit_file,
"edit-file" isn't a resolution choice that the library can then
"execute" in a black-box manner. Instead, the user wants the GUI to
help him/her prepare the final result with the aid of an editor, and
then the user may choose to confirm that resulting file as the final
resolved result. The resolution needs to be communicated to the Svn lib
in the form of a file, not "he chose 'edit' as the resolution".
> [...]
> svn_client_resolution_choice_scan_log_for_moves,
Not a resolution. Instead, there should be a way for the GUI to get the
suggestion(s) of what moves might be applicable, and to offer those as
suggestions to the user, and then for the user to select maybe one of
the choices or maybe another resolution.
> svn_client_resolution_choice_specify_local_target_for_incoming_move,
> svn_client_resolution_choice_merge_edits_into_local_file,
> [...]
> };
A GUI wants the user to be in control. The first obvious thing is the user should be able to select which file/dir to resolve next. If the GUI wants to have a button that enters a loop that iterates over all conflicts sequentially, that's it's business and we should make it possible to do that, but we should not implement this loop in the Svn lib.
The overall process the user goes through to resolve a particular conflict needs to include a reviewing phase, an editing phase, and finally a confirmation that the result is as the user wants it. How does this fit with the idea of there being a point where the user chooses a resolution such as "choose edit-file"?
While in the reviewing phase of resolving any particular conflict, the user should be able to bring up, on demand, a diff
between 'mine' and 'base' or 'mine:theirs' or 'base:theirs' or whatever
he/she chooses, jump to the location in their editor where the next text conflict occurs, review the log messages on the source and/or target
branch,
and so on. And all of this in parallel, in separate, non-modal windows.
To enable this, the application needs access to the metadata about the conflict. This metadata will include at least the WC location or repository location (URL, rev, etc.) of each of the three files/dirs involved. It needs to be sufficient to be able to display any required diffs, displaying friendly file
names (not the names of temp files) and labels (e.g. "Mine", "Theirs").
In short, it needs information like this:
svn_node_kind_t node_kind;
svn_wc_conflict_kind_t kind;
svn_wc_conflict_action_t action;
svn_wc_conflict_reason_t reason;
svn_wc_operation_t operation;
const char *base_abspath;
const char *their_abspath;
const char *my_abspath;
const char *merged_file;
const svn_wc_conflict_version_t *src_left_version;
const svn_wc_conflict_version_t *src_right_version;
... which is an extract from svn_wc_conflict_description2_t. Certainly we don't want the data to be presented in an "svn_wc_" struct, but I can't get away from the fact that that's the kind of metadata we need.
> There should be no reason for client UIs to implement custom resolution
> strategies. Any such strategies belong into the core. If UI developers
> have a special need they should talk to us and ask for a change in the
> resolver if need be. The current situation is too fragmented and confusing.
> I don't think it helps our users if every client developer keeps
> reinventing the wheel. That just leads to inconsistent behaviour and
> indicates a deficiency of the core library.
>
> So, the APIs should present the supported options, and then act upon
> the choice made by the user. I don't see why you would want to split
> these APIs into two separate categories. They belong together.
So, first the user needs to be presented with all the information so he/she can review the situation before making a choice. Then there are some choices that the Svn lib can simply execute (such as choose 'mine-full'). Then there are other choices that are not a simple matter of stating a one-of-N choice, but rather involve interactive editing in a 3-way (file or dir) merge tool and presenting Subversion with the final result from that.
> As I mentioned, if clients still want to mess with conflicts on their
> own using any of the existing APIs, they can do so. We won't force client
> API users to run the resolver, just strongly encourage them to do so.
>
> Note that all of this will run during an offline resolve, *not* during
> the current interactive conflict resolution during update/merge.
I like and fully support the idea of doing resolution in this way, as a separate pass after the update/merge has finished, if it's possible.
> I intend to remove interactive conflict resolution during update/merge
> for 1.8 and make 'svn resolve' handle all conflict resolution instead
> (at least to the same degree as interactive conflict resolution in 1.7,
> if not better).
That's a subject for a new thread.
- Julian
AW: new conflict callback API
Posted by Markus Schaber <m....@3s-software.com>.
Hi, Stefan,
Von: Stefan Sperling [stsp@elego.de]
On Wed, May 16, 2012 at 02:52:46PM +0000, Markus Schaber wrote:
>> Hmm, this may lead to _very_ special strategies implemented in the core.
>>
>> Like when a CoDeSys Device plugged into a Slot conflicts, and a neighbor slot is empty.
>>
>> On the SVN Working copy, this looks like (irrelevant files and directories omitted):
>>
>> Device (Device node directory)
>> + svnobj (Data file of the Device itself)
>> + [1] (Directory of Slot 1)
>> | + svnobj (Data file of Slot 1, containing plugged device)
>> + [2] (Directory of Slot 2)
>> + svnobj (Data file of Slot 2, containing an empty slot)
>>
>> Now the conflict can be either a "normal" conflict of Device/[1]/svnobj, or an "incoming addition and local addition" tree conflict on the Device/[1]/ directory (Maybe after resolving a similar conflict on the Device parent directory).
>>
>> I can offer the user the following two possibilities (amongst others):
>> 1) Move the existing device to the neighbor slot, so the slot is free for the incoming device.
>> 2) Redirect the incoming device to the neighbor slot.
>>
>> For resolution 1, I need to:
>> - remove Device/[2]/svnobj
>> - move Device/[1]/svnobj to Device/[2]/svnobj (retaining history)
>> - resolve the conflict of Device/[1] using "theirs".
>>
>> For resolution 2, I need to:
>> - remove Device/[2]/svnobj
>> - "redirect" the incoming version of Device/[1]/svnobj to Device/[2]/svnobj.
>> (My current strategy to achieve this is:
>> - first move Device/[1] into a "backup" location
>> - then resolve using "theirs"
>> - then move Device/[1] to Device/[2]
>> - and then restore the old Device/[1] from the backup location.
>
> What is special about that?
> Apart from terminology that is entirely foreign to Subversion, the above
> example describes some possible strategies to resolve conflicts.
If the set of built-in strategies will involve such strategies, then I'm fine with it, and I'll happily use such functionality.
Especially some standard way to "redirect" the incoming conflict candidate to a different local location (recording it as a local move / rename) would be appreciated. AFAICS, this is not offered yet by the SVN libraries, but would also potentially help "normal" users.
> Do I understand correctly that you've already implemented the above
> in your client? If so, whatever you've implemented will keep working.
> It is not like a new resolver would break any of that.
It is implemented prototypically, but not yet in the released versions.
And I know that the old way of resolving conflicts after the update operation will continue to happen.
I just wanted to show that there might be some strategies employed by clients which may seem very special. If you do not regard those strategies as special, and will include them in the default set of available operations, fine. I won't complain then, but happily use them once released, and enjoy the simplification of my code.
>> > I don't think it helps our users if every client developer keeps
>> > reinventing the wheel. That just leads to inconsistent behaviour and
>> > indicates a deficiency of the core library.
>>
>> This will work fine for "most" clients, where I define "most" as those simply working on a directory tree, and then the build toolchain / compiler / IDE / whatever has to cope what is in the directory tree.
>>
>> For clients which version something else (like the CoDeSys object database) which has their own structure and constraints, the semantic translation needed between that "something else" and the directory tree may prohibit some ways of conflict resolution, while offering some new ones on the other hand.
> Subversion versions files and directories. If your system requires
> an abstraction that doesn't fit the files and directories concept,
> you're way out of the intended scope of use anyway. If your conflicts
> can be expressed in terms of file and directories the resolver can
> support them. Else, you'll have to use something else to help users
> of your product with resolving these conflicts.
Yes, I guess what we're doing is out of scope of SVN, but I think it also is out of scope for most other version control systems.
So the only possibility for us is to build a mapping layer between the database and a file/directory hierarchy, and then use some VCS for files and directories.
At the end, I don't see the semantical difference to "classical" systems to be that big. It is only that, after some operations, the user is responsible for repairing the "damage" in the working copy, when Visual Studio / Eclipse & Co bark on what they find in their directory trees.
Thanks,
Markus
Re: new conflict callback API
Posted by Stefan Sperling <st...@elego.de>.
On Wed, May 16, 2012 at 02:52:46PM +0000, Markus Schaber wrote:
> Hmm, this may lead to _very_ special strategies implemented in the core.
>
> Like when a CoDeSys Device plugged into a Slot conflicts, and a neighbor slot is empty.
>
> On the SVN Working copy, this looks like (irrelevant files and directories omitted):
>
> Device (Device node directory)
> + svnobj (Data file of the Device itself)
> + [1] (Directory of Slot 1)
> | + svnobj (Data file of Slot 1, containing plugged device)
> + [2] (Directory of Slot 2)
> + svnobj (Data file of Slot 2, containing an empty slot)
>
> Now the conflict can be either a "normal" conflict of Device/[1]/svnobj, or an "incoming addition and local addition" tree conflict on the Device/[1]/ directory (Maybe after resolving a similar conflict on the Device parent directory).
>
> I can offer the user the following two possibilities (amongst others):
> 1) Move the existing device to the neighbor slot, so the slot is free for the incoming device.
> 2) Redirect the incoming device to the neighbor slot.
>
> For resolution 1, I need to:
> - remove Device/[2]/svnobj
> - move Device/[1]/svnobj to Device/[2]/svnobj (retaining history)
> - resolve the conflict of Device/[1] using "theirs".
>
> For resolution 2, I need to:
> - remove Device/[2]/svnobj
> - "redirect" the incoming version of Device/[1]/svnobj to Device/[2]/svnobj.
> (My current strategy to achieve this is:
> - first move Device/[1] into a "backup" location
> - then resolve using "theirs"
> - then move Device/[1] to Device/[2]
> - and then restore the old Device/[1] from the backup location.
What is special about that?
Apart from terminology that is entirely foreign to Subversion, the above
example describes some possible strategies to resolve conflicts.
Do I understand correctly that you've already implemented the above
in your client? If so, whatever you've implemented will keep working.
It is not like a new resolver would break any of that.
> > I don't think it helps our users if every client developer keeps
> > reinventing the wheel. That just leads to inconsistent behaviour and
> > indicates a deficiency of the core library.
>
> This will work fine for "most" clients, where I define "most" as those simply working on a directory tree, and then the build toolchain / compiler / IDE / whatever has to cope what is in the directory tree.
>
> For clients which version something else (like the CoDeSys object database) which has their own structure and constraints, the semantic translation needed between that "something else" and the directory tree may prohibit some ways of conflict resolution, while offering some new ones on the other hand.
Subversion versions files and directories. If your system requires
an abstraction that doesn't fit the files and directories concept,
you're way out of the intended scope of use anyway. If your conflicts
can be expressed in terms of file and directories the resolver can
support them. Else, you'll have to use something else to help users
of your product with resolving these conflicts.
AW: new conflict callback API
Posted by Markus Schaber <m....@3s-software.com>.
Hi, Stefan,
Von: Stefan Sperling [mailto:stsp@elego.de]
>
> On Wed, May 16, 2012 at 02:52:06PM +0100, Julian Foad wrote:
> > OK, I appreciate that issue. I see two ways the client lib can help:
> >
> > 1. providing a set of client-lib APIs to perform the most commonly
> > wanted resolutions;
> >
> > 2. providing a set of (APIs? comments?) that suggest what set of
> possible resolutions to offer for a given conflict.
> >
> > Does that match what you're aiming for?
> >
>
> Not quite. I am aiming for extensive coverage, not just the "most common"
> cases.
>
> The resolver should be good enough to handle any conflict people might run
> into, and offer all reasonable resolution strategies for a given conflict.
> We won't get there in one day, of course. It will evolve with time.
>
> There should be no reason for client UIs to implement custom resolution
> strategies. Any such strategies belong into the core. If UI developers
> have a special need they should talk to us and ask for a change in the
> resolver if need be. The current situation is too fragmented and
> confusing.
Hmm, this may lead to _very_ special strategies implemented in the core.
Like when a CoDeSys Device plugged into a Slot conflicts, and a neighbor slot is empty.
On the SVN Working copy, this looks like (irrelevant files and directories omitted):
Device (Device node directory)
+ svnobj (Data file of the Device itself)
+ [1] (Directory of Slot 1)
| + svnobj (Data file of Slot 1, containing plugged device)
+ [2] (Directory of Slot 2)
+ svnobj (Data file of Slot 2, containing an empty slot)
Now the conflict can be either a "normal" conflict of Device/[1]/svnobj, or an "incoming addition and local addition" tree conflict on the Device/[1]/ directory (Maybe after resolving a similar conflict on the Device parent directory).
I can offer the user the following two possibilities (amongst others):
1) Move the existing device to the neighbor slot, so the slot is free for the incoming device.
2) Redirect the incoming device to the neighbor slot.
For resolution 1, I need to:
- remove Device/[2]/svnobj
- move Device/[1]/svnobj to Device/[2]/svnobj (retaining history)
- resolve the conflict of Device/[1] using "theirs".
For resolution 2, I need to:
- remove Device/[2]/svnobj
- "redirect" the incoming version of Device/[1]/svnobj to Device/[2]/svnobj.
(My current strategy to achieve this is:
- first move Device/[1] into a "backup" location
- then resolve using "theirs"
- then move Device/[1] to Device/[2]
- and then restore the old Device/[1] from the backup location.
For our part, we're not interested in the history of directories as such at all (except that the're part of the path of the file), only the data files and their properties are of interest, and those have their very own structural semantics and constraints.
> I don't think it helps our users if every client developer keeps
> reinventing the wheel. That just leads to inconsistent behaviour and
> indicates a deficiency of the core library.
This will work fine for "most" clients, where I define "most" as those simply working on a directory tree, and then the build toolchain / compiler / IDE / whatever has to cope what is in the directory tree.
For clients which version something else (like the CoDeSys object database) which has their own structure and constraints, the semantic translation needed between that "something else" and the directory tree may prohibit some ways of conflict resolution, while offering some new ones on the other hand.
> So, the APIs should present the supported options, and then act upon the
> choice made by the user. I don't see why you would want to split these
> APIs into two separate categories. They belong together.
>
> As I mentioned, if clients still want to mess with conflicts on their own
> using any of the existing APIs, they can do so. We won't force client API
> users to run the resolver, just strongly encourage them to do so.
>
> Note that all of this will run during an offline resolve, *not* during the
> current interactive conflict resolution during update/merge.
> I intend to remove interactive conflict resolution during update/merge for
> 1.8 and make 'svn resolve' handle all conflict resolution instead (at
> least to the same degree as interactive conflict resolution in 1.7, if not
> better).
Best regards
Markus Schaber
--
___________________________
We software Automation.
3S-Smart Software Solutions GmbH
Markus Schaber | Developer
Memminger Str. 151 | 87439 Kempten | Germany | Tel. +49-831-54031-0 | Fax +49-831-54031-50
Email: m.schaber@3s-software.com | Web: http://www.3s-software.com
CoDeSys internet forum: http://forum.3s-software.com
Download CoDeSys sample projects: http://www.3s-software.com/index.shtml?sample_projects
Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915
Re: new conflict callback API
Posted by Stefan Sperling <st...@elego.de>.
On Wed, May 16, 2012 at 02:52:06PM +0100, Julian Foad wrote:
> OK, I appreciate that issue. I see two ways the client lib can help:
>
> 1. providing a set of client-lib APIs to perform the most commonly wanted resolutions;
>
> 2. providing a set of (APIs? comments?) that suggest what set of possible resolutions to offer for a given conflict.
>
> Does that match what you're aiming for?
>
Not quite. I am aiming for extensive coverage, not just the "most
common" cases.
The resolver should be good enough to handle any conflict people might
run into, and offer all reasonable resolution strategies for a given
conflict. We won't get there in one day, of course. It will evolve with time.
There should be no reason for client UIs to implement custom resolution
strategies. Any such strategies belong into the core. If UI developers
have a special need they should talk to us and ask for a change in the
resolver if need be. The current situation is too fragmented and confusing.
I don't think it helps our users if every client developer keeps
reinventing the wheel. That just leads to inconsistent behaviour and
indicates a deficiency of the core library.
So, the APIs should present the supported options, and then act upon
the choice made by the user. I don't see why you would want to split
these APIs into two separate categories. They belong together.
As I mentioned, if clients still want to mess with conflicts on their
own using any of the existing APIs, they can do so. We won't force client
API users to run the resolver, just strongly encourage them to do so.
Note that all of this will run during an offline resolve, *not* during
the current interactive conflict resolution during update/merge.
I intend to remove interactive conflict resolution during update/merge
for 1.8 and make 'svn resolve' handle all conflict resolution instead
(at least to the same degree as interactive conflict resolution in 1.7,
if not better).
Re: new conflict callback API
Posted by Julian Foad <ju...@btopenworld.com>.
----- Original Message -----
> From: Stefan Sperling <st...@elego.de>
> To: Julian Foad <ju...@btopenworld.com>
> Cc: "dev@subversion.apache.org" <de...@subversion.apache.org>; Bert Huijben <be...@qqmail.nl>
> Sent: Wednesday, 16 May 2012, 14:07
> Subject: Re: new conflict callback API
>
> On Wed, May 16, 2012 at 01:55:01PM +0100, Julian Foad wrote:
>> I think the way to avoid "offering options the client library is not
> prepared to handle" is that the application should resolve a conflict by
> calling a series of (mostly) normal client operations, rather than by telling
> the client library to do "the conflict-resolution-action named
> 'foo'".
>>
>> Certainly the set of available client-lib operations could include some of
> the common resolutions, but the application should not be restricted to offering
> only the options that are compiled into some list in the the Subversion library.
>>
>> It follows from what I'm saying that I don't believe we should be
> trying to make all clients display the same set of options. Instead, we should
> just show the client application authors what we think is a reasonable selection
> of options as a starting point to guide them, and let them deviate from that
> (especially by adding more interesting/complex ones).
>
> The tree conflict space is large. It is the cross product of all
> fundamental actions the version control system can perform on a local
> node (add, delete, edit, copy, move) and the same list of actions
> for the incoming change made to the node. And then for each box in
> the resulting matrix you have some number of N possible resolution
> options, to be determined on a case-by-case basis for each type of
> tree conflict.
>
> I don't think it is a good idea to leave the reasoning about any of
> this to client developers. Client developers are writing interfaces.
> They are not writing a version control system library. I strongly
> believe that the library should take extensive care of this problem.
>
> If clients decide not to use the conflict resolution API but perform
> custom API callls into the library to resolve a conflict, that's fine.
> They can already do that today and this possibility won't go away.
> But that is orthogonal to what I'm trying to do.
OK, I appreciate that issue. I see two ways the client lib can help:
1. providing a set of client-lib APIs to perform the most commonly wanted resolutions;
2. providing a set of (APIs? comments?) that suggest what set of possible resolutions to offer for a given conflict.
Does that match what you're aiming for?
- Julian
Re: new conflict callback API
Posted by Stefan Sperling <st...@elego.de>.
On Wed, May 16, 2012 at 01:55:01PM +0100, Julian Foad wrote:
> I think the way to avoid "offering options the client library is not prepared to handle" is that the application should resolve a conflict by calling a series of (mostly) normal client operations, rather than by telling the client library to do "the conflict-resolution-action named 'foo'".
>
> Certainly the set of available client-lib operations could include some of the common resolutions, but the application should not be restricted to offering only the options that are compiled into some list in the the Subversion library.
>
> It follows from what I'm saying that I don't believe we should be trying to make all clients display the same set of options. Instead, we should just show the client application authors what we think is a reasonable selection of options as a starting point to guide them, and let them deviate from that (especially by adding more interesting/complex ones).
The tree conflict space is large. It is the cross product of all
fundamental actions the version control system can perform on a local
node (add, delete, edit, copy, move) and the same list of actions
for the incoming change made to the node. And then for each box in
the resulting matrix you have some number of N possible resolution
options, to be determined on a case-by-case basis for each type of
tree conflict.
I don't think it is a good idea to leave the reasoning about any of
this to client developers. Client developers are writing interfaces.
They are not writing a version control system library. I strongly
believe that the library should take extensive care of this problem.
If clients decide not to use the conflict resolution API but perform
custom API callls into the library to resolve a conflict, that's fine.
They can already do that today and this possibility won't go away.
But that is orthogonal to what I'm trying to do.
Re: new conflict callback API
Posted by Mark Phippard <ma...@gmail.com>.
On Wed, May 16, 2012 at 8:55 AM, Julian Foad <ju...@btopenworld.com> wrote:
> Stefan Sperling wrote:
>
>> Bert Huijben wrote:
>>> 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.
>>>
>>> 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).
>>
>> So you mean that instead of calling into a GUI client via a set
>> of callbacks, the client library should provide several API functions
>> that GUI clients can use to gather conflict information and options to
>> display, once the client library calls a generic "please resolve
>> conflicts now" callback?
>>
>> In other words, keep my idea but invert the flow of control?
>>
>> I don't care much about how control flow is structured, more about
>> preventing clients from showing arbitrary conflict dialogs that
>> differ significantly from what other clients present, and which offer
>> options the client library is not prepared to handle.
>
> I think the way to avoid "offering options the client library is not prepared to handle" is that the application should resolve a conflict by calling a series of (mostly) normal client operations, rather than by telling the client library to do "the conflict-resolution-action named 'foo'".
>
> Certainly the set of available client-lib operations could include some of the common resolutions, but the application should not be restricted to offering only the options that are compiled into some list in the the Subversion library.
>
> It follows from what I'm saying that I don't believe we should be trying to make all clients display the same set of options. Instead, we should just show the client application authors what we think is a reasonable selection of options as a starting point to guide them, and let them deviate from that (especially by adding more interesting/complex ones).
To a degree, clients can already do this. When SVN 1.6 came out we
created a custom tree conflict resolution wizard for Subclipse that
does this. It just runs a series of high-level SVN options to resolve
conflicts in various ways. That said, I also like having built-in
resolutions that we can just call. Currently, the only way to do the
things that svn resolve --accept=XXX does is to call that API. I
guess it would be OK if those resolutions were separate API's that we
could call and then we would just follow them up with a general API
call that says we have resolved everything. But again, I think we can
do that today and the question is really just whether SVN could
provide more API's to resolve more scenarios.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
AW: new conflict callback API
Posted by Markus Schaber <m....@3s-software.com>.
Hi,
Von: Julian Foad [mailto:julianfoad@btopenworld.com]
> Stefan Sperling wrote:
> > Bert Huijben wrote:
> > So you mean that instead of calling into a GUI client via a set of
> > callbacks, the client library should provide several API functions
> > that GUI clients can use to gather conflict information and options to
> > display, once the client library calls a generic "please resolve
> > conflicts now" callback?
> >
> > In other words, keep my idea but invert the flow of control?
> >
> > I don't care much about how control flow is structured, more about
> > preventing clients from showing arbitrary conflict dialogs that differ
> > significantly from what other clients present, and which offer options
> > the client library is not prepared to handle.
>
> I think the way to avoid "offering options the client library is not
> prepared to handle" is that the application should resolve a conflict by
> calling a series of (mostly) normal client operations, rather than by
> telling the client library to do "the conflict-resolution-action named
> 'foo'".
I also think this is a good way to go. Some Clients may have additional semantic knowledge which helps to resolve the conflicts.
On the other hand, those clients can always decide to just skip all conflicts, and then use "svn status" and normal working copy operations to do whatever they want.
But I wonder whether it is a good, orthogonal design when the set of available conflict-resolving operations differs between "during the update operation" and "after the update operation".
I guess this is a nice topic for the hackathon, handling tree conflicts in a "nice" way is one of the next tasks in our SharpSVN-based "CoDeSys SVN".
> Certainly the set of available client-lib operations could include some of
> the common resolutions, but the application should not be restricted to
> offering only the options that are compiled into some list in the the
> Subversion library.
>
> It follows from what I'm saying that I don't believe we should be trying
> to make all clients display the same set of options. Instead, we should
> just show the client application authors what we think is a reasonable
> selection of options as a starting point to guide them, and let them
> deviate from that (especially by adding more interesting/complex ones).
Best regards
Markus Schaber
--
___________________________
We software Automation.
3S-Smart Software Solutions GmbH
Markus Schaber | Developer
Memminger Str. 151 | 87439 Kempten | Germany | Tel. +49-831-54031-0 | Fax +49-831-54031-50
Email: m.schaber@3s-software.com | Web: http://www.3s-software.com
CoDeSys internet forum: http://forum.3s-software.com
Download CoDeSys sample projects: http://www.3s-software.com/index.shtml?sample_projects
Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915
Re: new conflict callback API
Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Sperling wrote:
> Bert Huijben wrote:
>> 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.
>>
>> 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).
>
> So you mean that instead of calling into a GUI client via a set
> of callbacks, the client library should provide several API functions
> that GUI clients can use to gather conflict information and options to
> display, once the client library calls a generic "please resolve
> conflicts now" callback?
>
> In other words, keep my idea but invert the flow of control?
>
> I don't care much about how control flow is structured, more about
> preventing clients from showing arbitrary conflict dialogs that
> differ significantly from what other clients present, and which offer
> options the client library is not prepared to handle.
I think the way to avoid "offering options the client library is not prepared to handle" is that the application should resolve a conflict by calling a series of (mostly) normal client operations, rather than by telling the client library to do "the conflict-resolution-action named 'foo'".
Certainly the set of available client-lib operations could include some of the common resolutions, but the application should not be restricted to offering only the options that are compiled into some list in the the Subversion library.
It follows from what I'm saying that I don't believe we should be trying to make all clients display the same set of options. Instead, we should just show the client application authors what we think is a reasonable selection of options as a starting point to guide them, and let them deviate from that (especially by adding more interesting/complex ones).
- Julian
Re: new conflict callback API
Posted by Stefan Sperling <st...@elego.de>.
On Wed, May 16, 2012 at 01:59:43PM +0200, Bert Huijben wrote:
> 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.
> 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).
So you mean that instead of calling into a GUI client via a set
of callbacks, the client library should provide several API functions
that GUI clients can use to gather conflict information and options to
display, once the client library calls a generic "please resolve
conflicts now" callback?
In other words, keep my idea but invert the flow of control?
I don't care much about how control flow is structured, more about
preventing clients from showing arbitrary conflict dialogs that
differ significantly from what other clients present, and which offer
options the client library is not prepared to handle.
RE: new conflict callback API
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Sperling [mailto:stsp@elego.de]
> Sent: woensdag 16 mei 2012 13:25
> To: dev@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!