Re: Opening the repository hooks environment file

On 03.04.2013 13:01, Philip Martin wrote:
> Ivan Zhakov <ivan_at_visualsvn.com> writes:
>
>> On Wed, Apr 3, 2013 at 4:38 AM, Ben Reser <ben_at_reser.org> wrote:
>>> I've gone ahead and moved the default hooks-env to hooks-env.tmpl.
>>> We'll still try to open the file but we won't bother to read it in or
>>> parse a completely commented out file unless the user puts a file in
>>> place. The .tmpl is a common pattern with hooks already so it should
>>> add any confusion.
>>>
>> I'm going to implement reading hooks environment file just before hook
>> execution instead of repository open.
> The current API is:
>
> svn_error_t *
> svn_repos_hooks_setenv(svn_repos_t *repos,
> const char *hooks_env_path,
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool);
>
> With your change svn_repos_hooks_setenv no longer needs scratch_pool.
> It does still require result_pool to be the same pool used to allocate
> svn_repos_t and that's a bit ugly. Perhaps we should add a pool member
> to svn_repos_t and store the pool when allocating the struct? Perhaps
> we should remove svn_repos_hooks_setenv and pass the path to a new
> svn_repos_open3?

I thihk there has to be a way to say all of the following:

 1. do not use a hooks-env file at all
 2. use the default hooks-env file
 3. use a specific, non-default hooks-env file

Given that just calling svn_repos_open2 will (?) result in behaviour
(1), then svn_repos_hooks_setenv should result in (2) if hooks_env_path
is NULL, and (3) otherwise.

I think it would be a bit too involved to represent the tristate nature
of the option in a svn_repos_open3; although we could introduce a
"default"-type constant value (e.g., an empty string) to represent
either (1) or (2) (with NULL representing the other of these two options).

But in any case, there should never be an option to use a separate
result pool; any storage associated with the env file path must have the
same lifetime as the repository handle.

-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com