[PATCH (MINGW) Resend] Windows: Make OpenSSH properly detect tty detachment.

[PATCH (MINGW) Resend] Windows: Make OpenSSH properly detect tty detachment.

[Alexander Gavrilov]

Apparently, CREATE_NO_WINDOW makes the OS tell the process
that it has a console, but without actually creating the
window. As a result, when git is started from GUI, ssh
tries to ask its questions on the invisible console.

This patch uses DETACHED_PROCESS instead, which clearly
means that the process should be left without a console.
The downside is that if the process manually calls
AllocConsole, the window will appear. A similar thing
might occur if it calls another console executable.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
Acked-by: Johannes Sixt <johannes.sixt@telecom.at>
---

	This patch appears to have been overlooked, so I	resend
	it just in case. It fixes a long standing problem in msysgit.

	-- Alexander

 compat/mingw.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 1e29b88..b6fcf69 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -586,12 +586,16 @@ static pid_t mingw_spawnve(const char *cmd, const char **argv, char **env,
 		 * would normally create a console window. But
 		 * since we'll be redirecting std streams, we do
 		 * not need the console.
+		 * It is necessary to use DETACHED_PROCESS
+		 * instead of CREATE_NO_WINDOW to make ssh
+		 * recognize that it has no console.
 		 */
-		flags = CREATE_NO_WINDOW;
+		flags = DETACHED_PROCESS;
 	} else {
 		/* There is already a console. If we specified
-		 * CREATE_NO_WINDOW here, too, Windows would
+		 * DETACHED_PROCESS here, too, Windows would
 		 * disassociate the child from the console.
+		 * The same is true for CREATE_NO_WINDOW.
 		 * Go figure!
 		 */
 		flags = 0;
-- 
1.6.0.2.1256.ga12f0

Re: [PATCH (MINGW) Resend] Windows: Make OpenSSH properly detect tty detachment.

[Johannes Sixt]

Alexander Gavrilov schrieb:
> Apparently, CREATE_NO_WINDOW makes the OS tell the process
> that it has a console, but without actually creating the
> window. As a result, when git is started from GUI, ssh
> tries to ask its questions on the invisible console.
> 
> This patch uses DETACHED_PROCESS instead, which clearly
> means that the process should be left without a console.
> The downside is that if the process manually calls
> AllocConsole, the window will appear. A similar thing
> might occur if it calls another console executable.

The latest "What's cooking in git.git" report still doesn't mention this
patch. The reason might be that you mention some scary downsides in this
paragraph. I think you should either not mention the downsides, or
describe further why they do not make a difference in our case:

- We never call AllocConsole(). If some third-party program calls it, then
it gets what it requested. (But, yes, the behavior would indeed be
different from what it used to be.)

- There are two possible circumstances when another console executable is
called:

  * A program that has no console - e.g. git-gui - runs a git process.
Then we are entering the "then" branch, and the new process is started
with DETACH_PROCESS. No new console window appears.

  * A program that already has a console runs a git process. Then we are
entering the "else" branch, and we do not change the process flags nor are
we requesting a new console. No new console window appears.

I'm using this patch since weeks without noticing any downsides.

-- Hannes

> 
> Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
> Acked-by: Johannes Sixt <johannes.sixt@telecom.at>
> ---
> 
> 	This patch appears to have been overlooked, so I	resend
> 	it just in case. It fixes a long standing problem in msysgit.
> 
> 	-- Alexander
> 
>  compat/mingw.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 1e29b88..b6fcf69 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -586,12 +586,16 @@ static pid_t mingw_spawnve(const char *cmd, const char **argv, char **env,
>  		 * would normally create a console window. But
>  		 * since we'll be redirecting std streams, we do
>  		 * not need the console.
> +		 * It is necessary to use DETACHED_PROCESS
> +		 * instead of CREATE_NO_WINDOW to make ssh
> +		 * recognize that it has no console.
>  		 */
> -		flags = CREATE_NO_WINDOW;
> +		flags = DETACHED_PROCESS;
>  	} else {
>  		/* There is already a console. If we specified
> -		 * CREATE_NO_WINDOW here, too, Windows would
> +		 * DETACHED_PROCESS here, too, Windows would
>  		 * disassociate the child from the console.
> +		 * The same is true for CREATE_NO_WINDOW.
>  		 * Go figure!
>  		 */
>  		flags = 0;

Re: [PATCH (MINGW) Resend] Windows: Make OpenSSH properly detect tty detachment.

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> The latest "What's cooking in git.git" report still doesn't mention this
> patch. The reason might be ...

Sorry, my mistake.

>> Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
>> Acked-by: Johannes Sixt <johannes.sixt@telecom.at>
>> ---
>> 
>> 	This patch appears to have been overlooked,

Indeed.  It was lost in the noise because it was marked MinGW and I failed
to see your Ack.

Do people build and test MinGW port out of my 'next'?  If so, I'll queue
this on its own topic branch and have it go through the usual "cook then
graduate" cycle.  Otherwise, I'll apply this directly on top of 'master'.

Thanks.

Re: [PATCH (MINGW) Resend] Windows: Make OpenSSH properly detect tty detachment.

[Johannes Sixt]

Junio C Hamano schrieb:
> Do people build and test MinGW port out of my 'next'?  If so, I'll queue
> this on its own topic branch and have it go through the usual "cook then
> graduate" cycle.  Otherwise, I'll apply this directly on top of 'master'.

My own build is on top of your 'master', and I've cooked this patch to
death there already, so I don't think it needs another cycle. ;)

-- Hannes