On 12 February 2015 at 21:28, Philip Martin <philip.martin_at_wandisco.com> wrote:
> Ivan Zhakov <ivan_at_visualsvn.com> writes:
>
>> As far I understand out testsuite still may fail in some rare
>> circumstances with --parallel option, until we add
>> svn_dso_initialize2(). It's bad when tests are failing sometimes.
>
> I just seen a failure and it revealed the current bug. I think the
> tests explicitly clear the pools passed to RA/FS so they already follow
> the rules and will not have a problem. A test would have to fail to
> clear a pool for there to be a problem, and that would most likely be a
> bug that should be fixed.
>
>> We already documented that applications *should* call
>> svn_dso_initialize2() on startup. Why test some unrecommended usage of
>> our API that I known to lead problems?
>
> svn_dso_initialize2 didn't exist until 1.6 and is only documented in
> svn_dso.h. Do we expect people writing applications using the high
> level API to read that? Do we expect people using old applications to
> update them? Is it possible to call svn_dso_initialize2 when using the
> bindings?
>
svn_dso_initialize2() is just extended version of svn_dso_initialize()
which exists since 1.4. The svn_dso_load() also introduced in 1.4.
>> Do we want to fight problems
>> like we had with ra-test in future?
>
> I haven't followed the details but I think the ra-test problem was
> caused by failure to close a tunnel, nothing to do with initialization.
>
Probably, but we suspected BDB initialization problems and spend some
time to check it.
>> Testing on-the-fly
>> initialization() is a good goal, but not as expense of usual
>> development IMO. We may add special 'on-the-fly-initialization' test
>> program if you think that it's important to test this behavior. Just
>> my $0.02
>
> I'm still not seeing a gain, only the need to write another test.
>
The gain is to avoid unexpected behavior in our test suite. It's also
could be a problem if some application developer will use our test
application as starting point for his app.
Please understand me correctly: I'm not against improving
svn_dso_initialize2()/svn_dso_load() and I voted for r1659315 fix in
1.8.x. I'm just trying to demonstrate that there are still possible
circumstances for problems and do not why we should write our test
application in the not recommended way.
Btw svn_dso_load() is used for RA and FS modules. We already have
svn_ra_initialize() and svn_fs_initialize() with docstring that these
functions *must be* called before using any RA functions:
[[[
/**
* Initialize the RA library. This function must be called before using
* any function in this header, except the deprecated APIs based on
*/
]]]
This API is present from Subversion 1.2, so I suggest add call to
svn_dso_initialize2() in svn_ra_initialize()/svn_fs_inialize(): do you
have any objections against this approach?
--
Ivan Zhakov
Received on 2015-02-13 16:43:58 CET