You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2012/10/22 13:55:12 UTC
Re: svn commit: r1400545 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c
hwright@apache.org writes:
> Author: hwright
> Date: Sun Oct 21 01:18:26 2012
> New Revision: 1400545
>
> URL: http://svn.apache.org/viewvc?rev=1400545&view=rev
> Log:
> Refactor the pre-1.5 fallback code for replay_ranges into a separate function.
>
> * subversion/libsvn_ra/ra_loader.c
> (replay_range_from_replays): New.
> (svn_ra_replay_range): Call the factored out code.
This causes svnrdump_tests.py 43 and lots of svnsync_tests.py to FAIL
over ra_svn. It looks like an innocuous change, I can't see the error
in the code.
--
Join us this October at Subversion Live 2012
http://www.wandisco.com/svn-live-2012
Re: svn commit: r1400545 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Mon, Oct 22, 2012 at 11:46 AM, Philip Martin
<ph...@wandisco.com> wrote:
> Hyrum K Wright <hy...@hyrumwright.org> writes:
>
>> On Mon, Oct 22, 2012 at 7:55 AM, Philip Martin
>> <ph...@wandisco.com> wrote:
>>> hwright@apache.org writes:
>>>
>>>> Author: hwright
>>>> Date: Sun Oct 21 01:18:26 2012
>>>> New Revision: 1400545
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1400545&view=rev
>>>> Log:
>>>> Refactor the pre-1.5 fallback code for replay_ranges into a separate function.
>>>>
>>>> * subversion/libsvn_ra/ra_loader.c
>>>> (replay_range_from_replays): New.
>>>> (svn_ra_replay_range): Call the factored out code.
>>>
>>> This causes svnrdump_tests.py 43 and lots of svnsync_tests.py to FAIL
>>> over ra_svn. It looks like an innocuous change, I can't see the error
>>> in the code.
>>
>> Thanks for pointing out the failures. I won't be able to get around
>> to fixing it for (at least) several hours, so feel free to revert in
>> the meantime if it's a large issue.
>
> Worked it out:
>
> svn_error_t *err =
> session->vtable->replay_range(session, start_revision, end_revision,
> low_water_mark, text_deltas,
> revstart_func, revfinish_func,
> replay_baton, pool);
>
> if (err && (err->apr_err != SVN_ERR_RA_NOT_IMPLEMENTED))
> return svn_error_trace(err);
>
> svn_error_clear(err);
> return svn_error_trace(replay_range_from_replays(session, start_revision,
> end_revision,
> low_water_mark,
> text_deltas,
> revstart_func,
> revfinish_func,
> replay_baton, pool));
>
> When the vtable call returns SVN_NO_ERROR the code should return but
> instead goes on to call replay_range_from_replays.
Thanks. I generally like to use the early-return rather than
big-if-statement construction as I think it highlights readability,
but not if I sacrifice correctness to get there! Thanks for fixing.
-Hyrum
Re: svn commit: r1400545 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c
Posted by Philip Martin <ph...@wandisco.com>.
Hyrum K Wright <hy...@hyrumwright.org> writes:
> On Mon, Oct 22, 2012 at 7:55 AM, Philip Martin
> <ph...@wandisco.com> wrote:
>> hwright@apache.org writes:
>>
>>> Author: hwright
>>> Date: Sun Oct 21 01:18:26 2012
>>> New Revision: 1400545
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1400545&view=rev
>>> Log:
>>> Refactor the pre-1.5 fallback code for replay_ranges into a separate function.
>>>
>>> * subversion/libsvn_ra/ra_loader.c
>>> (replay_range_from_replays): New.
>>> (svn_ra_replay_range): Call the factored out code.
>>
>> This causes svnrdump_tests.py 43 and lots of svnsync_tests.py to FAIL
>> over ra_svn. It looks like an innocuous change, I can't see the error
>> in the code.
>
> Thanks for pointing out the failures. I won't be able to get around
> to fixing it for (at least) several hours, so feel free to revert in
> the meantime if it's a large issue.
Worked it out:
svn_error_t *err =
session->vtable->replay_range(session, start_revision, end_revision,
low_water_mark, text_deltas,
revstart_func, revfinish_func,
replay_baton, pool);
if (err && (err->apr_err != SVN_ERR_RA_NOT_IMPLEMENTED))
return svn_error_trace(err);
svn_error_clear(err);
return svn_error_trace(replay_range_from_replays(session, start_revision,
end_revision,
low_water_mark,
text_deltas,
revstart_func,
revfinish_func,
replay_baton, pool));
When the vtable call returns SVN_NO_ERROR the code should return but
instead goes on to call replay_range_from_replays.
--
Join us this October at Subversion Live 2012
http://www.wandisco.com/svn-live-2012
Re: svn commit: r1400545 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c
Posted by vijay <vi...@collab.net>.
On Monday 22 October 2012 06:53 PM, Hyrum K Wright wrote:
> On Mon, Oct 22, 2012 at 7:55 AM, Philip Martin
> <ph...@wandisco.com> wrote:
>> hwright@apache.org writes:
>>
>>> Author: hwright
>>> Date: Sun Oct 21 01:18:26 2012
>>> New Revision: 1400545
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1400545&view=rev
>>> Log:
>>> Refactor the pre-1.5 fallback code for replay_ranges into a separate function.
>>>
>>> * subversion/libsvn_ra/ra_loader.c
>>> (replay_range_from_replays): New.
>>> (svn_ra_replay_range): Call the factored out code.
>> This causes svnrdump_tests.py 43 and lots of svnsync_tests.py to FAIL
>> over ra_svn. It looks like an innocuous change, I can't see the error
>> in the code.
> Thanks for pointing out the failures. I won't be able to get around
> to fixing it for (at least) several hours, so feel free to revert in
> the meantime if it's a large issue.
If it is a serious issue, the attached patch will fix it. But I am
trying to understand what is wrong with the previous commit.
Thanks & Regards,
Vijayaguru
>
> -Hyrum
Re: svn commit: r1400545 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Mon, Oct 22, 2012 at 7:55 AM, Philip Martin
<ph...@wandisco.com> wrote:
> hwright@apache.org writes:
>
>> Author: hwright
>> Date: Sun Oct 21 01:18:26 2012
>> New Revision: 1400545
>>
>> URL: http://svn.apache.org/viewvc?rev=1400545&view=rev
>> Log:
>> Refactor the pre-1.5 fallback code for replay_ranges into a separate function.
>>
>> * subversion/libsvn_ra/ra_loader.c
>> (replay_range_from_replays): New.
>> (svn_ra_replay_range): Call the factored out code.
>
> This causes svnrdump_tests.py 43 and lots of svnsync_tests.py to FAIL
> over ra_svn. It looks like an innocuous change, I can't see the error
> in the code.
Thanks for pointing out the failures. I won't be able to get around
to fixing it for (at least) several hours, so feel free to revert in
the meantime if it's a large issue.
-Hyrum