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