See inline.
> -----Original Message-----
> From: s...@apache.org [mailto:s...@apache.org]
> Sent: maandag 15 februari 2016 16:04
> To: comm...@subversion.apache.org
> Subject: svn commit: r1730546 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h libsvn_client/resolved.c
> libsvn_wc/conflicts.c
>
> Author: stsp
> Date: Mon Feb 15 15:04:10 2016
> New Revision: 1730546
>
> URL: http://svn.apache.org/viewvc?rev=1730546&view=rev
> Log:
> Provide new private libsvn_wc APIs for resolving text and property conflicts.
> Use these new APIs from libsvn_client's new conflict resolver instead of
> calling the generic svn_wc__resolve_conflicts() function.
>
> This avoids going on a status walk for just one path at depth empty.
> The new functions added here provide sufficient functionality for the new
> conflict resolver: Marking a text/prop conflict resolved based on a choice
> made by the user, and sending a notification to the client.
>
> For now, tree conflicts are still resolved with svn_wc__resolve_conflicts().
> The plan is to add several new libsvn_wc APIs for resolving tree conflicts.
> These APIs will not be driven by a simple conflict choice argument. Rather,
> each API will implement a very specific resolution strategy for a particular
> kind of tree conflict.
> Eventually, libsvn_client will stop using svn_wc__resolve_conflicts() for
> anything but backwards compatibility.
>
> * subversion/include/private/svn_wc_private.h
> (svn_wc__conflict_text_mark_resolved,
> svn_wc__conflict_prop_mark_resolved): Declare.
>
> * subversion/libsvn_client/resolved.c
> (resolve_text_conflict): Call svn_wc__conflict_text_mark_resolved().
> (resolve_prop_conflict): Call svn_wc__conflict_prop_mark_resolved().
> (resolve_tree_conflict): Inline the body of resolve_conflict() here.
> (resolve_conflict): Remove.
>
> * subversion/libsvn_wc/conflicts.c
> (svn_wc__conflict_text_mark_resolved,
> svn_wc__conflict_prop_mark_resolved): Implement.
>
> Modified:
> subversion/trunk/subversion/include/private/svn_wc_private.h
> subversion/trunk/subversion/libsvn_client/resolved.c
> subversion/trunk/subversion/libsvn_wc/conflicts.c
>
> Modified: subversion/trunk/subversion/include/private/svn_wc_private.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private
> /svn_wc_private.h?rev=1730546&r1=1730545&r2=1730546&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/include/private/svn_wc_private.h (original)
> +++ subversion/trunk/subversion/include/private/svn_wc_private.h Mon
> Feb 15 15:04:10 2016
> @@ -1748,6 +1748,39 @@ svn_wc__resolve_conflicts(svn_wc_context
> void *notify_baton,
> apr_pool_t *scratch_pool);
>
> +/**
> + * Resolve the text conflict at LOCAL_ABSPATH as per CHOICE, and then
> + * mark the conflict resolved.
> + * The working copy must already be locked for resolving, e.g. by calling
> + * svn_wc__acquire_write_lock_for_resolve() first.
> + * @since New in 1.10.
> + */
> +svn_error_t *
> +svn_wc__conflict_text_mark_resolved(svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + svn_wc_conflict_choice_t choice,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + apr_pool_t *scratch_pool);
> +
> +/**
> + * Resolve the conflicted property PROPNAME at LOCAL_ABSPATH as per
> CHOICE,
> + * and then mark the conflict resolved.
> + * The working copy must already be locked for resolving, e.g. by calling
> + * svn_wc__acquire_write_lock_for_resolve() first.
> + * @since New in 1.10.
> + */
> +svn_error_t *
> +svn_wc__conflict_prop_mark_resolved(svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + const char *propname,
> + svn_wc_conflict_choice_t choice,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + apr_pool_t *scratch_pool);
> +
> /**
> * Move @a src_abspath to @a dst_abspath, by scheduling @a dst_abspath
> * for addition to the repository, remembering the history. Mark @a
> src_abspath
>
> Modified: subversion/trunk/subversion/libsvn_client/resolved.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/re
> solved.c?rev=1730546&r1=1730545&r2=1730546&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_client/resolved.c (original)
> +++ subversion/trunk/subversion/libsvn_client/resolved.c Mon Feb 15
> 15:04:10 2016
> @@ -674,62 +674,41 @@ svn_client_conflict_option_set_merged_pr
> option->type_data.prop.merged_propval = merged_propval;
> }
>
> -/*
> - * Resolve the conflict at LOCAL_ABSPATH. Currently only supports
> - * an OPTION_ID which can be mapped to svn_wc_conflict_choice_t and
> - * maps a single option_id to text, prop, and/or tree conflicts.
> - */
> +/* Implements conflict_option_resolve_func_t. */
> static svn_error_t *
> -resolve_conflict(svn_client_conflict_option_id_t option_id,
> - const char *local_abspath,
> - svn_boolean_t resolve_text,
> - const char * resolve_prop,
> - svn_boolean_t resolve_tree,
> - svn_client_ctx_t *ctx,
> - apr_pool_t *scratch_pool)
> +resolve_text_conflict(svn_client_conflict_option_t *option,
> + svn_client_conflict_t *conflict,
> + apr_pool_t *scratch_pool)
> {
> - svn_wc_conflict_choice_t conflict_choice;
> + svn_client_conflict_option_id_t option_id;
> + const char *local_abspath;
> const char *lock_abspath;
> + svn_wc_conflict_choice_t conflict_choice;
> + svn_client_ctx_t *ctx = conflict->ctx;
> svn_error_t *err;
>
> + option_id = svn_client_conflict_option_get_id(option);
> conflict_choice =
> svn_client_conflict_option_id_to_wc_conflict_choice(option_id);
> + local_abspath = svn_client_conflict_get_local_abspath(conflict);
> +
> SVN_ERR(svn_wc__acquire_write_lock_for_resolve(&lock_abspath, ctx-
> >wc_ctx,
> local_abspath,
> scratch_pool,
> scratch_pool));
> - err = svn_wc__resolve_conflicts(ctx->wc_ctx, local_abspath,
> - svn_depth_empty,
> - resolve_text, resolve_prop, resolve_tree,
> - conflict_choice,
> - NULL, NULL, /* legacy conflict_func/baton
> */
> - ctx->cancel_func,
> - ctx->cancel_baton,
> - ctx->notify_func2,
> - ctx->notify_baton2,
> - scratch_pool);
> + err = svn_wc__conflict_text_mark_resolved(conflict->ctx->wc_ctx,
> + local_abspath,
> + conflict_choice,
> + conflict->ctx->cancel_func,
> + conflict->ctx->cancel_baton,
> + conflict->ctx->notify_func2,
> + conflict->ctx->notify_baton2,
> + scratch_pool);
> err = svn_error_compose_create(err, svn_wc__release_write_lock(ctx-
> >wc_ctx,
>
> lock_abspath,
>
> scratch_pool));
> svn_io_sleep_for_timestamps(local_abspath, scratch_pool);
> -
> SVN_ERR(err);
>
> - return SVN_NO_ERROR;
> -}
> -
> -/* Implements conflict_option_resolve_func_t. */
> -static svn_error_t *
> -resolve_text_conflict(svn_client_conflict_option_t *option,
> - svn_client_conflict_t *conflict,
> - apr_pool_t *scratch_pool)
> -{
> - svn_client_conflict_option_id_t option_id;
> - const char *local_abspath;
> -
> - option_id = svn_client_conflict_option_get_id(option);
> - local_abspath = svn_client_conflict_get_local_abspath(conflict);
> - SVN_ERR(resolve_conflict(option_id, local_abspath, TRUE, NULL, FALSE,
> - conflict->ctx, scratch_pool));
> conflict->resolution_text = option_id;
>
> return SVN_NO_ERROR;
> @@ -742,14 +721,31 @@ resolve_prop_conflict(svn_client_conflic
> apr_pool_t *scratch_pool)
> {
> svn_client_conflict_option_id_t option_id;
> + svn_wc_conflict_choice_t conflict_choice;
> const char *local_abspath;
> + const char *lock_abspath;
> const char *propname = option->type_data.prop.propname;
> + svn_client_ctx_t *ctx = conflict->ctx;
> + svn_error_t *err;
>
> option_id = svn_client_conflict_option_get_id(option);
> + conflict_choice =
> + svn_client_conflict_option_id_to_wc_conflict_choice(option_id);
> local_abspath = svn_client_conflict_get_local_abspath(conflict);
> - SVN_ERR(resolve_conflict(option_id, local_abspath,
> - FALSE, propname, FALSE,
> - conflict->ctx, scratch_pool));
> +
> + SVN_ERR(svn_wc__acquire_write_lock_for_resolve(&lock_abspath, ctx-
> >wc_ctx,
> + local_abspath,
> + scratch_pool,
> scratch_pool));
> + err = svn_wc__conflict_prop_mark_resolved(ctx->wc_ctx, local_abspath,
> + propname, conflict_choice,
> + conflict->ctx->notify_func2,
> + conflict->ctx->notify_baton2,
> + scratch_pool);
> + err = svn_error_compose_create(err, svn_wc__release_write_lock(ctx-
> >wc_ctx,
> +
> lock_abspath,
> +
> scratch_pool));
> + svn_io_sleep_for_timestamps(local_abspath, scratch_pool);
> + SVN_ERR(err);
Why would you need to wait for timestamps here?
Can this change the working copy file?
If it can the wc function should probably return a boolean to notify the case
where it does, and only when it does we should wait for timestamps.
>
> if (propname[0] == '\0')
> {
> @@ -787,19 +783,43 @@ resolve_prop_conflict(svn_client_conflic
> return SVN_NO_ERROR;
> }
>
> -static svn_error_t *
> /* Implements conflict_option_resolve_func_t. */
> +static svn_error_t *
> resolve_tree_conflict(svn_client_conflict_option_t *option,
> svn_client_conflict_t *conflict,
> apr_pool_t *scratch_pool)
> {
> svn_client_conflict_option_id_t option_id;
> const char *local_abspath;
> + const char *lock_abspath;
> + svn_wc_conflict_choice_t conflict_choice;
> + svn_client_ctx_t *ctx = conflict->ctx;
> + svn_error_t *err;
>
> option_id = svn_client_conflict_option_get_id(option);
> local_abspath = svn_client_conflict_get_local_abspath(conflict);
> - SVN_ERR(resolve_conflict(option_id, local_abspath, FALSE, NULL, TRUE,
> - conflict->ctx, scratch_pool));
> + conflict_choice =
> + svn_client_conflict_option_id_to_wc_conflict_choice(option_id);
> +
> + SVN_ERR(svn_wc__acquire_write_lock_for_resolve(&lock_abspath, ctx-
> >wc_ctx,
> + local_abspath,
> + scratch_pool,
> scratch_pool));
> + err = svn_wc__resolve_conflicts(ctx->wc_ctx, local_abspath,
> + svn_depth_empty,
> + FALSE, FALSE, TRUE,
> + conflict_choice,
> + NULL, NULL, /* legacy conflict_func/baton
> */
> + ctx->cancel_func,
> + ctx->cancel_baton,
> + ctx->notify_func2,
> + ctx->notify_baton2,
> + scratch_pool);
> + err = svn_error_compose_create(err, svn_wc__release_write_lock(ctx-
> >wc_ctx,
> +
> lock_abspath,
> +
> scratch_pool));
> + svn_io_sleep_for_timestamps(local_abspath, scratch_pool);
> + SVN_ERR(err);
Same thing here... I don't think this sleep operation is necessary.
Same conditions, in case it is really necessary: add output argument to note
the cases where they are really necessary, etc. (Never needed on directories,
things that are already deleted, etc. etc.)
Bert
