From: Jeff King <peff@peff.net>
Date: Tue, 01 Apr 2025 09:16:06 GMT
Subject: Re: [PATCH v2 4/4] http.c: allow custom TCP keepalive behavior via config
Message-ID: <20250401091606.GC21089@coredump.intra.peff.net>
In-Reply-To: <3fe62181e563d011d4a6374ed7894cc4ea807316.1742423021.git.me@ttaylorr.com>

On Wed, Mar 19, 2025 at 06:23:56PM -0400, Taylor Blau wrote:

> For those users and others who wish to more finely tune the OS's
> keepalive behavior, expose configuration and environment variables which
> allow setting curl's KEEPIDLE, KEEPINTVL, and KEEPCNT options.

OK. I have never wanted those myself, but I have no problem with other
people using them. ;)

The implementation mostly looks as I'd expect, but...

> +http.keepAliveIdle::
> +	Specifies how long in seconds to wait on an idle connection
> +	before sending TCP keepalive probes (if supported by the OS). If
> +	unset, curl's default value is used. Can be overridden by the
> +	`GIT_HTTP_KEEPALIVE_IDLE` environment variable.

...while this environment variable (and its siblings) makes sense to
me....

> @@ -1382,6 +1409,10 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
>  			ssl_cert_password_required = 1;
>  	}
>  
> +	set_long_from_env(&curl_tcp_keepidle, "GIT_TCP_KEEPIDLE");
> +	set_long_from_env(&curl_tcp_keepintvl, "GIT_TCP_KEEPINTVL");
> +	set_long_from_env(&curl_tcp_keepcnt, "GIT_TCP_KEEPCNT");

...we seem to use different names in the code.

I think the ones mentioning HTTP make the most sense, to match the
config (and since this only affects curl).

-Peff