Re: [PATCH 2/1] support -4 and -6 switches for remote operations

["Torsten Bögershausen" <tboegi@web.de>]

On 2016-01-30 14.13, Eric Wong wrote:
Nicely done, some minor questions inline.
> Sometimes, it is necessary to force IPv4-only or IPv6-only
> operation on networks where name lookups may return a
> non-routable address and stall remote operations.
>
> The ssh(1) command has an equivalent switches which we may
> pass when we run them.
Should we mention that putty and tortoiseplink don't have these options ?
At least in the commit message ?
> rsync support is untouched for now since it is deprecated
> and scheduled to be removed.
>
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ---
>  Documentation/fetch-options.txt |  8 ++++++++
>  Documentation/git-push.txt      |  7 +++++++
>  builtin/clone.c                 |  4 ++++
>  builtin/fetch.c                 |  4 ++++
>  builtin/push.c                  |  4 ++++
>  connect.c                       |  8 ++++++++
>  connect.h                       |  2 ++
>  http.c                          |  9 +++++++++
>  http.h                          |  1 +
>  remote-curl.c                   | 19 +++++++++++++++++++
>  transport-helper.c              |  7 +++++++
>  transport.c                     | 18 ++++++++++++++++++
>  transport.h                     | 11 +++++++++++
>  13 files changed, 102 insertions(+)
>
> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
> index 952dfdf..6ec7dde 100644
> --- a/Documentation/fetch-options.txt
> +++ b/Documentation/fetch-options.txt
> @@ -158,3 +158,11 @@ endif::git-pull[]
>  	by default when it is attached to a terminal, unless -q
>  	is specified. This flag forces progress status even if the
>  	standard error stream is not directed to a terminal.
> +
> +-4::
> +--ipv4::
> +	Resolve IPv4 addresses only, ignoring IPv6 addresses.
> +
> +-6::
> +--ipv6::
> +	Resolve IPv6 addresses only, ignoring IPv4 addresses.
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 32482ce..559c166 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -277,6 +277,13 @@ origin +master` to force a push to the `master` branch). See the
>  	default is --verify, giving the hook a chance to prevent the
>  	push.  With --no-verify, the hook is bypassed completely.
>  
> +-4::
> +--ipv4::
> +	Resolve IPv4 addresses only, ignoring IPv6 addresses.
> +
> +-6::
> +--ipv6::
> +	Resolve IPv6 addresses only, ignoring IPv4 addresses.
>  
>  include::urls-remotes.txt[]
>  
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 81e238f..3feae64 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -47,6 +47,7 @@ static const char *real_git_dir;
>  static char *option_upload_pack = "git-upload-pack";
>  static int option_verbosity;
>  static int option_progress = -1;
> +static int ipv4, ipv6;
Do we need 2 variables here ?

Or would
int preferred_ip_version
be better ?
>  static struct string_list option_config;
>  static struct string_list option_reference;
>  static int option_dissociate;
> @@ -92,6 +93,8 @@ static struct option builtin_clone_options[] = {
>  		   N_("separate git dir from working tree")),
>  	OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
>  			N_("set config inside the new repository")),
> +	OPT_BOOL('4', "ipv4", &ipv4, N_("resolve IPv4 addresses only")),
> +	OPT_BOOL('6', "ipv6", &ipv6, N_("resolve IPv6 addresses only")),
Technically OK to mention resolve, but does it give any information to the user ?
s/resolve IPv4 addresses only/use IPv4 addresses only/

>  	OPT_END()
>  };
>  
> @@ -970,6 +973,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  	remote = remote_get(option_origin);
>  	transport = transport_get(remote, remote->url[0]);
>  	transport_set_verbosity(transport, option_verbosity, option_progress);
> +	transport_set_family(transport, ipv4, ipv6);
>  
Does it make sense to name the variable into
ipv4only (to make clear that it does not mean ipv4_allowed ?)
(and similar in the rest of the code)

[snip]