You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2012/06/19 21:52:16 UTC
svn commit: r1351822 - /subversion/trunk/subversion/libsvn_client/switch.c
Author: stsp
Date: Tue Jun 19 19:52:15 2012
New Revision: 1351822
URL: http://svn.apache.org/viewvc?rev=1351822&view=rev
Log:
Teach 'svn switch' to invoke the interactive conflict resolution callback
when the switch operation has completed, rather than invoking it during the
switch operation. Similar to r1350795 and r1351792.
* subversion/libsvn_client/switch.c
(switch_internal): For 1.7 API users, invoke the conflict resolution
callback after, rather than during, the switch operation.
Modified:
subversion/trunk/subversion/libsvn_client/switch.c
Modified: subversion/trunk/subversion/libsvn_client/switch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/switch.c?rev=1351822&r1=1351821&r2=1351822&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/switch.c (original)
+++ subversion/trunk/subversion/libsvn_client/switch.c Tue Jun 19 19:52:15 2012
@@ -92,6 +92,10 @@ switch_internal(svn_revnum_t *result_rev
SVN_CONFIG_CATEGORY_CONFIG,
APR_HASH_KEY_STRING)
: NULL;
+ /* Resolve conflicts post-switch for 1.7 and above API users. */
+ svn_boolean_t resolve_conflicts_post_switch = (ctx->conflict_func2 != NULL);
+ svn_wc_conflict_resolver_func2_t conflict_func2;
+ void *conflict_baton2;
/* An unknown depth can't be sticky. */
if (depth == svn_depth_unknown)
@@ -218,6 +222,16 @@ switch_internal(svn_revnum_t *result_rev
svn_dirent_dirname(local_abspath, pool));
}
+ if (resolve_conflicts_post_switch)
+ {
+ /* Remove the conflict resolution callback from the client context.
+ * We invoke it after of the switch instead of during the switch. */
+ conflict_func2 = ctx->conflict_func2;
+ conflict_baton2 = ctx->conflict_baton2;
+ ctx->conflict_func2 = NULL;
+ ctx->conflict_baton2 = NULL;
+ }
+
SVN_ERR(svn_ra_reparent(ra_session, anchor_url, pool));
@@ -324,6 +338,21 @@ switch_internal(svn_revnum_t *result_rev
if (result_rev)
*result_rev = revnum;
+ if (resolve_conflicts_post_switch)
+ {
+ /* Resolve conflicts within the switched target. */
+ SVN_ERR(svn_wc__resolve_conflicts(ctx->wc_ctx, local_abspath,
+ depth,
+ TRUE /* resolve_text */,
+ "" /* resolve_prop (ALL props) */,
+ TRUE /* resolve_tree */,
+ svn_wc_conflict_choose_unspecified,
+ conflict_func2, conflict_baton2,
+ ctx->cancel_func, ctx->cancel_baton,
+ ctx->notify_func2, ctx->notify_baton2,
+ pool));
+ }
+
return SVN_NO_ERROR;
}
Re: svn commit: r1351822 -
/subversion/trunk/subversion/libsvn_client/switch.c
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jun 20, 2012 at 01:54:29AM -0400, Greg Stein wrote:
> On Jun 19, 2012 9:52 PM, <st...@apache.org> wrote:
> >...
> > + if (resolve_conflicts_post_switch)
> > + {
> > + /* Remove the conflict resolution callback from the client context.
> > + * We invoke it after of the switch instead of during the switch.
> */
> > + conflict_func2 = ctx->conflict_func2;
> > + conflict_baton2 = ctx->conflict_baton2;
> > + ctx->conflict_func2 = NULL;
> > + ctx->conflict_baton2 = NULL;
> > + }
>
> This feels like a terrible hack, of the worst kind.
>
> The caller gives you a client context, and you change it on them. It's
> theirs. Not yours to redefine.
> Even worse, you never restore it! The context is now damaged because you
> monkeyed with it. The context is NOT a single-use structure. It can live
> for hours, over multiple operations, by design. But the above hack destroys
> it.
>
> I think you need to find a new approach.
Agreed, plus I'm not happy with changing the behaviour for 1.7 API users.
Keeping 1.5 behaviour the same while changing behaviour of already released
1.7 APIs is... inconsistent.
Maybe we need a new-in-1.8 conflict_func3 that has different invocation
rules than conflict_func2, but otherwise the same signature (unless we
decide to change it before release for some reason)? Would that be better?
Re: svn commit: r1351822 - /subversion/trunk/subversion/libsvn_client/switch.c
Posted by Greg Stein <gs...@gmail.com>.
On Jun 19, 2012 9:52 PM, <st...@apache.org> wrote:
>...
> + if (resolve_conflicts_post_switch)
> + {
> + /* Remove the conflict resolution callback from the client context.
> + * We invoke it after of the switch instead of during the switch.
*/
> + conflict_func2 = ctx->conflict_func2;
> + conflict_baton2 = ctx->conflict_baton2;
> + ctx->conflict_func2 = NULL;
> + ctx->conflict_baton2 = NULL;
> + }
This feels like a terrible hack, of the worst kind.
The caller gives you a client context, and you change it on them. It's
theirs. Not yours to redefine.
Even worse, you never restore it! The context is now damaged because you
monkeyed with it. The context is NOT a single-use structure. It can live
for hours, over multiple operations, by design. But the above hack destroys
it.
I think you need to find a new approach.
Cheers,
-g