git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Torsten Bögershausen <tboegi@web.de>
Date
Jan 30, 2016, 23:34 UTC
Message-ID
<56AD4887.3070207@web.de>
In-Reply-To
<20160130131353.GA20429@dcvr.yhbt.net>

On 2016-01-30 14.13, Eric Wong wrote: Nicely done, some minor questions inline.

Show 6 quoted lines
> 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 ?

Show 63 quoted lines
> 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 ?

Show 9 quoted lines
>  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/

Show 9 quoted lines
>  	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]
Previous: Eric WongNext: Eric Wong
Message 6 of 18 in “pass transport verbosity down to git_connect”
  1. pass transport verbosity down to git_connectEric Wong, Jan 28, 2016
  2. Junio C HamanoJan 28, 2016
  3. pass transport verbosity down to git_connectEric Wong, Jan 30, 2016
  4. 2/1 support -4 and -6 switches for remote operationsEric Wong, Jan 30, 2016
  5. Eric WongJan 30, 2016
  6. Torsten BögershausenJan 30, 2016
  7. Eric WongJan 31, 2016
  8. Jeff KingJan 31, 2016
  9. 2/1 support -4 and -6 switches for remote operationsEric Wong, Feb 3, 2016
  10. Eric WongFeb 12, 2016
  11. Torsten BögershausenFeb 12, 2016
  12. Torsten BögershausenJan 31, 2016
  13. Jeff KingJan 28, 2016
  14. Eric WongJan 29, 2016
  15. Junio C HamanoJan 29, 2016
  16. Jeff KingJan 29, 2016
  17. Junio C HamanoJan 29, 2016
  18. Jeff KingJan 29, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.