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

Re: svn_fs_commit_txn and svn_repos_fs_commit_txn inconsistency

From: Blair Zajac <blair_at_orcaware.com>
Date: Tue, 21 Dec 2010 10:55:37 -0800

On 12/21/10 10:40 AM, Daniel Shahaf wrote:
> Blair Zajac wrote on Tue, Dec 21, 2010 at 10:16:56 -0800:
>> On 12/20/10 11:32 AM, C. Michael Pilato wrote:
>>> On 12/20/2010 02:14 PM, Blair Zajac wrote:
>>>> Shouldn't svn_repos_fs_commit_txn() always run the post-commit hook if
>>>> new_rev is a valid rev?
>>> That does seem reasonable, yes.
>> Looking through our code, no existing use of svn_fs_commit_txn() and
>> svn_repos_fs_commit_txn() use SVN_IS_VALID_REVNUM(new_rev), the code
>> checks for a non-NULL svn_error_t * and checks if the parent error is a
>> SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED. It also doesn't scan the chain
>> for that error.
> svn_error_has_cause() could be used for that.

Thanks, I'll use that. For 1.6.x, have to make a private version of it.

>> 4) In svn_repos_fs_commit_txn(), which order should errors be composed?
>> svn_fs_commit_txn()'s error as the parent followed by the
>> SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED error as a child? This seems to be
>> the standard ordering of chained errors. On the other hand, it makes it
>> harder to find a post-commit script error.
> Actually, it will make it impossible to detect post-commit errors over
> ra_dav, since that RA layer marshals only the outermost error code in an
> error chain.

ra_dav already checks for SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED, it could use
svn_error_has_cause() to find it. If ra_dav cannot find a
SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED but there is an error and the commit
succeeded, then I think the parent error should be returned to warn people there
may be an issue with the server.

Received on 2010-12-21 19:56:18 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.