Re: [PATCH] Add unix domain socket support to HTTP transport.

From: Eric Wong <e@80x24.org>
Date: Feb 21, 2024, 22:09 UTC
Message-ID: <20240221220945.M686016@dcvr>
In-Reply-To: <pull.1681.git.git.1708506863243.gitgitgadget@gmail.com>

Leslie Cheng via GitGitGadget <gitgitgadget@gmail.com> wrote:

> Subject: Re: [PATCH] Add unix domain socket support to HTTP transport.

No need for trailing `.' in commit message titles

<snip>

Show 17 quoted lines

> @@ -455,6 +458,20 @@ static int http_options(const char *var, const char *value,
>  		return 0;
>  	}
>  
> +	if (!strcmp("http.unixsocket", var)) {
> +#ifdef GIT_CURL_HAVE_CURLOPT_UNIX_SOCKET_PATH
> +#ifndef NO_UNIX_SOCKETS
> +		return git_config_string(&curl_unix_socket_path, var, value);
> +#else
> +		warning(_("Unix socket support unavailable in this build of Git"));
> +		return 0;
> +#endif
> +#else
> +		warning(_("Unix socket support is not supported with cURL < 7.40.0"));
> +		return 0;
> +#endif
> +	}

Personally, I'd hoist the #ifdef part into a standalone function since I find mixing CPP and C conditionals confusing.

disclaimer: I'm an easily confused person and don't usually program in C, though.

<snip>

> --- /dev/null
> +++ b/t/t5565-http-unix-domain-socket.sh

<snip>

> +    nc -klU "$socket_path" <tcp_to_uds >uds_to_tcp &
> +    UDS_PID=$!
> +
> +    nc "$host" "$port" >tcp_to_uds <uds_to_tcp &

`nc' isn't widely installed, its supported flags vary between implementations, and our test suite doesn't currently use it. I suggest either using a small bit of Perl or writing a t/helper program to do its job.

Finally, hard tabs should be used for indentation throughout.

I'll wait on others to comment since I haven't looked at git hacking in a while.

Anyways, I think this feature could be useful for me, too :> Thanks.