git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Alex Riesen <alexander.riesen@cetitec.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	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 16:02:03 -0400	[thread overview]
Message-ID: <20200916200203.GA37225@coredump.intra.peff.net> (raw)
In-Reply-To: <20200915135428.GA28038@pflmari>

On Tue, Sep 15, 2020 at 03:54:28PM +0200, Alex Riesen wrote:

> Jeff King, Tue, Sep 15, 2020 15:05:06 +0200:
> > On Tue, Sep 15, 2020 at 01:50:25PM +0200, Alex Riesen wrote:
> > > Sure! Thinking about it, I actually would have preferred to have both: a
> > > config option and a command-line option. So that I can set --ipv4 in, say,
> > > ~/.config/git/config file, but still have the option to try --ipv6 from time
> > > to time to check if the network setup magically fixed itself.
> > > 
> > > What would the preferred name for that config option be? fetch.ipv?
> > 
> > It looks like we've got similar options for clone/pull (which are really
> > fetch under the hood of course) and push. We have the "transfer.*"
> > namespace which applies to both already. So maybe "transfer.ipversion"
> > or something?
> 
> Something like this?

That's the right direction, but I think we'd want to make sure it
impacted all of the spots that allow switching. "clone" on the fetching
side, but probably also "push".

Each of those commands could learn the same config, but it might be
easier to just enforce it in the transport layer. Something like this
maybe (compiled but not tested):

diff --git a/builtin/fetch.c b/builtin/fetch.c
index a6d3268661..2f7a734eb2 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1267,7 +1267,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
 
 	transport = transport_get(remote, NULL);
 	transport_set_verbosity(transport, verbosity, progress);
-	transport->family = family;
+	if (family)
+		transport->family = family;
 	if (upload_pack)
 		set_option(transport, TRANS_OPT_UPLOADPACK, upload_pack);
 	if (keep)
diff --git a/builtin/push.c b/builtin/push.c
index bc94078e72..f7a40b65cd 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -343,7 +343,8 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
 	char *anon_url = transport_anonymize_url(transport->url);
 
 	transport_set_verbosity(transport, verbosity, progress);
-	transport->family = family;
+	if (family)
+		transport->family = family;
 
 	if (receivepack)
 		transport_set_option(transport,
diff --git a/transport.c b/transport.c
index 43e24bf1e5..92f81b414d 100644
--- a/transport.c
+++ b/transport.c
@@ -919,6 +919,20 @@ static struct transport_vtable builtin_smart_vtable = {
 	disconnect_git
 };
 
+enum transport_family default_transport_family(void)
+{
+	const char *v;
+
+	if (git_config_get_string_tmp("core.ipversion", &v))
+		return TRANSPORT_FAMILY_ALL;
+	else if (!strcmp(v, "ipv4"))
+		return TRANSPORT_FAMILY_IPV4;
+	else if (!strcmp(v, "ipv6"))
+		return TRANSPORT_FAMILY_IPV6;
+
+	die(_("invalid core.ipversion: %s"), v);
+}
+
 struct transport *transport_get(struct remote *remote, const char *url)
 {
 	const char *helper;
@@ -948,6 +962,8 @@ struct transport *transport_get(struct remote *remote, const char *url)
 			helper = xstrndup(url, p - url);
 	}
 
+	ret->family = default_transport_family();
+
 	if (helper) {
 		transport_helper_init(ret, helper);
 	} else if (starts_with(url, "rsync:")) {

A few notes:

  - it cheats a little by noting that command-line options can't get it
    to "ALL". It might be a bit cleaner if we have an explicit
    TRANSPORT_FAMILY_UNSET, and then resolve the default at look-up
    time.

  - it probably needs more "if (family)" spots in each command, which is
    unfortunate (which again might go away if "UNSET" is 0).

  - I waffled on the name. transfer.* is where we put things that apply
    to both push/fetch, but it's usually more related to the git layer.
    This is more of a core networking decision, and if we added other
    network programs, I'd expect them to respect it. So maybe "core." is
    a better namespace.

-Peff

  reply	other threads:[~2020-09-16 20:02 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 [this message]
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
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=20200916200203.GA37225@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).