Re: [PATCH] Re: Hooks on Windows

D.J. Heap wrote:

> Would someone mind doing quick review of this patch?
>
> Log:
>
> Fix the messy stderr output of hooks on Windows when
> a script file is used.
>
> * subversion/libsvn_repos/hooks.c
> (run_hook_cmd): On Windows, create a 'nul' file handle
> and pass it as the stdout handle when creating hook
> processes to prevent the shell from writing error
> messages to stderr and polluting stderr output.
>
>------------------------------------------------------------------------
>
>Index: subversion/libsvn_repos/hooks.c
>===================================================================
>--- subversion/libsvn_repos/hooks.c (revision 6787)
>+++ subversion/libsvn_repos/hooks.c (working copy)
>@@ -50,7 +50,7 @@
> svn_boolean_t check_exitcode,
> apr_pool_t *pool)
> {
>- apr_file_t *read_errhandle, *write_errhandle;
>+ apr_file_t *read_errhandle, *write_errhandle, *null_handle;
> apr_status_t apr_err;
> svn_error_t *err;
> int exitcode;
>@@ -62,8 +62,19 @@
> return svn_error_createf
> (apr_err, NULL, "can't create pipe for '%s' hook", cmd);
>
>+ null_handle = NULL;
>+#ifdef SVN_WIN32
>+ /* Create a 'nul' file handle for Windows, so that an invalid stdout handle
>+ won't cause cmd.exe to garbage up our stderr output with messages */
>+ apr_err = apr_file_open (&null_handle, "nul", APR_WRITE, APR_OS_DEFAULT,
>+ pool);
>+ if (apr_err)
>+ return svn_error_createf
>+ (apr_err, NULL, "can't create nul stdout for '%s' hook", cmd);
>+#endif
>
There's no reason to make this windows-specific. What we really want
here is an apr_file_open_null that returns a writable file to the
system-specific /dev/null, and use that. Unix people, do we have to
detect if /dev/null exists, or can we just fail if it doesn't?

>+
> err = svn_io_run_cmd (".", cmd, args, &exitcode, &exitwhy, FALSE,
>- NULL, NULL, write_errhandle, pool);
>+ NULL, null_handle, 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 does
>@@ -72,6 +83,13 @@
> if (!err && apr_err)
> return svn_error_create
> (apr_err, NULL, "can't close write end of stderr pipe");
>+ if (null_handle)
>+ {
>+ apr_err = apr_file_close (null_handle);
>+ if (!err && apr_err)
>+ return svn_error_create
>+ (apr_err, NULL, "can't close write end of null file");
>+ }
>
> /* Function failed. */
> if (err)
>
>

-- 
Brane Čibej   <brane_at_xbc.nu>   http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org