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

Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step 6

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2005-10-20 18:16:04 CEST

"Peter N. Lundblad" <peter@famlundblad.se> writes:

> On Thu, 20 Oct 2005, Madan U Sreenivasan wrote:
>
>> On Thu, 2005-10-20 at 03:10, Philip Martin wrote:
>> > Madan U Sreenivasan <madan@collab.net> writes:
>> >
>> > > @@ -1694,7 +1695,11 @@
>> > > return dav_svn_convert_err(serr, HTTP_CONFLICT, msg, pool);
>> > > }
>> > > else if (serr)
>> > > - svn_error_clear(serr);
>> > > + {
>> > > + if (serr->child->message)
>> > > + post_commit_err = apr_pstrdup (pool, serr->child->message);
>> >
>> > are building detailed knowledge of the structure of the repository
>> > error chain into mod_dav_svn. I'm not quite sure what you are trying
>> > to do, but if you are looking for a particular error then perhaps you
>> > should iterate through the list checking apr_err?
>> theres only one level of error message, in this case.. I think iterating
>> would be overkill...
>
> You're missing the main point that you're building this assumption about
> how libsvn_repos reports errors into mod_dav_svn. That's not guaranteed.
> If you want to provide the post-commit hook error back from an error with
> a specific error code as Philip suggests, that's fine, but you can't
> just assume that the next error in the chain is what it is today; it's an
> implementation detail of libsvn_repos.

I've now read the documentation for svn_repos_fs_commit_txn and it
explicitly states that any post hook error is returned wrapped in
SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED. I think it is acceptable for
mod_dav_svn to rely on that beahviour, although it should still verify
that err->child and err->child->message are non-NULL.

One other point, there are two calls to svn_repos_fs_commit_txn in
version.c but the patch only seems to change one of them. The other
call is in dav_svn_checkin and post-commit errors don't appear to be
handled at all, that might be an existing bug.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Oct 20 18:16:57 2005

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