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.
