Does this look ok for commiting, now? Any objections?
DJ
dj wrote:
> Here's a revised version.
>
> Log:
>
> Windows scripts require a valid stdout handle or else 'bad handle'
> messages are sent to stderr, cluttering up the error message that is
> sent back to the client. This patch sets up a stdout handle pointing to
> the null device when hooks are run to remedy the problem.
>
>
> *** Note that autogen.sh/configure (or gen-make.py on Windows) will need
> to be run to generate the null device definitions needed to compile.
>
>
> * configure.in
> Add a definition for the null device on unixy OS'.
>
> * subversion/libsvn_repos/hooks.c
> (run_hook_cmd): Setup stdout to point to the null device
> rather than leaving it as an invalid handle. Modified error
> handling/cleanup slightly to leave most file closing up to
> the pools.
>
> * svn_private_config.hw
> Add a definition for the null device on Windows.
>
>
>
> ------------------------------------------------------------------------
>
> Index: configure.in
> ===================================================================
> --- configure.in (revision 6898)
> +++ configure.in (working copy)
> @@ -422,6 +422,9 @@
> AC_DEFINE_UNQUOTED(SVN_PATH_LOCAL_SEPARATOR, '/',
> [Defined to be the path separator used on your local filesystem])
>
> +AC_DEFINE_UNQUOTED(SVN_NULL_DEVICE_NAME, "/dev/null",
> + [Defined to be the null device for the system])
> +
> dnl Find a python 2.X binary, test cases will not run with Python 1.X
> AC_PATH_PROG(PYTHON2, python2, none)
> if test "$PYTHON2" = "none"; then
> Index: subversion/libsvn_repos/hooks.c
> ===================================================================
> --- subversion/libsvn_repos/hooks.c (revision 6898)
> +++ subversion/libsvn_repos/hooks.c (working copy)
> @@ -42,7 +42,8 @@
> program that is to be run. If CHECK_EXITCODE is TRUE then the hook's
> exit status will be checked, and if an error occurred the hook's stderr
> output will be added to the returned error. If CHECK_EXITCODE is FALSE
> - the hook's exit status will be ignored. */
> + the hook's exit status will be ignored. Unclosed files and pipes will
> + be handled at pool cleanup. */
> static svn_error_t *
> run_hook_cmd (const char *name,
> const char *cmd,
> @@ -50,7 +51,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,12 +63,20 @@
> return svn_error_createf
> (apr_err, NULL, "can't create pipe for '%s' hook", cmd);
>
> + /* Redirect stdout to the null device */
> + apr_err = apr_file_open (&null_handle, SVN_NULL_DEVICE_NAME, APR_WRITE,
> + APR_OS_DEFAULT, pool);
> + if (apr_err)
> + return svn_error_createf
> + (apr_err, NULL, "can't create null stdout for '%s' hook", cmd);
> +
> 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
> - not support those parameters. */
> + not support those parameters. We need to close the write handle
> + so we don't hang if we need to read it later. */
> apr_err = apr_file_close (write_errhandle);
> if (!err && apr_err)
> return svn_error_create
> @@ -97,13 +106,6 @@
> }
> }
>
> - /* Hooks are fallible, and so hook failure is "expected" to occur at
> - times. When such a failure happens we still want to close the pipe */
> - apr_err = apr_file_close (read_errhandle);
> - if (!err && apr_err)
> - return svn_error_create
> - (apr_err, NULL, "can't close read end of stdout pipe");
> -
> return err;
> }
>
> Index: svn_private_config.hw
> ===================================================================
> --- svn_private_config.hw (revision 6898)
> +++ svn_private_config.hw (working copy)
> @@ -39,6 +39,9 @@
> /* Path separator for local filesystem */
> #define SVN_PATH_LOCAL_SEPARATOR '\\'
>
> +/* Name of system's null device */
> +#define SVN_NULL_DEVICE_NAME "nul"
> +
> /* Link local repos access library to client */
> #define SVN_LIBSVN_CLIENT_LINKS_RA_LOCAL
>
>
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Sep 5 16:31:09 2003