To reproduce:
% svnadmin create r
% ln -s /bin/false r/hooks/post-commit
% svnmucc put -mm r/format file://$PWD/r/$RANDOM
r1 committed by daniel at 2016-09-08T21:13:46.645621Z
%
The problem here is that the "post-commit hook failed" error isn't
printed.
The corresponding error message is available to the svn_commit_callback_t,
called deltify_etc(), in the svn_commit_info_t::post_commit_err struct
member, which is a string.
(I note in passing that this problem also affects post-commit FS
processing errors and that for them, the svn_error_t is also passed to
to svn_fs_warning_func_t that ra_local registers, called ignore_warnings().
However, that function is not invoked for post-commit hook errors [since
those are a repos-layer concern, not an FS-layer one], so I won't
discuss it further.)
I see three different approaches to fixing this:
1. Have ra_local's svn_commit_callback_t, called deltify_etc(),
wrap 'post_commit_err' in an svn_error_t struct, using an arbitrary,
hardcoded apr_err value.
2. Have svn_repos__post_commit_error_str() serialize the apr_err error
numbers into the svn_commit_callback_t::post_commit_err string — for
example, by setting that string to a skel containing a list of
(err->apr_err, err->message) pairs — then have deltify_etc() parse the
skel and use the apr_err values for constructing the error object.¹
3. Add to svn_commit_info_t an svn_error_t representation of the error
chain that svn_commit_callback_t::post_commit_err summarizes, and
a boolean member that, if true, tells svn_commit_info_dup() to install
on the duplicated struct a pool cleanup handler that clears the
duplicated svn_error_t. Any existing API function that produces
svn_commit_info_t objects will set the new boolean member so existing
consumers will be unaffected. Future API functions will be at liberty
to decide whether it is the producer's or consumer's responsibility to
clear the svn_error_t member.
Assessment: #1 is undesirable since it doesn't let API consumers
distinguish post-commit hook errors, post-commit FS processing errors,
and other errors. #2 would be robust, but it's unquestionably hacky;
there is no reason to resort to such tricks in this case. So I think we
have to take #3.
Makes sense?
Cheers,
Daniel
¹ I say "skel" because that scans better, but since ra_svn has code for
marshalling error chains over the wire, it might be easier to reuse that
code and serialize the error chain into ra_svn tuples rather than skels.
Received on 2016-09-09 08:45:50 CEST