You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2014/08/18 18:40:16 UTC

RE: svn commit: r1618643 - in /subversion/trunk/subversion:include/svn_wc.h libsvn_wc/info.c

Can you document how property conflicts are stored in this array. For the previous versions we have multiple ways to encode these.

An info call is pretty performance critical for many gui clients so it shouldn't start creating temp files for every property conflict it notes, or many different db calls for data that not everybody needs.

Returning everything from generic APIs, potentially including expensive to obtain data is not the recommended way to do things. It is better to obtain the expensive parts on demand via different functions.

The best conflict descriptor might be a simple reference that can just be used by a different function set. We don't want to recreate the problems of svn_wc_entry_t.

Bert

-----Original Message-----
From: "stsp@apache.org" <st...@apache.org>
Sent: ‎18/‎08/‎2014 18:01
To: "commits@subversion.apache.org" <co...@subversion.apache.org>
Subject: svn commit: r1618643 - in /subversion/trunk/subversion:include/svn_wc.h libsvn_wc/info.c

Author: stsp
Date: Mon Aug 18 16:01:07 2014
New Revision: 1618643

URL: http://svn.apache.org/r1618643
Log:
Extend svn_wc_info_t with an array of svn_wc_conflict_description3_t items.
This will allow the svn client to use conflict descriptions of this type.

* subversion/include/svn_wc.h
  (svn_wc_info_t): Add 'conflicts2'.

* subversion/libsvn_wc/info.c
  (svn_wc_info_dup): Duplicate the new svn_wc_conflict_description3_t array.
  (build_info_for_node): Populate the svn_wc_conflict_description3_t array.

Modified:
    subversion/trunk/subversion/include/svn_wc.h
    subversion/trunk/subversion/libsvn_wc/info.c

Modified: subversion/trunk/subversion/include/svn_wc.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=1618643&r1=1618642&r2=1618643&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_wc.h (original)
+++ subversion/trunk/subversion/include/svn_wc.h Mon Aug 18 16:01:07 2014
@@ -3363,6 +3363,10 @@ typedef struct svn_wc_info_t
   /** The path the node was moved to, if it was moved away. Else NULL.
    * @since New in 1.8. */
   const char *moved_to_abspath;
+
+  /** Array of const svn_wc_conflict_description3_t * which contains info
+   * on any conflict of which this node is a victim. Otherwise NULL.  */
+  const apr_array_header_t *conflicts2;
 } svn_wc_info_t;
 
 /**

Modified: subversion/trunk/subversion/libsvn_wc/info.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/info.c?rev=1618643&r1=1618642&r2=1618643&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/info.c (original)
+++ subversion/trunk/subversion/libsvn_wc/info.c Mon Aug 18 16:01:07 2014
@@ -67,6 +67,22 @@ svn_wc_info_dup(const svn_wc_info_t *inf
     new_info->moved_from_abspath = apr_pstrdup(pool, info->moved_from_abspath);
   if (info->moved_to_abspath)
     new_info->moved_to_abspath = apr_pstrdup(pool, info->moved_to_abspath);
+  if (info->conflicts2)
+    {
+      int i;
+
+      apr_array_header_t *new_conflicts
+        = apr_array_make(pool, info->conflicts2->nelts, info->conflicts2->elt_size);
+      for (i = 0; i < info->conflicts2->nelts; i++)
+        {
+          APR_ARRAY_PUSH(new_conflicts, svn_wc_conflict_description3_t *)
+            = svn_wc__conflict_description3_dup(
+                APR_ARRAY_IDX(info->conflicts2, i,
+                              const svn_wc_conflict_description3_t *),
+                pool);
+        }
+      new_info->conflicts2 = new_conflicts;
+    }
 
   return new_info;
 }
@@ -319,9 +335,13 @@ build_info_for_node(svn_wc__info2_t **in
                                      result_pool, scratch_pool));
       wc_info->conflicts = svn_wc__cd3_array_to_cd2_array(conflicts,
                                                           result_pool);
+      wc_info->conflicts2 = conflicts;
     }
   else
-    wc_info->conflicts = NULL;
+    {
+      wc_info->conflicts = NULL;
+      wc_info->conflicts2 = NULL;
+    }
 
   /* lock stuff */
   if (lock != NULL)



Re: svn commit: r1618643 - in /subversion/trunk/subversion:include/svn_wc.h libsvn_wc/info.c

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Aug 18, 2014 at 06:40:16PM +0200, Bert Huijben wrote:
> Can you document how property conflicts are stored in this array. For the previous versions we have multiple ways to encode these.
> 
> An info call is pretty performance critical for many gui clients so it shouldn't start creating temp files for every property conflict it notes, or many different db calls for data that not everybody needs.
> 
> Returning everything from generic APIs, potentially including expensive to obtain data is not the recommended way to do things. It is better to obtain the expensive parts on demand via different functions.
> 
> The best conflict descriptor might be a simple reference that can just be used by a different function set. We don't want to recreate the problems of svn_wc_entry_t.
> 
> Bert

You make a good point but this goes beyond what svn_wc_conflict_description3_t
does. It's just a better version of svn_wc_conflict_description2_t without
some of the quirks, such as storing the prop reject file path in the wrong
field. And it has some fields renamed for clarity. It's *not* a better API.

Going forward, I agree a much better API to describe conflicts will be
needed. But we're not quite there yet, I think.

Re: svn commit: r1618643 - in /subversion/trunk/subversion:include/svn_wc.h libsvn_wc/info.c

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Aug 18, 2014 at 06:40:16PM +0200, Bert Huijben wrote:
> Can you document how property conflicts are stored in this array. For the previous versions we have multiple ways to encode these.
> 
> An info call is pretty performance critical for many gui clients so it shouldn't start creating temp files for every property conflict it notes, or many different db calls for data that not everybody needs.
> 
> Returning everything from generic APIs, potentially including expensive to obtain data is not the recommended way to do things. It is better to obtain the expensive parts on demand via different functions.
> 
> The best conflict descriptor might be a simple reference that can just be used by a different function set. We don't want to recreate the problems of svn_wc_entry_t.
> 
> Bert

You make a good point but this goes beyond what svn_wc_conflict_description3_t
does. It's just a better version of svn_wc_conflict_description2_t without
some of the quirks, such as storing the prop reject file path in the wrong
field. And it has some fields renamed for clarity. It's *not* a better API.

Going forward, I agree a much better API to describe conflicts will be
needed. But we're not quite there yet, I think.