You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2015/03/17 11:55:46 UTC

svn commit: r1667258 - in /subversion/trunk/subversion/libsvn_wc: conflicts.c conflicts.h merge.c props.c update_editor.c

Author: rhuijben
Date: Tue Mar 17 10:55:45 2015
New Revision: 1667258

URL: http://svn.apache.org/r1667258
Log:
Avoid a db operation for each property conflict in some invocations of
the interactive conflict resolver.

* subversion/libsvn_wc/conflicts.c
  (generate_propconflict): Add kind argument.
  (svn_wc__conflict_invoke_resolver): Update caller. Add argument.

* subversion/libsvn_wc/conflicts.h
  (svn_wc__conflict_invoke_resolver): Add kind argument.

* subversion/libsvn_wc/merge.c
  (svn_wc_merge5): Extend scope of variable. Update caller.

* subversion/libsvn_wc/props.c
  (svn_wc_merge_props3): Update caller.

* subversion/libsvn_wc/update_editor.c
  (delete_entry,
   close_directory,
   absent_node,
   close_file): Update caller.

Modified:
    subversion/trunk/subversion/libsvn_wc/conflicts.c
    subversion/trunk/subversion/libsvn_wc/conflicts.h
    subversion/trunk/subversion/libsvn_wc/merge.c
    subversion/trunk/subversion/libsvn_wc/props.c
    subversion/trunk/subversion/libsvn_wc/update_editor.c

Modified: subversion/trunk/subversion/libsvn_wc/conflicts.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/conflicts.c?rev=1667258&r1=1667257&r2=1667258&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/conflicts.c (original)
+++ subversion/trunk/subversion/libsvn_wc/conflicts.c Tue Mar 17 10:55:45 2015
@@ -1142,6 +1142,7 @@ static svn_error_t *
 generate_propconflict(svn_boolean_t *conflict_remains,
                       svn_wc__db_t *db,
                       const char *local_abspath,
+                      svn_node_kind_t kind,
                       svn_wc_operation_t operation,
                       const svn_wc_conflict_version_t *left_version,
                       const svn_wc_conflict_version_t *right_version,
@@ -1159,24 +1160,11 @@ generate_propconflict(svn_boolean_t *con
   svn_wc_conflict_result_t *result = NULL;
   svn_wc_conflict_description2_t *cdesc;
   const char *dirpath = svn_dirent_dirname(local_abspath, scratch_pool);
-  svn_node_kind_t kind;
   const svn_string_t *new_value = NULL;
 
-  SVN_ERR(svn_wc__db_read_kind(&kind, db, local_abspath,
-                               FALSE /* allow_missing */,
-                               FALSE /* show_deleted */,
-                               FALSE /* show_hidden */,
-                               scratch_pool));
-
-  if (kind == svn_node_none)
-    return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
-                             _("The node '%s' was not found."),
-                             svn_dirent_local_style(local_abspath,
-                                                    scratch_pool));
-
   cdesc = svn_wc_conflict_description_create_prop2(
                 local_abspath,
-                (kind == svn_node_dir) ? svn_node_dir : svn_node_file,
+                kind,
                 propname, scratch_pool);
 
   cdesc->operation = operation;
@@ -1838,6 +1826,7 @@ resolve_tree_conflict_on_node(svn_boolea
 svn_error_t *
 svn_wc__conflict_invoke_resolver(svn_wc__db_t *db,
                                  const char *local_abspath,
+                                 svn_node_kind_t kind,
                                  const svn_skel_t *conflict_skel,
                                  const apr_array_header_t *merge_options,
                                  svn_wc_conflict_resolver_func2_t resolver_func,
@@ -1914,7 +1903,7 @@ svn_wc__conflict_invoke_resolver(svn_wc_
             SVN_ERR(cancel_func(cancel_baton));
 
           SVN_ERR(generate_propconflict(&conflict_remains,
-                                        db, local_abspath,
+                                        db, local_abspath, kind,
                                         operation,
                                         left_version,
                                         right_version,

Modified: subversion/trunk/subversion/libsvn_wc/conflicts.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/conflicts.h?rev=1667258&r1=1667257&r2=1667258&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/conflicts.h (original)
+++ subversion/trunk/subversion/libsvn_wc/conflicts.h Tue Mar 17 10:55:45 2015
@@ -419,6 +419,7 @@ svn_wc__conflict_create_markers(svn_skel
 svn_error_t *
 svn_wc__conflict_invoke_resolver(svn_wc__db_t *db,
                                  const char *local_abspath,
+                                 svn_node_kind_t kind,
                                  const svn_skel_t *conflict_skel,
                                  const apr_array_header_t *merge_options,
                                  svn_wc_conflict_resolver_func2_t resolver_func,

Modified: subversion/trunk/subversion/libsvn_wc/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/merge.c?rev=1667258&r1=1667257&r2=1667258&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/merge.c (original)
+++ subversion/trunk/subversion/libsvn_wc/merge.c Tue Mar 17 10:55:45 2015
@@ -1230,6 +1230,7 @@ svn_wc_merge5(enum svn_wc_merge_outcome_
   apr_hash_t *pristine_props = NULL;
   apr_hash_t *old_actual_props;
   apr_hash_t *new_actual_props = NULL;
+  svn_node_kind_t kind;
 
   SVN_ERR_ASSERT(svn_dirent_is_absolute(left_abspath));
   SVN_ERR_ASSERT(svn_dirent_is_absolute(right_abspath));
@@ -1242,7 +1243,6 @@ svn_wc_merge5(enum svn_wc_merge_outcome_
   /* Sanity check:  the merge target must be a file under revision control */
   {
     svn_wc__db_status_t status;
-    svn_node_kind_t kind;
     svn_boolean_t had_props;
     svn_boolean_t props_mod;
     svn_boolean_t conflicted;
@@ -1405,7 +1405,7 @@ svn_wc_merge5(enum svn_wc_merge_outcome_
           svn_boolean_t text_conflicted, prop_conflicted;
 
           SVN_ERR(svn_wc__conflict_invoke_resolver(
-                    wc_ctx->db, target_abspath,
+                    wc_ctx->db, target_abspath, kind,
                     conflict_skel, merge_options,
                     conflict_func, conflict_baton,
                     cancel_func, cancel_baton,

Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=1667258&r1=1667257&r2=1667258&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Tue Mar 17 10:55:45 2015
@@ -327,7 +327,8 @@ svn_wc_merge_props3(svn_wc_notify_state_
     {
       svn_boolean_t prop_conflicted;
 
-      SVN_ERR(svn_wc__conflict_invoke_resolver(db, local_abspath, conflict_skel,
+      SVN_ERR(svn_wc__conflict_invoke_resolver(db, local_abspath, kind,
+                                               conflict_skel,
                                                NULL /* merge_options */,
                                                conflict_func, conflict_baton,
                                                cancel_func, cancel_baton,

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1667258&r1=1667257&r2=1667258&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Tue Mar 17 10:55:45 2015
@@ -1812,6 +1812,7 @@ delete_entry(const char *path,
     {
       if (eb->conflict_func)
         SVN_ERR(svn_wc__conflict_invoke_resolver(eb->db, local_abspath,
+                                                 kind,
                                                  tree_conflict,
                                                  NULL /* merge_options */,
                                                  eb->conflict_func,
@@ -2815,6 +2816,7 @@ close_directory(void *dir_baton,
 
   if (conflict_skel && eb->conflict_func)
     SVN_ERR(svn_wc__conflict_invoke_resolver(eb->db, db->local_abspath,
+                                             svn_node_dir,
                                              conflict_skel,
                                              NULL /* merge_options */,
                                              eb->conflict_func,
@@ -3000,6 +3002,7 @@ absent_node(const char *path,
       {
         if (eb->conflict_func)
           SVN_ERR(svn_wc__conflict_invoke_resolver(eb->db, local_abspath,
+                                                   kind,
                                                    tree_conflict,
                                                    NULL /* merge_options */,
                                                    eb->conflict_func,
@@ -4539,6 +4542,7 @@ close_file(void *file_baton,
 
   if (conflict_skel && eb->conflict_func)
     SVN_ERR(svn_wc__conflict_invoke_resolver(eb->db, fb->local_abspath,
+                                             svn_node_file,
                                              conflict_skel,
                                              NULL /* merge_options */,
                                              eb->conflict_func,



Re: svn commit: r1667258 - in /subversion/trunk/subversion/libsvn_wc: conflicts.c conflicts.h merge.c props.c update_editor.c

Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Sperling wrote:
>> URL: http://svn.apache.org/r1667258
>> Log:
>> Avoid a db operation for each property conflict in some invocations of
>> the interactive conflict resolver.
>
> Are we sure the hard-coded node kinds (svn_node_dir, svn_node_file)
> below are always correct? These are supposed to represent the node
> kind of the tree conflict victim, which is not necessarily the same
> as the server thinks it should be.

The new 'kind' argument added to svn_wc__conflict_invoke_resolver()
need to be be documented, as it is not obvious whether it is supposed
to represent the kind of the source, the target, the YCA, or the
on-disk node-version. In fact it appears to be only used within that
function for prop conflicts, where the node kind must be the same for
all versions, but that function can also be called for tree conflicts,
and it is not clear what the caller should pass in that case.

- Julian

RE: svn commit: r1667258 - in /subversion/trunk/subversion/libsvn_wc: conflicts.c conflicts.h merge.c props.c update_editor.c

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

> -----Original Message-----
> From: Stefan Sperling [mailto:stsp@elego.de]
> Sent: donderdag 16 april 2015 00:15
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1667258 - in
/subversion/trunk/subversion/libsvn_wc:
> conflicts.c conflicts.h merge.c props.c update_editor.c
> 
> On Tue, Mar 17, 2015 at 10:55:46AM -0000, rhuijben@apache.org wrote:
> > Author: rhuijben
> > Date: Tue Mar 17 10:55:45 2015
> > New Revision: 1667258
> >
> > URL: http://svn.apache.org/r1667258
> > Log:
> > Avoid a db operation for each property conflict in some invocations of
> > the interactive conflict resolver.
> 
> > Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
> > URL:
>
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_e
> ditor.c?rev=1667258&r1=1667257&r2=1667258&view=diff
> >
> ================================================================
> ==============
> > --- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
> > +++ subversion/trunk/subversion/libsvn_wc/update_editor.c Tue Mar 17
> 10:55:45 2015
> 
> Are we sure the hard-coded node kinds (svn_node_dir, svn_node_file)
> below are always correct? These are supposed to represent the node
> kind of the tree conflict victim, which is not necessarily the same
> as the server thinks it should be.

Yes.

Tree conflict detection happens in <add/open>_<directory/file>() and this
doesn't invoke the conflict resolver callback.

In the cases where we do invoke the resolver we just read and pass the
actual kind, or we just process the BASE kind where we know it is not
different from what there is in the working copy(or we already raised a tree
conflict before).

BTW: libsvn_client doesn't use this callback infrastructure for anything but
collecting the paths that are conflicted. The actual resolving happens via
the standard conflict resolving code, unless you implement your own update
invocation via deprecated libsvn_wc functions.

This also explains why we don't call the callback for tree conflicts: nobody
is really using this, and old versions didn't call into the callback either.

	Bert


Re: svn commit: r1667258 - in /subversion/trunk/subversion/libsvn_wc: conflicts.c conflicts.h merge.c props.c update_editor.c

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Mar 17, 2015 at 10:55:46AM -0000, rhuijben@apache.org wrote:
> Author: rhuijben
> Date: Tue Mar 17 10:55:45 2015
> New Revision: 1667258
> 
> URL: http://svn.apache.org/r1667258
> Log:
> Avoid a db operation for each property conflict in some invocations of
> the interactive conflict resolver.

> Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1667258&r1=1667257&r2=1667258&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/update_editor.c Tue Mar 17 10:55:45 2015

Are we sure the hard-coded node kinds (svn_node_dir, svn_node_file)
below are always correct? These are supposed to represent the node
kind of the tree conflict victim, which is not necessarily the same
as the server thinks it should be.

> @@ -2815,6 +2816,7 @@ close_directory(void *dir_baton,
>  
>    if (conflict_skel && eb->conflict_func)
>      SVN_ERR(svn_wc__conflict_invoke_resolver(eb->db, db->local_abspath,
> +                                             svn_node_dir,
>                                               conflict_skel,
>                                               NULL /* merge_options */,
>                                               eb->conflict_func,
> @@ -3000,6 +3002,7 @@ absent_node(const char *path,
>        {
>          if (eb->conflict_func)
>            SVN_ERR(svn_wc__conflict_invoke_resolver(eb->db, local_abspath,
> +                                                   kind,
>                                                     tree_conflict,
>                                                     NULL /* merge_options */,
>                                                     eb->conflict_func,
> @@ -4539,6 +4542,7 @@ close_file(void *file_baton,
>  
>    if (conflict_skel && eb->conflict_func)
>      SVN_ERR(svn_wc__conflict_invoke_resolver(eb->db, fb->local_abspath,
> +                                             svn_node_file,
>                                               conflict_skel,
>                                               NULL /* merge_options */,
>                                               eb->conflict_func,
>