diff mbox series

Pass --ipv4 and --ipv6 options to sub-fetches when fetching multiple remotes and submodules

Message ID 20200915115407.GA31786@pflmari
State Accepted
Commit 4e735c13267e65f8c37fc3e7dfaacb36e5d51ab8
Headers show
Series Pass --ipv4 and --ipv6 options to sub-fetches when fetching multiple remotes and submodules | expand

Commit Message

Alex Riesen Sept. 15, 2020, 11:54 a.m. UTC
The options indicate user intent for the whole fetch operation, and
ignoring them in sub-fetches is quite unexpected when, for instance,
it is intended to limit all of the communication to a specific transport
protocol for some reason.

Signed-off-by: Alex Riesen <alexander.riesen@cetitec.com>
---

 builtin/fetch.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jeff King Sept. 15, 2020, 1:06 p.m. UTC | #1
On Tue, Sep 15, 2020 at 01:54:07PM +0200, Alex Riesen wrote:

> The options indicate user intent for the whole fetch operation, and
> ignoring them in sub-fetches is quite unexpected when, for instance,
> it is intended to limit all of the communication to a specific transport
> protocol for some reason.
> 
> Signed-off-by: Alex Riesen <alexander.riesen@cetitec.com>
> ---

Regardless of whether we move forward with the parse-options flag or
config discussed in the other thread, I think this is an obvious
improvement that we should take in the meantime.

-Peff
Junio C Hamano Sept. 15, 2020, 9:19 p.m. UTC | #2
Alex Riesen <alexander.riesen@cetitec.com> writes:

> The options indicate user intent for the whole fetch operation, and
> ignoring them in sub-fetches is quite unexpected when, for instance,
> it is intended to limit all of the communication to a specific transport
> protocol for some reason.
>
> Signed-off-by: Alex Riesen <alexander.riesen@cetitec.com>
> ---

To avoid an overlong title and conform to project convention (aka
"easier to read 'git shortlog --no-merges' output), I shortened the
title and tweaked the text a bit to compensate for the change.

Thanks.


-- >8 --
From: Alex Riesen <alexander.riesen@cetitec.com>
Date: Tue, 15 Sep 2020 13:54:07 +0200
Subject: [PATCH] fetch: pass --ipv4 and --ipv6 options to sub-fetches

The options indicate user intent for the whole fetch operation, and
ignoring them in sub-fetches (i.e. "--all" and recursive fetching of
submodules) is quite unexpected when, for instance, it is intended
to limit all of the communication to a specific transport protocol
for some reason.

Signed-off-by: Alex Riesen <alexander.riesen@cetitec.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fetch.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 82ac4be8a5..447d28ac29 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1531,6 +1531,10 @@ static void add_options_to_argv(struct argv_array *argv)
 		argv_array_push(argv, "-v");
 	else if (verbosity < 0)
 		argv_array_push(argv, "-q");
+	if (family == TRANSPORT_FAMILY_IPV4)
+		argv_array_push(argv, "--ipv4");
+	else if (family == TRANSPORT_FAMILY_IPV6)
+		argv_array_push(argv, "--ipv6");
 
 }
Junio C Hamano Sept. 16, 2020, 4:17 a.m. UTC | #3
Jeff King <peff@peff.net> writes:

> On Tue, Sep 15, 2020 at 01:54:07PM +0200, Alex Riesen wrote:
>
>> The options indicate user intent for the whole fetch operation, and
>> ignoring them in sub-fetches is quite unexpected when, for instance,
>> it is intended to limit all of the communication to a specific transport
>> protocol for some reason.
>> 
>> Signed-off-by: Alex Riesen <alexander.riesen@cetitec.com>
>> ---
>
> Regardless of whether we move forward with the parse-options flag or
> config discussed in the other thread, I think this is an obvious
> improvement that we should take in the meantime.

Yes.  Others can wait.  ipversion configuration variable is probably
easier to sell; parse_options thing deserves a longer and deeper
thought as it will affect the API future codebase would rely on.

Thanks.
Alex Riesen Sept. 16, 2020, 7:25 a.m. UTC | #4
Junio C Hamano, Tue, Sep 15, 2020 23:19:11 +0200:
> Alex Riesen <alexander.riesen@cetitec.com> writes:
> 
> > The options indicate user intent for the whole fetch operation, and
> > ignoring them in sub-fetches is quite unexpected when, for instance,
> > it is intended to limit all of the communication to a specific transport
> > protocol for some reason.
> >
> 
> To avoid an overlong title and conform to project convention (aka
> "easier to read 'git shortlog --no-merges' output), I shortened the
> title and tweaked the text a bit to compensate for the change.

Thanks :)
Alex Riesen Sept. 16, 2020, 7:27 a.m. UTC | #5
Junio C Hamano, Wed, Sep 16, 2020 06:17:41 +0200:
> Jeff King <peff@peff.net> writes:
> 
> > On Tue, Sep 15, 2020 at 01:54:07PM +0200, Alex Riesen wrote:
> >
> >> The options indicate user intent for the whole fetch operation, and
> >> ignoring them in sub-fetches is quite unexpected when, for instance,
> >> it is intended to limit all of the communication to a specific transport
> >> protocol for some reason.
> >> 
> >> Signed-off-by: Alex Riesen <alexander.riesen@cetitec.com>
> >> ---
> >
> > Regardless of whether we move forward with the parse-options flag or
> > config discussed in the other thread, I think this is an obvious
> > improvement that we should take in the meantime.
> 
> Yes.  Others can wait.  ipversion configuration variable is probably
> easier to sell; ...

Is its choice of namespace (transfer.) alright, too?

> ... parse_options thing deserves a longer and deeper
> thought as it will affect the API future codebase would rely on.

Of course.

Regards,
Alex
diff mbox series

Patch

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 82ac4be8a5..447d28ac29 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1531,6 +1531,10 @@  static void add_options_to_argv(struct argv_array *argv)
 		argv_array_push(argv, "-v");
 	else if (verbosity < 0)
 		argv_array_push(argv, "-q");
+	if (family == TRANSPORT_FAMILY_IPV4)
+		argv_array_push(argv, "--ipv4");
+	else if (family == TRANSPORT_FAMILY_IPV6)
+		argv_array_push(argv, "--ipv6");
 
 }