git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: Leslie Cheng via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Leslie Cheng <leslie@lc.fyi>,
	Leslie Cheng <leslie.cheng5@gmail.com>
Subject: Re: [PATCH] Add unix domain socket support to HTTP transport.
Date: Wed, 21 Feb 2024 22:09:45 +0000	[thread overview]
Message-ID: <20240221220945.M686016@dcvr> (raw)
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>

> @@ -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.

  reply	other threads:[~2024-02-21 22:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-21  9:14 [PATCH] Add unix domain socket support to HTTP transport Leslie Cheng via GitGitGadget
2024-02-21 22:09 ` Eric Wong [this message]
2024-02-22  3:04   ` Leslie Cheng
2024-02-23  1:58 ` [PATCH v2] " Leslie Cheng via GitGitGadget
2024-02-23  8:37   ` Junio C Hamano
2024-02-23 15:43     ` Junio C Hamano
2024-02-23 22:24       ` Leslie Cheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240221220945.M686016@dcvr \
    --to=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=leslie.cheng5@gmail.com \
    --cc=leslie@lc.fyi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).