RE: svn commit: r1730546 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/resolved.c libsvn_wc/conflicts.c

2016-02-15 Thread Bert Huijben
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/tr

Re: svn commit: r1730546 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/resolved.c libsvn_wc/conflicts.c

2016-02-15 Thread Stefan Sperling
On Mon, Feb 15, 2016 at 05:30:55PM +0100, Bert Huijben wrote:
> 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.
> 

I agree that this should be fixed.

I'd like to spend some more time exploring the new APIs before addressing
performance issues like this. Hunting down all the code paths where we may
or may not modify a working copy file probably takes some time. I'm willing
to invest that time eventually, but not right now.

Please feel free to fix this yourself.
If you don't have time to deal with this problem right away, could you
please file an issue so we don't forget? Thanks!