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@xxxxxxxx>
> ---
> 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]
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html