[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1376201 - in /subversion/trunk/subversion/libsvn_repos: commit.c fs-wrap.c hooks.c repos.c repos.h

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Fri, 14 Mar 2014 13:10:10 +0400

On 22 August 2012 23:19, <cmpilato_at_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

Received on 2014-03-14 10:11:08 CET

This is an archived mail posted to the Subversion Dev mailing list.