You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2010/04/23 03:39:05 UTC

svn commit: r937128 - in /subversion/trunk/subversion/libsvn_wc: props.c props.h update_editor.c

Author: gstein
Date: Fri Apr 23 01:39:05 2010
New Revision: 937128

URL: http://svn.apache.org/viewvc?rev=937128&view=rev
Log:
Change some basics around the svn_wc__merge_props before digging in for
some further work.

Note: this restricts svn_wc_merge_props3() to ONLY "normal" props. The
prior behavior of allowing wc/entry props would result in installing these
as real properties (oops!).

* subversion/libsvn_wc/props.h:
  (svn_wc__merge_props): pass the node's KIND information (or the intended
    KIND if it has not (yet) been created)

* subversion/libsvn_wc/props.c:
  (svn_wc_merge_props3): disallow non-normal propchanges. fetch the KIND
    earlier and pass to merge_props. fetch the pristine/actual props and
    pass them along, too.
  (svn_wc__merge_props): accept the KIND rather than fetching it (and then
    compensating for "unknown"). assert that we always get the set of
    BASE/ACTUAL properties rather than attempting to fetch these or fill
    in some empty defaults. remove all the IS_NORMAL stuff. we only ever
    get normal properties.

* subversion/libsvn_wc/update_editor.c:
  (struct dir_baton): add an ADDING_DIR member
  (make_dir_baton): store the ADDING parameter
  (close_directory): rename WORKING_PROPS localvar to ACTUAL_PROPS. ensure
    that both BASE_PROPS and ACTUAL_PROPS initialized, then pass them.
    pass svn_wc__db_kind_dir into svn_wc__merge_props.
  (close_file): declare and set up CURRENT_BASE_PROPS and
    CURRENT_ACTUAL_PROPS, then pass then to merge_props, along with the
    svn_wc__db_kind_file value.

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

Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=937128&r1=937127&r2=937128&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Fri Apr 23 01:39:05 2010
@@ -629,27 +629,54 @@ svn_wc_merge_props3(svn_wc_notify_state_
                     apr_pool_t *pool /* scratch_pool */)
 {
   svn_boolean_t hidden;
+  int i;
+  svn_wc__db_kind_t kind;
   apr_hash_t *new_base_props;
   apr_hash_t *new_actual_props;
+  apr_hash_t *base_props;
+  apr_hash_t *actual_props;
 
   /* IMPORTANT: svn_wc_merge_prop_diffs relies on the fact that baseprops
      may be NULL. */
 
   /* Checks whether the node exists and returns the hidden flag */
   SVN_ERR(svn_wc__db_node_hidden(&hidden, wc_ctx->db, local_abspath, pool));
-
   if (hidden)
     return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
                              _("The node '%s' was not found."),
                              svn_dirent_local_style(local_abspath, pool));
 
+  /* The PROPCHANGES may not have non-"normal" properties in it. If entry
+     or wc props were allowed, then the following code would install them
+     into the BASE and/or WORKING properties(!).  */
+  for (i = propchanges->nelts; i--; )
+    {
+      const svn_prop_t *change = &APR_ARRAY_IDX(propchanges, i, svn_prop_t);
+
+      if (!svn_wc_is_normal_prop(change->name))
+        return svn_error_createf(SVN_ERR_BAD_PROP_KIND, NULL,
+                                 _("The property '%s' may not be merged "
+                                   "into '%s'."),
+                                 change->name,
+                                 svn_dirent_local_style(local_abspath, pool));
+    }
+
+  SVN_ERR(svn_wc__db_read_kind(&kind, wc_ctx->db, local_abspath, FALSE, pool));
+
+  SVN_ERR(svn_wc__get_pristine_props(&base_props, wc_ctx->db, local_abspath,
+                                     pool, pool));
+  SVN_ERR(svn_wc__get_actual_props(&actual_props, wc_ctx->db, local_abspath,
+                                   pool, pool));
+
   /* Note that while this routine does the "real" work, it's only
      prepping tempfiles and writing log commands.  */
   SVN_ERR(svn_wc__merge_props(state,
                               &new_base_props, &new_actual_props,
-                              wc_ctx->db, local_abspath,
+                              wc_ctx->db, local_abspath, kind,
                               left_version, right_version,
-                              baseprops, NULL, NULL,
+                              baseprops /* server_baseprops */,
+                              base_props,
+                              actual_props,
                               propchanges, base_merge, dry_run,
                               conflict_func, conflict_baton,
                               cancel_func, cancel_baton,
@@ -657,11 +684,8 @@ svn_wc_merge_props3(svn_wc_notify_state_
 
   if (!dry_run)
     {
-      svn_wc__db_kind_t kind;
       const char *dir_abspath;
 
-      SVN_ERR(svn_wc__db_read_kind(&kind, wc_ctx->db, local_abspath,
-                                   FALSE, pool));
       if (kind == svn_wc__db_kind_dir)
         dir_abspath = local_abspath;
       else
@@ -1497,6 +1521,7 @@ svn_wc__merge_props(svn_wc_notify_state_
                     apr_hash_t **new_actual_props,
                     svn_wc__db_t *db,
                     const char *local_abspath,
+                    svn_wc__db_kind_t kind,
                     const svn_wc_conflict_version_t *left_version,
                     const svn_wc_conflict_version_t *right_version,
                     apr_hash_t *server_baseprops,
@@ -1518,52 +1543,21 @@ svn_wc__merge_props(svn_wc_notify_state_
   const char *reject_path = NULL;
   svn_stream_t *reject_tmp_stream = NULL;  /* the temporary conflicts stream */
   const char *reject_tmp_path = NULL;
-  svn_wc__db_kind_t kind;
   const char *adm_abspath;
 
+  SVN_ERR_ASSERT(base_props != NULL);
+  SVN_ERR_ASSERT(working_props != NULL);
+
   *new_base_props = NULL;
   *new_actual_props = NULL;
 
-  /* ### shouldn't ALLOW_MISSING be FALSE? how can we merge props into
-     ### a node that doesn't exist?!  */
-  /* ### BH: In some cases we allow merging into missing to create a new
-             node. */
-  SVN_ERR(svn_wc__db_read_kind(&kind, db, local_abspath, TRUE, scratch_pool));
+  is_dir = (kind == svn_wc__db_kind_dir);
 
-  if (kind == svn_wc__db_kind_dir)
-    {
-      is_dir = TRUE;
-      adm_abspath = local_abspath;
-    }
+  if (is_dir)
+    adm_abspath = local_abspath;
   else
-    {
-      is_dir = FALSE;
-      adm_abspath = svn_dirent_dirname(local_abspath, scratch_pool);
-    }
+    adm_abspath = svn_dirent_dirname(local_abspath, scratch_pool);
 
-  /* If not provided, load the base & working property files into hashes */
-  if (! base_props || ! working_props)
-    {
-      if (kind == svn_wc__db_kind_unknown)
-        {
-          /* No entry... no props.  */
-          if (base_props == NULL)
-            base_props = apr_hash_make(result_pool);
-          if (working_props == NULL)
-            working_props = apr_hash_make(result_pool);
-        }
-      else
-        {
-          if (base_props == NULL)
-            SVN_ERR(load_pristine_props(&base_props,
-                                        db, local_abspath,
-                                        result_pool, scratch_pool));
-          if (working_props == NULL)
-            SVN_ERR(load_actual_props(&working_props,
-                                      db, local_abspath,
-                                      result_pool, scratch_pool));
-        }
-    }
   if (!server_baseprops)
     server_baseprops = base_props;
 
@@ -1584,14 +1578,12 @@ svn_wc__merge_props(svn_wc_notify_state_
       svn_string_t *conflict = NULL;
       const svn_prop_t *incoming_change;
       const svn_string_t *from_val, *to_val, *base_val;
-      svn_boolean_t is_normal;
 
       svn_pool_clear(iterpool);
 
       /* For the incoming propchange, figure out the TO and FROM values. */
       incoming_change = &APR_ARRAY_IDX(propchanges, i, svn_prop_t);
       propname = incoming_change->name;
-      is_normal = svn_wc_is_normal_prop(propname);
       to_val = incoming_change->value
         ? svn_string_dup(incoming_change->value, result_pool) : NULL;
       from_val = apr_hash_get(server_baseprops, propname, APR_HASH_KEY_STRING);
@@ -1604,11 +1596,10 @@ svn_wc__merge_props(svn_wc_notify_state_
       /* We already know that state is at least `changed', so mark
          that, but remember that we may later upgrade to `merged' or
          even `conflicted'. */
-      if (is_normal)
-        set_prop_merge_state(state, svn_wc_notify_state_changed);
+      set_prop_merge_state(state, svn_wc_notify_state_changed);
 
       if (! from_val)  /* adding a new property */
-        SVN_ERR(apply_single_prop_add(is_normal ? state : NULL, &conflict,
+        SVN_ERR(apply_single_prop_add(state, &conflict,
                                       db, local_abspath,
                                       left_version, right_version,
                                       is_dir, working_props,
@@ -1618,7 +1609,7 @@ svn_wc__merge_props(svn_wc_notify_state_
                                       dry_run, result_pool, iterpool));
 
       else if (! to_val) /* delete an existing property */
-        SVN_ERR(apply_single_prop_delete(is_normal ? state : NULL, &conflict,
+        SVN_ERR(apply_single_prop_delete(state, &conflict,
                                          db, local_abspath,
                                          left_version, right_version,
                                          is_dir,
@@ -1629,7 +1620,7 @@ svn_wc__merge_props(svn_wc_notify_state_
                                          dry_run, result_pool, iterpool));
 
       else  /* changing an existing property */
-        SVN_ERR(apply_single_prop_change(is_normal ? state : NULL, &conflict,
+        SVN_ERR(apply_single_prop_change(state, &conflict,
                                          db, local_abspath,
                                          left_version, right_version,
                                          is_dir,
@@ -1645,8 +1636,7 @@ svn_wc__merge_props(svn_wc_notify_state_
 
       if (conflict)
         {
-          if (is_normal)
-            set_prop_merge_state(state, svn_wc_notify_state_conflicted);
+          set_prop_merge_state(state, svn_wc_notify_state_conflicted);
 
           if (dry_run)
             continue;   /* skip to next incoming change */

Modified: subversion/trunk/subversion/libsvn_wc/props.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.h?rev=937128&r1=937127&r2=937128&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.h (original)
+++ subversion/trunk/subversion/libsvn_wc/props.h Fri Apr 23 01:39:05 2010
@@ -120,6 +120,7 @@ svn_wc__merge_props(svn_wc_notify_state_
                     apr_hash_t **new_actual_props,
                     svn_wc__db_t *db,
                     const char *local_abspath,
+                    svn_wc__db_kind_t kind,
                     const svn_wc_conflict_version_t *left_version,
                     const svn_wc_conflict_version_t *right_version,
                     apr_hash_t *server_baseprops,

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=937128&r1=937127&r2=937128&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Fri Apr 23 01:39:05 2010
@@ -300,6 +300,9 @@ struct dir_baton
   /* Set if there was a previous notification for this directory */
   svn_boolean_t already_notified;
 
+  /* Set if this directory is being added during this editor drive. */
+  svn_boolean_t adding_dir;
+
   /* Set on a node and its descendants when a node gets tree conflicted
      and descendants should still be updated (not skipped).
      These nodes should all be marked as deleted. */
@@ -661,6 +664,7 @@ make_dir_baton(struct dir_baton **d_p,
   d->add_existed  = FALSE;
   d->bump_info    = bdi;
   d->old_revision = SVN_INVALID_REVNUM;
+  d->adding_dir   = adding;
 
   /* The caller of this function needs to fill these in. */
   d->ambient_depth = svn_depth_unknown;
@@ -2884,7 +2888,8 @@ close_directory(void *dir_baton,
   struct last_change_info *last_change = NULL;
   svn_wc_notify_state_t prop_state = svn_wc_notify_state_unknown;
   apr_array_header_t *entry_props, *wc_props, *regular_props;
-  apr_hash_t *base_props = NULL, *working_props = NULL;
+  apr_hash_t *base_props;
+  apr_hash_t *actual_props;
   apr_hash_t *new_base_props = NULL, *new_actual_props = NULL;
   struct bump_dir_info *bdi;
 
@@ -2905,6 +2910,23 @@ close_directory(void *dir_baton,
   SVN_ERR(svn_categorize_props(db->propchanges, &entry_props, &wc_props,
                                &regular_props, pool));
 
+  if (db->adding_dir)
+    {
+      /* A newly added directory has no properties.  */
+      base_props = apr_hash_make(pool);
+      actual_props = apr_hash_make(pool);
+    }
+  else
+    {
+      /* Fetch the existing properties.  */
+      SVN_ERR(svn_wc__get_pristine_props(&base_props,
+                                         eb->db, db->local_abspath,
+                                         pool, pool));
+      SVN_ERR(svn_wc__get_actual_props(&actual_props,
+                                       eb->db, db->local_abspath,
+                                       pool, pool));
+    }
+
   /* An incomplete directory might have props which were supposed to be
      deleted but weren't.  Because the server sent us all the props we're
      supposed to have, any previous base props not in this list must be
@@ -2913,26 +2935,8 @@ close_directory(void *dir_baton,
     {
       int i;
       apr_hash_t *props_to_delete;
-      svn_wc__db_kind_t kind;
       apr_hash_index_t *hi;
 
-      SVN_ERR(svn_wc__db_read_kind(&kind, eb->db,
-                                   db->local_abspath, TRUE, pool));
-      if (kind == svn_wc__db_kind_unknown)
-        {
-          base_props = apr_hash_make(pool);
-          working_props = apr_hash_make(pool);
-        }
-      else
-        {
-          SVN_ERR(svn_wc__get_pristine_props(&base_props,
-                                             eb->db, db->local_abspath,
-                                             pool, pool));
-          SVN_ERR(svn_wc__get_actual_props(&working_props,
-                                           eb->db, db->local_abspath,
-                                           pool, pool));
-        }
-
       /* In a copy of the BASE props, remove every property that we see an
          incoming change for. The remaining unmentioned properties are those
          which need to be deleted.  */
@@ -3007,10 +3011,12 @@ close_directory(void *dir_baton,
                                         &new_actual_props,
                                         eb->db,
                                         db->local_abspath,
+                                        svn_wc__db_kind_dir,
                                         NULL, /* left_version */
                                         NULL, /* right_version */
                                         NULL /* use baseprops */,
-                                        base_props, working_props,
+                                        base_props,
+                                        actual_props,
                                         regular_props,
                                         TRUE /* base_merge */,
                                         FALSE /* dry_run */,
@@ -4683,6 +4689,8 @@ close_file(void *file_baton,
   const svn_wc_entry_t *entry;
   svn_boolean_t install_pristine;
   const char *install_from;
+  apr_hash_t *current_base_props;
+  apr_hash_t *current_actual_props;
 
   if (fb->skip_this)
     {
@@ -4787,6 +4795,25 @@ close_file(void *file_baton,
      BASE_PROPS and WORKING_PROPS are hashes of the base and
      working props of the file; if NULL they are read from the wc.  */
 
+  if (fb->adding_file)
+    {
+      current_base_props = (fb->copied_base_props
+                            ? fb->copied_base_props
+                            : apr_hash_make(pool));
+      current_actual_props = (fb->copied_working_props
+                              ? fb->copied_working_props
+                              : apr_hash_make(pool));
+    }
+  else
+    {
+      SVN_ERR(svn_wc__get_pristine_props(&current_base_props,
+                                         eb->db, fb->local_abspath,
+                                         pool, pool));
+      SVN_ERR(svn_wc__get_actual_props(&current_actual_props,
+                                       eb->db, fb->local_abspath,
+                                       pool, pool));
+    }
+
   prop_state = svn_wc_notify_state_unknown;
 
   /* Merge the 'regular' props into the existing working proplist. */
@@ -4798,11 +4825,12 @@ close_file(void *file_baton,
                               &new_actual_props,
                               eb->db,
                               fb->local_abspath,
+                              svn_wc__db_kind_file,
                               NULL /* left_version */,
                               NULL /* right_version */,
                               NULL /* server_baseprops (update, not merge)  */,
-                              fb->copied_base_props,
-                              fb->copied_working_props,
+                              current_base_props,
+                              current_actual_props,
                               regular_props,
                               TRUE /* base_merge */,
                               FALSE /* dry_run */,



Re: svn commit: r937128 - in /subversion/trunk/subversion/libsvn_wc: props.c props.h update_editor.c

Posted by Greg Stein <gs...@gmail.com>.
On Fri, Apr 23, 2010 at 06:48, Philip Martin <ph...@wandisco.com> wrote:
> gstein@apache.org writes:
>
>> Author: gstein
>> Date: Fri Apr 23 01:39:05 2010
>> New Revision: 937128
>>
>> URL: http://svn.apache.org/viewvc?rev=937128&view=rev
>> Log:
>> Change some basics around the svn_wc__merge_props before digging in for
>> some further work.
>>
>> Note: this restricts svn_wc_merge_props3() to ONLY "normal" props. The
>> prior behavior of allowing wc/entry props would result in installing these
>> as real properties (oops!).
>>
>> * subversion/libsvn_wc/props.h:
>>   (svn_wc__merge_props): pass the node's KIND information (or the intended
>>     KIND if it has not (yet) been created)
>>
>> * subversion/libsvn_wc/props.c:
>>   (svn_wc_merge_props3): disallow non-normal propchanges. fetch the KIND
>>     earlier and pass to merge_props. fetch the pristine/actual props and
>>     pass them along, too.
>>   (svn_wc__merge_props): accept the KIND rather than fetching it (and then
>>     compensating for "unknown"). assert that we always get the set of
>>     BASE/ACTUAL properties rather than attempting to fetch these or fill
>>     in some empty defaults. remove all the IS_NORMAL stuff. we only ever
>>     get normal properties.
>
> This causes
>
> FAIL:  merge_tests.py 54: set no mergeinfo when merging from foreign repos
> FAIL:  update_tests.py 33: update wc containing a replaced-with-history file
> FAIL:  update_tests.py 34: update handles obstructing paths scheduled for add
>
> all in svn_wc__merge_props with
>
> ../src/subversion/libsvn_subr/io.c:2935: (apr_err=235000)
> svn: In file '../src/subversion/libsvn_wc/props.c' line 1548: assertion failed (base_props != NULL)

Yup, thanks. I just discovered that after my full test run completed.

update-34 is not due to the assertion, however, so this isn't as
straightforward...

Cheers,
-g

Re: svn commit: r937128 - in /subversion/trunk/subversion/libsvn_wc: props.c props.h update_editor.c

Posted by Philip Martin <ph...@wandisco.com>.
gstein@apache.org writes:

> Author: gstein
> Date: Fri Apr 23 01:39:05 2010
> New Revision: 937128
>
> URL: http://svn.apache.org/viewvc?rev=937128&view=rev
> Log:
> Change some basics around the svn_wc__merge_props before digging in for
> some further work.
>
> Note: this restricts svn_wc_merge_props3() to ONLY "normal" props. The
> prior behavior of allowing wc/entry props would result in installing these
> as real properties (oops!).
>
> * subversion/libsvn_wc/props.h:
>   (svn_wc__merge_props): pass the node's KIND information (or the intended
>     KIND if it has not (yet) been created)
>
> * subversion/libsvn_wc/props.c:
>   (svn_wc_merge_props3): disallow non-normal propchanges. fetch the KIND
>     earlier and pass to merge_props. fetch the pristine/actual props and
>     pass them along, too.
>   (svn_wc__merge_props): accept the KIND rather than fetching it (and then
>     compensating for "unknown"). assert that we always get the set of
>     BASE/ACTUAL properties rather than attempting to fetch these or fill
>     in some empty defaults. remove all the IS_NORMAL stuff. we only ever
>     get normal properties.

This causes

FAIL:  merge_tests.py 54: set no mergeinfo when merging from foreign repos
FAIL:  update_tests.py 33: update wc containing a replaced-with-history file
FAIL:  update_tests.py 34: update handles obstructing paths scheduled for add

all in svn_wc__merge_props with

../src/subversion/libsvn_subr/io.c:2935: (apr_err=235000)
svn: In file '../src/subversion/libsvn_wc/props.c' line 1548: assertion failed (base_props != NULL)

-- 
Philip