Re: t0301-credential-cache test failure on cygwin

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 7, 2022, 06:15 UTC
Message-ID: <xmqqtu7t30uv.fsf@gitster.g>
In-Reply-To: <9dc3e85f-a532-6cff-de11-1dfb2e4bc6b6@ramsayjones.plus.com>

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

Show 14 quoted lines

> However, I had some time to kill tonight, so I decided to take a _quick_ look
> to see if there was something that could be done ... (famous last words).
> ...
>   diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c
>   index 78c02ad531..84fd513c62 100644
>   --- a/builtin/credential-cache.c
>   +++ b/builtin/credential-cache.c
>   @@ -27,7 +27,7 @@ static int connection_fatally_broken(int error)
>    
>    static int connection_closed(int error)
>    {
>   -	return (error == ECONNRESET);
>   +	return (error == ECONNRESET) || (error == ECONNABORTED);
>    }

This feels like papering over the problem.

Show 7 quoted lines

> Having noticed that the 'timeout' test was not failing, I decided to try
> making the 'action=exit' code-path behave more like the timeout code, as
> far as exiting the server is concerned. Indeed, you might ask why the
> timeout code doesn't just 'exit(0)' as well ...
>
> Anyway, the following patch does that, and it also provides a 'fix' for this
> issue!

If this codepath was written like this (i.e. [PATCH 1C]) from the beginning, I would have found it very sensible (i.e. instead of caling exit() in the middle of the infinite client serving loop, exiting the loop cleanly is easier to follow and maintain), even if we didn't know the issue on Cygwin you investigated.