Re: [PATCH] Fix for issue #2982

Hi Kamesh,

Based on your comments and inputs from dlr, I am attaching an updated patch
along with this email. I shall look into the revision renumbering in "svnadmin
load" as suggested by glasser.

[[[
Fix issue #2982.

  Reflect dropped/renumbered revisions in modified svn:mergeinfo data

  * subversion/svndumpfilter/main.c
    (): Include svn_mergeinfo.h
    (parse_baton_t): Add skip_missing_merge_sources boolean for the new option
    (renumber_merge_source_rev_range): New function to skip missing merge
     sources in mergeinfo
    (set_node_property): If we have svn:mergeinfo then skip missing merge
     sources or renumber mergeinfo revs
    (svn_opt_subcommand_t): Add --skip-missing-merge-sources option
    (options_table): Add description for --skip-missing-merge-sources option
    (cmd_table): Add the new option --skip-missing-merge-sources
    (svndumpfilter_opt_state): Add skip_missing_merge_sources boolean for the
     new option
    (parse_baton_initialize): Initialize skip_missing_merge_sources
    (main): Add svndumpfilter__skip_missing_merge_sources to switch case

Patch by: Senthil Kumaran <senthil@collab.net>
]]]

Thank You.

--
Senthil Kumaran S
http://www.stylesen.org/
Kamesh Jayachandran wrote:
> 
>> [[[
>> Fix issue #2982.
>>
>>  Reflect dropped/renumbered revisions in modified svn:mergeinfo data
>>
>>  * subversion/svndumpfilter/main.c
>>    (include): Include svn_mergeinfo.h
> No *(include)*, it should be ().
> 
>  >+/* Skip missing merge sources in mergeinfo. */
> 
> Fix the docstring , purpose of this helper function is not matching the 
> doc string.
> 
>  > static svn_error_t *
>  >+skip_missing_merge_sources(const char **mergeinfo_val, const char 
> *mergeinfo_orig,
>  >+ struct revision_baton_t *rb, apr_pool_t *pool)
> 
> Fix the alignment issue here.
> 
> This helper function name is not correct, it should be 
> 'renumber_merge_source_rev_range'.
> 
> 
>  >+ /* Renumber mergeinfo rangelist. */
>  >+ for (i = 0; i < ((apr_array_header_t *)rangelist)->nelts; i++)
>  >+ {
>  >+ struct revmap_t *renum_start;
>  >+ struct revmap_t *renum_end;
> 
> Probably you can name this as 'revmap_start' and 'revmap_end', my first 
> look at it just by name in the later part of the code made me to believe 
> that it was a revnum which was wrong.
> 
>  >+ svn_merge_range_t *range;
>  >+ svn_merge_range_t *newrange = apr_pcalloc(pool, sizeof(*range));
> 
> No need to allocate 'newrange', you can just overwrite range's members.
> 
>  >+ SVN_ERR(svn_mergeinfo_sort(modified_mergeinfo, pool));
> 
> I don't think we need to sort here, as the incoming range themselves 
> should be in a sorted order. Anyways resorting should not hurt.
> 
> 
>  >+ if (strcmp(name, SVN_PROP_MERGE_INFO) == 0)
>  >+ {
>  >+ /* Skip missing merge sources. */
>  >+ const char *mergeinfo_val;
>  >+ SVN_ERR(skip_missing_merge_sources(&mergeinfo_val, value->data, rb,
>  >+ subpool));
> 
> Formatting issue with arguments.
> 
> With regards
> Kamesh Jayachandran

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