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/26 16:21:20 UTC

svn commit: r1354029 - in /subversion/trunk/subversion/libsvn_client: merge.c switch.c update.c

Author: stsp
Date: Tue Jun 26 14:21:18 2012
New Revision: 1354029

URL: http://svn.apache.org/viewvc?rev=1354029&view=rev
Log:
Remove some redundant local variables. We're now effectively always postponing
conflict resolution until after an update/switch/merge has completed.

The intent of resolving conflicts during an operation for 1.5 API users
won't work anyway, since our backwards compat wrappers will always pass
a 1.7-style conflict_func2 which wraps a 1.5 conflict_func.

* subversion/libsvn_client/merge.c
  (merge_locked, merge_peg_locked, do_symmetric_merge_locked): Get rid of
   the local variable resolve_conflicts_post_merge, check the
   ctx->conflict_func2 pointer directly.

* subversion/libsvn_client/switch.c
  (switch_internal): Get rid of local variable resolve_conflicts_post_switch,
   check the ctx->conflict_func2 pointer directly.

* subversion/libsvn_client/update.c
  (svn_client_update4): Get rid of local variable resolve_conflicts_post_update,
   check the ctx->conflict_func2 pointer directly.

Modified:
    subversion/trunk/subversion/libsvn_client/merge.c
    subversion/trunk/subversion/libsvn_client/switch.c
    subversion/trunk/subversion/libsvn_client/update.c

Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1354029&r1=1354028&r2=1354029&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Tue Jun 26 14:21:18 2012
@@ -9546,8 +9546,6 @@ merge_locked(const char *source1,
   svn_client__pathrev_t *yca = NULL;
   apr_pool_t *sesspool;
   svn_boolean_t same_repos;
-  /* Resolve conflicts post-merge for 1.7 and above API users. */
-  svn_boolean_t resolve_conflicts_post_merge = (ctx->conflict_func2 != NULL);
 
   /* ### FIXME: This function really ought to do a history check on
      the left and right sides of the merge source, and -- if one is an
@@ -9694,7 +9692,7 @@ merge_locked(const char *source1,
   if (err)
     return svn_error_trace(err);
 
-  if (resolve_conflicts_post_merge)
+  if (ctx->conflict_func2)
     {
       /* Resolve conflicts within the merge target. */
       SVN_ERR(svn_wc__resolve_conflicts(ctx->wc_ctx, target_abspath,
@@ -10970,8 +10968,6 @@ merge_peg_locked(const char *source_path
   svn_boolean_t use_sleep = FALSE;
   svn_error_t *err;
   svn_boolean_t same_repos;
-  /* Resolve conflicts post-merge for 1.7 and above API users. */
-  svn_boolean_t resolve_conflicts_post_merge = (ctx->conflict_func2 != NULL);
 
   SVN_ERR_ASSERT(svn_dirent_is_absolute(target_abspath));
 
@@ -11009,7 +11005,7 @@ merge_peg_locked(const char *source_path
   /* We're done with our RA session. */
   svn_pool_destroy(sesspool);
 
-  if (resolve_conflicts_post_merge)
+  if (ctx->conflict_func2)
     {
       /* Resolve conflicts within the merge target. */
       SVN_ERR(svn_wc__resolve_conflicts(ctx->wc_ctx, target_abspath,
@@ -11618,8 +11614,6 @@ do_symmetric_merge_locked(const svn_clie
   merge_target_t *target;
   svn_boolean_t use_sleep = FALSE;
   svn_error_t *err;
-  /* Resolve conflicts post-merge for 1.7 and above API users. */
-  svn_boolean_t resolve_conflicts_post_merge = (ctx->conflict_func2 != NULL);
 
   SVN_ERR(open_target_wc(&target, target_abspath, TRUE, TRUE, TRUE,
                          ctx, scratch_pool, scratch_pool));
@@ -11680,7 +11674,7 @@ do_symmetric_merge_locked(const svn_clie
 
   SVN_ERR(err);
 
-  if (resolve_conflicts_post_merge)
+  if (ctx->conflict_func2)
     {
       /* Resolve conflicts within the merge target. */
       SVN_ERR(svn_wc__resolve_conflicts(ctx->wc_ctx, target_abspath,

Modified: subversion/trunk/subversion/libsvn_client/switch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/switch.c?rev=1354029&r1=1354028&r2=1354029&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/switch.c (original)
+++ subversion/trunk/subversion/libsvn_client/switch.c Tue Jun 26 14:21:18 2012
@@ -92,8 +92,6 @@ 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);
 
   /* An unknown depth can't be sticky. */
   if (depth == svn_depth_unknown)
@@ -325,7 +323,7 @@ switch_internal(svn_revnum_t *result_rev
   if (result_rev)
     *result_rev = revnum;
 
-  if (resolve_conflicts_post_switch)
+  if (ctx->conflict_func2)
     {
       /* Resolve conflicts within the switched target. */
       SVN_ERR(svn_wc__resolve_conflicts(ctx->wc_ctx, local_abspath,

Modified: subversion/trunk/subversion/libsvn_client/update.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/update.c?rev=1354029&r1=1354028&r2=1354029&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/update.c (original)
+++ subversion/trunk/subversion/libsvn_client/update.c Tue Jun 26 14:21:18 2012
@@ -570,8 +570,6 @@ svn_client_update4(apr_array_header_t **
   apr_pool_t *iterpool = svn_pool_create(pool);
   const char *path = NULL;
   svn_boolean_t sleep = FALSE;
-  /* Resolve conflicts post-update for 1.7 and above API users. */
-  svn_boolean_t resolve_conflicts_post_update = (ctx->conflict_func2 != NULL);
 
   if (result_revs)
     *result_revs = apr_array_make(pool, paths->nelts, sizeof(svn_revnum_t));
@@ -635,7 +633,7 @@ svn_client_update4(apr_array_header_t **
   if (sleep)
     svn_io_sleep_for_timestamps((paths->nelts == 1) ? path : NULL, pool);
 
-  if (resolve_conflicts_post_update)
+  if (ctx->conflict_func2)
     {
       for (i = 0; i < paths->nelts; ++i)
         {



Re: svn commit: r1354029 - in /subversion/trunk/subversion/libsvn_client: merge.c switch.c update.c

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Jun 26, 2012 at 06:00:06PM +0200, Bert Huijben wrote:
> > > The intent of resolving conflicts during an operation for 1.5 API users
> > > won't work anyway, since our backwards compat wrappers will always pass
> > > a 1.7-style conflict_func2 which wraps a 1.5 conflict_func.
> > 
> > I'm unclear on this. It seems that if no conflict2_func is passed,
> > then we can pass a wrapper for 1.7-style resolution during the
> > operation. ie. stop hard-coding NULL and possibly provide a wrapper
> > for the 1.5 API.
> > 
> > (not sure what we'd do for 1.6 and 1.7, but it seems pretty clear we
> > could support 1.5 and before)
> 
> For the libsvn_client api we already wrap the 1.7 api, by setting a default
> wrapper in the svn_client_ctx_t that calls the conflict_func. 
> (Before 1.5 we didn't use any conflict callbacks)
> 
> I think the behavior difference Stephan talks about is that he can ignore an
> older style conflict handler without a newer one.

Yes, it seems with our libsvn_client APIs callers will always pass in a
conflict_func2 even if a 1.5 API users passes a conflict_func.
So we cannot detect what API the caller is really using by inspecting
the conflict_func2 pointer.

And even if we could, it seems that trying to keep the exact 1.5
behaviour for 1.5 API users is more trouble than it is worth. It's not
as if the new behaviour will fundamentally break things. The callback will
just be invoked at a different time. And if we're willing to expose code
compiled against the 1.7 API to this, why treat code compiled against 1.5
differently?

RE: svn commit: r1354029 - in /subversion/trunk/subversion/libsvn_client: merge.c switch.c update.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: dinsdag 26 juni 2012 17:48
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1354029 - in
> /subversion/trunk/subversion/libsvn_client: merge.c switch.c update.c
> 
> On Tue, Jun 26, 2012 at 10:21 AM,  <st...@apache.org> wrote:
> > Author: stsp
> > Date: Tue Jun 26 14:21:18 2012
> > New Revision: 1354029
> >
> > URL: http://svn.apache.org/viewvc?rev=1354029&view=rev
> > Log:
> > Remove some redundant local variables. We're now effectively always
> postponing
> > conflict resolution until after an update/switch/merge has completed.
> >
> > The intent of resolving conflicts during an operation for 1.5 API users
> > won't work anyway, since our backwards compat wrappers will always pass
> > a 1.7-style conflict_func2 which wraps a 1.5 conflict_func.
> 
> I'm unclear on this. It seems that if no conflict2_func is passed,
> then we can pass a wrapper for 1.7-style resolution during the
> operation. ie. stop hard-coding NULL and possibly provide a wrapper
> for the 1.5 API.
> 
> (not sure what we'd do for 1.6 and 1.7, but it seems pretty clear we
> could support 1.5 and before)

For the libsvn_client api we already wrap the 1.7 api, by setting a default
wrapper in the svn_client_ctx_t that calls the conflict_func. 
(Before 1.5 we didn't use any conflict callbacks)

I think the behavior difference Stephan talks about is that he can ignore an
older style conflict handler without a newer one.

	Bert
> 
> >...
> 
> Cheers,
> -g


Re: svn commit: r1354029 - in /subversion/trunk/subversion/libsvn_client: merge.c switch.c update.c

Posted by Greg Stein <gs...@gmail.com>.
On Tue, Jun 26, 2012 at 10:21 AM,  <st...@apache.org> wrote:
> Author: stsp
> Date: Tue Jun 26 14:21:18 2012
> New Revision: 1354029
>
> URL: http://svn.apache.org/viewvc?rev=1354029&view=rev
> Log:
> Remove some redundant local variables. We're now effectively always postponing
> conflict resolution until after an update/switch/merge has completed.
>
> The intent of resolving conflicts during an operation for 1.5 API users
> won't work anyway, since our backwards compat wrappers will always pass
> a 1.7-style conflict_func2 which wraps a 1.5 conflict_func.

I'm unclear on this. It seems that if no conflict2_func is passed,
then we can pass a wrapper for 1.7-style resolution during the
operation. ie. stop hard-coding NULL and possibly provide a wrapper
for the 1.5 API.

(not sure what we'd do for 1.6 and 1.7, but it seems pretty clear we
could support 1.5 and before)

>...

Cheers,
-g