[PATCH] show hook errors on client side

Fix issue 443:

* hooks.c
  (run_cmd_with_output) Function renamed to run_hook_cmd, and changed
  its interface. Now command error checking is done inside the function.
  Also, the stderr from the command run is appended to the error msg,
  so that remote clients can see the error as well.
  
  (run_start_commit_hook,run_pre_commit_hook,run_post_commit_hook):
  Changed to use new run_hook_cmd interface.

  (run_start_commit_hook) Fixed behavior. Now considers hook exit code,
  as stated in the documentation.

--- subversion/subversion/libsvn_repos/hooks.c.reporterror 2002-08-26 11:16:20.000000000 -0300
+++ subversion/subversion/libsvn_repos/hooks.c 2002-08-30 18:13:20.000000000 -0300
@@ -38,29 +38,74 @@
 /*** Hook drivers. ***/
 
 static svn_error_t *
-run_cmd_with_output (const char *cmd,
- const char **args,
- int *exitcode,
- apr_exit_why_e *exitwhy,
- apr_pool_t *pool)
+run_hook_cmd (const char *name,
+ const char *cmd,
+ const char **args,
+ svn_boolean_t check_exitcode,
+ apr_pool_t *pool)
 {
- apr_file_t *outhandle, *errhandle;
+ apr_file_t *read_outhandle, *write_outhandle;
+ apr_file_t *read_errhandle, *write_errhandle;
   apr_status_t apr_err;
+ svn_error_t *err;
+ int exitcode;
+ apr_exit_why_e exitwhy;
+
+ /* Create pipes to access stdout and stderr of the child. */
+ apr_err = apr_file_pipe_create (&read_outhandle, &write_outhandle, pool);
+ if (apr_err)
+ return svn_error_create
+ (apr_err, 0, NULL, pool,
+ "run_hook_cmd: can't create pipe for stdout");
+ apr_err = apr_file_pipe_create(&read_errhandle, &write_errhandle, pool);
+ if (apr_err)
+ return svn_error_create
+ (apr_err, 0, NULL, pool,
+ "run_hook_cmd: can't create pipe for stderr");
   
- /* Get an apr_file_t representing stdout and stderr. */
- apr_err = apr_file_open_stdout (&outhandle, pool);
+ err = svn_io_run_cmd (".", cmd, args, &exitcode, &exitwhy, FALSE,
+ NULL, write_outhandle, write_errhandle, pool);
+
+ /* This seems to be done automatically if we pass the third parameter
+ of apr_procattr_child_in/out_set(), but svn_io_run_cmd()'s interface
+ is currently not supporting these parameters. */
+ apr_err = apr_file_close(write_outhandle);
   if (apr_err)
     return svn_error_create
       (apr_err, 0, NULL, pool,
- "run_cmd_with_output: can't open handle to stdout");
- apr_err = apr_file_open_stderr (&errhandle, pool);
+ "run_hook_cmd: can't close parent side stdout pipe");
+ apr_err = apr_file_close(write_errhandle);
   if (apr_err)
     return svn_error_create
       (apr_err, 0, NULL, pool,
- "run_cmd_with_output: can't open handle to stderr");
+ "run_hook_cmd: can't close parent side stderr pipe");
+
+ /* Function failed. */
+ if (err)
+ {
+ return svn_error_createf
+ (SVN_ERR_REPOS_HOOK_FAILURE, 0, err, pool,
+ "run_hook_cmd: error running cmd `%s'", cmd);
+ }
+
+ if (check_exitcode)
+ {
+ /* Command failed. */
+ if (! APR_PROC_CHECK_EXIT (exitwhy) || exitcode != 0)
+ {
+ svn_stringbuf_t *error = svn_stringbuf_create ("", pool);
+
+ /* Read the file's contents into a stringbuf, allocated in POOL. */
+ SVN_ERR (svn_string_from_aprfile (&error, read_errhandle, pool));
 
- return svn_io_run_cmd (".", cmd, args, exitcode, exitwhy, FALSE,
- NULL, outhandle, errhandle, pool);
+ return svn_error_createf
+ (SVN_ERR_REPOS_HOOK_FAILURE, 0, err, pool,
+ "%s hook return non-zero status. Aborting txn.\n"
+ "Command output:\n%s", name, error->data);
+ }
+ }
+
+ return SVN_NO_ERROR;
 }
 
 
@@ -85,12 +130,7 @@
       args[2] = user;
       args[3] = NULL;
 
- if ((err = run_cmd_with_output (hook, args, NULL, NULL, pool)))
- {
- return svn_error_createf
- (SVN_ERR_REPOS_HOOK_FAILURE, 0, err, pool,
- "run_start_commit_hook: error running cmd `%s'", hook);
- }
+ SVN_ERR (run_hook_cmd ("start-commit", hook, args, TRUE, pool));
     }
 
   return SVN_NO_ERROR;
@@ -110,9 +150,6 @@
   if ((! svn_io_check_path (hook, &kind, pool))
       && (kind == svn_node_file))
     {
- svn_error_t *err;
- int exitcode;
- apr_exit_why_e exitwhy;
       const char *args[4];
 
       args[0] = hook;
@@ -120,18 +157,7 @@
       args[2] = txn_name;
       args[3] = NULL;
 
- if ((err = run_cmd_with_output (hook, args, &exitcode, &exitwhy, pool)))
- {
- return svn_error_createf
- (SVN_ERR_REPOS_HOOK_FAILURE, 0, err, pool,
- "run_pre_commit_hook: error running cmd `%s'", hook);
- }
- if (! APR_PROC_CHECK_EXIT (exitwhy) || exitcode != 0)
- {
- return svn_error_create
- (SVN_ERR_REPOS_HOOK_FAILURE, 0, err, pool,
- "pre-commit hook return non-zero status. Aborting txn.");
- }
+ SVN_ERR (run_hook_cmd ("pre-commit", hook, args, TRUE, pool));
     }
 
   return SVN_NO_ERROR;
@@ -151,7 +177,6 @@
   if ((! svn_io_check_path (hook, &kind, pool))
       && (kind == svn_node_file))
     {
- svn_error_t *err;
       const char *args[4];
 
       args[0] = hook;
@@ -159,12 +184,7 @@
       args[2] = apr_psprintf (pool, "%" SVN_REVNUM_T_FMT, rev);
       args[3] = NULL;
 
- if ((err = run_cmd_with_output (hook, args, NULL, NULL, pool)))
- {
- return svn_error_createf
- (SVN_ERR_REPOS_HOOK_FAILURE, 0, err, pool,
- "run_post_commit_hook: error running cmd `%s'", hook);
- }
+ SVN_ERR (run_hook_cmd ("post-commit", hook, args, FALSE, pool));
     }
 
   return SVN_NO_ERROR;

-- 
Gustavo Niemeyer
[ 2AAC 7928 0FBF 0299 5EB5  60E2 2253 B29A 6664 3A0C ]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org