Re: Specification of hook script interface

Julian Foad <julianfoad@btopenworld.com> writes:

> The interfaces for post-* hooks are similar but a bit different, with
> the primary difference that stderr and the exit code are ignored.

That's no longer true for 1.4, it returns post-commit errors back to
the client.

Since I didn't know what the other post-* hooks did I decided to
investigate. The first thing I see is that libsvn_repos/hooks.c
throws away stderr for these hooks, so the default hook error message
"hook failed; no error output available" is used.

First ra_svn:

$ svn lock svn://localhost/repo/foo
../svn/subversion/libsvn_repos/fs-wrap.c:453: (apr_err=165008)
svn: Lock succeeded, but post-lock hook failed
../svn/subversion/libsvn_repos/hooks.c:144: (apr_err=165001)
svn: 'post-lock' hook failed; no error output available

$ svn unlock svn://localhost/repo/foo
../svn/subversion/libsvn_repos/fs-wrap.c:496: (apr_err=165009)
svn: Unlock succeeded, but post-unlock hook failed
../svn/subversion/libsvn_repos/hooks.c:144: (apr_err=165001)
svn: 'post-unlock' hook failed; no error output available

$ svn ps --revprop -r1 xxx yyy svn://localhost/repo/foo
../svn/subversion/libsvn_repos/hooks.c:144: (apr_err=165001)
svn: 'post-revprop-change' hook failed; no error output available

That's not so bad, the failure is reported but with the default hook
error message, it looks as if a minor tweak to hooks.c would cause the
hook's stderr to be returned.

Next ra_dav:

$ svn lock http://localhost/repo
../svn/subversion/libsvn_ra_dav/util.c:389: (apr_err=165008)
svn: Lock succeeded, but post-lock hook failed

$ svn lock http://localhost/repo
../svn/subversion/libsvn_ra_dav/util.c:296: (apr_err=175002)
svn: Unlock request failed: 500 Internal Server Error (http://localhost:8888)

$ svn ps --revprop -r1 xxx yyy http://localhost/repo
../svn/subversion/libsvn_ra_dav/fetch.c:1906: (apr_err=175002)
svn: DAV request failed; it's possible that the repository's pre-revprop-change hook either failed or is non-existent
../svn/subversion/libsvn_ra_dav/props.c:1104: (apr_err=175008)
svn: At least one property change failed; repository is unchanged

That's not so good. Lock gives an error, but it's not the default
hook error message. Unlock looks to have a real bug. The propset
gives a very misleading error message considering it's a
post-revprop-change failure after the change has been made.

Looking in the error log I see this for lock:

[Thu Mar 30 19:40:31 2006] [error] [client 127.0.0.1] Failed to create new lock. [500, #165008]
[Thu Mar 30 19:40:31 2006] [error] [client 127.0.0.1] Lock succeeded, but post-lock hook failed [500, #165008]
[Thu Mar 30 19:40:31 2006] [error] [client 127.0.0.1] 'post-lock' hook failed; no error output available [500, #165001]

nothing for unlock, and this for propset:

[Thu Mar 30 19:41:58 2006] [error] [client 127.0.0.1] 'post-revprop-change' hook failed; no error output available [500, #165001]
[Thu Mar 30 19:41:58 2006] [error] [client 127.0.0.1] Could not execute PROPPATCH. [500, #206]
[Thu Mar 30 19:41:58 2006] [error] [client 127.0.0.1] 'post-revprop-change' hook failed; no error output available [500, #165001]

There's the default hook error message, so once again a small tweak to
hooks.c should cause the hook's stderr to get reported. Does anyone
know why we are throwing away the hook's stderr? This is what I mean:

Index: subversion/libsvn_repos/hooks.c
===================================================================
--- subversion/libsvn_repos/hooks.c (revision 19083)
+++ subversion/libsvn_repos/hooks.c (working copy)
@@ -690,7 +690,7 @@
       args[5] = action_string;
       args[6] = NULL;
 
- SVN_ERR(run_hook_cmd("post-revprop-change", hook, args, FALSE,
+ SVN_ERR(run_hook_cmd("post-revprop-change", hook, args, TRUE,
                            stdin_handle, pool));
       
       SVN_ERR(svn_io_file_close(stdin_handle, pool));
@@ -760,7 +760,7 @@
       args[3] = NULL;
       args[4] = NULL;
 
- SVN_ERR(run_hook_cmd("post-lock", hook, args, FALSE,
+ SVN_ERR(run_hook_cmd("post-lock", hook, args, TRUE,
                            stdin_handle, pool));
 
       SVN_ERR(svn_io_file_close(stdin_handle, pool));
@@ -829,7 +829,7 @@
       args[3] = NULL;
       args[4] = NULL;
 
- SVN_ERR(run_hook_cmd("post-unlock", hook, args, FALSE,
+ SVN_ERR(run_hook_cmd("post-unlock", hook, args, TRUE,
                            stdin_handle, pool));
 
       SVN_ERR(svn_io_file_close(stdin_handle, pool));

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org