Re: [PATCH] Implement svnadmin verify --force

On 10/29/2012 02:51 PM, Stefan Sperling wrote:
> On Mon, Oct 29, 2012 at 12:27:32PM +0530, Prabhu Gnana Sundar wrote:
>> On 10/28/2012 01:54 AM, Daniel Shahaf wrote:
>>>> Index: subversion/libsvn_fs/fs-loader.c
>>>> ===================================================================
>>>> --- subversion/libsvn_fs/fs-loader.c (revision 1402414)
>>>> +++ subversion/libsvn_fs/fs-loader.c (working copy)
>>>> @@ -487,17 +487,21 @@
>>>> void *cancel_baton,
>>>> svn_revnum_t start,
>>>> svn_revnum_t end,
>>>> + svn_boolean_t keep_going,
>>>> apr_pool_t *pool)
>>>> {
>>>> - fs_library_vtable_t *vtable;
>>>> - svn_fs_t *fs;
>>>> + if (!keep_going)
>>>> + {
>>> That's wrong for the same reason as last time
>>
>> I am not sure on what has to be done. Can you please guide me with
>> some pointers on what has to be done.
>>
>> Thanks
>> Prabhu
>
> With some context added back it's easier to see what's wrong:
>
>> Index: subversion/libsvn_fs/fs-loader.c
>> ===================================================================
>> --- subversion/libsvn_fs/fs-loader.c (revision 1402414)
>> +++ subversion/libsvn_fs/fs-loader.c (working copy)
>> @@ -487,17 +487,21 @@
>> void *cancel_baton,
>> svn_revnum_t start,
>> svn_revnum_t end,
>> + svn_boolean_t keep_going,
>> apr_pool_t *pool)
>> {
>> - fs_library_vtable_t *vtable;
>> - svn_fs_t *fs;
>> + if (!keep_going)
>> + {
>> + fs_library_vtable_t *vtable;
>> + svn_fs_t *fs;
>>
>> - SVN_ERR(fs_library_vtable(&vtable, path, pool));
>> - fs = fs_new(NULL, pool);
>> + SVN_ERR(fs_library_vtable(&vtable, path, pool));
>> + fs = fs_new(NULL, pool);
> You are effectively disabling the verify_fs() function call below if the
> user passes --keep-going. That doesn't seem right.
>
> This patch is supposed to change the way we treat errors encountered
> during verification. The verification process itself shouldn't be changed.

Thanks much Stefan,

Now, I have changed the code in libsvn_fs/fs-loader.c to handle the
error. Passed the keep_going to the verify_fs so that it can be handled
in both libsvn_fs_fs and libsvn_fs_base. I have attached the updated
patch with this mail. Please share your reviews.

Thanks and regards
Prabhu