cmpilato_at_apache.org wrote on Wed, Apr 03, 2013 at 17:51:56 -0000:
> Author: cmpilato
> Date: Wed Apr 3 17:51:56 2013
> New Revision: 1464122
>
> URL: http://svn.apache.org/r1464122
> Log:
> Avoid parsing the hooks-env file multiple times for closely-knit hook
> invocations, specifically the pre-/post- pairs for commit, revprop
> change, lock, and unlock operations.
>
> * subversion/libsvn_repos/repos.h,
> * subversion/libsvn_repos/hooks.c
> (svn_repos__parse_hooks_env): Was parse_hooks_env().
> (svn_repos__hooks_start_commit,
> svn_repos__hooks_pre_commit,
> svn_repos__hooks_post_commit,
> svn_repos__hooks_pre_revprop_change,
> svn_repos__hooks_post_revprop_change,
> svn_repos__hooks_pre_lock,
> svn_repos__hooks_post_lock,
> svn_repos__hooks_pre_unlock,
> svn_repos__hooks_post_unlock): Add 'hooks_env' parameter, used now
> instead of calling parse_hooks_env() from within.
>
> * subversion/libsvn_repos/commit.c
> (complete_cb, svn_repos__get_commit_ev2): Call
> svn_repos__parse_hooks_env(), and update calls to hook wrapper
> functions.
>
> * subversion/libsvn_repos/fs-wrap.c
> (svn_repos_fs_commit_txn, svn_repos_fs_begin_txn_for_commit2,
> svn_repos_fs_change_rev_prop4, svn_repos_fs_lock, svn_repos_fs_unlock):
> Call svn_repos__parse_hooks_env(), and update calls to hook wrapper
> functions.
>
> * subversion/libsvn_repos/load-fs-vtable.c
> (close_revision): Call svn_repos__parse_hooks_env(), and update
> calls to hook wrapper functions.
>
> Modified:
> subversion/trunk/subversion/libsvn_repos/commit.c
> subversion/trunk/subversion/libsvn_repos/fs-wrap.c
> subversion/trunk/subversion/libsvn_repos/hooks.c
> subversion/trunk/subversion/libsvn_repos/load-fs-vtable.c
> subversion/trunk/subversion/libsvn_repos/repos.h
>
> Modified: subversion/trunk/subversion/libsvn_repos/commit.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/commit.c?rev=1464122&r1=1464121&r2=1464122&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/commit.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/commit.c Wed Apr 3 17:51:56 2013
> @@ -1295,10 +1295,16 @@ complete_cb(void *baton,
> const char *conflict_path;
> svn_error_t *err;
> const char *post_commit_errstr;
> + apr_hash_t *hooks_env;
> +
> + /* Parse the hooks-env file (if any). */
> + SVN_ERR(svn_repos__parse_hooks_env(&hooks_env, eb->repos->hooks_env_path,
> + scratch_pool, scratch_pool));
>
> /* The transaction has been fully edited. Let the pre-commit hook
> have a look at the thing. */
> - SVN_ERR(svn_repos__hooks_pre_commit(eb->repos, eb->txn_name, scratch_pool));
> + SVN_ERR(svn_repos__hooks_pre_commit(eb->repos, hooks_env,
> + eb->txn_name, scratch_pool));
>
> /* Hook is done. Let's do the actual commit. */
> SVN_ERR(svn_fs__editor_commit(&revision, &post_commit_err, &conflict_path,
> @@ -1314,8 +1320,8 @@ complete_cb(void *baton,
> Other errors may have occurred within the FS (specified by the
> POST_COMMIT_ERR localvar), but we need to run the hooks. */
> SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
> - err = svn_repos__hooks_post_commit(eb->repos, revision, eb->txn_name,
> - scratch_pool);
> + err = svn_repos__hooks_post_commit(eb->repos, hooks_env, revision,
> + eb->txn_name, scratch_pool);
> if (err)
> err = svn_error_create(SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED, err,
> _("Commit succeeded, but post-commit hook failed"));
> @@ -1405,6 +1411,11 @@ svn_repos__get_commit_ev2(svn_editor_t *
> };
> struct ev2_baton *eb;
> const svn_string_t *author;
> + apr_hash_t *hooks_env;
> +
> + /* Parse the hooks-env file (if any). */
> + SVN_ERR(svn_repos__parse_hooks_env(&hooks_env, repos->hooks_env_path,
> + scratch_pool, scratch_pool));
>
> /* Can the user modify the repository at all? */
> /* ### check against AUTHZ. */
> @@ -1428,7 +1439,8 @@ svn_repos__get_commit_ev2(svn_editor_t *
> SVN_ERR(apply_revprops(repos->fs, eb->txn_name, revprops, scratch_pool));
>
> /* Okay... some access is allowed. Let's run the start-commit hook. */
> - SVN_ERR(svn_repos__hooks_start_commit(repos, author ? author->data : NULL,
> + SVN_ERR(svn_repos__hooks_start_commit(repos, hooks_env,
> + author ? author->data : NULL,
> repos->client_capabilities,
> eb->txn_name, scratch_pool));
>
>
> Modified: subversion/trunk/subversion/libsvn_repos/fs-wrap.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/fs-wrap.c?rev=1464122&r1=1464121&r2=1464122&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/fs-wrap.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/fs-wrap.c Wed Apr 3 17:51:56 2013
> @@ -54,12 +54,17 @@ svn_repos_fs_commit_txn(const char **con
> apr_hash_t *props;
> apr_pool_t *iterpool;
> apr_hash_index_t *hi;
> + apr_hash_t *hooks_env;
>
> *new_rev = SVN_INVALID_REVNUM;
>
> + /* Parse the hooks-env file (if any). */
> + SVN_ERR(svn_repos__parse_hooks_env(&hooks_env, repos->hooks_env_path,
> + pool, pool));
> +
> /* Run pre-commit hooks. */
> SVN_ERR(svn_fs_txn_name(&txn_name, txn, pool));
> - SVN_ERR(svn_repos__hooks_pre_commit(repos, txn_name, pool));
> + SVN_ERR(svn_repos__hooks_pre_commit(repos, hooks_env, txn_name, pool));
>
> /* Remove any ephemeral transaction properties. */
> SVN_ERR(svn_fs_txn_proplist(&props, txn, pool));
> @@ -85,7 +90,8 @@ svn_repos_fs_commit_txn(const char **con
> return err;
>
> /* Run post-commit hooks. */
> - if ((err2 = svn_repos__hooks_post_commit(repos, *new_rev, txn_name, pool)))
> + if ((err2 = svn_repos__hooks_post_commit(repos, hooks_env,
> + *new_rev, txn_name, pool)))
> {
> err2 = svn_error_create
> (SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED, err2,
> @@ -110,6 +116,11 @@ svn_repos_fs_begin_txn_for_commit2(svn_f
> apr_array_header_t *revprops;
> const char *txn_name;
> svn_string_t *author = svn_hash_gets(revprop_table, SVN_PROP_REVISION_AUTHOR);
> + apr_hash_t *hooks_env;
> +
> + /* Parse the hooks-env file (if any). */
> + SVN_ERR(svn_repos__parse_hooks_env(&hooks_env, repos->hooks_env_path,
> + pool, pool));
>
> /* Begin the transaction, ask for the fs to do on-the-fly lock checks.
> We fetch its name, too, so the start-commit hook can use it. */
> @@ -124,7 +135,8 @@ svn_repos_fs_begin_txn_for_commit2(svn_f
> SVN_ERR(svn_repos_fs_change_txn_props(*txn_p, revprops, pool));
>
> /* Run start-commit hooks. */
> - SVN_ERR(svn_repos__hooks_start_commit(repos, author ? author->data : NULL,
> + SVN_ERR(svn_repos__hooks_start_commit(repos, hooks_env,
> + author ? author->data : NULL,
> repos->client_capabilities, txn_name,
> pool));
> return SVN_NO_ERROR;
> @@ -317,6 +329,7 @@ svn_repos_fs_change_rev_prop4(svn_repos_
> {
> const svn_string_t *old_value;
> char action;
> + apr_hash_t *hooks_env = NULL;
>
IMO would be better not to initialise this at declaration: all codepaths
that use the value also initialise it first.
The same might apply to the load-fs-vtable.c:close_revision() changes.
> Modified: subversion/trunk/subversion/libsvn_repos/repos.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/repos.h?rev=1464122&r1=1464121&r2=1464122&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/repos.h (original)
> +++ subversion/trunk/subversion/libsvn_repos/repos.h Wed Apr 3 17:51:56 2013
> @@ -161,9 +161,25 @@ struct svn_repos_t
>
> /*** Hook-running Functions ***/
>
> +/* Set *HOOKS_ENV_P to the parsed contents of the hooks-env file
> + LOCAL_ABSPATH, allocated in RESULT_POOL. (This result is suitable
> + for delivery to the various hook wrapper functions which accept a
> + 'hooks_env' parameter.)
> +
As noted on IRC, suggesting to promise setting HOOKS_ENV_P to NULL if
local_abspath is NULL, since callers depend on that.
> + Use SCRATCH_POOL for temporary allocations. */
> +svn_error_t *
> +svn_repos__parse_hooks_env(apr_hash_t **hooks_env_p,
> + const char *local_abspath,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool);
> +
Received on 2013-04-03 20:04:32 CEST