Re: [PATCH] config: option transfer.ipversion to set transport protocol version for network fetches

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 16, 2020, 20:14 UTC
Message-ID: <xmqqtuvxwkbz.fsf@gitster.c.googlers.com>
In-Reply-To: <20200915135428.GA28038@pflmari>

Alex Riesen <alexander.riesen@cetitec.com> writes:

Show 5 quoted lines

> Affecting the transfers caused by git-fetch, the
> option allows to control network operations similar
> to --ipv4 and --ipv6 options.
> ...
> Something like this?

Good start.

If I configure it to "4", I do not see a way to override it and say "I don't care which one is used". With the introduction of this configuration variable, we'd need a bit more support on the command line side.

How about introducing a new command line option

	--transfer-protocol-family=("any"|<protocol>)

where <protocol> is either "ipv4" or "ipv6" [*1*], and make existing "--ipv4" a synonym for "--transfer-protocol-family=ipv4" and "--ipv6" for "--transfer-protocol-family=ipv6"

With such an extended command line override, we can override configured

	[transfer]
		ipversion = 6

with "--transfer-protocol-family=any" from the command line.

Also, we should follow the usual "the last one wins" for a configuration variable like this, which is *not* a multi-valued variable. So the config parsing would look more like this:

	if (!strcmp(k, "transfer.ipversion")) {
		if (!v)
			return config_error_nonbool("transfer.ipversion");
		if (!strcmp(v, "any"))
			family = 0;
		else if (!strcmp(v, "4") || !strcmp(v, "ipv4"))
			family = TRANSPORT_FAMILY_IPV4;
		else if (!strcmp(v, "6") || !strcmp(v, "ipv6"))
			family = TRANSPORT_FAMILY_IPV6;
		else
			return error("transfer.ipversion: unknown value '%s'", v);
	}

Would we regret to choose 'ipversion' as the variable name, by the way? On the command line side, --transfer-protocol-family=ipv4 makes it clear that we leave room to support protocols outside the Internet protocol family, and existing --ipv4 is grandfathered in by making it a synonym to --transfer-protocol-family=ipv4. Calling the variable "transfer.ipversion" and still allowing future protocols outside the Internet protocol family is rather awkward.

Calling "transfer.protocolFamily" would not have such a problem, though.

[Footnote]

*1* But leave a room to extend it in the future to a comma-separated
    list of them to allow something like "ipv6,ipv7,ipv8" (i.e. "not
    just 'any'---we want to say that 'ipv4' is not welcomed").

Show 40 quoted lines

>  Documentation/config/transfer.txt |  7 +++++++
>  builtin/fetch.c                   | 11 +++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/Documentation/config/transfer.txt b/Documentation/config/transfer.txt
> index f5b6245270..cc0e97fbb1 100644
> --- a/Documentation/config/transfer.txt
> +++ b/Documentation/config/transfer.txt
> @@ -69,3 +69,10 @@ transfer.unpackLimit::
>  	When `fetch.unpackLimit` or `receive.unpackLimit` are
>  	not set, the value of this variable is used instead.
>  	The default value is 100.
> +
> +transfer.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..da01c8f7b3 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -118,6 +118,17 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
>  		return 0;
>  	}
>  
> +	if (!strcmp(k, "transfer.ipversion")) {
> +		if (!strcmp(v, "all"))
> +			;
> +		else if (!strcmp(v, "4"))
> +			family = TRANSPORT_FAMILY_IPV4;
> +		else if (!strcmp(v, "6"))
> +			family = TRANSPORT_FAMILY_IPV6;
> +		else
> +			die(_("transfer.ipversion can be only 4, 6, or any"));
> +		return 0;
> +	}
>  	return git_default_config(k, v, cb);
>  }