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 2011/04/18 16:02:41 UTC

svn commit: r1094581 - in /subversion/trunk/subversion: libsvn_wc/update_editor.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/db-test.c tests/libsvn_wc/op-depth-test.c

Author: rhuijben
Date: Mon Apr 18 14:02:40 2011
New Revision: 1094581

URL: http://svn.apache.org/viewvc?rev=1094581&view=rev
Log:
Resolve a mayor TODO in the update editor: Make node updates atomic by setting
the ACTUAL properties in the same SQLite transaction as where we update the
BASE node.

* subversion/libsvn_wc/update_editor.c
  (close_directory): Set properties using svn_wc__db_base_add_directory and
    simplify conditions of when to set.
  (close_file): Set properties using svn_wc__db_base_add_file and simplify
    conditions of when to set.

* subversion/libsvn_wc/wc_db.c
  (insert_base_baton_t): Add two fields.
  (set_actual_props): Add prototype.
  (insert_base_node): Call set_actual_props if necessary.
  (svn_wc__db_base_add_directory,
   svn_wc__db_base_add_file,
   svn_wc__db_base_add_symlink): Put property set information in baton.

* subversion/libsvn_wc/wc_db.h
  (svn_wc__db_base_add_directory,
   svn_wc__db_base_add_file,
   svn_wc__db_base_add_symlink): Add set_actual_props, new_actual_props.

* subversion/tests/libsvn_wc/db-test.c
  (test_inserting_nodes): Update caller.

* subversion/tests/libsvn_wc/op-depth-test.c
  (base_dir_insert_remove): Update caller.

Modified:
    subversion/trunk/subversion/libsvn_wc/update_editor.c
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/libsvn_wc/wc_db.h
    subversion/trunk/subversion/tests/libsvn_wc/db-test.c
    subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1094581&r1=1094580&r2=1094581&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Mon Apr 18 14:02:40 2011
@@ -2642,11 +2642,6 @@ close_directory(void *dir_baton,
       if (props == NULL)
         props = base_props;
 
-      /* ### NOTE: from this point onwards, we make TWO changes to the
-         ### database in a non-transactional way. some kind of revamp
-         ### needs to happend to bring this down to a single DB transaction
-         ### to perform the changes and install all the needed work items.  */
-
       SVN_ERR(svn_wc__db_base_add_directory(
                 eb->db, db->local_abspath,
                 db->new_relpath,
@@ -2660,24 +2655,10 @@ close_directory(void *dir_baton,
                     ? prop_hash_from_array(dav_prop_changes, pool)
                     : NULL,
                 NULL /* conflict */,
+                (! db->shadowed) && new_base_props != NULL,
+                new_actual_props,
                 NULL /* work_items */,
                 pool));
-
-      /* If we updated the BASE properties, then we also have ACTUAL
-         properties to update. Do that now, along with queueing a work
-         item to write out an old-style props file.  */
-      if (!db->shadowed
-          && !(db->adding_dir && !db->add_existed)
-          && new_base_props != NULL)
-        {
-          SVN_ERR_ASSERT(new_actual_props != NULL);
-
-          SVN_ERR(svn_wc__db_op_set_props(eb->db, db->local_abspath,
-                                          new_actual_props,
-                                          NULL /* conflict */,
-                                          NULL /* work_items */,
-                                          pool));
-        }
     }
 
   /* Process all of the queued work items for this directory.  */
@@ -4035,6 +4016,8 @@ close_file(void *file_baton,
                                                               scratch_pool)
                                        : NULL,
                                      NULL /* conflict */,
+                                     (! fb->shadowed) && new_base_props,
+                                     new_actual_props,
                                      all_work_items,
                                      scratch_pool));
 
@@ -4077,20 +4060,6 @@ close_file(void *file_baton,
                                                 scratch_pool));
     }
 
-  /* Now we might have to update the ACTUAL tree, with the result of the
-     properties merge. */
-  if (! (fb->adding_file && !fb->add_existed)
-      && ! fb->shadowed)
-    {
-      SVN_ERR_ASSERT(new_actual_props != NULL);
-
-      SVN_ERR(svn_wc__db_op_set_props(eb->db, fb->local_abspath,
-                                      new_actual_props,
-                                      NULL /* conflict */,
-                                      NULL /* work_item */,
-                                      scratch_pool));
-    }
-
   /* ### we may as well run whatever is in the queue right now. this
      ### starts out with some crap node data via construct_base_node(),
      ### so we can't really monkey things up too badly here. all tests

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1094581&r1=1094580&r2=1094581&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Apr 18 14:02:40 2011
@@ -168,6 +168,10 @@ typedef struct insert_base_baton_t {
   /* may need to insert/update ACTUAL to record a conflict  */
   const svn_skel_t *conflict;
 
+  /* may need to insert/update ACTUAL to record new properties */
+  svn_boolean_t update_actual_props;
+  const apr_hash_t *new_actual_props;
+
   /* may have work items to queue in this transaction  */
   const svn_skel_t *work_items;
 
@@ -238,6 +242,13 @@ add_work_items(svn_sqlite__db_t *sdb,
                apr_pool_t *scratch_pool);
 
 static svn_error_t *
+set_actual_props(apr_int64_t wc_id,
+                 const char *local_relpath,
+                 apr_hash_t *props,
+                 svn_sqlite__db_t *db,
+                 apr_pool_t *scratch_pool);
+
+static svn_error_t *
 insert_incomplete_children(svn_sqlite__db_t *sdb,
                            apr_int64_t wc_id,
                            const char *local_relpath,
@@ -826,6 +837,29 @@ insert_base_node(void *baton,
 
   SVN_ERR(svn_sqlite__insert(NULL, stmt));
 
+  if (pibb->update_actual_props)
+    {
+      /* Cast away const, to allow calling property helpers */
+      apr_hash_t *base_props = (apr_hash_t *)pibb->props;
+      apr_hash_t *new_actual_props = (apr_hash_t *)pibb->new_actual_props;
+
+      if (base_props != NULL
+          && new_actual_props != NULL
+          && (apr_hash_count(base_props) == apr_hash_count(new_actual_props)))
+        {
+          apr_array_header_t *diffs;
+
+          SVN_ERR(svn_prop_diffs(&diffs, (apr_hash_t *)new_actual_props,
+                                 (apr_hash_t *)pibb->props, scratch_pool));
+
+          if (diffs->nelts == 0)
+            new_actual_props = NULL;
+        }
+
+      SVN_ERR(set_actual_props(wcroot->wc_id, local_relpath, new_actual_props,
+                               wcroot->sdb, scratch_pool));
+    }
+
   if (pibb->kind == svn_wc__db_kind_dir && pibb->children)
     SVN_ERR(insert_incomplete_children(wcroot->sdb, wcroot->wc_id,
                                        local_relpath,
@@ -1456,6 +1490,8 @@ svn_wc__db_base_add_directory(svn_wc__db
                               svn_depth_t depth,
                               apr_hash_t *dav_cache,
                               const svn_skel_t *conflict,
+                              svn_boolean_t update_actual_props,
+                              apr_hash_t *new_actual_props,
                               const svn_skel_t *work_items,
                               apr_pool_t *scratch_pool)
 {
@@ -1501,6 +1537,12 @@ svn_wc__db_base_add_directory(svn_wc__db
   ibb.conflict = conflict;
   ibb.work_items = work_items;
 
+  if (update_actual_props)
+    {
+      ibb.update_actual_props = TRUE;
+      ibb.new_actual_props = new_actual_props;
+    }
+
   /* Insert the directory and all its children transactionally.
 
      Note: old children can stick around, even if they are no longer present
@@ -1529,6 +1571,8 @@ svn_wc__db_base_add_file(svn_wc__db_t *d
                          svn_filesize_t translated_size,
                          apr_hash_t *dav_cache,
                          const svn_skel_t *conflict,
+                         svn_boolean_t update_actual_props,
+                         apr_hash_t *new_actual_props,
                          const svn_skel_t *work_items,
                          apr_pool_t *scratch_pool)
 {
@@ -1572,6 +1616,13 @@ svn_wc__db_base_add_file(svn_wc__db_t *d
   ibb.conflict = conflict;
   ibb.work_items = work_items;
 
+  if (update_actual_props)
+    {
+      ibb.update_actual_props = TRUE;
+      ibb.new_actual_props = new_actual_props;
+    }
+
+
   /* ### hmm. if this used to be a directory, we should remove children.
      ### or maybe let caller deal with that, if there is a possibility
      ### of a node kind change (rather than eat an extra lookup here).  */
@@ -1598,6 +1649,8 @@ svn_wc__db_base_add_symlink(svn_wc__db_t
                             const char *target,
                             apr_hash_t *dav_cache,
                             const svn_skel_t *conflict,
+                            svn_boolean_t update_actual_props,
+                            apr_hash_t *new_actual_props,
                             const svn_skel_t *work_items,
                             apr_pool_t *scratch_pool)
 {
@@ -1640,6 +1693,12 @@ svn_wc__db_base_add_symlink(svn_wc__db_t
   ibb.conflict = conflict;
   ibb.work_items = work_items;
 
+  if (update_actual_props)
+    {
+      ibb.update_actual_props = TRUE;
+      ibb.new_actual_props = new_actual_props;
+    }
+
   /* ### hmm. if this used to be a directory, we should remove children.
      ### or maybe let caller deal with that, if there is a possibility
      ### of a node kind change (rather than eat an extra lookup here).  */

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1094581&r1=1094580&r2=1094581&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Mon Apr 18 14:02:40 2011
@@ -433,6 +433,11 @@ svn_wc__db_get_wcroot(const char **wcroo
    If CONFLICT is not NULL, then it describes a conflict for this node. The
    node will be record as conflicted (in ACTUAL).
 
+   If SET_ACTUAL_PROPS is TRUE, set the properties store NEW_ACTUAL_PROPS
+   as the new set of properties in ACTUAL. If NEW_ACTUAL_PROPS is NULL or
+   when the value of NEW_ACTUAL_PROPS matches NEW_PROPS, store NULL in
+   ACTUAL, to mark the properties unmodified.
+
    Any work items that are necessary as part of this node construction may
    be passed in WORK_ITEMS.
 
@@ -453,6 +458,8 @@ svn_wc__db_base_add_directory(svn_wc__db
                               svn_depth_t depth,
                               apr_hash_t *dav_cache,
                               const svn_skel_t *conflict,
+                              svn_boolean_t set_actual_props,
+                              apr_hash_t *new_actual_props,
                               const svn_skel_t *work_items,
                               apr_pool_t *scratch_pool);
 
@@ -482,6 +489,11 @@ svn_wc__db_base_add_directory(svn_wc__db
    If CONFLICT is not NULL, then it describes a conflict for this node. The
    node will be record as conflicted (in ACTUAL).
 
+   If SET_ACTUAL_PROPS is TRUE, set the properties store NEW_ACTUAL_PROPS
+   as the new set of properties in ACTUAL. If NEW_ACTUAL_PROPS is NULL or
+   when the value of NEW_ACTUAL_PROPS matches NEW_PROPS, store NULL in
+   ACTUAL, to mark the properties unmodified.
+
    Any work items that are necessary as part of this node construction may
    be passed in WORK_ITEMS.
 
@@ -502,6 +514,8 @@ svn_wc__db_base_add_file(svn_wc__db_t *d
                          svn_filesize_t translated_size,
                          apr_hash_t *dav_cache,
                          const svn_skel_t *conflict,
+                         svn_boolean_t set_actual_props,
+                         apr_hash_t *new_actual_props,
                          const svn_skel_t *work_items,
                          apr_pool_t *scratch_pool);
 
@@ -526,6 +540,11 @@ svn_wc__db_base_add_file(svn_wc__db_t *d
    If CONFLICT is not NULL, then it describes a conflict for this node. The
    node will be record as conflicted (in ACTUAL).
 
+   If SET_ACTUAL_PROPS is TRUE, set the properties store NEW_ACTUAL_PROPS
+   as the new set of properties in ACTUAL. If NEW_ACTUAL_PROPS is NULL or
+   when the value of NEW_ACTUAL_PROPS matches NEW_PROPS, store NULL in
+   ACTUAL, to mark the properties unmodified.
+
    Any work items that are necessary as part of this node construction may
    be passed in WORK_ITEMS.
 
@@ -573,6 +592,8 @@ svn_wc__db_base_add_symlink(svn_wc__db_t
                             const char *target,
                             apr_hash_t *dav_cache,
                             const svn_skel_t *conflict,
+                            svn_boolean_t set_actual_props,
+                            apr_hash_t *new_actual_props,
                             const svn_skel_t *work_items,
                             apr_pool_t *scratch_pool);
 

Modified: subversion/trunk/subversion/tests/libsvn_wc/db-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/db-test.c?rev=1094581&r1=1094580&r2=1094581&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/db-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/db-test.c Mon Apr 18 14:02:40 2011
@@ -690,7 +690,7 @@ test_inserting_nodes(apr_pool_t *pool)
             props,
             1, TIME_1a, AUTHOR_1,
             children, svn_depth_infinity,
-            NULL, NULL, NULL,
+            NULL, NULL, FALSE, NULL, NULL,
             pool));
 
   /* Replace an incomplete node with a file node. */
@@ -701,7 +701,7 @@ test_inserting_nodes(apr_pool_t *pool)
             props,
             1, TIME_1a, AUTHOR_1,
             checksum, 10,
-            NULL, NULL, NULL,
+            NULL, NULL, FALSE, NULL, NULL,
             pool));
 
   /* Create a new symlink node. */
@@ -712,7 +712,7 @@ test_inserting_nodes(apr_pool_t *pool)
             props,
             1, TIME_1a, AUTHOR_1,
             "O-target",
-            NULL, NULL, NULL,
+            NULL, NULL, FALSE, NULL, NULL,
             pool));
 
   /* Replace an incomplete node with an absent file node. */

Modified: subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c?rev=1094581&r1=1094580&r2=1094581&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c Mon Apr 18 14:02:40 2011
@@ -1215,7 +1215,8 @@ base_dir_insert_remove(svn_test__sandbox
                                         "not-even-a-uuid", revision,
                                         apr_hash_make(b->pool), revision,
                                         0, NULL, NULL, svn_depth_infinity,
-                                        NULL, NULL, NULL, b->pool));
+                                        NULL, NULL, FALSE, NULL, NULL,
+                                        b->pool));
 
   after = apr_palloc(b->pool, sizeof(*after) * (num_before + num_added + 1));
   for (i = 0; i < num_before; ++i)