Thread (22 messages) read the whole thread 22 messages, 2 authors, 2020-09-22
STALE2141d

[PATCH] Config option to set the transport protocol version for network fetches

From: Alex Riesen <hidden>
Date: 2020-09-17 13:25:46
Subsystem: documentation, the rest · Maintainers: Jonathan Corbet, Linus Torvalds

Affecting the transfers initiated by fetch and push,
the option allows to control network operations similar
to --ipv4 and --ipv6 options.

Suggested-by: Jeff King <redacted>
Signed-off-by: Alex Riesen <redacted>
---

Jeff King, Wed, Sep 16, 2020 22:02:03 +0200:
On Tue, Sep 15, 2020 at 03:54:28PM +0200, Alex Riesen wrote:
quoted
Jeff King, Tue, Sep 15, 2020 15:05:06 +0200:
quoted
On Tue, Sep 15, 2020 at 01:50:25PM +0200, Alex Riesen wrote:
quoted
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):
I have merged the patches.
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).
As this is still being discussed, and because I still imagine the
transport protocol family as a list of allowed protocols, this part
is not in the patch below.
  - 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.
I agree.

 Documentation/config/core.txt |  7 +++++++
 builtin/fetch.c               |  3 ++-
 builtin/push.c                |  3 ++-
 transport.c                   | 19 +++++++++++++++++++
 4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 74619a9c03..dcb7db9799 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -626,3 +626,10 @@ core.abbrev::
 	in your repository, which hopefully is enough for
 	abbreviated object names to stay unique for some time.
 	The minimum length is 4.
+
+core.ipversion::
+	Limit the network operations to the specified version of the transport
+	protocol. Can be specified as `4` to allow IPv4 only, `6` for IPv6, or
+	`all` to allow all protocols.
+	See also linkgit:git-fetch[1] options `--ipv4` and `--ipv6`.
+	The default value is `all` to allow all protocols.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 447d28ac29..41f82d61d7 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1248,7 +1248,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 b41386eccb..e16c339f3e 100644
--- a/transport.c
+++ b/transport.c
@@ -922,6 +922,23 @@ static struct transport_vtable builtin_smart_vtable = {
 	disconnect_git
 };
 
+static enum transport_family default_transport_family(void)
+{
+	static const char key[] = "core.ipversion";
+	const char *v;
+
+	if (git_config_get_string_const(key, &v))
+		return TRANSPORT_FAMILY_ALL;
+	if (!strcmp(v, "all"))
+		return TRANSPORT_FAMILY_ALL;
+	if (!strcmp(v, "ipv4"))
+		return TRANSPORT_FAMILY_IPV4;
+	if (!strcmp(v, "ipv6"))
+		return TRANSPORT_FAMILY_IPV6;
+
+	die(_("%s: unknown value '%s'"), key, v);
+}
+
 struct transport *transport_get(struct remote *remote, const char *url)
 {
 	const char *helper;
@@ -951,6 +968,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:")) {
-- 
2.28.0.22.gfab0d4627e
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help