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 2015/05/29 13:16:02 UTC
[RFC] new svn_client_conflict API
I would like to start transitioning away from svn_wc_conflict_description2_t
in public APIs and introduce a higher-level API to eventually replace it.
My end goal is to use an opaque type with methods that make it possible
to implement conflict resolution in a way that is easier to use.
For example, the way tree conflict are currently presented to the user is
universally poor across all SVN clients and very poor in our command line
client in particular.
Another sore point is property conflicts. There's still no way in our API
to resolve individual property conflicts on the node, even though the
underlying WC storage can support this.
As a first step, I'd like to introduce new APIs that still use
svn_wc_conflict_description2_t for passing information around but treat it
as if it was an opaque type. This way we can gradually switch the command
line client over to these new APIs without changing existing behaviour.
Once that's done, a new opaque type can be introduced and we can
deprecate svn_wc_conflict_description2_t from the public API.
'svn resolve' would then stop using the conflict callback and instead
call into the client library, actively steering the resolution process.
Below is a sketch of an API I'd like to wrap around the
svn_wc_conflict_description2_t struct.
The API uses terminology that I'd like clients to mirror in their user
interfaces. It breaks with some naming conventions that are misleading
or insufficient to describe complex conflicts (e.g. "yours/mine/theirs").
It also tries to explain things in a way that prevents misunderstandings
of what the information returned by the API really means, so that SVN
client authors can make informed choices about how to present the information.
The intention is to later extend this API to produce detailed data about
tree conflicts and other complex scenarios so that users can make more
informed choices about how to resolve conflicts.
See http://mail-archives.apache.org/mod_mbox/subversion-users/201505.mbox/%3C20150520105235.GE4388%40jim.stsp.name%3E
for an example of what I'd eventually like to see at the conflict prompt.
Any comments on this?
I'm expecting people to point out that all switch functions are duplicates
of the update ones :-) At the API level I'd like to keep them seperate for
clarity even if they map to the same internal implementation.
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 1682370)
+++ subversion/include/svn_client.h (working copy)
@@ -4356,6 +4356,400 @@ svn_client_revert(const apr_array_header_t *paths,
/** @} */
/**
+ * @defgroup Conflicts Dealing with conflicted paths.
+ *
+ * @{
+ */
+
+/**
+ * Return conflict information for the working copy node at @a local_abspath.
+ * Allocate @a *conflict @a result_pool.
+ *
+ * Use @a scratch_pool for temporary allocations.
+ *
+ * @since New in 1.10.
+ * ### Should return a new opaque type such as "svn_client_conflict_t"
+ * ### instead of svn_wc_conflict_description2_t. For now, we use the old
+ * ### type to allow for smooth API transition within Subversion, though
+ * ### some internal use of svn_wc_conflict_description2_t may remain.
+ */
+svn_error_t *
+svn_client_conflict_get(svn_wc_conflict_description2_t **conflict,
+ const char *local_abspath,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return conflict information for all conflicted paths at or beneath
+ * @a local_abspath within the specified @a depth.
+ *
+ * @a conflicts will be set to a hash keyed by a local absolute path.
+ * Hash table values are pointers to svn_wc_conflict_description2_t.
+ * Theh hash table and values are allocated in @a result_pool.
+ *
+ * Use @a scratch_pool for temporary allocations.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_get_multiple(apr_hash_t *conflicts,
+ const char *local_abspath,
+ svn_depth_t depth,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return the absolute path to the conflicted working copy node described
+ * by @a conflict.
+ *
+ * @since New in 1.10.
+ */
+const char *
+svn_client_conflict_get_local_abspath(svn_wc_conflict_description2_t *conflict);
+
+/**
+ * Return the operation during which the conflict described by @a
+ * conflict was recorded.
+ *
+ * @since New in 1.10.
+ */
+svn_wc_operation_t
+svn_client_conflict_get_operation(svn_wc_conflict_description2_t *conflict);
+
+/**
+ * Return the action an update, switch, or merge operation attempted to
+ * perform on the working copy node described by @a conflict.
+ *
+ * @since New in 1.10.
+ */
+svn_wc_conflict_action_t
+svn_client_conflict_get_incoming_change(
+ svn_wc_conflict_description2_t *conflict);
+
+/**
+ * Return the reason why the attempted action performed by an update, switch,
+ * or merge operation conflicted with the state of the node in the working copy.
+ *
+ * During update and switch operations this local change is part of uncommitted
+ * modifications in the working copy. During merge operations it may
+ * additionally be part of the history of the merge target branch, anywhere
+ * between the common ancestor revision and the working copy revision.
+ *
+ * @since New in 1.10.
+ */
+svn_wc_conflict_reason_t
+svn_client_conflict_get_local_change(svn_wc_conflict_description2_t *conflict);
+
+/**
+ * Indicate the types of conflicts present on the working copy node
+ * described by @a conflict. Any output argument may be @c NULL if
+ * the caller is not interested in the status of a particular type.
+ *
+ * While multiple text and/or property changes may exist, only one
+ * tree conflict can be recorded on a node.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_get_types(svn_boolean_t *has_text_conflicts,
+ svn_boolean_t *has_prop_conflicts,
+ svn_boolean_t *has_tree_conflict,
+ svn_wc_conflict_description2_t *conflict,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return the URLs and revisions of repository nodes involved in the
+ * merge operation which recorded @a conflict.
+ *
+ * Note that this does not provide any information about the actual
+ * changes which caused the conflict. These are merely the repository
+ * nodes involved in driving the merge operation. The conflicting changes
+ * are contained somewhere within the set of differences between these nodes
+ * and perhaps also the local modifications of the working copy.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_merge_get_versions(const char **url_source,
+ svn_revnum_t *rev_source,
+ const char **url_target,
+ svn_revnum_t *rev_target,
+ const char **url_common_ancestor,
+ svn_revnum_t *rev_common_ancestor,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+/**
+ * Return the URLs and revisions of repository nodes involved in the
+ * update operation which recorded @a conflict.
+ *
+ * Note that this does not provide any information about the actual
+ * changes which caused the conflict. These are merely the repository
+ * nodes involved in driving the update operation. The conflicting changes
+ * are contained somewhere within the set of differences between these nodes
+ * and the local modifications of the working copy.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_update_get_versions(
+ const char **url_before,
+ svn_revnum_t *rev_before,
+ const char **url_after,
+ svn_revnum_t *rev_after,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return the URLs and revisions of repository nodes involved in the
+ * switch operation which recorded @a conflict.
+ *
+ * Note that this does not provide any information about the actual
+ * changes which caused the conflict. These are merely the repository
+ * nodes involved in driving the switch operation. The conflicting changes
+ * are contained somewhere within the set of differences between these nodes
+ * and the local modifications of the working copy.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_switch_get_versions(
+ const char **url_before,
+ svn_revnum_t *rev_before,
+ const char **url_after,
+ svn_revnum_t *rev_after,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+
+/**
+ * Return the kinds of nodes involved in a conflict recorded during
+ * a merge operation.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_merge_get_kinds(svn_node_kind_t **kind_source,
+ svn_node_kind_t **kind_target,
+ svn_node_kind_t **kind_common_ancestor,
+ svn_node_kind_t **kind_during,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return the kinds of nodes involved in a conflict recorded during
+ * an update operation.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_update_get_kinds(svn_node_kind_t **kind_before,
+ svn_node_kind_t **kind_after,
+ svn_node_kind_t **kind_during,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return the kinds of nodes involved in a conflict recorded during
+ * an switch operation.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_switch_get_kinds(svn_node_kind_t **kind_before,
+ svn_node_kind_t **kind_after,
+ svn_node_kind_t **kind_during,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+/**
+ * If @a conflict describes one or more property conflicts, return an array
+ * of const char * in @a *propnames, allocated in @a result_pool.
+ * If there is no property conflict set @a *propnames to NULL.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_get_propnames(apr_array_header **propnames,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return property values for the conflicted property @propname for which
+ * a conflict was recorded during a merge operation.
+ *
+ * Set @a *val_source to the merge target's property value as seen in the
+ * repository on the merge source branch at the merged-in revision.
+ *
+ * Set @a *val_target to the merge target's property value as seen in
+ * the repository at the merge target's revision. In other words, it is the
+ * value as seen in the merge target working copy's BASE revision and it is
+ * unaffected by any local modifications in the working copy.
+ *
+ * Set @a *val_common_ancestor to the value as seen in the repository at
+ * merge operation's common ancestor path and revision.
+ *
+ * Set @a *val_during to the value as seen in the working copy during the
+ * merge operation, including any local modifications. It may be identical to
+ * @a *val_target if no local modifications were present during the operation.
+ *
+ * These values do not contain conflict markers. A diff between versions can
+ * be generated with svn_diff_mem_string_output_unified3(). A merged version
+ * can be generated using svn_diff_mem_string_output_merge3().
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_merge_get_propvals(const svn_string_t **val_source,
+ const svn_string_t **val_target,
+ const svn_string_t **val_common_ancestor,
+ const svn_string_t **val_during,
+ const char *propname,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return property values for the conflicted property @propname for which
+ * a conflict was recorded during an update operation.
+ *
+ * Set @a *val_before to the pre-update value as seen in the
+ * repository at the former BASE revision of the working copy.
+ * It is unaffected by any local modifications in the working copy.
+ *
+ * Set @a *val_after to the post-update value as seen in the
+ * repository at the update target revision. In other words, it
+ * is the value the working copy has been updated to and it is also
+ * unaffected by any local modifications in the working copy.
+ *
+ * Set @a *val_during to the property value as seen in the
+ * working copy at the time the conflict was recorded, including
+ * local modifications, if any.
+ *
+ * These values do not contain conflict markers. A diff between versions can
+ * be generated with svn_diff_mem_string_output_unified3(). A merged version
+ * can be generated using svn_diff_mem_string_output_merge3().
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_update_get_propvals(
+ const svn_string_t **val_before,
+ const svn_string_t **val_after,
+ const svn_string_t **val_during,
+ const char *propname,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return property values for the conflicted property @propname for which
+ * a conflict was recorded during a switch operation.
+ *
+ * Set @a *val_before to the pre-switch value as seen in the
+ * repository at the former BASE revision and branch of the working copy.
+ * It is unaffected by any local modifications in the working copy.
+ *
+ * Set @a *val_after to the post-switch value as seen in the
+ * repository at the switch target branch and revision. In other words,
+ * it is the value the working copy has been switched to and it is also
+ * unaffected by any local modifications in the working copy.
+ *
+ * @a *val_during contains the property value as seen in the
+ * working copy at the time the conflict was recorded, including
+ * local modifications, if any.
+ *
+ * These values do not contain conflict markers. A diff between versions can
+ * be generated with svn_diff_mem_string_output_unified3(). A merged version
+ * can be generated using svn_diff_mem_string_output_merge3().
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_switch_get_propvals(
+ const svn_string_t **val_before,
+ const svn_string_t **val_after,
+ const svn_string_t **val_during,
+ const char *propname,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return the content of the file versions involved in a text conflict
+ * recorded during a merge operation. The returned content does not
+ * contain conflict markers. A merged version can be generated using
+ * ### need to implement a diff3 function which accepts streams as input
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_update_get_file_contents(
+ svn_stream_t **content_source,
+ svn_stream_t **content_target,
+ svn_stream_t **content_common_ancestor,
+ svn_stream_t **content_during,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return the content of the file versions involved in a text conflict
+ * recorded during an update operation. The returned content does not
+ * contain conflict markers. A merged version can be generated using
+ * ### need to implement a diff3 function which accepts streams as input
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_update_get_file_contents(
+ svn_stream_t **content_before,
+ svn_stream_t **content_after,
+ svn_stream_t **content_during,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Return the content of the file versions involved in a text conflict
+ * recorded during a switch operation. The returned content does not
+ * contain conflict markers. A merged version can be generated using
+ * ### need to implement a diff3 function which accepts streams as input
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_client_conflict_switch_get_file_contents(
+ svn_stream_t **content_before,
+ svn_stream_t **content_after,
+ svn_stream_t **content_during,
+ svn_wc_conflict_description2_t *conflict,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/** @} */
+
+/**
* @defgroup Resolved Mark conflicted paths as resolved.
*
* @{
Re: [RFC] new svn_client_conflict API
Posted by Julian Foad <ju...@btopenworld.com>.
Hi, Stefan. Glad to see this!
Changing the API syntactic form to an opaque struct + accessor
functions sounds fine, as I mentioned on IRC. I see you've done that
already.
As for the semantic changes, I want to help clarify our understanding
of the high level 'shape' of merge scenarios. We need clear conceptual
model in mind, in order to be able to suggest an understandable UI for
merge conflicts.
(We must continually be on the look out for conceptually simple ways
to understand the system, and never accept that it has to be complex
beyond understanding.)
I've drawn a picture. The attached diagram 'merge corners 2.png' shows
two branches, B1 and B2; let's say they are branch root directories,
and they have children which are not shown. B2@60 is checked out into
a WC. We are merging r30:50 from B1 to the WC.
If this is an automatic merge, then B1@30 is the youngest common
ancestor, both in reality and in what we tell the 3-way merge
algorithm. That is true because an automatic merge requires that
r10:30 from B1 is already merged to the target.
Otherwise, this is a cherry-pick merge; B1@30 is an *assumed* common
ancestor for the 3-way merge algorithm, while B1@10 is the *real*
youngest common ancestor. Note that if we chose to use a 4-way merge
algorithm for cherry-pick merges, aka 'variance-adjusted patching',
such as the one implemented in our 'tools/diff/diff4', then we would
pass it both the real YCA = B1@10 and also the source-left B1@30, as
well as source-right and target.
The three points marked in red (B1@30, B1@50, WC@working) are the main
input 'corners' of the 3-way merge. Other revisions of interest are
the WC base and the true YCA B1@10. The main point I want to make is
that the WC base is not a high-level input to the merge. Rather, it is
a point in the practical work flow that has to be negotiated to get to
the desired logical endpoint (the WC working state).
So where does this take us?
I believe it's helpful to see two categories of conflicts:
* conflicts between the two sets of version-controlled changes --
that is, the kinds of changes supported by the repository
* inconsistent WC state -- for example, no file actually on disk
where the working state metadata says there is a file, etc. (I don't
even like to call these 'conflicts')
Conflicts in category (1) are a result of the logical high-level merge
operation. They are exactly the same conflicts that could occur if we
were to implement an option to perform the merge on the server writing
its result directly into a commit transaction. The most relevant data,
in my opinion, are the three "corners" of the 3-way merge, followed
next by the real YCA if different from source-left, and finally
followed by the WC base state.
The category (2) 'conflicts', on the other hand, are absolutely
related to the WC state. The WC base state and working state are
pretty much the only thing that needs to be shown. The merge itself is
merely the reason why we found this inconsistency in the WC -- the
merge is not the primary interesting thing about these.
Thus, a conflict with reason 'svn_wc_conflict_reason_unversioned' for
example is in category (2).
By keeping these two categories conceptually separated, we can provide
a clearer view of the relevant information.
(Another thing I notice about the 'action' and 'reason' descriptions
is they're documented as applying to an 'object' -- 'Object is
unversioned', for example. The current merge system, and thus the
conflict system, is mainly based on paths. In any tree change, one
'object' does not map cleanly to one path. This is a source of
confusion.)
Does that help a bit?
- Julian
Stefan Sperling wrote:
> I would like to start transitioning away from svn_wc_conflict_description2_t
> in public APIs and introduce a higher-level API to eventually replace it.
[...]
> Below is a sketch of an API I'd like to wrap around the
> svn_wc_conflict_description2_t struct.
>
> The API uses terminology that I'd like clients to mirror in their user
> interfaces. It breaks with some naming conventions that are misleading
> or insufficient to describe complex conflicts (e.g. "yours/mine/theirs").
> It also tries to explain things in a way that prevents misunderstandings
> of what the information returned by the API really means, so that SVN
> client authors can make informed choices about how to present the information.
>
> The intention is to later extend this API to produce detailed data about
> tree conflicts and other complex scenarios so that users can make more
> informed choices about how to resolve conflicts.
> See http://mail-archives.apache.org/mod_mbox/subversion-users/201505.mbox/%3C20150520105235.GE4388%40jim.stsp.name%3E
> for an example of what I'd eventually like to see at the conflict prompt.
[...]