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/23 23:44:21 UTC

Re: svn commit: r28978 - in trunk/subversion: libsvn_repos tests/cmdline

On Jan 21, 2008 11:42 PM,  <st...@tigris.org> wrote:
> Author: stylesen
> Date: Mon Jan 21 23:42:56 2008
> New Revision: 28978
>
> Log:
> Fix issue #3020.
>
> Reflect dropped/renumbered revisions in svn:mergeinfo during svnadmin load
>
> * subversion/tests/cmdline/svnadmin_tests.py
>   (load_with_parent_dir): Change test case expectation for svn:mergeinfo
>
> * subversion/libsvn_repos/load.c
>   (): include private/svn_mergeinfo_private.h
>   (renumber_mergeinfo_revs): New function to renumber svn:mergeinfo
>   (set_node_property): If we have svn:mergeinfo then renumber revs in
>    svn:mergeinfo as appropriate
>
> Patch by: me
> Approved by: kameshj
> Reviewed by: kameshj

Thanks for fixing this one, Senthil!

>
>
> Modified:
>    trunk/subversion/libsvn_repos/load.c
>    trunk/subversion/tests/cmdline/svnadmin_tests.py
>
> Modified: trunk/subversion/libsvn_repos/load.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_repos/load.c?pathrev=28978&r1=28977&r2=28978
> ==============================================================================
> --- trunk/subversion/libsvn_repos/load.c        (original)
> +++ trunk/subversion/libsvn_repos/load.c        Mon Jan 21 23:42:56 2008
> @@ -31,6 +31,7 @@
>
>  #include <apr_lib.h>
>
> +#include "private/svn_mergeinfo_private.h"
>
>  /*----------------------------------------------------------------------*/
>
> @@ -263,6 +264,63 @@
>    return SVN_NO_ERROR;
>  }
>
> +/* Examine the mergeinfo in INITIAL_VAL, renumber revisions in rangelists
> +   as appropriate, and return the (possibly new) mergeinfo in *FINAL_VAL
> +   (allocated from POOL). */
> +static svn_error_t *
> +renumber_mergeinfo_revs(svn_string_t **final_val,
> +                        const svn_string_t *initial_val,
> +                        struct revision_baton *rb,
> +                        apr_pool_t *pool)
> +{
> +  apr_hash_t *mergeinfo;
> +  apr_hash_t *final_mergeinfo = apr_hash_make(pool);

This hash could be in the subpool.

> +  apr_hash_index_t *hi;
> +  apr_pool_t *subpool = svn_pool_create(pool);
> +
> +  SVN_ERR(svn_mergeinfo_parse(&mergeinfo, initial_val->data, subpool));
> +  for (hi = apr_hash_first(NULL, mergeinfo); hi; hi = apr_hash_next(hi))
> +    {
> +      const char *merge_source;
> +      apr_array_header_t *rangelist;
> +      struct parse_baton *pb = rb->pb;
> +      int i;
> +      const void *key;
> +      void *val;
> +
> +      apr_hash_this(hi, &key, NULL, &val);
> +      merge_source = (const char *) key;
> +      rangelist = (apr_array_header_t *) val;
> +
> +      /* Possibly renumber revisions in merge source's rangelist. */
> +      for (i = 0; i < rangelist->nelts; i++)
> +        {
> +          svn_revnum_t *rev_from_map;
> +          svn_merge_range_t *range = APR_ARRAY_IDX(rangelist, i,
> +                                                   svn_merge_range_t *);
> +
> +          if ((rev_from_map = apr_hash_get(pb->rev_map, &range->start,
> +                                           sizeof(svn_revnum_t))))
> +            if (SVN_IS_VALID_REVNUM(*rev_from_map))
> +              range->start = *rev_from_map;;

Just do an assignment followed by "if (rev_from_map &&
SVN_IS_VALID_REVNUM(*rev_from_map)" here rather than the
nested-if-with-assignment.  Also ditch the doubled semicolon.

> +
> +          if ((rev_from_map = apr_hash_get(pb->rev_map, &range->end,
> +                                           sizeof(svn_revnum_t))))
> +            if (SVN_IS_VALID_REVNUM(*rev_from_map))
> +              range->end = *rev_from_map;

Ditto.

> +         }
> +       apr_hash_set(final_mergeinfo, merge_source,
> +                    APR_HASH_KEY_STRING, rangelist);
> +     }
> +
> +  SVN_ERR(svn_mergeinfo_sort(final_mergeinfo, subpool));
> +  SVN_ERR(svn_mergeinfo__to_string(final_val, final_mergeinfo, pool));
> +  svn_pool_destroy(subpool);
> +
> +  return SVN_NO_ERROR;
> +}
> +
> +
>  /* Read CONTENT_LENGTH bytes from STREAM, parsing the bytes as an
>     encoded Subversion properties hash, and making multiple calls to
>     PARSE_FNS->set_*_property on RECORD_BATON (depending on the value
> @@ -1134,14 +1192,22 @@
>    struct revision_baton *rb = nb->rb;
>    const char *parent_dir = rb->pb->parent_dir;
>
> -  if (parent_dir && strcmp(name, SVN_PROP_MERGEINFO) == 0)
> +  if (strcmp(name, SVN_PROP_MERGEINFO) == 0)
>      {
> -      /* Prefix the merge source paths with PARENT_DIR. */
> -      /* ASSUMPTION: All source paths are included in the dump stream. */
> -      const char *mergeinfo_val;
> -      SVN_ERR(prefix_mergeinfo_paths(&mergeinfo_val, value->data,
> -                                     parent_dir, nb->pool));
> -      value = svn_string_create(mergeinfo_val, nb->pool);
> +      /* Renumber mergeinfo as appropriate. */
> +      svn_string_t *renumbered_mergeinfo;
> +      SVN_ERR(renumber_mergeinfo_revs(&renumbered_mergeinfo, value, rb,
> +                                      nb->pool));
> +      value = renumbered_mergeinfo;
> +      if (parent_dir)
> +        {
> +          /* Prefix the merge source paths with PARENT_DIR. */
> +          /* ASSUMPTION: All source paths are included in the dump stream. */
> +          const char *mergeinfo_val;
> +          SVN_ERR(prefix_mergeinfo_paths(&mergeinfo_val, value->data,
> +                                         parent_dir, nb->pool));
> +          value = svn_string_create(mergeinfo_val, nb->pool);
> +        }
>      }
>
>    SVN_ERR(svn_fs_change_node_prop(rb->txn_root, nb->path,
>
> Modified: trunk/subversion/tests/cmdline/svnadmin_tests.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/svnadmin_tests.py?pathrev=28978&r1=28977&r2=28978
> ==============================================================================
> --- trunk/subversion/tests/cmdline/svnadmin_tests.py    (original)
> +++ trunk/subversion/tests/cmdline/svnadmin_tests.py    Mon Jan 21 23:42:56 2008
> @@ -481,13 +481,13 @@
>    # Verify the svn:mergeinfo properties for '--parent-dir'
>    svntest.actions.run_and_verify_svn(None,
>                                       [sbox.repo_url +
> -                                      "/sample/branch - /sample/trunk:4-6\n"],
> +                                      "/sample/branch - /sample/trunk:5-7\n"],
>                                       [], 'propget', 'svn:mergeinfo', '-R',
>                                       sbox.repo_url + '/sample/branch')
>    svntest.actions.run_and_verify_svn(None,
>                                       [sbox.repo_url +
>                                        "/sample/branch1 - " +
> -                                      "/sample/branch:5-8\n"],
> +                                      "/sample/branch:6-9\n"],
>                                       [], 'propget', 'svn:mergeinfo', '-R',
>                                       sbox.repo_url + '/sample/branch1')
>
>
> ---------------------------------------------------------------------
> 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

Re: svn commit: r28978 - in trunk/subversion: libsvn_repos tests/cmdline

Posted by Senthil Kumaran S <se...@collab.net>.
Hi Glasser,

Thanks for the review. I ve made the changes and committed in r29035.

David Glasser wrote:
> On Jan 21, 2008 11:42 PM,  <st...@tigris.org> wrote:
>> Author: stylesen
>> Date: Mon Jan 21 23:42:56 2008
>> New Revision: 28978
>>
>> Log:
>> Fix issue #3020.
>>
>> Reflect dropped/renumbered revisions in svn:mergeinfo during svnadmin load
>>
>> * subversion/tests/cmdline/svnadmin_tests.py
>>   (load_with_parent_dir): Change test case expectation for svn:mergeinfo
>>
>> * subversion/libsvn_repos/load.c
>>   (): include private/svn_mergeinfo_private.h
>>   (renumber_mergeinfo_revs): New function to renumber svn:mergeinfo
>>   (set_node_property): If we have svn:mergeinfo then renumber revs in
>>    svn:mergeinfo as appropriate
>>
>> Patch by: me
>> Approved by: kameshj
>> Reviewed by: kameshj
> 
> Thanks for fixing this one, Senthil!
> 
>>
>> Modified:
>>    trunk/subversion/libsvn_repos/load.c
>>    trunk/subversion/tests/cmdline/svnadmin_tests.py
>>
>> Modified: trunk/subversion/libsvn_repos/load.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_repos/load.c?pathrev=28978&r1=28977&r2=28978
>> ==============================================================================
>> --- trunk/subversion/libsvn_repos/load.c        (original)
>> +++ trunk/subversion/libsvn_repos/load.c        Mon Jan 21 23:42:56 2008
>> @@ -31,6 +31,7 @@
>>
>>  #include <apr_lib.h>
>>
>> +#include "private/svn_mergeinfo_private.h"
>>
>>  /*----------------------------------------------------------------------*/
>>
>> @@ -263,6 +264,63 @@
>>    return SVN_NO_ERROR;
>>  }
>>
>> +/* Examine the mergeinfo in INITIAL_VAL, renumber revisions in rangelists
>> +   as appropriate, and return the (possibly new) mergeinfo in *FINAL_VAL
>> +   (allocated from POOL). */
>> +static svn_error_t *
>> +renumber_mergeinfo_revs(svn_string_t **final_val,
>> +                        const svn_string_t *initial_val,
>> +                        struct revision_baton *rb,
>> +                        apr_pool_t *pool)
>> +{
>> +  apr_hash_t *mergeinfo;
>> +  apr_hash_t *final_mergeinfo = apr_hash_make(pool);
> 
> This hash could be in the subpool.
> 
>> +  apr_hash_index_t *hi;
>> +  apr_pool_t *subpool = svn_pool_create(pool);
>> +
>> +  SVN_ERR(svn_mergeinfo_parse(&mergeinfo, initial_val->data, subpool));
>> +  for (hi = apr_hash_first(NULL, mergeinfo); hi; hi = apr_hash_next(hi))
>> +    {
>> +      const char *merge_source;
>> +      apr_array_header_t *rangelist;
>> +      struct parse_baton *pb = rb->pb;
>> +      int i;
>> +      const void *key;
>> +      void *val;
>> +
>> +      apr_hash_this(hi, &key, NULL, &val);
>> +      merge_source = (const char *) key;
>> +      rangelist = (apr_array_header_t *) val;
>> +
>> +      /* Possibly renumber revisions in merge source's rangelist. */
>> +      for (i = 0; i < rangelist->nelts; i++)
>> +        {
>> +          svn_revnum_t *rev_from_map;
>> +          svn_merge_range_t *range = APR_ARRAY_IDX(rangelist, i,
>> +                                                   svn_merge_range_t *);
>> +
>> +          if ((rev_from_map = apr_hash_get(pb->rev_map, &range->start,
>> +                                           sizeof(svn_revnum_t))))
>> +            if (SVN_IS_VALID_REVNUM(*rev_from_map))
>> +              range->start = *rev_from_map;;
> 
> Just do an assignment followed by "if (rev_from_map &&
> SVN_IS_VALID_REVNUM(*rev_from_map)" here rather than the
> nested-if-with-assignment.  Also ditch the doubled semicolon.
> 
>> +
>> +          if ((rev_from_map = apr_hash_get(pb->rev_map, &range->end,
>> +                                           sizeof(svn_revnum_t))))
>> +            if (SVN_IS_VALID_REVNUM(*rev_from_map))
>> +              range->end = *rev_from_map;
> 
> Ditto.
> 
>> +         }
>> +       apr_hash_set(final_mergeinfo, merge_source,
>> +                    APR_HASH_KEY_STRING, rangelist);
>> +     }
>> +
>> +  SVN_ERR(svn_mergeinfo_sort(final_mergeinfo, subpool));
>> +  SVN_ERR(svn_mergeinfo__to_string(final_val, final_mergeinfo, pool));
>> +  svn_pool_destroy(subpool);
>> +
>> +  return SVN_NO_ERROR;
>> +}
>> +
>> +
>>  /* Read CONTENT_LENGTH bytes from STREAM, parsing the bytes as an
>>     encoded Subversion properties hash, and making multiple calls to
>>     PARSE_FNS->set_*_property on RECORD_BATON (depending on the value
>> @@ -1134,14 +1192,22 @@
>>    struct revision_baton *rb = nb->rb;
>>    const char *parent_dir = rb->pb->parent_dir;
>>
>> -  if (parent_dir && strcmp(name, SVN_PROP_MERGEINFO) == 0)
>> +  if (strcmp(name, SVN_PROP_MERGEINFO) == 0)
>>      {
>> -      /* Prefix the merge source paths with PARENT_DIR. */
>> -      /* ASSUMPTION: All source paths are included in the dump stream. */
>> -      const char *mergeinfo_val;
>> -      SVN_ERR(prefix_mergeinfo_paths(&mergeinfo_val, value->data,
>> -                                     parent_dir, nb->pool));
>> -      value = svn_string_create(mergeinfo_val, nb->pool);
>> +      /* Renumber mergeinfo as appropriate. */
>> +      svn_string_t *renumbered_mergeinfo;
>> +      SVN_ERR(renumber_mergeinfo_revs(&renumbered_mergeinfo, value, rb,
>> +                                      nb->pool));
>> +      value = renumbered_mergeinfo;
>> +      if (parent_dir)
>> +        {
>> +          /* Prefix the merge source paths with PARENT_DIR. */
>> +          /* ASSUMPTION: All source paths are included in the dump stream. */
>> +          const char *mergeinfo_val;
>> +          SVN_ERR(prefix_mergeinfo_paths(&mergeinfo_val, value->data,
>> +                                         parent_dir, nb->pool));
>> +          value = svn_string_create(mergeinfo_val, nb->pool);
>> +        }
>>      }
>>
>>    SVN_ERR(svn_fs_change_node_prop(rb->txn_root, nb->path,
>>
>> Modified: trunk/subversion/tests/cmdline/svnadmin_tests.py
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/svnadmin_tests.py?pathrev=28978&r1=28977&r2=28978
>> ==============================================================================
>> --- trunk/subversion/tests/cmdline/svnadmin_tests.py    (original)
>> +++ trunk/subversion/tests/cmdline/svnadmin_tests.py    Mon Jan 21 23:42:56 2008
>> @@ -481,13 +481,13 @@
>>    # Verify the svn:mergeinfo properties for '--parent-dir'
>>    svntest.actions.run_and_verify_svn(None,
>>                                       [sbox.repo_url +
>> -                                      "/sample/branch - /sample/trunk:4-6\n"],
>> +                                      "/sample/branch - /sample/trunk:5-7\n"],
>>                                       [], 'propget', 'svn:mergeinfo', '-R',
>>                                       sbox.repo_url + '/sample/branch')
>>    svntest.actions.run_and_verify_svn(None,
>>                                       [sbox.repo_url +
>>                                        "/sample/branch1 - " +
>> -                                      "/sample/branch:5-8\n"],
>> +                                      "/sample/branch:6-9\n"],
>>                                       [], 'propget', 'svn:mergeinfo', '-R',
>>                                       sbox.repo_url + '/sample/branch1')
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: svn-help@subversion.tigris.org
>>
>>
> 
> 
> 


-- 
Senthil Kumaran S
http://www.stylesen.org/

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