You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "D.J. Heap" <dj...@shadyvale.net> on 2003/08/15 00:18:01 UTC
Re: Hooks on Windows]
Ok, after more investigating I've found the problem, I believe.
Windows appears to expect ALL std handles to be set to valid handles if
the STARTF_USESTDHANDLES flag is set in the STARTUPINFO -- but *only*
for .cmd and .bat files...exe's don't seem to have a problem.
When Subversion launches a pre-commit hook, it doesn't specify a stdout
handle, only a stderr one -- so apr_proc_create sets the stdout handle
to INVALID_HANDLE_VALUE. Then what's happening is the shell script
writes to stdout, the stdout handle is bad, and Windows (cmd.exe, I
suspect) spits out a 'Invalid handle' message to stderr. Thus, stderr
output is littered with invalid handle messages as well as the normal
stderr output.
Very weird, since it doesn't seem to apply to exe's (probably, cmd.exe
is 'helping' by printing error messages to stderr or something and
normal exe's just ignore the write failure).
Anyway, I'm not sure what exactly should be done to fix it...change APR
to provide a valid handle if the caller doesn't, or if Subversion should
just specify stdout as well or...?
I've inlined the little test program I made to play with it. Just
comment/uncomment setting the stdout handle in the STARTUPINFO to see
the differences when it is valid or not valid. Note that it is a
hastily written little test program and leaks handles, has hardcoded
paths, etc...The test.cmd script just echoes a line to stdout and one to
stderr.
DJ
----- stderr.cpp -----
#include <tchar.h>
#include <stdio.h>
#include <windows.h>
int _tmain(int argc, _TCHAR* argv[])
{
SECURITY_ATTRIBUTES sa = {sizeof(SECURITY_ATTRIBUTES), 0, true};
HANDLE out = CreateFile( "c:\\temp\\stdout.txt",
GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE, &sa,
CREATE_ALWAYS, 0, 0 );
if ( out == INVALID_HANDLE_VALUE )
{
printf( "CreateFile for out failed: 0x%x\n", GetLastError() );
return -1;
}
HANDLE err = CreateFile( "c:\\temp\\stderr.txt",
GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE, &sa,
CREATE_ALWAYS, 0, 0 );
if ( err == INVALID_HANDLE_VALUE )
{
printf( "CreateFile for err failed: 0x%x\n", GetLastError() );
return -1;
}
STARTUPINFO si = {sizeof(STARTUPINFO)};
si.dwFlags = STARTF_USESTDHANDLES;
// si.hStdOutput = out;
si.hStdError = err;
PROCESS_INFORMATION pi = {0};
BOOL rc = CreateProcess( "c:\\temp\\test.cmd", "c:\\temp\\test.cmd",
0, 0, true, 0, 0, 0, &si, &pi );
if ( rc == FALSE )
{
printf( "CreateProcess failed: 0x%x\n", GetLastError() );
return -1;
}
WaitForSingleObject( pi.hThread, INFINITE );
printf( "Complete.\n" );
return 0;
}
----- test.cmd -----
@echo off
echo hello stdout
echo 1>&2 hello stderr
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: Hooks on Windows
Posted by br...@xbc.nu.
Quoting dj <dj...@shadyvale.net>:
> Ok, after playing with this some, the hooks work fine on Linux if you
> have the #!/bin/shell line in a script, or if the hook is a binary.
>
> So, if you don't specify a shell/interpreter with a #! line, does the
> OS assume you want to use your default shell? And it must cause
> execution to behave differently somehow...? Just curious and trying
> to add to my limited (obviously) unixy OS knowledge.
So, you have a file with the execute bit set, but it's not a program binary, and
doesn't have a #! line telling the kernel how to process it? I can't say for
sure what is supposed to happen. If you start such a script from the shell, then
the shell will definitely interper it. Our hooks aren't started from the shell,
though, and what the kernel does is anyone's guess. The best suggestion I have
is "Don't do that". :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: Hooks on Windows
Posted by dj <dj...@shadyvale.net>.
Ok, after playing with this some, the hooks work fine on Linux if you
have the #!/bin/shell line in a script, or if the hook is a binary.
So, if you don't specify a shell/interpreter with a #! line, does the OS
assume you want to use your default shell? And it must cause execution
to behave differently somehow...? Just curious and trying to add to my
limited (obviously) unixy OS knowledge.
DJ
On Thu, 2003-08-21 at 18:54, D.J. Heap wrote:
> Is this patch what you had in mind? It builds and works well on
> Windows. It builds on my RH9 box, but hook output is lost (before and
> after this patch). Issue 443 implies that it was working at one time,
> but it seems to be broken now on Linux. I'm going to take a look at it
> this weekend and see if my meager Linux programming knowledge is up to
> the task.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] Re: Hooks on Windows
Posted by "D.J. Heap" <dj...@shadyvale.net>.
Branko Čibej wrote:
[snip]
>
> For the interim, I'd suggest defining the name of the null device in
> svn_private_config.h, for both Unix and Windows, then just use that
> define, and the rest of your patch can remain unchanged.
>
Is this patch what you had in mind? It builds and works well on
Windows. It builds on my RH9 box, but hook output is lost (before and
after this patch). Issue 443 implies that it was working at one time,
but it seems to be broken now on Linux. I'm going to take a look at it
this weekend and see if my meager Linux programming knowledge is up to
the task.
DJ
Re: [PATCH] Re: Hooks on Windows
Posted by Branko Čibej <br...@xbc.nu>.
D.J. Heap wrote:
> Branko Čibej wrote:
> [snip]
>
>>
>> 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?
>>
>>
> [snip]
>
> That sounds like a better idea. #ifdef's in code make me nervous.
For the interim, I'd suggest defining the name of the null device in
svn_private_config.h, for both Unix and Windows, then just use that
define, and the rest of your patch can remain unchanged.
--
Brane Čibej <br...@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
Re: [PATCH] Re: Hooks on Windows
Posted by "D.J. Heap" <dj...@shadyvale.net>.
Branko Čibej wrote:
[snip]
>
> 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?
>
>
[snip]
That sounds like a better idea. #ifdef's in code make me nervous.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: Hooks on Windows
Posted by Branko Čibej <br...@xbc.nu>.
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 <br...@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
[PATCH] Re: Hooks on Windows
Posted by "D.J. Heap" <dj...@shadyvale.net>.
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.
Re: Hooks on Windows
Posted by "D.J. Heap" <dj...@shadyvale.net>.
Ah, thanks, that is looking like it will work.
DJ
James Goodall wrote:
> There's a special DOS device called NUL that acts like *nix /dev/null. I
> don't know if it's a cmd.exe-specific feature or more general. It's in the
> same class as CON, PRN, COM1, COM2, etc.
>
> - James
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Hooks on Windows
Posted by James Goodall <jg...@dmetrix.com>.
There's a special DOS device called NUL that acts like *nix /dev/null. I
don't know if it's a cmd.exe-specific feature or more general. It's in the
same class as CON, PRN, COM1, COM2, etc.
- James
-----Original Message-----
From: D.J. Heap [mailto:dj@shadyvale.net]
Sent: Monday, August 18, 2003 6:51 PM
To: Branko Cibej
Cc: Subversion
Subject: Re: Hooks on Windows
Branko Čibej wrote:
[snip]
>
> Nope, I'd say we should only add a note about this to the APR
> documentation. SVN should provide a stdout handle, even if it's to
> /dev/null (or equivalent).
>
>
Is there a standard equivalent to /dev/null in Windows? I'm not aware
of one, but I would be very happy if someone could point one out.
DJ
---------------------------------------------------------------------
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
Re: Hooks on Windows
Posted by "D.J. Heap" <dj...@shadyvale.net>.
Branko Čibej wrote:
[snip]
>
> Nope, I'd say we should only add a note about this to the APR
> documentation. SVN should provide a stdout handle, even if it's to
> /dev/null (or equivalent).
>
>
Is there a standard equivalent to /dev/null in Windows? I'm not aware
of one, but I would be very happy if someone could point one out.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Hooks on Windows]
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Branko Čibej wrote:
> Probably, yes. Most exe's use stdio for output, anyway, and I've yet to
> see a program that checks ferror(stdout) after a write. :-)
Actually I believe we've got an issue somewhere about Subversion not
checking errors from writing to stdout ;-)
Not that I expect anyone to be closing that issue any time soon...
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Hooks on Windows]
Posted by Branko Čibej <br...@xbc.nu>.
D.J. Heap wrote:
> Ok, after more investigating I've found the problem, I believe.
>
> Windows appears to expect ALL std handles to be set to valid handles if
> the STARTF_USESTDHANDLES flag is set in the STARTUPINFO -- but *only*
> for .cmd and .bat files...exe's don't seem to have a problem.
Nice catch!
> When Subversion launches a pre-commit hook, it doesn't specify a stdout
> handle, only a stderr one -- so apr_proc_create sets the stdout handle
> to INVALID_HANDLE_VALUE. Then what's happening is the shell script
> writes to stdout, the stdout handle is bad, and Windows (cmd.exe, I
> suspect) spits out a 'Invalid handle' message to stderr. Thus, stderr
> output is littered with invalid handle messages as well as the normal
> stderr output.
>
> Very weird, since it doesn't seem to apply to exe's (probably, cmd.exe
> is 'helping' by printing error messages to stderr or something and
> normal exe's just ignore the write failure).
Probably, yes. Most exe's use stdio for output, anyway, and I've yet to
see a program that checks ferror(stdout) after a write. :-)
> Anyway, I'm not sure what exactly should be done to fix it...change APR
> to provide a valid handle if the caller doesn't, or if Subversion should
> just specify stdout as well or...?
Nope, I'd say we should only add a note about this to the APR
documentation. SVN should provide a stdout handle, even if it's to
/dev/null (or equivalent).
--
Brane Čibej <br...@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
Re: Hooks on Windows]
Posted by "D.J. Heap" <dj...@shadyvale.net>.
I believe that is exactly what happens underneath, anyway, and indeed,
the results are the same. It seems that cmd.exe is paying special
attention to scripts writing to stdout, and if a failure occurs it
writes the failure message to stderr.
DJ
Greg Thomas wrote:
> On Thu, 14 Aug 2003 18:18:01 -0600, "D.J. Heap" <dj...@shadyvale.net>
> wrote:
>
>
>>Very weird, since it doesn't seem to apply to exe's (probably, cmd.exe
>>is 'helping' by printing error messages to stderr or something and
>>normal exe's just ignore the write failure).
>
>
> Could you run the .bat/.cmd as a .exe, making them the same, by
> calling the command interpreter (which is a .exe) with the /C flag -
> i.e. something like
>
> BOOL rc = CreateProcess( "%COMSPEC% /C c:\\temp\\test.cmd" );
>
> Greg
>
> ---------------------------------------------------------------------
> 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
Re: Hooks on Windows]
Posted by Greg Thomas <Gr...@TheThomasHome.co.uk>.
On Thu, 14 Aug 2003 18:18:01 -0600, "D.J. Heap" <dj...@shadyvale.net>
wrote:
> Very weird, since it doesn't seem to apply to exe's (probably, cmd.exe
> is 'helping' by printing error messages to stderr or something and
> normal exe's just ignore the write failure).
Could you run the .bat/.cmd as a .exe, making them the same, by
calling the command interpreter (which is a .exe) with the /C flag -
i.e. something like
BOOL rc = CreateProcess( "%COMSPEC% /C c:\\temp\\test.cmd" );
Greg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org