Re: [PATCH] Fix for issue #4234, svndumpfilter exclude --targets wants pathname to start with '/'

On Wednesday 26 September 2012 03:19 PM, Stefan Sperling wrote:
> On Wed, Sep 26, 2012 at 03:52:57AM +0530, vijay wrote:
>> From the issue[1] description,
>>
>> <snip>
>>
>> When svndumpfilter exclude command accepts path prefixes starting with either /<pathname>
>> or just <pathname>, I expect it to work the same way when we use with --targets option.
>>
>> For Eg :
>>
>> Both of the following command works fine when we directly provide the
>> path prefix
>>
>> $ cat sample.dump | svndumpfilter exclude /tags > filtered4.dump
>>
>> $ cat sample.dump | svndumpfilter exclude tags > filtered4.dump
>>
>> However, when we use targets, it seems to be a must to prefix all the path prefixed with
>> a / which is not mentioned anywhere in the documentation as well.
>>
>> </snip>
>>
>> Attaching the patch and log message to fix this issue.
>>
>> Thanks & Regards,
>> Vijayaguru
>>
>> [1] http://subversion.tigris.org/issues/show_bug.cgi?id=4234
> Hi Vijay,
>
> looks good to me. Can you also add a regression test for this,
> perhaps in a second patch?

Attached the patch for test and log message.

Thanks & Regards,
Vijayaguru

>
> Thanks!
>
>> Index: subversion/svndumpfilter/main.c
>> ===================================================================
>> --- subversion/svndumpfilter/main.c (revision 1389492)
>> +++ subversion/svndumpfilter/main.c (working copy)
>> @@ -1526,6 +1526,9 @@
>> {
>> svn_stringbuf_t *buffer, *buffer_utf8;
>> const char *utf8_targets_file;
>> + apr_array_header_t *targets = apr_array_make(pool, 0,
>> + sizeof(const char *));
>> + int i;
>>
>> /* We need to convert to UTF-8 now, even before we divide
>> the targets into an array, because otherwise we wouldn't
>> @@ -1538,10 +1541,18 @@
>> pool));
>> SVN_INT_ERR(svn_utf_stringbuf_to_utf8(&buffer_utf8, buffer, pool));
>>
>> - opt_state.prefixes = apr_array_append(pool,
>> - svn_cstring_split(buffer_utf8->data, "\n\r",
>> - TRUE, pool),
>> - opt_state.prefixes);
>> + targets = apr_array_append(pool,
>> + svn_cstring_split(buffer_utf8->data, "\n\r",
>> + TRUE, pool),
>> + targets);
>> +
>> + for (i = 0; i < targets->nelts; i++)
>> + {
>> + const char *prefix = APR_ARRAY_IDX(targets, i, const char *);
>> + if (prefix[0] != '/')
>> + prefix = apr_pstrcat(pool, "/", prefix, (char *)NULL);
>> + APR_ARRAY_PUSH(opt_state.prefixes, const char *) = prefix;
>> + }
>> }
>>
>> if (apr_is_empty_array(opt_state.prefixes))
>> Fix issue #4234, "svndumpfilter exclude --targets wants pathname to start
>> with '/'"
>>
>> * subversion/svndumpfilter/main.c
>> (main): While parsing through the 'targets' file, check for a leading
>> slash('/') in every path prefix. If it is not there, prepend a '/'.
>>
>> Found by: Jeyanthan <jeyanthan{_AT_}collab.net>
>> Patch by: Vijayaguru G <vijay{_AT_}collab.net>