[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] show hook errors on client side

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2002-09-02 02:48:35 CEST

Gustavo Niemeyer <niemeyer@conectiva.com> writes:

> Fix issue 443:

Great!

>
> * hooks.c

* subversion/libsvn_repos/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)
> {

[snip]

> + 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));

[snip]

> @@ -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));

Why did you choose to ignore post-commit errors? I suppose it's so
that the client carries out the post-commit processing and doesn't
abort. I think it would be better to define a specific error for this
case, return that instead of SVN_ERR_REPOS_HOOK_FAILURE, and get the
client to detect it, print the error message (svn_handle_error with
FATAL set FALSE probably) and continue.

> }
>
> return SVN_NO_ERROR;

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Sep 2 02:49:34 2002

This is an archived mail posted to the Subversion Dev mailing list.