On 12/22/10 5:47 AM, Daniel Shahaf wrote:
bl...@apache.org wrote on Wed, Dec 22, 2010 at 05:46:45 -0000:
Author: blair
Date: Wed Dec 22 05:46:45 2010
New Revision: 1051763
URL: http://svn.apache.org/viewvc?rev=1051763&view=rev
Log:
Modified: subversion/trunk/subversion/libsvn_repos/commit.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/commit.c?rev=1051763&r1=1051762&r2=1051763&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/commit.c (original)
+++ subversion/trunk/subversion/libsvn_repos/commit.c Wed Dec 22 05:46:45 2010
@@ -644,7 +644,56 @@ change_dir_prop(void *dir_baton,
name, value, pool);
}
+const char *
+svn_repos__post_commit_error_str(svn_error_t *err,
+ apr_pool_t *pool)
+{
+ svn_error_t *hook_err1, *hook_err2;
+
+ if (! err)
+ return "(no error)";
+
+ err = svn_error_purge_tracing(err);
This modifies the provided error. Either the doc string should warn
that this is being done (i.e., the provided ERR is not safe for use
after calling this helper), or this call should be removed and another
means used to locate the first non-tracing error message.
Where do you clear ERR? You must do so in this function, since the
caller's ERR may not be cleared (according to svn_error_purge_tracing()'s
promise).
Is this promise a defensive promise, as the original err still looks usable by
reviewing svn_error_purge_tracing()'s implementation? It looks like only one of
the errors should ever be cleared and it doesn't matter which one. True, it's
conceptually easier to remember that one should only clear the error returned
from svn_error_purge_tracing().
This function is meant to consume the error, so I've updated the docs to say
that it shouldn't be used after it and clear it.
r1051978.
+ hook_err1 = svn_error_find_cause(err, SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED);
+ if (hook_err1&& hook_err1->child)
+ hook_err2 = hook_err1->child;
+ else
+ hook_err2 = hook_err1;
+
So, ERR is the svn_fs_commit_txn() error (which has the hook error
somewhere down the chain), and HOOK_ERR1 is the post-commit hook error?
Yes, and HOOK_ERR2 is the un-wrapped error from the hook script.
+ /* This implementation counts on svn_repos_fs_commit_txn() returning
+ svn_fs_commit_txn() as the parent error with a child
+ SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED error. If the parent error
+ is SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED then there was no error
+ in svn_fs_commit_txn(). */
+ if (hook_err1)
+ {
+ if (err == hook_err1)
+ {
+ if (hook_err2->message)
+ return apr_pstrdup(pool, hook_err2->message);
+ else
+ return "(no error message)";
+ }
+ else
+ {
+ return apr_psprintf(pool,
+ "Post commit processing had error and '%s' "
s/and '%s'/'%s' and/
+ "post-commit hook had error '%s'.",
+ err->message ? err->message
+ : "(no error message)",
+ hook_err2->message ? hook_err2->message
+ : "(no error message)");
+ }
+ }
+ else
+ {
+ if (err->message)
+ return apr_pstrdup(pool, err->message);
In this case, and in the apr_pstrdup() above, might it be useful to say
explicitly whether the given error is from the post-commit hook or from
some FS post-commit fiddling? You've skipped over the
SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED link, so the "post-commit hook
failed" message that used to be there would be gone.
Fixes in r1051988.
Should we have an official term for FS fiddling, or call it post-commit
processing? So we'll have to distinguish between "hook" and "processing".
Blair
