You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2010/10/18 11:39:12 UTC

svn commit: r1023708 - in /subversion/trunk/subversion: libsvn_wc/props.c libsvn_wc/update_editor.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/db-test.c

Author: julianfoad
Date: Mon Oct 18 09:39:11 2010
New Revision: 1023708

URL: http://svn.apache.org/viewvc?rev=1023708&view=rev
Log:
Factor out some repeated code.  The ACTUAL_NODE table stores the properties
only when they differ from the pristine properties, and this comparison was
being performed in several places.  Do it in svn_wc__db_op_set_props()
instead.

* subversion/libsvn_wc/props.c
  (immediate_install_props, svn_wc__perform_props_merge): Don't compare
    against pristine props; instead, pass the pristine props to
    svn_wc__db_op_set_props().

* subversion/libsvn_wc/update_editor.c
  (close_directory, close_file, svn_wc_add_repos_file4): Same.

* subversion/libsvn_wc/wc_db.c,
  subversion/libsvn_wc/wc_db.h
  (svn_wc__db_op_set_props): Take the pristine props as an extra parameter,
    and set the ACTUAL_NODE props to null if they're the same as that.

* subversion/tests/libsvn_wc/db-test.c
  (validate_node): Pass the pristine props to svn_wc__db_op_set_props().

Modified:
    subversion/trunk/subversion/libsvn_wc/props.c
    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

Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=1023708&r1=1023707&r2=1023708&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Mon Oct 18 09:39:11 2010
@@ -149,21 +149,8 @@ immediate_install_props(svn_wc__db_t *db
                                            scratch_pool, scratch_pool),
             _("Failed to load pristine properties"));
 
-  /* Check if the props are modified. If no changes, then wipe out
-     the ACTUAL props. No pristines defined means that any ACTUAL
-     props are okay, so go ahead and set them.  */
-  if (base_props != NULL)
-    {
-      apr_array_header_t *prop_diffs;
-
-      SVN_ERR(svn_prop_diffs(&prop_diffs, working_props, base_props,
-                             scratch_pool));
-      if (prop_diffs->nelts == 0)
-        working_props = NULL;
-    }
-
   SVN_ERR(svn_wc__db_op_set_props(db, local_abspath,
-                                  working_props,
+                                  working_props, base_props,
                                   NULL /* conflict */,
                                   NULL, /* work_items */
                                   scratch_pool));
@@ -349,7 +336,6 @@ svn_wc__perform_props_merge(svn_wc_notif
       {
         svn_wc__db_status_t status;
         svn_boolean_t have_base;
-        apr_array_header_t *prop_diffs;
 
         SVN_ERR(svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL,
                                      NULL, NULL, NULL, NULL, NULL, NULL, NULL,
@@ -364,41 +350,20 @@ svn_wc__perform_props_merge(svn_wc_notif
           SVN_ERR(svn_wc__db_temp_base_set_props(db, local_abspath,
                                                  new_base_props, pool));
 
-        /* Check if the props are modified. */
-        SVN_ERR(svn_prop_diffs(&prop_diffs, actual_props, new_base_props, pool));
-
-        /* Save the actual properties file if it differs from base. */
-        if (prop_diffs->nelts == 0)
-          SVN_ERR(svn_wc__db_op_set_props(db, local_abspath, NULL, NULL, NULL,
-                                          pool));
-        else
-          SVN_ERR(svn_wc__db_op_set_props(db, local_abspath, actual_props,
-                                          NULL, NULL, pool));
+        SVN_ERR(svn_wc__db_op_set_props(db, local_abspath, actual_props,
+                                        new_base_props,
+                                        NULL, NULL, pool));
       }
 #else
       if (base_merge)
         return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
                                 U_("base_merge=TRUE is no longer supported"));
 
-      {
-        apr_array_header_t *prop_diffs;
-
-        SVN_ERR(svn_prop_diffs(&prop_diffs, new_actual_props, new_base_props,
-                               pool));
-
-        /* Save the actual properties file if it differs from base. */
-        if (prop_diffs->nelts == 0)
-          new_actual_props = NULL; /* Remove actual properties*/
-
-        /* For the old school: write the properties into the "working"
-           (aka ACTUAL) location. Note that PROPS may be NULL, indicating
-           a removal of the props file.  */
-
-        SVN_ERR(svn_wc__db_op_set_props(db, local_abspath, new_actual_props,
-                                        NULL /* conflict */,
-                                        NULL /* work_item */,
-                                        pool));
-      }
+      SVN_ERR(svn_wc__db_op_set_props(db, local_abspath, new_actual_props,
+                                      new_base_props,
+                                      NULL /* conflict */,
+                                      NULL /* work_item */,
+                                      pool));
 #endif
 
       SVN_ERR(svn_wc__wq_run(db, local_abspath,

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1023708&r1=1023707&r2=1023708&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Mon Oct 18 09:39:11 2010
@@ -2925,22 +2925,10 @@ close_directory(void *dir_baton,
          item to write out an old-style props file.  */
       if (new_base_props != NULL)
         {
-          apr_array_header_t *prop_diffs;
-
           SVN_ERR_ASSERT(new_actual_props != NULL);
 
-          /* If the ACTUAL props are the same as the BASE props, then we
-             should "write" a NULL. This will remove the props from the
-             ACTUAL_NODE row, and remove the old-style props file, indicating
-             "no change".  */
-          props = new_actual_props;
-          SVN_ERR(svn_prop_diffs(&prop_diffs, new_actual_props, new_base_props,
-                                 pool));
-          if (prop_diffs->nelts == 0)
-            props = NULL;
-
           SVN_ERR(svn_wc__db_op_set_props(eb->db, db->local_abspath,
-                                          props,
+                                          new_actual_props, new_base_props,
                                           NULL /* conflict */,
                                           NULL /* work_items */,
                                           pool));
@@ -4586,23 +4574,10 @@ close_file(void *file_baton,
      properties merge. */
   if (! fb->adding_base_under_local_add)
     {
-      apr_hash_t *props;
-      apr_array_header_t *prop_diffs;
-
       SVN_ERR_ASSERT(new_actual_props != NULL);
 
-      /* If the ACTUAL props are the same as the BASE props, then we
-         should "write" a NULL. This will remove the props from the
-         ACTUAL_NODE row, and remove the old-style props file, indicating
-         "no change".  */
-      props = new_actual_props;
-      SVN_ERR(svn_prop_diffs(&prop_diffs, new_actual_props, new_base_props,
-                             pool));
-      if (prop_diffs->nelts == 0)
-        props = NULL;
-
       SVN_ERR(svn_wc__db_op_set_props(eb->db, fb->local_abspath,
-                                      props,
+                                      new_actual_props, new_base_props,
                                       NULL /* conflict */,
                                       NULL /* work_item */,
                                       pool));
@@ -5591,7 +5566,6 @@ svn_wc_add_repos_file4(svn_wc_context_t 
   const char *original_root_url;
   const char *original_repos_relpath;
   const char *original_uuid;
-  apr_hash_t *actual_props;
   svn_revnum_t changed_rev;
   apr_time_t changed_date;
   const char *changed_author;
@@ -5713,25 +5687,6 @@ svn_wc_add_repos_file4(svn_wc_context_t 
                                    entry_props, pool, pool));
   }
 
-  /* Add some work items to install the properties.  */
-  {
-    if (new_props == NULL)
-      {
-        actual_props = NULL;
-      }
-    else
-      {
-        apr_array_header_t *prop_diffs;
-
-        SVN_ERR(svn_prop_diffs(&prop_diffs, new_props, new_base_props,
-                               pool));
-        if (prop_diffs->nelts == 0)
-          actual_props = NULL;
-        else
-          actual_props = new_props;
-      }
-  }
-
   /* Copy NEW_BASE_CONTENTS into a temporary file so our log can refer to
      it, and set TMP_TEXT_BASE_ABSPATH to its path.  Compute its
      NEW_TEXT_BASE_MD5_CHECKSUM and NEW_TEXT_BASE_SHA1_CHECKSUM as we copy. */
@@ -5856,7 +5811,7 @@ svn_wc_add_repos_file4(svn_wc_context_t 
   /* ### if below fails, then the above db change would remain :-(  */
 
   SVN_ERR(svn_wc__db_op_set_props(db, local_abspath,
-                                  actual_props,
+                                  new_props, new_base_props,
                                   NULL /* conflict */,
                                   all_work_items,
                                   pool));

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1023708&r1=1023707&r2=1023708&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Oct 18 09:39:11 2010
@@ -3338,6 +3338,7 @@ svn_error_t *
 svn_wc__db_op_set_props(svn_wc__db_t *db,
                         const char *local_abspath,
                         apr_hash_t *props,
+                        apr_hash_t *pristine_props,
                         const svn_skel_t *conflict,
                         const svn_skel_t *work_items,
                         apr_pool_t *scratch_pool)
@@ -3352,6 +3353,19 @@ svn_wc__db_op_set_props(svn_wc__db_t *db
                               scratch_pool, scratch_pool));
   VERIFY_USABLE_PDH(pdh);
 
+  /* Check if the props are modified. If no changes, then wipe out the
+     ACTUAL props.  PRISTINE_PROPS==NULL means the caller knows that any
+     ACTUAL props are okay as provided, so go ahead and set them.  */
+  if (props && pristine_props)
+    {
+      apr_array_header_t *prop_diffs;
+
+      SVN_ERR(svn_prop_diffs(&prop_diffs, props, pristine_props,
+                             scratch_pool));
+      if (prop_diffs->nelts == 0)
+        props = NULL;
+    }
+
   spb.props = props;
   spb.wc_id = pdh->wcroot->wc_id;
   spb.conflict = conflict;

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1023708&r1=1023707&r2=1023708&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Mon Oct 18 09:39:11 2010
@@ -1122,10 +1122,16 @@ svn_wc__db_op_add_symlink(svn_wc__db_t *
    To specify no properties, PROPS must be an empty hash, not NULL.
    If the node is not present, return an error.
 
-   ### All the callers are doing a comparison against the current 'pristine'
-       props before calling this, and are passing NULL if the actual props
-       are to be the same as the pristine props. This behaviour should be
-       encapsulated.
+   If PROPS is NULL, set the properties to be the same as the pristine
+   properties.
+
+   PRISTINE_PROPS must be the pristine props against which the actual props
+   may be elided in the DB, or NULL if the caller knows that the actual
+   props should not be elided.
+
+   ### TODO: The caller should not have to provide PRISTINE_PROPS.
+     Encapsulate this functionality, either at this level or in a higher-
+     level API.
 
    CONFLICT is used to register a conflict on this node at the same time
    the properties are changed.
@@ -1145,6 +1151,7 @@ svn_error_t *
 svn_wc__db_op_set_props(svn_wc__db_t *db,
                         const char *local_abspath,
                         apr_hash_t *props,
+                        apr_hash_t *pristine_props,
                         const svn_skel_t *conflict,
                         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=1023708&r1=1023707&r2=1023708&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/db-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/db-test.c Mon Oct 18 09:39:11 2010
@@ -604,14 +604,17 @@ validate_node(svn_wc__db_t *db,
   SVN_TEST_STRING_ASSERT(value->data, "v1");
 
   /* Now add a property value and read it back (all on actual) */
-  apr_hash_set(props, "p999", APR_HASH_KEY_STRING, value);
-
-  SVN_ERR(svn_wc__db_op_set_props(db, path, props, NULL, NULL, scratch_pool));
-  SVN_ERR(svn_wc__db_read_props(&props, db, path,
-                                scratch_pool, scratch_pool));
-  SVN_TEST_ASSERT(props != NULL);
-  value = apr_hash_get(props, "p999", APR_HASH_KEY_STRING);
-  SVN_TEST_STRING_ASSERT(value->data, "v1");
+  {
+    apr_hash_t *actual_props = apr_hash_copy(scratch_pool, props);
+    apr_hash_set(actual_props, "p999", APR_HASH_KEY_STRING, value);
+    SVN_ERR(svn_wc__db_op_set_props(db, path, actual_props, props,
+                                    NULL, NULL, scratch_pool));
+    SVN_ERR(svn_wc__db_read_props(&props, db, path,
+                                  scratch_pool, scratch_pool));
+    SVN_TEST_ASSERT(props != NULL);
+    value = apr_hash_get(props, "p999", APR_HASH_KEY_STRING);
+    SVN_TEST_STRING_ASSERT(value->data, "v1");
+  }
 
   return SVN_NO_ERROR;
 }