Blair Zajac wrote on Mon, Dec 20, 2010 at 11:14:34 -0800:
> The docs for svn_fs_commit_txn() for read:
>
>  * @note Success or failure of the commit of @a txn is determined by
>  * examining the value of @a *new_rev upon this function's return.  If
>  * the value is a valid revision number, the commit was successful,
>  * even though a n...@c NULL function return value may indicate that
>  * something else went wrong.
>
> However, svn_repos_fs_commit_txn() will only run the post-commit hook if  
> SVN_NO_ERROR was returned:
>
>   /* Commit. */
>   SVN_ERR(svn_fs_commit_txn(conflict_p, new_rev, txn, pool));
>
>   /* Run post-commit hooks.   Notice that we're wrapping the error
>      with a -specific- errorcode, so that our caller knows not to try
>      and abort the transaction. */
>   if ((err = svn_repos__hooks_post_commit(repos, *new_rev, pool)))
>     return svn_error_create
>       (SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED, err,
>        _("Commit succeeded, but post-commit hook failed"));
>
>   return SVN_NO_ERROR;
>
> Shouldn't svn_repos_fs_commit_txn() always run the post-commit hook if 
> new_rev is a valid rev?
>

That matters when NEW_REV is a valid rev but there is a non-SVN_NO_ERROR
return value.  When can that happen?

(just on Saturday I drafted a patch to make failing to update
rep-cache.db after the commit itself succeeded not be considered
a fatal error; that would be one case when that can happen.)

> BTW, we should have the docs for svn_fs_commit_txn mention that *new_rev 
> is always modified, so the caller doesn't have to set *new_rev to 
> SVN_INVALID_REVNUM.
>
> Blair

Reply via email to