git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Alex Riesen <alexander.riesen@cetitec.com>,
	git@vger.kernel.org, Eric Wong <normalperson@yhbt.net>
Subject: Re: [PATCH] config: option transfer.ipversion to set transport protocol version for network fetches
Date: Wed, 16 Sep 2020 20:48:28 -0400	[thread overview]
Message-ID: <20200917004828.GA2442845@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqq4knxuyfz.fsf@gitster.c.googlers.com>

On Wed, Sep 16, 2020 at 03:52:16PM -0700, Junio C Hamano wrote:

> >> Adding a command-line option for "all" is a good idea, but will probably
> >> mean needing to add the "unset" sentinel value I mentioned in the other
> >> email.
> >
> > Sorry, I do not quite follow.  I thought that assigning the
> > (misnamed --- see other mail) ALL to the "family" variable would be
> > sufficient?
> >
> >     enum transport_family {
> >             TRANSPORT_FAMILY_ALL = 0,
> >             TRANSPORT_FAMILY_IPV4,
> >             TRANSPORT_FAMILY_IPV6
> >     };
> 
> Ah, I see.  We want a way to tell "nobody has set it from the command
> line or the config" and "we were explicitly told to accept any"
> apart.
> 
> But wouldn't the usual "read config first and then override from the
> command line" handle that without "not yet set" value?  I thought we
> by default accept any.

It would, if each individual program does it in that order. But that
means every caller of the transport code needs to be updated to handle
the config. That might not be that bad (after all, they have to take
--ipv6 etc, options, and I guess they'd need a new "any" option).

My suggestion elsewhere was to have an "unset" value, and then resolve
it at the time-of-use, something like:

diff --git a/transport.c b/transport.c
index 43e24bf1e5..6414a847ae 100644
--- a/transport.c
+++ b/transport.c
@@ -248,6 +248,9 @@ static int connect_setup(struct transport *transport, int for_push)
 	if (data->conn)
 		return 0;
 
+	if (transport->family == TRANSPORT_FAMILY_UNSET)
+		transport->family = transport_family_config;
+
 	switch (transport->family) {
 	case TRANSPORT_FAMILY_ALL: break;
 	case TRANSPORT_FAMILY_IPV4: flags |= CONNECT_IPV4; break;

but I am happy either way as long as the code does the right thing.

-Peff

  reply	other threads:[~2020-09-17  0:55 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-14 12:19 sub-fetches discard --ipv4|6 option Alex Riesen
2020-09-14 19:49 ` Jeff King
2020-09-15 11:50   ` Alex Riesen
2020-09-15 11:54     ` [PATCH] Pass --ipv4 and --ipv6 options to sub-fetches when fetching multiple remotes and submodules Alex Riesen
2020-09-15 13:06       ` Jeff King
2020-09-16  4:17         ` Junio C Hamano
2020-09-16  7:27           ` Alex Riesen
2020-09-15 21:19       ` Junio C Hamano
2020-09-16  7:25         ` Alex Riesen
2020-09-15 13:05     ` sub-fetches discard --ipv4|6 option Jeff King
2020-09-15 13:54       ` [PATCH] config: option transfer.ipversion to set transport protocol version for network fetches Alex Riesen
2020-09-16 20:02         ` Jeff King
2020-09-17  8:07           ` Alex Riesen
2020-09-17 13:20           ` [PATCH] Config option to set the " Alex Riesen
2020-09-17 13:26             ` Alex Riesen
2020-09-17 13:31             ` Jeff King
2020-09-17 13:35               ` Alex Riesen
2020-09-17 14:51                 ` Jeff King
2020-09-17 15:17                   ` Alex Riesen
2020-12-22 19:55                     ` Junio C Hamano
2021-01-07 10:06                       ` Alex Riesen
2020-09-17 16:05             ` Alex Riesen
2020-09-16 20:14         ` [PATCH] config: option transfer.ipversion to set " Junio C Hamano
2020-09-16 20:18           ` Jeff King
2020-09-16 22:50             ` Junio C Hamano
2020-09-16 22:52               ` Junio C Hamano
2020-09-17  0:48                 ` Jeff King [this message]
2020-09-17  0:57                   ` Junio C Hamano
2020-09-16 21:35           ` Junio C Hamano
2020-09-17 14:02             ` Alex Riesen
2020-09-17 22:31               ` Junio C Hamano
2020-09-18  7:16                 ` Alex Riesen
2020-09-18 16:37                   ` Junio C Hamano
2020-09-21 16:39                     ` Alex Riesen
2020-09-22  5:03                       ` Jeff King
2020-09-17  8:04           ` Alex Riesen
2020-09-17  8:18           ` Alex Riesen
2020-09-15 14:06       ` sub-fetches discard --ipv4|6 option Alex Riesen
2020-09-15 15:27         ` Jeff King
2020-09-15 16:03           ` Alex Riesen
2020-09-16 16:32             ` Jeff King
2020-09-17 14:33               ` Alex Riesen
2020-09-22  5:08                 ` Jeff King
2020-09-15 20:09           ` Junio C Hamano
2020-09-15 21:23             ` Jeff King
2020-09-15 21:32               ` Junio C Hamano
2020-09-16 16:34                 ` Jeff King
2020-09-16 21:20                   ` Junio C Hamano
2020-09-14 20:06 ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20200917004828.GA2442845@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=alexander.riesen@cetitec.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=normalperson@yhbt.net \
    --subject='Re: [PATCH] config: option transfer.ipversion to set transport protocol version for network fetches' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).