You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David Glasser <gl...@davidglasser.net> on 2008/01/04 22:03:51 UTC

Re: svn commit: r28752 - in branches/reintegrate/subversion: libsvn_fs_base tests/libsvn_fs

On Jan 4, 2008 4:36 PM,  <cm...@tigris.org> wrote:
> Author: cmpilato
> Date: Fri Jan  4 13:36:51 2008
> New Revision: 28752
>
> Log:
> In libsvn_fs_base on the 'reintegrate' branch, implement some logic
> for getting and setting the new HAS-MERGEINFO and MERGEINFO-COUNT node
> revision members, and teach that provider's implementations of
> 'svn_fs_change_node_prop', 'svn_fs_copy', 'svn_fs_revision_link', and
> 'svn_fs_delete' to use them.

Yay!

> NOTE: I've added some unused functions because they were simple to
>       add.  Will trim out the ones that remain unused once finished
>       with this work.
>
> * subversion/libsvn_fs_base/tree.c
>   (adjust_parent_mergeinfo_counts): New helper function.
>   (change_txn_mergeinfo): Removed.
>   (txn_body_change_node_prop): Call svn_fs_base__dag_set_has_mergeinfo()
>     to adjust the node's HAS-MERGEINFO and MERGEINFO-COUNT members, and
>     then use adjust_parent_mergeinfo_counts() on its parents (if any).
>   (txn_body_delete, txn_body_copy): Adjust the mergeinfo counts of the
>     deleted node's parents.
>
> * subversion/libsvn_fs_base/dag.h,
> * subversion/libsvn_fs_base/dag.c
>   (svn_fs_base__dag_get_has_mergeinfo, svn_fs_base__dag_set_has_mergeinfo,
>    svn_fs_base__dag_get_mergeinfo_count, svn_fs_base__dag_set_mergeinfo_count,
>    svn_fs_base__dag_adjust_mergeinfo_count):  New functions.
>
> * subversion/tests/libsvn_fs/fs-test.c
>   (unordered_txn_dirprops): Tweak this test to use the svn:mergeinfo
>     property, just so there's some FS code doing so.

Is it worth a comment saying "btw, we're using svn:mergeinfo here to
make sure that the special code doesn't crash"?

>
> Modified:
>    branches/reintegrate/subversion/libsvn_fs_base/dag.c
>    branches/reintegrate/subversion/libsvn_fs_base/dag.h
>    branches/reintegrate/subversion/libsvn_fs_base/tree.c
>    branches/reintegrate/subversion/tests/libsvn_fs/fs-test.c
>
> Modified: branches/reintegrate/subversion/libsvn_fs_base/dag.c
> URL: http://svn.collab.net/viewvc/svn/branches/reintegrate/subversion/libsvn_fs_base/dag.c?pathrev=28752&r1=28751&r2=28752
> ==============================================================================
> --- branches/reintegrate/subversion/libsvn_fs_base/dag.c        (original)
> +++ branches/reintegrate/subversion/libsvn_fs_base/dag.c        Fri Jan  4 13:36:51 2008
> @@ -1515,3 +1515,124 @@
>
>    return SVN_NO_ERROR;
>  }
> +
> +
> +
> +/*** Mergeinfo tracking stuff ***/
> +
> +svn_error_t *svn_fs_base__dag_get_has_mergeinfo(svn_boolean_t *has_mergeinfo,
> +                                                dag_node_t *node,
> +                                                trail_t *trail,
> +                                                apr_pool_t *pool)
> +{
> +  node_revision_t *node_rev;
> +  svn_fs_t *fs = svn_fs_base__dag_get_fs(node);
> +  const svn_fs_id_t *id = svn_fs_base__dag_get_id(node);
> +
> +  SVN_ERR(svn_fs_bdb__get_node_revision(&node_rev, fs, id, trail, pool));
> +  *has_mergeinfo = node_rev->has_mergeinfo;
> +  return SVN_NO_ERROR;
> +}
> +
> +
> +svn_error_t *svn_fs_base__dag_set_has_mergeinfo(dag_node_t *node,
> +                                                svn_boolean_t has_mergeinfo,
> +                                                svn_boolean_t *had_mergeinfo,
> +                                                const char *txn_id,
> +                                                trail_t *trail,
> +                                                apr_pool_t *pool)
> +{
> +  node_revision_t *node_rev;
> +  svn_fs_t *fs = svn_fs_base__dag_get_fs(node);
> +  const svn_fs_id_t *id = svn_fs_base__dag_get_id(node);
> +
> +  if (! svn_fs_base__dag_check_mutable(node, txn_id))
> +    return svn_error_createf(SVN_ERR_FS_NOT_MUTABLE, NULL,
> +                             _("Attempted merge tracking info change on "
> +                               "immutable node"));
> +
> +  SVN_ERR(svn_fs_bdb__get_node_revision(&node_rev, fs, id, trail, pool));
> +  *had_mergeinfo = node_rev->has_mergeinfo;
> +
> +  /* Are we changing the node? */
> +  if ((! has_mergeinfo) != (! *had_mergeinfo))
> +    {
> +      /* Not the new has-mergeinfo state. */

"Note"

> +      node_rev->has_mergeinfo = has_mergeinfo;
> +
> +      /* Increment or decrement the mergeinfo count as necessary. */
> +      if (has_mergeinfo)
> +        node_rev->mergeinfo_count++;
> +      else
> +        node_rev->mergeinfo_count--;
> +
> +      SVN_ERR(svn_fs_bdb__put_node_revision(fs, id, node_rev, trail, pool));
> +    }
> +  return SVN_NO_ERROR;
> +}
> +
> +
> +svn_error_t *svn_fs_base__dag_get_mergeinfo_count(apr_int64_t *count,
> +                                                  dag_node_t *node,
> +                                                  trail_t *trail,
> +                                                  apr_pool_t *pool)
> +{
> +  node_revision_t *node_rev;
> +  svn_fs_t *fs = svn_fs_base__dag_get_fs(node);
> +  const svn_fs_id_t *id = svn_fs_base__dag_get_id(node);
> +
> +  SVN_ERR(svn_fs_bdb__get_node_revision(&node_rev, fs, id, trail, pool));
> +  *count = node_rev->mergeinfo_count;
> +  return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *svn_fs_base__dag_set_mergeinfo_count(dag_node_t *node,
> +                                                  apr_int64_t count,
> +                                                  const char *txn_id,
> +                                                  trail_t *trail,
> +                                                  apr_pool_t *pool)
> +{
> +  node_revision_t *node_rev;
> +  svn_fs_t *fs = svn_fs_base__dag_get_fs(node);
> +  const svn_fs_id_t *id = svn_fs_base__dag_get_id(node);
> +
> +  if (! svn_fs_base__dag_check_mutable(node, txn_id))
> +    return svn_error_createf(SVN_ERR_FS_NOT_MUTABLE, NULL,
> +                             _("Attempted mergeinfo count change on "
> +                               "immutable node"));
> +
> +  SVN_ERR(svn_fs_bdb__get_node_revision(&node_rev, fs, id, trail, pool));
> +  if (node_rev->mergeinfo_count == count)

Is this comparison backwards?

> +    {
> +      node_rev->mergeinfo_count = count;
> +      SVN_ERR(svn_fs_bdb__put_node_revision(fs, id, node_rev, trail, pool));
> +    }
> +  return SVN_NO_ERROR;
> +}
> +
> +
> +svn_error_t *svn_fs_base__dag_adjust_mergeinfo_count(dag_node_t *node,
> +                                                     apr_int64_t count_delta,
> +                                                     const char *txn_id,
> +                                                     trail_t *trail,
> +                                                     apr_pool_t *pool)
> +{
> +  node_revision_t *node_rev;
> +  svn_fs_t *fs = svn_fs_base__dag_get_fs(node);
> +  const svn_fs_id_t *id = svn_fs_base__dag_get_id(node);
> +
> +  if (! svn_fs_base__dag_check_mutable(node, txn_id))
> +    return svn_error_createf(SVN_ERR_FS_NOT_MUTABLE, NULL,
> +                             _("Attempted mergeinfo count change on "
> +                               "immutable node"));
> +
> +  if (count_delta == 0)
> +    return SVN_NO_ERROR;
> +
> +  SVN_ERR(svn_fs_bdb__get_node_revision(&node_rev, fs, id, trail, pool));
> +  node_rev->mergeinfo_count = node_rev->mergeinfo_count + count_delta;
> +  assert(node_rev->mergeinfo_count >= 0);

This error can be caused by corrupted data, not just by programmer
failure, so I'd use an SVN error instead of an assert.  It might also
be reasonable to put a similar check in the set_mergeinfo_count, and
to do a "files have at most 1" check.

> +  SVN_ERR(svn_fs_bdb__put_node_revision(fs, id, node_rev, trail, pool));
> +
> +  return SVN_NO_ERROR;
> +}
>
> Modified: branches/reintegrate/subversion/libsvn_fs_base/dag.h
> URL: http://svn.collab.net/viewvc/svn/branches/reintegrate/subversion/libsvn_fs_base/dag.h?pathrev=28752&r1=28751&r2=28752
> ==============================================================================
> --- branches/reintegrate/subversion/libsvn_fs_base/dag.h        (original)
> +++ branches/reintegrate/subversion/libsvn_fs_base/dag.h        Fri Jan  4 13:36:51 2008
> @@ -152,6 +152,83 @@
>
>
>
> +/* Mergeinfo tracking stuff. */
> +
> +/* Set *HAS_MERGEINFO to TRUE iff NODE does not record that it's
> +   property list contains merge tracking information, as part of
> +   TRAIL.  Use POOL for necessary allocations.
> +
> +   NOTE:  No validation against NODE's actual property list is
> +   performed. */
> +svn_error_t *svn_fs_base__dag_get_has_mergeinfo(svn_boolean_t *has_mergeinfo,
> +                                                dag_node_t *node,
> +                                                trail_t *trail,
> +                                                apr_pool_t *pool);
> +
> +/* If HAS_MERGEINFO is set, record on NODE that its property list
> +   carries merge tracking information.  Otherwise, record on NODE its
> +   property list does *not* carry merge tracking information.  NODE
> +   must be mutable under TXN_ID (the Subversion transaction under
> +   which this operation occurs).  Set *HAD_MERGEINFO to the previous
> +   state of this record.
> +
> +   Update the mergeinfo count on NODE as necessary.
> +
> +   Do all of this as part of TRAIL, and use POOL for necessary
> +   allocations.
> +
> +   NOTE:  No validation against NODE's actual property list is
> +   performed. */
> +svn_error_t *svn_fs_base__dag_set_has_mergeinfo(dag_node_t *node,
> +                                                svn_boolean_t has_mergeinfo,
> +                                                svn_boolean_t *had_mergeinfo,
> +                                                const char *txn_id,
> +                                                trail_t *trail,
> +                                                apr_pool_t *pool);
> +
> +/* Set *COUNT to the number of nodes -- including NODE itself -- in
> +   the subtree rooted at NODE which claim to carry merge tracking
> +   information.  Do this as part of TRAIL, and use POOL for necessary
> +   allocations.
> +
> +   NOTE:  No validation of these claims is performed. */
> +svn_error_t *svn_fs_base__dag_get_mergeinfo_count(apr_int64_t *count,
> +                                                  dag_node_t *node,
> +                                                  trail_t *trail,
> +                                                  apr_pool_t *pool);
> +
> +/* Record on NODE that COUNT nodes -- including NODE itself -- in the
> +   subtree rooted at NODE claim to carry merge tracking information.
> +   NODE must be mutable under TXN_ID (the Subversion transaction under
> +   which this operation occurs).  Do this as part of TRAIL, and use
> +   POOL for necessary allocations.
> +
> +   NOTE:  No validation of these claims is performed. */
> +svn_error_t *svn_fs_base__dag_set_mergeinfo_count(dag_node_t *node,
> +                                                  apr_int64_t count,
> +                                                  const char *txn_id,
> +                                                  trail_t *trail,
> +                                                  apr_pool_t *pool);
> +
> +
> +/* Record on NODE a change of COUNT_DELTA nodes -- including NODE
> +   itself -- in the subtree rooted at NODE claim to carry merge
> +   tracking information.  That is, add COUNT_DELTA to NODE's current
> +   mergeinfo count (regardless of whether COUNT_DELTA is a positive or
> +   negative integer).
> +
> +   NODE must be mutable under TXN_ID (the Subversion transaction under
> +   which this operation occurs).  Do this as part of TRAIL, and use
> +   POOL for necessary allocations.
> +
> +   NOTE:  No validation of these claims is performed. */
> +svn_error_t *svn_fs_base__dag_adjust_mergeinfo_count(dag_node_t *node,
> +                                                     apr_int64_t count_delta,
> +                                                     const char *txn_id,
> +                                                     trail_t *trail,
> +                                                     apr_pool_t *pool);
> +
> +
>  /* Revision and transaction roots.  */
>
>
>
> Modified: branches/reintegrate/subversion/libsvn_fs_base/tree.c
> URL: http://svn.collab.net/viewvc/svn/branches/reintegrate/subversion/libsvn_fs_base/tree.c?pathrev=28752&r1=28751&r2=28752
> ==============================================================================
> --- branches/reintegrate/subversion/libsvn_fs_base/tree.c       (original)
> +++ branches/reintegrate/subversion/libsvn_fs_base/tree.c       Fri Jan  4 13:36:51 2008
> @@ -848,6 +848,35 @@
>  }
>
>
> +/* Walk up PARENT_PATH to the root of the tree, adjusting each node's
> +   mergeinfo count by COUNT_DELTA as part of Subversion transaction
> +   TXN_ID and TRAIL.  Use POOL for allocations. */
> +static svn_error_t *
> +adjust_parent_mergeinfo_counts(parent_path_t *parent_path,
> +                               apr_int64_t count_delta,
> +                               const char *txn_id,
> +                               trail_t *trail,
> +                               apr_pool_t *pool)
> +{
> +  parent_path_t *pp = parent_path;
> +  apr_pool_t *subpool = svn_pool_create(pool);
> +
> +  if (count_delta == 0)
> +    return SVN_NO_ERROR;
> +
> +  while (pp)
> +    {
> +      svn_pool_clear(subpool);
> +      SVN_ERR(svn_fs_base__dag_adjust_mergeinfo_count(pp->node, count_delta,
> +                                                      txn_id, trail, subpool));
> +      pp = pp->parent;
> +    }
> +  svn_pool_destroy(subpool);
> +
> +  return SVN_NO_ERROR;
> +}
> +
> +
>  /* Open the node identified by PATH in ROOT, as part of TRAIL.  Set
>     *DAG_NODE_P to the node we find, allocated in TRAIL->pool.  Return
>     the error SVN_ERR_FS_NOT_FOUND if this node doesn't exist. */
> @@ -1265,6 +1294,25 @@
>    SVN_ERR(svn_fs_base__dag_set_proplist(parent_path->node, proplist,
>                                          txn_id, trail, trail->pool));
>
> +  /* If this was a change to the mergeinfo property, we have some
> +     extra recording to do. */
> +  if (strcmp(args->name, SVN_PROP_MERGEINFO) == 0)
> +    {
> +      svn_boolean_t had_mergeinfo, has_mergeinfo = args->value ? TRUE : FALSE;
> +
> +      /* First, note on our node that it has mergeinfo. */
> +      SVN_ERR(svn_fs_base__dag_set_has_mergeinfo(parent_path->node,
> +                                                 has_mergeinfo, &had_mergeinfo,
> +                                                 txn_id, trail, trail->pool));
> +
> +      /* If this is a change from the old state, we need to update our
> +         node's parents' mergeinfo counts by a factor of 1. */
> +      if (parent_path->parent && ((! had_mergeinfo) != (! has_mergeinfo)))
> +        SVN_ERR(adjust_parent_mergeinfo_counts(parent_path->parent,
> +                                               has_mergeinfo ? 1 : -1,
> +                                               txn_id, trail, trail->pool));
> +    }
> +
>    /* Make a record of this modification in the changes table. */
>    SVN_ERR(add_change(args->root->fs, txn_id,
>                       args->path, svn_fs_base__dag_get_id(parent_path->node),
> @@ -2675,6 +2723,7 @@
>    const char *path = args->path;
>    parent_path_t *parent_path;
>    const char *txn_id = root->txn;
> +  apr_int64_t mergeinfo_count;
>
>    if (! root->is_txn_root)
>      return SVN_FS__NOT_TXN(root);
> @@ -2695,13 +2744,26 @@
>                                                    trail, trail->pool));
>      }
>
> -  /* Make the parent directory mutable, and do the deletion.  */
> +  /* Make the parent directory mutable. */
>    SVN_ERR(make_path_mutable(root, parent_path->parent, path,
>                              trail, trail->pool));
> +
> +  /* Squirrel away the mergeinfo count that the node carries. */
> +  SVN_ERR(svn_fs_base__dag_get_mergeinfo_count(&mergeinfo_count,
> +                                               parent_path->node,
> +                                               trail, trail->pool));
> +
> +  /* Do the deletion. */
>    SVN_ERR(svn_fs_base__dag_delete(parent_path->parent->node,
>                                    parent_path->entry,
>                                    txn_id, trail, trail->pool));
>
> +  /* Decrement mergeinfo counts on the parents of this node by the
> +     count it currently carried. */
> +  SVN_ERR(adjust_parent_mergeinfo_counts(parent_path->parent,
> +                                         -mergeinfo_count, txn_id,
> +                                         trail, trail->pool));
> +
>    /* Make a record of this modification in the changes table. */
>    SVN_ERR(add_change(root->fs, txn_id, path,
>                       svn_fs_base__dag_get_id(parent_path->node),
> @@ -2780,6 +2842,7 @@
>      {
>        svn_fs_path_change_kind_t kind;
>        dag_node_t *new_node;
> +      apr_int64_t old_mergeinfo_count = 0, mergeinfo_count;
>
>        /* If TO_PATH already existed prior to the copy, note that this
>           operation is a replacement, not an addition. */
> @@ -2792,6 +2855,13 @@
>        SVN_ERR(make_path_mutable(to_root, to_parent_path->parent,
>                                  to_path, trail, trail->pool));
>
> +      /* If this is a replacement operation, we need to know the old
> +         node's mergeinfo count. */
> +      if (to_parent_path->node)
> +        SVN_ERR(svn_fs_base__dag_get_mergeinfo_count(&old_mergeinfo_count,
> +                                                     to_parent_path->node,
> +                                                     trail, trail->pool));
> +      /* Do the copy. */
>        SVN_ERR(svn_fs_base__dag_copy(to_parent_path->parent->node,
>                                      to_parent_path->entry,
>                                      from_node,
> @@ -2799,6 +2869,13 @@
>                                      from_root->rev,
>                                      from_path, txn_id, trail, trail->pool));
>
> +      /* Adjust the mergeinfo counts of the destination's parents. */
> +      SVN_ERR(svn_fs_base__dag_get_mergeinfo_count(&mergeinfo_count, from_node,
> +                                                   trail, trail->pool));
> +      SVN_ERR(adjust_parent_mergeinfo_counts
> +              (to_parent_path->parent, mergeinfo_count - old_mergeinfo_count,
> +               txn_id, trail, trail->pool));
> +
>        /* Make a record of this modification in the changes table. */
>        SVN_ERR(get_dag(&new_node, to_root, to_path, trail, trail->pool));
>        SVN_ERR(add_change(to_root->fs, txn_id, to_path,
>
> Modified: branches/reintegrate/subversion/tests/libsvn_fs/fs-test.c
> URL: http://svn.collab.net/viewvc/svn/branches/reintegrate/subversion/tests/libsvn_fs/fs-test.c?pathrev=28752&r1=28751&r2=28752
> ==============================================================================
> --- branches/reintegrate/subversion/tests/libsvn_fs/fs-test.c   (original)
> +++ branches/reintegrate/subversion/tests/libsvn_fs/fs-test.c   Fri Jan  4 13:36:51 2008
> @@ -4670,8 +4670,9 @@
>                                        "New contents", pool));
>
>    /* Change dir props in the other. */
> -  SET_STR(&pval, "value");
> -  SVN_ERR(svn_fs_change_node_prop(txn_root2, "/A/B", "name", &pval, pool));
> +  SET_STR(&pval, "/A/C:1");
> +  SVN_ERR(svn_fs_change_node_prop(txn_root2, "/A/B", "svn:mergeinfo",
> +                                  &pval, pool));
>
>    /* Commit the second one first. */
>    SVN_ERR(test_commit_txn(&new_rev, txn2, NULL, pool));
> @@ -4692,8 +4693,9 @@
>                                        "New contents", pool));
>
>    /* Change dir props in the other. */
> -  SET_STR(&pval, "value");
> -  SVN_ERR(svn_fs_change_node_prop(txn_root2, "/A/B", "name", &pval, pool));
> +  SET_STR(&pval, "/A/C:1");
> +  SVN_ERR(svn_fs_change_node_prop(txn_root2, "/A/B", "svn:mergeinfo",
> +                                  &pval, pool));
>
>    /* Commit the first one first. */
>    SVN_ERR(test_commit_txn(&new_rev, txn, NULL, pool));
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
>
>



-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org