You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2012/08/22 21:19:49 UTC
svn commit: r1376201 - in /subversion/trunk/subversion/libsvn_repos:
commit.c fs-wrap.c hooks.c repos.c repos.h
Author: cmpilato
Date: Wed Aug 22 19:19:48 2012
New Revision: 1376201
URL: http://svn.apache.org/viewvc?rev=1376201&view=rev
Log:
Tweak the 'start-commit' hook to run *after* txn creation and initial
revprop population, and to receive as a new command-line argument the
txn-id (so that scripts can examine those revprops).
* subversion/libsvn_repos/repos.h,
* subversion/libsvn_repos/hooks.c
(svn_repos__hooks_start_commit): Add 'txn_name' parameter, the value
of which is passed as the new final argument to the 'start-commit'
hook.
* subversion/libsvn_repos/commit.c
(svn_repos__get_commit_ev2): Update call to svn_repos__hooks_start_commit(),
and now run it after setting txnprops.
* subversion/libsvn_repos/fs-wrap.c
(svn_repos_fs_begin_txn_for_commit2): Fetch the transaction name.
Update call to svn_repos__hooks_start_commit(), and now run it
after setting txnprops.
* subversion/libsvn_repos/repos.c
(create_hooks): Update 'start-commit' hook template text to reflect
changes.
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/repos.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=1376201&r1=1376200&r2=1376201&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/commit.c (original)
+++ subversion/trunk/subversion/libsvn_repos/commit.c Wed Aug 22 19:19:48 2012
@@ -1334,12 +1334,8 @@ svn_repos__get_commit_ev2(svn_editor_t *
/* Can the user modify the repository at all? */
/* ### check against AUTHZ. */
- /* Okay... some access is allowed. Let's run the start-commit hook. */
author = apr_hash_get(revprops, SVN_PROP_REVISION_AUTHOR,
APR_HASH_KEY_STRING);
- SVN_ERR(svn_repos__hooks_start_commit(repos, author ? author->data : NULL,
- repos->client_capabilities,
- scratch_pool));
eb = apr_palloc(result_pool, sizeof(*eb));
eb->repos = repos;
@@ -1357,6 +1353,11 @@ svn_repos__get_commit_ev2(svn_editor_t *
/* The TXN has been created. Go ahead and apply all revision properties. */
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,
+ repos->client_capabilities,
+ eb->txn_name, scratch_pool));
+
/* Wrap the FS editor within our editor. */
SVN_ERR(svn_editor_create(editor, eb, cancel_func, cancel_baton,
result_pool, 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=1376201&r1=1376200&r2=1376201&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/fs-wrap.c (original)
+++ subversion/trunk/subversion/libsvn_repos/fs-wrap.c Wed Aug 22 19:19:48 2012
@@ -83,23 +83,29 @@ svn_repos_fs_begin_txn_for_commit2(svn_f
apr_hash_t *revprop_table,
apr_pool_t *pool)
{
- svn_string_t *author = apr_hash_get(revprop_table, SVN_PROP_REVISION_AUTHOR,
- APR_HASH_KEY_STRING);
apr_array_header_t *revprops;
+ const char *txn_name;
+ svn_string_t *author = apr_hash_get(revprop_table,
+ SVN_PROP_REVISION_AUTHOR,
+ APR_HASH_KEY_STRING);
- /* Run start-commit hooks. */
- SVN_ERR(svn_repos__hooks_start_commit(repos, author ? author->data : NULL,
- repos->client_capabilities, pool));
-
- /* Begin the transaction, ask for the fs to do on-the-fly lock checks. */
+ /* 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. */
SVN_ERR(svn_fs_begin_txn2(txn_p, repos->fs, rev,
SVN_FS_TXN_CHECK_LOCKS, pool));
+ SVN_ERR(svn_fs_txn_name(&txn_name, *txn_p, pool));
/* We pass the revision properties to the filesystem by adding them
as properties on the txn. Later, when we commit the txn, these
properties will be copied into the newly created revision. */
revprops = svn_prop_hash_to_array(revprop_table, pool);
- return svn_repos_fs_change_txn_props(*txn_p, revprops, pool);
+ 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,
+ repos->client_capabilities, txn_name,
+ pool));
+ return SVN_NO_ERROR;
}
Modified: subversion/trunk/subversion/libsvn_repos/hooks.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/hooks.c?rev=1376201&r1=1376200&r2=1376201&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/hooks.c (original)
+++ subversion/trunk/subversion/libsvn_repos/hooks.c Wed Aug 22 19:19:48 2012
@@ -357,6 +357,7 @@ svn_error_t *
svn_repos__hooks_start_commit(svn_repos_t *repos,
const char *user,
const apr_array_header_t *capabilities,
+ const char *txn_name,
apr_pool_t *pool)
{
const char *hook = svn_repos_start_commit_hook(repos, pool);
@@ -368,7 +369,7 @@ svn_repos__hooks_start_commit(svn_repos_
}
else if (hook)
{
- const char *args[5];
+ const char *args[6];
char *capabilities_string;
if (capabilities)
@@ -388,7 +389,8 @@ svn_repos__hooks_start_commit(svn_repos_
args[1] = svn_dirent_local_style(svn_repos_path(repos, pool), pool);
args[2] = user ? user : "";
args[3] = capabilities_string;
- args[4] = NULL;
+ args[4] = txn_name;
+ args[5] = NULL;
SVN_ERR(run_hook_cmd(NULL, SVN_REPOS__HOOK_START_COMMIT, hook, args,
repos->hooks_env, NULL, pool));
Modified: subversion/trunk/subversion/libsvn_repos/repos.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/repos.c?rev=1376201&r1=1376200&r2=1376201&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/repos.c (original)
+++ subversion/trunk/subversion/libsvn_repos/repos.c Wed Aug 22 19:19:48 2012
@@ -311,16 +311,17 @@ create_hooks(svn_repos_t *repos, apr_poo
"" NL
"# START-COMMIT HOOK" NL
"#" NL
-"# The start-commit hook is invoked before a Subversion txn is created" NL
-"# in the process of doing a commit. Subversion runs this hook" NL
-"# by invoking a program (script, executable, binary, etc.) named" NL
-"# '"SCRIPT_NAME"' (for which this file is a template)" NL
-"# with the following ordered arguments:" NL
+"# The start-commit hook is invoked immediately after a Subversion txn is" NL
+"# created and populated with initial revprops in the process of doing a" NL
+"# commit. Subversion runs this hook by invoking a program (script, " NL
+"# executable, binary, etc.) named '"SCRIPT_NAME"' (for which this file" NL
+"# is a template) with the following ordered arguments:" NL
"#" NL
"# [1] REPOS-PATH (the path to this repository)" NL
"# [2] USER (the authenticated user attempting to commit)" NL
"# [3] CAPABILITIES (a colon-separated list of capabilities reported" NL
"# by the client; see note below)" NL
+"# [4] TXN-NAME (the name of the commit txn just created)" NL
"#" NL
"# Note: The CAPABILITIES parameter is new in Subversion 1.5, and 1.5" NL
"# clients will typically report at least the \"" \
@@ -352,6 +353,10 @@ create_hooks(svn_repos_t *repos, apr_poo
"# '"SCRIPT_NAME".bat' or '"SCRIPT_NAME".exe'," NL
"# but the basic idea is the same." NL
"# " NL
+"# COMPATIBILITY NOTE: Prior to Subversion 1.8, the start-commit hook was" NL
+"# invoked before the commit txn was even created, and it did not accept" NL
+"# the TXN-NAME argument at all." NL
+"# " NL
HOOKS_ENVIRONMENT_TEXT
"# " NL
"# Here is an example hook script, for a Unix /bin/sh interpreter." NL
Modified: subversion/trunk/subversion/libsvn_repos/repos.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/repos.h?rev=1376201&r1=1376200&r2=1376201&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/repos.h (original)
+++ subversion/trunk/subversion/libsvn_repos/repos.h Wed Aug 22 19:19:48 2012
@@ -153,14 +153,19 @@ struct svn_repos_t
allocations. If the hook fails, return SVN_ERR_REPOS_HOOK_FAILURE.
USER is the authenticated name of the user starting the commit.
+
CAPABILITIES is a list of 'const char *' capability names (using
SVN_RA_CAPABILITY_*) that the client has self-reported. Note that
there is no guarantee the client is telling the truth: the hook
- should not make security assumptions based on the capabilities. */
+ should not make security assumptions based on the capabilities.
+
+ TXN_NAME is the name of the commit transaction that's just been
+ created. */
svn_error_t *
svn_repos__hooks_start_commit(svn_repos_t *repos,
const char *user,
const apr_array_header_t *capabilities,
+ const char *txn_name,
apr_pool_t *pool);
/* Run the pre-commit hook for REPOS. Use POOL for any temporary
Re: svn commit: r1376201 - in /subversion/trunk/subversion/libsvn_repos:
commit.c fs-wrap.c hooks.c repos.c repos.h
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 14 March 2014 13:10, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On 22 August 2012 23:19, <cm...@apache.org> wrote:
>> Author: cmpilato
>> Date: Wed Aug 22 19:19:48 2012
>> New Revision: 1376201
>>
>> URL: http://svn.apache.org/viewvc?rev=1376201&view=rev
>> Log:
>> Tweak the 'start-commit' hook to run *after* txn creation and initial
>> revprop population, and to receive as a new command-line argument the
>> txn-id (so that scripts can examine those revprops).
>>
>> * subversion/libsvn_repos/repos.h,
>> * subversion/libsvn_repos/hooks.c
>> (svn_repos__hooks_start_commit): Add 'txn_name' parameter, the value
>> of which is passed as the new final argument to the 'start-commit'
>> hook.
>>
>> * subversion/libsvn_repos/commit.c
>> (svn_repos__get_commit_ev2): Update call to svn_repos__hooks_start_commit(),
>> and now run it after setting txnprops.
>>
>> * subversion/libsvn_repos/fs-wrap.c
>> (svn_repos_fs_begin_txn_for_commit2): Fetch the transaction name.
>> Update call to svn_repos__hooks_start_commit(), and now run it
>> after setting txnprops.
>>
>> * subversion/libsvn_repos/repos.c
>> (create_hooks): Update 'start-commit' hook template text to reflect
>> changes.
>>
> Hi Mike,
>
> This commit broke several things:
> 1. The dead transaction will remain if start commit fails for any
> reason, while it should not.
> 2. The docstring promises that *TXN_P unaffected in case of hook
> failure, while it changes it. We rely on documented behavior in
> open_root at libsvn_repos/commit.c:401
>
> 3. Docstring for svn_repos_fs_begin_txn_for_commit2() explicitly
> states that start-commit hook run *before* transaction created:
> [[[
> * Before a txn is created, the repository's start-commit hooks are
> * run; if any of them fail, no txn is created, @a *txn_p is unaffected,
> * and #SVN_ERR_REPOS_HOOK_FAILURE is returned.
> ]]]
> I doubt that our policy allow to change such API promise. Anyway this
> code is already released, so we have to deal with this somehow.
>
> I'm working on patch to solve (1) and (2), see attached file.
>
> Btw I wonder why you decided to change existing hook behavior, instead
> of create separate hook like 'post-start-commit' or 'validate-txn' to
> give opportunity to validate client capabilities early as possible.
>
>
Fixed in r1583977.
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Re: svn commit: r1376201 - in /subversion/trunk/subversion/libsvn_repos:
commit.c fs-wrap.c hooks.c repos.c repos.h
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 22 August 2012 23:19, <cm...@apache.org> wrote:
> Author: cmpilato
> Date: Wed Aug 22 19:19:48 2012
> New Revision: 1376201
>
> URL: http://svn.apache.org/viewvc?rev=1376201&view=rev
> Log:
> Tweak the 'start-commit' hook to run *after* txn creation and initial
> revprop population, and to receive as a new command-line argument the
> txn-id (so that scripts can examine those revprops).
>
> * subversion/libsvn_repos/repos.h,
> * subversion/libsvn_repos/hooks.c
> (svn_repos__hooks_start_commit): Add 'txn_name' parameter, the value
> of which is passed as the new final argument to the 'start-commit'
> hook.
>
> * subversion/libsvn_repos/commit.c
> (svn_repos__get_commit_ev2): Update call to svn_repos__hooks_start_commit(),
> and now run it after setting txnprops.
>
> * subversion/libsvn_repos/fs-wrap.c
> (svn_repos_fs_begin_txn_for_commit2): Fetch the transaction name.
> Update call to svn_repos__hooks_start_commit(), and now run it
> after setting txnprops.
>
> * subversion/libsvn_repos/repos.c
> (create_hooks): Update 'start-commit' hook template text to reflect
> changes.
>
Hi Mike,
This commit broke several things:
1. The dead transaction will remain if start commit fails for any
reason, while it should not.
2. The docstring promises that *TXN_P unaffected in case of hook
failure, while it changes it. We rely on documented behavior in
open_root at libsvn_repos/commit.c:401
3. Docstring for svn_repos_fs_begin_txn_for_commit2() explicitly
states that start-commit hook run *before* transaction created:
[[[
* Before a txn is created, the repository's start-commit hooks are
* run; if any of them fail, no txn is created, @a *txn_p is unaffected,
* and #SVN_ERR_REPOS_HOOK_FAILURE is returned.
]]]
I doubt that our policy allow to change such API promise. Anyway this
code is already released, so we have to deal with this somehow.
I'm working on patch to solve (1) and (2), see attached file.
Btw I wonder why you decided to change existing hook behavior, instead
of create separate hook like 'post-start-commit' or 'validate-txn' to
give opportunity to validate client capabilities early as possible.
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com