All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pass transport verbosity down to git_connect
@ 2016-01-28 22:51 Eric Wong
  2016-01-28 23:45 ` Junio C Hamano
  2016-01-28 23:53 ` [PATCH] pass transport verbosity down to git_connect Jeff King
  0 siblings, 2 replies; 18+ messages in thread
From: Eric Wong @ 2016-01-28 22:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

While working in connect.c to perform non-blocking connections,
I noticed calling "git fetch -v" was not causing the progress
messages inside git_tcp_connect_sock to be emitted as I
expected.

Looking at history, it seems connect_setup has never been called
with the verbose parameter.  Since transport already has a
"verbose" field, use that field instead of another parameter
in connect_setup.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
  Related, but independent of my other change to connect.c:
  http://mid.gmane.org/20160128115720.GA1827@dcvr.yhbt.net
  ("attempt connects in parallel for IPv6-capable builds")

 transport.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/transport.c b/transport.c
index 67f3666..9ae7184 100644
--- a/transport.c
+++ b/transport.c
@@ -481,9 +481,10 @@ static int set_git_option(struct git_transport_options *opts,
 	return 1;
 }
 
-static int connect_setup(struct transport *transport, int for_push, int verbose)
+static int connect_setup(struct transport *transport, int for_push)
 {
 	struct git_transport_data *data = transport->data;
+	int flags = transport->verbose > 0 ? CONNECT_VERBOSE : 0;
 
 	if (data->conn)
 		return 0;
@@ -491,7 +492,7 @@ static int connect_setup(struct transport *transport, int for_push, int verbose)
 	data->conn = git_connect(data->fd, transport->url,
 				 for_push ? data->options.receivepack :
 				 data->options.uploadpack,
-				 verbose ? CONNECT_VERBOSE : 0);
+				 flags);
 
 	return 0;
 }
@@ -501,7 +502,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
 	struct git_transport_data *data = transport->data;
 	struct ref *refs;
 
-	connect_setup(transport, for_push, 0);
+	connect_setup(transport, for_push);
 	get_remote_heads(data->fd[0], NULL, 0, &refs,
 			 for_push ? REF_NORMAL : 0,
 			 &data->extra_have,
@@ -536,7 +537,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 	args.update_shallow = data->options.update_shallow;
 
 	if (!data->got_remote_heads) {
-		connect_setup(transport, 0, 0);
+		connect_setup(transport, 0);
 		get_remote_heads(data->fd[0], NULL, 0, &refs_tmp, 0,
 				 NULL, &data->shallow);
 		data->got_remote_heads = 1;
@@ -812,7 +813,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 
 	if (!data->got_remote_heads) {
 		struct ref *tmp_refs;
-		connect_setup(transport, 1, 0);
+		connect_setup(transport, 1);
 
 		get_remote_heads(data->fd[0], NULL, 0, &tmp_refs, REF_NORMAL,
 				 NULL, &data->shallow);
-- 
EW

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] pass transport verbosity down to git_connect
  2016-01-28 22:51 [PATCH] pass transport verbosity down to git_connect Eric Wong
@ 2016-01-28 23:45 ` Junio C Hamano
  2016-01-30  8:50   ` [PATCH v2] " Eric Wong
  2016-01-28 23:53 ` [PATCH] pass transport verbosity down to git_connect Jeff King
  1 sibling, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2016-01-28 23:45 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

Eric Wong <normalperson@yhbt.net> writes:

> While working in connect.c to perform non-blocking connections,
> I noticed calling "git fetch -v" was not causing the progress
> messages inside git_tcp_connect_sock to be emitted as I
> expected.

Nice.  Can we demonstrate and protect this fix with simple tests?

Thanks.  Will queue.

>  transport.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/transport.c b/transport.c
> index 67f3666..9ae7184 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -481,9 +481,10 @@ static int set_git_option(struct git_transport_options *opts,
>  	return 1;
>  }
>  
> -static int connect_setup(struct transport *transport, int for_push, int verbose)
> +static int connect_setup(struct transport *transport, int for_push)
>  {
>  	struct git_transport_data *data = transport->data;
> +	int flags = transport->verbose > 0 ? CONNECT_VERBOSE : 0;
>  
>  	if (data->conn)
>  		return 0;
> @@ -491,7 +492,7 @@ static int connect_setup(struct transport *transport, int for_push, int verbose)
>  	data->conn = git_connect(data->fd, transport->url,
>  				 for_push ? data->options.receivepack :
>  				 data->options.uploadpack,
> -				 verbose ? CONNECT_VERBOSE : 0);
> +				 flags);
>  
>  	return 0;
>  }
> @@ -501,7 +502,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
>  	struct git_transport_data *data = transport->data;
>  	struct ref *refs;
>  
> -	connect_setup(transport, for_push, 0);
> +	connect_setup(transport, for_push);
>  	get_remote_heads(data->fd[0], NULL, 0, &refs,
>  			 for_push ? REF_NORMAL : 0,
>  			 &data->extra_have,
> @@ -536,7 +537,7 @@ static int fetch_refs_via_pack(struct transport *transport,
>  	args.update_shallow = data->options.update_shallow;
>  
>  	if (!data->got_remote_heads) {
> -		connect_setup(transport, 0, 0);
> +		connect_setup(transport, 0);
>  		get_remote_heads(data->fd[0], NULL, 0, &refs_tmp, 0,
>  				 NULL, &data->shallow);
>  		data->got_remote_heads = 1;
> @@ -812,7 +813,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
>  
>  	if (!data->got_remote_heads) {
>  		struct ref *tmp_refs;
> -		connect_setup(transport, 1, 0);
> +		connect_setup(transport, 1);
>  
>  		get_remote_heads(data->fd[0], NULL, 0, &tmp_refs, REF_NORMAL,
>  				 NULL, &data->shallow);

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] pass transport verbosity down to git_connect
  2016-01-28 22:51 [PATCH] pass transport verbosity down to git_connect Eric Wong
  2016-01-28 23:45 ` Junio C Hamano
@ 2016-01-28 23:53 ` Jeff King
  2016-01-29  0:38   ` Eric Wong
  1 sibling, 1 reply; 18+ messages in thread
From: Jeff King @ 2016-01-28 23:53 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git

On Thu, Jan 28, 2016 at 10:51:23PM +0000, Eric Wong wrote:

> While working in connect.c to perform non-blocking connections,
> I noticed calling "git fetch -v" was not causing the progress
> messages inside git_tcp_connect_sock to be emitted as I
> expected.
> 
> Looking at history, it seems connect_setup has never been called
> with the verbose parameter.  Since transport already has a
> "verbose" field, use that field instead of another parameter
> in connect_setup.

That makes sense, but...

> -static int connect_setup(struct transport *transport, int for_push, int verbose)
> +static int connect_setup(struct transport *transport, int for_push)
>  {
>  	struct git_transport_data *data = transport->data;
> +	int flags = transport->verbose > 0 ? CONNECT_VERBOSE : 0;

Do we want to trigger this only for "transport->verbose > 1"?

Right now, "git fetch -v" gives us a verbose status table (i.e.,
includes up-to-date refs), but no more debugging than that. Should we
reserve more debug-ish information like for "git fetch -vv"?

-Peff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] pass transport verbosity down to git_connect
  2016-01-28 23:53 ` [PATCH] pass transport verbosity down to git_connect Jeff King
@ 2016-01-29  0:38   ` Eric Wong
  2016-01-29  3:19     ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Wong @ 2016-01-29  0:38 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

Jeff King <peff@peff.net> wrote:
> On Thu, Jan 28, 2016 at 10:51:23PM +0000, Eric Wong wrote:
> > -static int connect_setup(struct transport *transport, int for_push, int verbose)
> > +static int connect_setup(struct transport *transport, int for_push)
> >  {
> >  	struct git_transport_data *data = transport->data;
> > +	int flags = transport->verbose > 0 ? CONNECT_VERBOSE : 0;
> 
> Do we want to trigger this only for "transport->verbose > 1"?
> 
> Right now, "git fetch -v" gives us a verbose status table (i.e.,
> includes up-to-date refs), but no more debugging than that. Should we
> reserve more debug-ish information like for "git fetch -vv"?

I'm not sure, I've never used "-v" at all in the past with fetch.

On one hand, I suspect someone who looks up "-v" and uses it is likely
wondering: "why is it so slow?"  At least, that's what I did before
resorting to strace :)

On the other hand, I'm not sure if there's anything parsing the stderr
out of "git fetch -v" right now.  In that case, perhaps only changing
"-vv" (and documenting it) is a better idea.  I've always been of the
opinion stderr is for humans and test suites, only; and not considered
an interface somebody should be parsing.

For reference, "curl -v" includes connection info which I rely on
all the time.

Junio:

  I'm leaning towards putting the test into t/t5570-git-daemon.sh
  to avoid potential port conflicts if that's OK with you.
  It doesn't seem like we have a lot of fetch tests on the git://
  at all.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] pass transport verbosity down to git_connect
  2016-01-29  0:38   ` Eric Wong
@ 2016-01-29  3:19     ` Junio C Hamano
  2016-01-29  3:47       ` Jeff King
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2016-01-29  3:19 UTC (permalink / raw)
  To: Eric Wong; +Cc: Jeff King, git

Eric Wong <normalperson@yhbt.net> writes:

> On the other hand, I'm not sure if there's anything parsing the stderr
> out of "git fetch -v" right now.

It would also affect end-user experience, depending on what new
pieces of lines are emitted to the terminal.  From "git fetch -v", I
expect to see the transfer progress and the final listing of fetched
and updated refs, and as long as the line "remote: Compressing
objects" and the lines that follow do not get garbled, I would
imagine it would be fine.

  ...
  remote: Compressing objects: 100% (1195/1195), done.
  remote: Total 1869 (delta 1551), reused 841 (delta 672)
  Receiving objects: 100% (1869/1869), 462.80 KiB | 0 bytes/s, done.
  Resolving deltas: 100% (1551/1551), completed with 335 local objects.
  From $over_there
   * branch            pu         -> FETCH_HEAD

>  In that case, perhaps only changing
> "-vv" (and documenting it) is a better idea.

I just reviewed the output that are protected by CONNECT_VERBOSE;
they look somewhere between pure debugging aid (like the protocol
dump that are shown by "fetch -vv") and progress display, and at
least to me they are much closer to the latter than the former, in
the sense that they are not _so_ annoying as the protocol dump that
are clearly not meant for the end users, and that they say "I am
looking up this host's address", "Now connecting to this host:port",
etc.

So, I personally find this addtional output not _too_ bad if we give
it with "fetch -v" (not limiting to "fetch -vv").

I dunno.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] pass transport verbosity down to git_connect
  2016-01-29  3:19     ` Junio C Hamano
@ 2016-01-29  3:47       ` Jeff King
  2016-01-29 17:34         ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2016-01-29  3:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Wong, git

On Thu, Jan 28, 2016 at 07:19:30PM -0800, Junio C Hamano wrote:

> I just reviewed the output that are protected by CONNECT_VERBOSE;
> they look somewhere between pure debugging aid (like the protocol
> dump that are shown by "fetch -vv") and progress display, and at
> least to me they are much closer to the latter than the former, in
> the sense that they are not _so_ annoying as the protocol dump that
> are clearly not meant for the end users, and that they say "I am
> looking up this host's address", "Now connecting to this host:port",
> etc.
> 
> So, I personally find this addtional output not _too_ bad if we give
> it with "fetch -v" (not limiting to "fetch -vv").

Yeah, I do not feel that strongly, and am OK if it is attached to a
single "-v". I don't think we make any promises about scraping stderr,
so it is really just about end-user experience.  It is mostly just my
gut feeling on what I'd expect based on other parts of git (especially
"fetch -vv" in other circumstances).

-Peff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] pass transport verbosity down to git_connect
  2016-01-29  3:47       ` Jeff King
@ 2016-01-29 17:34         ` Junio C Hamano
  2016-01-29 17:41           ` Jeff King
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2016-01-29 17:34 UTC (permalink / raw)
  To: Jeff King; +Cc: Eric Wong, git

Jeff King <peff@peff.net> writes:

> On Thu, Jan 28, 2016 at 07:19:30PM -0800, Junio C Hamano wrote:
>
>> I just reviewed the output that are protected by CONNECT_VERBOSE;
>> they look somewhere between pure debugging aid (like the protocol
>> dump that are shown by "fetch -vv") and progress display, and at
>> least to me they are much closer to the latter than the former, in
>> the sense that they are not _so_ annoying as the protocol dump that
>> are clearly not meant for the end users, and that they say "I am
>> looking up this host's address", "Now connecting to this host:port",
>> etc.
>> 
>> So, I personally find this addtional output not _too_ bad if we give
>> it with "fetch -v" (not limiting to "fetch -vv").
>
> Yeah, I do not feel that strongly, and am OK if it is attached to a
> single "-v". I don't think we make any promises about scraping stderr,
> so it is really just about end-user experience.  It is mostly just my
> gut feeling on what I'd expect based on other parts of git (especially
> "fetch -vv" in other circumstances).

Yeah, after re-reading the messages in this thread, I realize that I
missed the fact that you do consider these CONNECT_VERBOSE messages
as debugging aid and from that point of view "fetch -v" that shows
these messagse in addition to what you get from "fetch" may be a bad
idea.

But after inspecting what CONNECT_VERBOSE would add to the output, I
am inclined to say that, especially if some of these steps can
exhibit multi-second stalls, they fall more into the "progress
indicator" (aka "do not worry, we are not stuck, be patient")
category than "debugging aid" category.  The former includes
"remote: Counting objects..." that is shown by default (you need to
give --quiet to squelch).  So I think it is OK to show the
CONNECT_VERBOSE messages with "fetch -v"; if somebody feels strongly
that these messages should be shown unless --quiet is given, I might
even be persuaded to agree, but I am not that somebody, so...

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] pass transport verbosity down to git_connect
  2016-01-29 17:34         ` Junio C Hamano
@ 2016-01-29 17:41           ` Jeff King
  0 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2016-01-29 17:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Wong, git

On Fri, Jan 29, 2016 at 09:34:27AM -0800, Junio C Hamano wrote:

> Yeah, after re-reading the messages in this thread, I realize that I
> missed the fact that you do consider these CONNECT_VERBOSE messages
> as debugging aid and from that point of view "fetch -v" that shows
> these messagse in addition to what you get from "fetch" may be a bad
> idea.
> 
> But after inspecting what CONNECT_VERBOSE would add to the output, I
> am inclined to say that, especially if some of these steps can
> exhibit multi-second stalls, they fall more into the "progress
> indicator" (aka "do not worry, we are not stuck, be patient")
> category than "debugging aid" category.

OK, I can buy that line of reasoning (and I agree it implies showing
with a single "-v").

-Peff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] pass transport verbosity down to git_connect
  2016-01-28 23:45 ` Junio C Hamano
@ 2016-01-30  8:50   ` Eric Wong
  2016-01-30 13:13     ` [PATCH 2/1] support -4 and -6 switches for remote operations Eric Wong
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Wong @ 2016-01-30  8:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Junio C Hamano <gitster@pobox.com> wrote:
> Nice.  Can we demonstrate and protect this fix with simple tests?

I just added the tests to t5570 since we don't use git://
much in the tests and I didn't want to introduce potential
port conflicts.

----------------8<----------------
Subject: [PATCH] pass transport verbosity down to git_connect

While working in connect.c to perform non-blocking connections,
I noticed calling "git fetch -v" was not causing the progress
messages inside git_tcp_connect_sock to be emitted as I
expected.

Looking at history, it seems connect_setup has never been called
with the verbose parameter.  Since transport already has a
"verbose" field, use that field instead of another parameter
in connect_setup.

v2: add "-v" tests for clone/fetch/pull to t5570-git-daemon.sh

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 t/t5570-git-daemon.sh | 25 +++++++++++++++++++++++--
 transport.c           | 11 ++++++-----
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index b7e2832..678c8ba 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -6,6 +6,13 @@ test_description='test fetching over git protocol'
 . "$TEST_DIRECTORY"/lib-git-daemon.sh
 start_git_daemon
 
+check_verbose_connect () {
+	grep -qF "Looking up 127.0.0.1 ..." stderr &&
+	grep -qF "Connecting to 127.0.0.1 (port " stderr &&
+	grep -qF "done." stderr &&
+	rm stderr
+}
+
 test_expect_success 'setup repository' '
 	git config push.default matching &&
 	echo content >file &&
@@ -24,18 +31,32 @@ test_expect_success 'create git-accessible bare repository' '
 '
 
 test_expect_success 'clone git repository' '
-	git clone "$GIT_DAEMON_URL/repo.git" clone &&
+	git clone -v "$GIT_DAEMON_URL/repo.git" clone 2>stderr &&
 	test_cmp file clone/file
 '
 
+test_expect_success 'clone -v stderr is as expected' check_verbose_connect
+
 test_expect_success 'fetch changes via git protocol' '
 	echo content >>file &&
 	git commit -a -m two &&
 	git push public &&
-	(cd clone && git pull) &&
+	(cd clone && git pull -v) 2>stderr &&
 	test_cmp file clone/file
 '
 
+test_expect_success 'pull -v stderr is as expected' check_verbose_connect
+
+test_expect_success 'no-op fetch -v stderr is as expected' '
+	(cd clone && git fetch -v) 2>stderr &&
+	check_verbose_connect
+'
+
+test_expect_success 'no-op fetch without "-v" is quiet' '
+	(cd clone && git fetch) 2>stderr &&
+	! test -s stderr
+'
+
 test_expect_success 'remote detects correct HEAD' '
 	git push public master:other &&
 	(cd clone &&
diff --git a/transport.c b/transport.c
index 67f3666..9ae7184 100644
--- a/transport.c
+++ b/transport.c
@@ -481,9 +481,10 @@ static int set_git_option(struct git_transport_options *opts,
 	return 1;
 }
 
-static int connect_setup(struct transport *transport, int for_push, int verbose)
+static int connect_setup(struct transport *transport, int for_push)
 {
 	struct git_transport_data *data = transport->data;
+	int flags = transport->verbose > 0 ? CONNECT_VERBOSE : 0;
 
 	if (data->conn)
 		return 0;
@@ -491,7 +492,7 @@ static int connect_setup(struct transport *transport, int for_push, int verbose)
 	data->conn = git_connect(data->fd, transport->url,
 				 for_push ? data->options.receivepack :
 				 data->options.uploadpack,
-				 verbose ? CONNECT_VERBOSE : 0);
+				 flags);
 
 	return 0;
 }
@@ -501,7 +502,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
 	struct git_transport_data *data = transport->data;
 	struct ref *refs;
 
-	connect_setup(transport, for_push, 0);
+	connect_setup(transport, for_push);
 	get_remote_heads(data->fd[0], NULL, 0, &refs,
 			 for_push ? REF_NORMAL : 0,
 			 &data->extra_have,
@@ -536,7 +537,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 	args.update_shallow = data->options.update_shallow;
 
 	if (!data->got_remote_heads) {
-		connect_setup(transport, 0, 0);
+		connect_setup(transport, 0);
 		get_remote_heads(data->fd[0], NULL, 0, &refs_tmp, 0,
 				 NULL, &data->shallow);
 		data->got_remote_heads = 1;
@@ -812,7 +813,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 
 	if (!data->got_remote_heads) {
 		struct ref *tmp_refs;
-		connect_setup(transport, 1, 0);
+		connect_setup(transport, 1);
 
 		get_remote_heads(data->fd[0], NULL, 0, &tmp_refs, REF_NORMAL,
 				 NULL, &data->shallow);
-- 
EW

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/1] support -4 and -6 switches for remote operations
  2016-01-30  8:50   ` [PATCH v2] " Eric Wong
@ 2016-01-30 13:13     ` Eric Wong
  2016-01-30 13:28       ` Eric Wong
  2016-01-30 23:34       ` Torsten Bögershausen
  0 siblings, 2 replies; 18+ messages in thread
From: Eric Wong @ 2016-01-30 13:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Torsten Bögershausen, Jeff King

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.

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;
 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")),
 	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);
 
 	path = get_repo_path(remote->url[0], &is_bundle);
 	is_local = option_local != 0 && path && !is_bundle;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8e74213..c77af86 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -37,6 +37,7 @@ static int prune = -1; /* unspecified */
 static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow, update_shallow;
+static int ipv4, ipv6;
 static int max_children = 1;
 static const char *depth;
 static const char *upload_pack;
@@ -127,6 +128,8 @@ static struct option builtin_fetch_options[] = {
 		 N_("accept refs that update .git/shallow")),
 	{ OPTION_CALLBACK, 0, "refmap", NULL, N_("refmap"),
 	  N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg },
+	OPT_BOOL('4', "ipv4", &ipv4, N_("resolve IPv4 addresses only")),
+	OPT_BOOL('6', "ipv6", &ipv6, N_("resolve IPv6 addresses only")),
 	OPT_END()
 };
 
@@ -864,6 +867,7 @@ static struct transport *prepare_transport(struct remote *remote)
 	struct transport *transport;
 	transport = transport_get(remote, NULL);
 	transport_set_verbosity(transport, verbosity, progress);
+	transport_set_family(transport, ipv4, ipv6);
 	if (upload_pack)
 		set_option(transport, TRANS_OPT_UPLOADPACK, upload_pack);
 	if (keep)
diff --git a/builtin/push.c b/builtin/push.c
index 960ffc3..e0e8b40 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -23,6 +23,7 @@ static const char *receivepack;
 static int verbosity;
 static int progress = -1;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int ipv4, ipv6;
 
 static struct push_cas_option cas;
 
@@ -346,6 +347,7 @@ static int push_with_options(struct transport *transport, int flags)
 	unsigned int reject_reasons;
 
 	transport_set_verbosity(transport, verbosity, progress);
+	transport_set_family(transport, ipv4, ipv6);
 
 	if (receivepack)
 		transport_set_option(transport,
@@ -565,6 +567,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		  0, "signed", &push_cert, "yes|no|if-asked", N_("GPG sign the push"),
 		  PARSE_OPT_OPTARG, option_parse_push_signed },
 		OPT_BIT(0, "atomic", &flags, N_("request atomic transaction on remote side"), TRANSPORT_PUSH_ATOMIC),
+		OPT_BOOL('4', "ipv4", &ipv4, N_("resolve IPv4 addresses only")),
+		OPT_BOOL('6', "ipv6", &ipv6, N_("resolve IPv6 addresses only")),
 		OPT_END()
 	};
 
diff --git a/connect.c b/connect.c
index fd7ffe1..99178d0 100644
--- a/connect.c
+++ b/connect.c
@@ -357,6 +357,10 @@ static int git_tcp_connect_sock(char *host, int flags)
 		port = "<none>";
 
 	memset(&hints, 0, sizeof(hints));
+	if (flags & CONNECT_IPV4ONLY)
+		hints.ai_family = AF_INET;
+	else if (flags & CONNECT_IPV6ONLY)
+		hints.ai_family = AF_INET6;
 	hints.ai_socktype = SOCK_STREAM;
 	hints.ai_protocol = IPPROTO_TCP;
 
@@ -783,6 +787,10 @@ struct child_process *git_connect(int fd[2], const char *url,
 			}
 
 			argv_array_push(&conn->args, ssh);
+			if (flags & CONNECT_IPV4ONLY)
+				argv_array_push(&conn->args, "-4");
+			else if (flags & CONNECT_IPV6ONLY)
+				argv_array_push(&conn->args, "-6");
 			if (tortoiseplink)
 				argv_array_push(&conn->args, "-batch");
 			if (port) {
diff --git a/connect.h b/connect.h
index c41a685..1cf2412 100644
--- a/connect.h
+++ b/connect.h
@@ -3,6 +3,8 @@
 
 #define CONNECT_VERBOSE       (1u << 0)
 #define CONNECT_DIAG_URL      (1u << 1)
+#define CONNECT_IPV4ONLY      (1u << 2)
+#define CONNECT_IPV6ONLY      (1u << 3)
 extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
 extern int finish_connect(struct child_process *conn);
 extern int git_connection_is_socket(struct child_process *conn);
diff --git a/http.c b/http.c
index 0da9e66..67e7bc2 100644
--- a/http.c
+++ b/http.c
@@ -11,6 +11,11 @@
 #include "gettext.h"
 #include "transport.h"
 
+#if LIBCURL_VERSION_NUM >= 0x070a08
+long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
+#else
+long int git_curl_ipresolve;
+#endif
 int active_requests;
 int http_is_verbose;
 size_t http_post_buffer = 16 * LARGE_PACKET_MAX;
@@ -692,6 +697,10 @@ struct active_request_slot *get_active_slot(void)
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);
+
+#if LIBCURL_VERSION_NUM >= 0x070a08
+	curl_easy_setopt(slot->curl, CURLOPT_IPRESOLVE, git_curl_ipresolve);
+#endif
 #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);
 #endif
diff --git a/http.h b/http.h
index 4f97b60..fa45c2b 100644
--- a/http.h
+++ b/http.h
@@ -106,6 +106,7 @@ extern void http_init(struct remote *remote, const char *url,
 		      int proactive_auth);
 extern void http_cleanup(void);
 
+extern long int git_curl_ipresolve;
 extern int active_requests;
 extern int http_is_verbose;
 extern size_t http_post_buffer;
diff --git a/remote-curl.c b/remote-curl.c
index c704857..2297fa1 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -119,6 +119,25 @@ static int set_option(const char *name, const char *value)
 		else
 			return -1;
 		return 0;
+
+#if LIBCURL_VERSION_NUM >= 0x070a08
+	} else if (!strcmp(name, "ipv4")) {
+		if (!strcmp(value, "true"))
+			git_curl_ipresolve = CURL_IPRESOLVE_V4;
+		else if (!strcmp(value, "false"))
+			git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
+		else
+			return -1;
+		return 0;
+	} else if (!strcmp(name, "ipv6")) {
+		if (!strcmp(value, "true"))
+			git_curl_ipresolve = CURL_IPRESOLVE_V6;
+		else if (!strcmp(value, "false"))
+			git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
+		else
+			return -1;
+		return 0;
+#endif /* LIBCURL_VERSION_NUM >= 0x070a08 */
 	} else {
 		return 1 /* unsupported */;
 	}
diff --git a/transport-helper.c b/transport-helper.c
index e45d88f..e6ffb9a 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -258,6 +258,8 @@ static const char *boolean_options[] = {
 	TRANS_OPT_THIN,
 	TRANS_OPT_KEEP,
 	TRANS_OPT_FOLLOWTAGS,
+	TRANS_OPT_IPV4,
+	TRANS_OPT_IPV6,
 	};
 
 static int set_helper_option(struct transport *transport,
@@ -321,6 +323,11 @@ static void standard_options(struct transport *t)
 	if (n >= sizeof(buf))
 		die("impossibly large verbosity value");
 	set_helper_option(t, "verbosity", buf);
+
+	if (t->ipv4)
+		set_helper_option(t, "ipv4", "true");
+	if (t->ipv6)
+		set_helper_option(t, "ipv6", "true");
 }
 
 static int release_helper(struct transport *transport)
diff --git a/transport.c b/transport.c
index 9ae7184..8fd31d3 100644
--- a/transport.c
+++ b/transport.c
@@ -489,6 +489,10 @@ static int connect_setup(struct transport *transport, int for_push)
 	if (data->conn)
 		return 0;
 
+	if (transport->ipv4)
+		flags |= CONNECT_IPV4ONLY;
+	if (transport->ipv6)
+		flags |= CONNECT_IPV6ONLY;
 	data->conn = git_connect(data->fd, transport->url,
 				 for_push ? data->options.receivepack :
 				 data->options.uploadpack,
@@ -1086,6 +1090,20 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 		transport->progress = verbosity >= 0 && isatty(2);
 }
 
+void transport_set_family(struct transport *transport, int ipv4, int ipv6)
+{
+	if (ipv4 && ipv6)
+		die("-4/--ipv4 and -6/--ipv6 are incompatible");
+	if (ipv4) {
+		transport->ipv4 = 1;
+		transport_set_option(transport, TRANS_OPT_IPV4, "true");
+	}
+	if (ipv6) {
+		transport->ipv6 = 1;
+		transport_set_option(transport, TRANS_OPT_IPV6, "true");
+	}
+}
+
 static void die_with_unpushed_submodules(struct string_list *needs_pushing)
 {
 	int i;
diff --git a/transport.h b/transport.h
index 8ebaaf2..f9f3c00 100644
--- a/transport.h
+++ b/transport.h
@@ -104,6 +104,10 @@ struct transport {
 	 * in transport_set_verbosity().
 	 **/
 	unsigned progress : 1;
+
+	/* mutually exclusive, set by transport_set_family */
+	unsigned ipv4 : 1;
+	unsigned ipv6 : 1;
 	/*
 	 * If transport is at least potentially smart, this points to
 	 * git_transport_options structure to use in case transport
@@ -180,6 +184,12 @@ int transport_restrict_protocols(void);
 /* Send push certificates */
 #define TRANS_OPT_PUSH_CERT "pushcert"
 
+/* Limit to IPv4 only */
+#define TRANS_OPT_IPV4 "ipv4"
+
+/* Limit to IPv6 only */
+#define TRANS_OPT_IPV6 "ipv6"
+
 /**
  * Returns 0 if the option was used, non-zero otherwise. Prints a
  * message to stderr if the option is not used.
@@ -188,6 +198,7 @@ int transport_set_option(struct transport *transport, const char *name,
 			 const char *value);
 void transport_set_verbosity(struct transport *transport, int verbosity,
 	int force_progress);
+void transport_set_family(struct transport *transport, int ipv4, int ipv6);
 
 #define REJECT_NON_FF_HEAD     0x01
 #define REJECT_NON_FF_OTHER    0x02
-- 
EW

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/1] support -4 and -6 switches for remote operations
  2016-01-30 13:13     ` [PATCH 2/1] support -4 and -6 switches for remote operations Eric Wong
@ 2016-01-30 13:28       ` Eric Wong
  2016-01-30 23:34       ` Torsten Bögershausen
  1 sibling, 0 replies; 18+ messages in thread
From: Eric Wong @ 2016-01-30 13:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Torsten Bögershausen, Jeff King

Eric Wong <normalperson@yhbt.net> wrote:
> rsync support is untouched for now since it is deprecated
> and scheduled to be removed.

I forgot add I'm not sure how to actually go about testing these changes
automatically since they involve DNS setups.  And the test suite seems
really slow nowadays, I guess/hope our test coverage has improved much
in the past few years?

I did test the ssh push case manually and all the fetch/clone cases,
including rsync (where I encountered the breakage from 2007).  So I
reverted my rsync change for now to avoid conflicting with its removal.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/1] support -4 and -6 switches for remote operations
  2016-01-30 13:13     ` [PATCH 2/1] support -4 and -6 switches for remote operations Eric Wong
  2016-01-30 13:28       ` Eric Wong
@ 2016-01-30 23:34       ` Torsten Bögershausen
  2016-01-31  0:01         ` Eric Wong
  1 sibling, 1 reply; 18+ messages in thread
From: Torsten Bögershausen @ 2016-01-30 23:34 UTC (permalink / raw)
  To: Eric Wong, Junio C Hamano; +Cc: git, Jeff King

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]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/1] support -4 and -6 switches for remote operations
  2016-01-30 23:34       ` Torsten Bögershausen
@ 2016-01-31  0:01         ` Eric Wong
  2016-01-31  1:13           ` Jeff King
  2016-01-31 16:03           ` [PATCH " Torsten Bögershausen
  0 siblings, 2 replies; 18+ messages in thread
From: Eric Wong @ 2016-01-31  0:01 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Junio C Hamano, git, Jeff King

Torsten Bögershausen <tboegi@web.de> wrote:
> On 2016-01-30 14.13, Eric Wong wrote:
> > 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 ?

Sure, will remember for v2 and document in the manpages.

Curious, do these ssh implementations throw out a meaningful error
message when given these options?

> > --- 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 ?

Yes, I'm not sure how else to use OPT_BOOL below...

> 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/

I suppose "use" is shorter and just as informational.
Will prepare that for v2.

> > @@ -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)

I actually had "only" in the variables originally, but didn't want to
line-wrap in builtin/push.c

Furthermore, the non-"only" name is used by the long switch (just like
in both curl(1) and rsync(1)), so I figured we should remain consistent
with what the user will see in documentation.

I think I will drop "ONLY" from the CONNECT_* macros instead...

Will await further comments and prepare v2 in a day or two.
Thanks for the comments.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/1] support -4 and -6 switches for remote operations
  2016-01-31  0:01         ` Eric Wong
@ 2016-01-31  1:13           ` Jeff King
  2016-02-03  4:09             ` [PATCH v2 " Eric Wong
  2016-01-31 16:03           ` [PATCH " Torsten Bögershausen
  1 sibling, 1 reply; 18+ messages in thread
From: Jeff King @ 2016-01-31  1:13 UTC (permalink / raw)
  To: Eric Wong; +Cc: Torsten Bögershausen, Junio C Hamano, git

On Sun, Jan 31, 2016 at 12:01:44AM +0000, Eric Wong wrote:

> > > --- 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 ?
> 
> Yes, I'm not sure how else to use OPT_BOOL below...

If they're mutually exclusive, and saying "-6" cancels "-4", you
probably want something like:

  enum {
	USE_NET_ALL = 0,
	USE_NET_IPV4,
	USE_NET_IPV6,
  } use_net;

  ...

  OPT_SET_INT('4', "ipv4", &use_net,
              N_("resolve IPv4 addresses only"), USE_NET_IPV4),
  OPT_SET_INT('6', "ipv6", &use_net,
              N_("resolve IPv6 addresses only"), USE_NET_IPV6),

Using --no-ipv4 will set it back to USE_NET_ALL, which is probably OK.
It will cancel a previous "--ipv4", which is logically consistent,
though I guess some people might assume that "--no-ipv4" means "do not
use ipv4 addresses". Supporting that would be more complicated.

-Peff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/1] support -4 and -6 switches for remote operations
  2016-01-31  0:01         ` Eric Wong
  2016-01-31  1:13           ` Jeff King
@ 2016-01-31 16:03           ` Torsten Bögershausen
  1 sibling, 0 replies; 18+ messages in thread
From: Torsten Bögershausen @ 2016-01-31 16:03 UTC (permalink / raw)
  To: Eric Wong, Torsten Bögershausen; +Cc: Junio C Hamano, git, Jeff King

On 2016-01-31 01.01, Eric Wong wrote:
> Torsten Bögershausen <tboegi@web.de> wrote:
>> On 2016-01-30 14.13, Eric Wong wrote:
>>> 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 ?

I may need to take that back;
Just did a test with putty (under Debian)

And both -4 and -6 are supported, nice.

I couldn't find it first, but here seems to be the latest documentation,
which mentions -4 and -6.
http://the.earth.li/~sgtatham/putty/0.66/puttydoc.txt

And even plink acceptes -4 and -6 (again under Debian)

Sorry for the noise.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 2/1] support -4 and -6 switches for remote operations
  2016-01-31  1:13           ` Jeff King
@ 2016-02-03  4:09             ` Eric Wong
  2016-02-12 11:31               ` Eric Wong
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Wong @ 2016-02-03  4:09 UTC (permalink / raw)
  To: Jeff King; +Cc: Torsten Bögershausen, Junio C Hamano, git

Jeff King <peff@peff.net> wrote:
>   OPT_SET_INT('4', "ipv4", &use_net,
>               N_("resolve IPv4 addresses only"), USE_NET_IPV4),
>   OPT_SET_INT('6', "ipv6", &use_net,
>               N_("resolve IPv6 addresses only"), USE_NET_IPV6),

Thanks, I missed OPT_SET_INT before.

> Using --no-ipv4 will set it back to USE_NET_ALL, which is probably OK.
> It will cancel a previous "--ipv4", which is logically consistent,
> though I guess some people might assume that "--no-ipv4" means "do not
> use ipv4 addresses". Supporting that would be more complicated.

Right, not worth the effort to support that and I prefer leaving
the "--no-" variants undocumented as an implementation artifact.

(no rush to review this, unlikely to be around the next few days)

-----------------8<------------------
Subject: [PATCH] support -4 and -6 switches for remote operations

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.  There may be old ssh(1) implementations
out there which do not support these switches; they should
report the appropriate error in that case.

rsync support is untouched for now since it is deprecated
and scheduled to be removed.

v2:
 - switch from boolean ints to enum
 - shorted CONNECT_* macro names to be consistent with switches
 - remove unneeded TRANS_OPT_* macros and usage
 - remove transport_set_family wrapper, enum is enough validation
 - s/resolve/use/ in documentation
 - document potential (but unconfirmed) problems with some ssh(1)

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Documentation/fetch-options.txt |  8 ++++++++
 Documentation/git-push.txt      |  7 +++++++
 builtin/clone.c                 |  6 ++++++
 builtin/fetch.c                 |  6 ++++++
 builtin/push.c                  |  6 ++++++
 connect.c                       |  8 ++++++++
 connect.h                       |  2 ++
 http.c                          |  9 +++++++++
 http.h                          |  1 +
 remote-curl.c                   | 13 +++++++++++++
 transport-helper.c              | 15 +++++++++++++++
 transport.c                     |  6 ++++++
 transport.h                     |  8 ++++++++
 13 files changed, 95 insertions(+)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 952dfdf..036edfb 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::
+	Use IPv4 addresses only, ignoring IPv6 addresses.
+
+-6::
+--ipv6::
+	Use IPv6 addresses only, ignoring IPv4 addresses.
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 32482ce..669a357 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::
+	Use IPv4 addresses only, ignoring IPv6 addresses.
+
+-6::
+--ipv6::
+	Use IPv6 addresses only, ignoring IPv4 addresses.
 
 include::urls-remotes.txt[]
 
diff --git a/builtin/clone.c b/builtin/clone.c
index 81e238f..592e6db 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 enum transport_family family;
 static struct string_list option_config;
 static struct string_list option_reference;
 static int option_dissociate;
@@ -92,6 +93,10 @@ 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_SET_INT('4', "ipv4", &family, N_("use IPv4 addresses only"),
+			TRANSPORT_FAMILY_IPV4),
+	OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
+			TRANSPORT_FAMILY_IPV6),
 	OPT_END()
 };
 
@@ -970,6 +975,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->family = family;
 
 	path = get_repo_path(remote->url[0], &is_bundle);
 	is_local = option_local != 0 && path && !is_bundle;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8e74213..03ba709 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -37,6 +37,7 @@ static int prune = -1; /* unspecified */
 static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow, update_shallow;
+static enum transport_family family;
 static int max_children = 1;
 static const char *depth;
 static const char *upload_pack;
@@ -127,6 +128,10 @@ static struct option builtin_fetch_options[] = {
 		 N_("accept refs that update .git/shallow")),
 	{ OPTION_CALLBACK, 0, "refmap", NULL, N_("refmap"),
 	  N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg },
+	OPT_SET_INT('4', "ipv4", &family, N_("use IPv4 addresses only"),
+			TRANSPORT_FAMILY_IPV4),
+	OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
+			TRANSPORT_FAMILY_IPV6),
 	OPT_END()
 };
 
@@ -864,6 +869,7 @@ static struct transport *prepare_transport(struct remote *remote)
 	struct transport *transport;
 	transport = transport_get(remote, NULL);
 	transport_set_verbosity(transport, verbosity, progress);
+	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 960ffc3..6e13b3c 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -23,6 +23,7 @@ static const char *receivepack;
 static int verbosity;
 static int progress = -1;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static enum transport_family family;
 
 static struct push_cas_option cas;
 
@@ -346,6 +347,7 @@ static int push_with_options(struct transport *transport, int flags)
 	unsigned int reject_reasons;
 
 	transport_set_verbosity(transport, verbosity, progress);
+	transport->family = family;
 
 	if (receivepack)
 		transport_set_option(transport,
@@ -565,6 +567,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		  0, "signed", &push_cert, "yes|no|if-asked", N_("GPG sign the push"),
 		  PARSE_OPT_OPTARG, option_parse_push_signed },
 		OPT_BIT(0, "atomic", &flags, N_("request atomic transaction on remote side"), TRANSPORT_PUSH_ATOMIC),
+		OPT_SET_INT('4', "ipv4", &family, N_("use IPv4 addresses only"),
+				TRANSPORT_FAMILY_IPV4),
+		OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
+				TRANSPORT_FAMILY_IPV6),
 		OPT_END()
 	};
 
diff --git a/connect.c b/connect.c
index fd7ffe1..0478631 100644
--- a/connect.c
+++ b/connect.c
@@ -357,6 +357,10 @@ static int git_tcp_connect_sock(char *host, int flags)
 		port = "<none>";
 
 	memset(&hints, 0, sizeof(hints));
+	if (flags & CONNECT_IPV4)
+		hints.ai_family = AF_INET;
+	else if (flags & CONNECT_IPV6)
+		hints.ai_family = AF_INET6;
 	hints.ai_socktype = SOCK_STREAM;
 	hints.ai_protocol = IPPROTO_TCP;
 
@@ -783,6 +787,10 @@ struct child_process *git_connect(int fd[2], const char *url,
 			}
 
 			argv_array_push(&conn->args, ssh);
+			if (flags & CONNECT_IPV4)
+				argv_array_push(&conn->args, "-4");
+			else if (flags & CONNECT_IPV6)
+				argv_array_push(&conn->args, "-6");
 			if (tortoiseplink)
 				argv_array_push(&conn->args, "-batch");
 			if (port) {
diff --git a/connect.h b/connect.h
index c41a685..01f14cd 100644
--- a/connect.h
+++ b/connect.h
@@ -3,6 +3,8 @@
 
 #define CONNECT_VERBOSE       (1u << 0)
 #define CONNECT_DIAG_URL      (1u << 1)
+#define CONNECT_IPV4          (1u << 2)
+#define CONNECT_IPV6          (1u << 3)
 extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
 extern int finish_connect(struct child_process *conn);
 extern int git_connection_is_socket(struct child_process *conn);
diff --git a/http.c b/http.c
index 0da9e66..67e7bc2 100644
--- a/http.c
+++ b/http.c
@@ -11,6 +11,11 @@
 #include "gettext.h"
 #include "transport.h"
 
+#if LIBCURL_VERSION_NUM >= 0x070a08
+long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
+#else
+long int git_curl_ipresolve;
+#endif
 int active_requests;
 int http_is_verbose;
 size_t http_post_buffer = 16 * LARGE_PACKET_MAX;
@@ -692,6 +697,10 @@ struct active_request_slot *get_active_slot(void)
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);
+
+#if LIBCURL_VERSION_NUM >= 0x070a08
+	curl_easy_setopt(slot->curl, CURLOPT_IPRESOLVE, git_curl_ipresolve);
+#endif
 #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);
 #endif
diff --git a/http.h b/http.h
index 4f97b60..fa45c2b 100644
--- a/http.h
+++ b/http.h
@@ -106,6 +106,7 @@ extern void http_init(struct remote *remote, const char *url,
 		      int proactive_auth);
 extern void http_cleanup(void);
 
+extern long int git_curl_ipresolve;
 extern int active_requests;
 extern int http_is_verbose;
 extern size_t http_post_buffer;
diff --git a/remote-curl.c b/remote-curl.c
index c704857..b33a1e4 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -119,6 +119,19 @@ static int set_option(const char *name, const char *value)
 		else
 			return -1;
 		return 0;
+
+#if LIBCURL_VERSION_NUM >= 0x070a08
+	} else if (!strcmp(name, "family")) {
+		if (!strcmp(value, "ipv4"))
+			git_curl_ipresolve = CURL_IPRESOLVE_V4;
+		else if (!strcmp(value, "ipv6"))
+			git_curl_ipresolve = CURL_IPRESOLVE_V6;
+		else if (!strcmp(value, "all"))
+			git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
+		else
+			return -1;
+		return 0;
+#endif /* LIBCURL_VERSION_NUM >= 0x070a08 */
 	} else {
 		return 1 /* unsupported */;
 	}
diff --git a/transport-helper.c b/transport-helper.c
index e45d88f..f6b8e7b 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -321,6 +321,21 @@ static void standard_options(struct transport *t)
 	if (n >= sizeof(buf))
 		die("impossibly large verbosity value");
 	set_helper_option(t, "verbosity", buf);
+
+	switch (t->family) {
+	case TRANSPORT_FAMILY_ALL:
+		/*
+		 * this is already the default,
+		 * do not break old remote helpers by setting "all" here
+		 */
+		break;
+	case TRANSPORT_FAMILY_IPV4:
+		set_helper_option(t, "family", "ipv4");
+		break;
+	case TRANSPORT_FAMILY_IPV6:
+		set_helper_option(t, "family", "ipv6");
+		break;
+	}
 }
 
 static int release_helper(struct transport *transport)
diff --git a/transport.c b/transport.c
index 9ae7184..afcec43 100644
--- a/transport.c
+++ b/transport.c
@@ -489,6 +489,12 @@ static int connect_setup(struct transport *transport, int for_push)
 	if (data->conn)
 		return 0;
 
+	switch (transport->family) {
+	case TRANSPORT_FAMILY_ALL: break;
+	case TRANSPORT_FAMILY_IPV4: flags |= CONNECT_IPV4; break;
+	case TRANSPORT_FAMILY_IPV6: flags |= CONNECT_IPV6; break;
+	}
+
 	data->conn = git_connect(data->fd, transport->url,
 				 for_push ? data->options.receivepack :
 				 data->options.uploadpack,
diff --git a/transport.h b/transport.h
index 8ebaaf2..c681408 100644
--- a/transport.h
+++ b/transport.h
@@ -18,6 +18,12 @@ struct git_transport_options {
 	struct push_cas_option *cas;
 };
 
+enum transport_family {
+	TRANSPORT_FAMILY_ALL = 0,
+	TRANSPORT_FAMILY_IPV4,
+	TRANSPORT_FAMILY_IPV6
+};
+
 struct transport {
 	struct remote *remote;
 	const char *url;
@@ -110,6 +116,8 @@ struct transport {
 	 * actually turns out to be smart.
 	 */
 	struct git_transport_options *smart_options;
+
+	enum transport_family family;
 };
 
 #define TRANSPORT_PUSH_ALL 1
-- 
EW

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/1] support -4 and -6 switches for remote operations
  2016-02-03  4:09             ` [PATCH v2 " Eric Wong
@ 2016-02-12 11:31               ` Eric Wong
  2016-02-12 15:43                 ` Torsten Bögershausen
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Wong @ 2016-02-12 11:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Torsten Bögershausen, Junio C Hamano

Eric Wong <normalperson@yhbt.net> wrote:
> (no rush to review this, unlikely to be around the next few days)

Ping on v2.  I will be online the next few days and likely
working on some other git stuff anyways :)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/1] support -4 and -6 switches for remote operations
  2016-02-12 11:31               ` Eric Wong
@ 2016-02-12 15:43                 ` Torsten Bögershausen
  0 siblings, 0 replies; 18+ messages in thread
From: Torsten Bögershausen @ 2016-02-12 15:43 UTC (permalink / raw)
  To: Eric Wong, git; +Cc: Jeff King, Torsten Bögershausen, Junio C Hamano



On 12.02.16 12:31, Eric Wong wrote:
> Eric Wong <normalperson@yhbt.net> wrote:
>> (no rush to review this, unlikely to be around the next few days)
> Ping on v2.  I will be online the next few days and likely
> working on some other git stuff anyways :)
Patch V2 looks OK for me.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2016-02-12 15:44 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-28 22:51 [PATCH] pass transport verbosity down to git_connect Eric Wong
2016-01-28 23:45 ` Junio C Hamano
2016-01-30  8:50   ` [PATCH v2] " Eric Wong
2016-01-30 13:13     ` [PATCH 2/1] support -4 and -6 switches for remote operations Eric Wong
2016-01-30 13:28       ` Eric Wong
2016-01-30 23:34       ` Torsten Bögershausen
2016-01-31  0:01         ` Eric Wong
2016-01-31  1:13           ` Jeff King
2016-02-03  4:09             ` [PATCH v2 " Eric Wong
2016-02-12 11:31               ` Eric Wong
2016-02-12 15:43                 ` Torsten Bögershausen
2016-01-31 16:03           ` [PATCH " Torsten Bögershausen
2016-01-28 23:53 ` [PATCH] pass transport verbosity down to git_connect Jeff King
2016-01-29  0:38   ` Eric Wong
2016-01-29  3:19     ` Junio C Hamano
2016-01-29  3:47       ` Jeff King
2016-01-29 17:34         ` Junio C Hamano
2016-01-29 17:41           ` Jeff King

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.