cmpil...@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); > +
