git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Alex Riesen <alexander.riesen@cetitec.com>
Cc: git@vger.kernel.org, Eric Wong <normalperson@yhbt.net>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: sub-fetches discard --ipv4|6 option
Date: Mon, 14 Sep 2020 15:49:51 -0400	[thread overview]
Message-ID: <20200914194951.GA2819729@coredump.intra.peff.net> (raw)
In-Reply-To: <20200914121906.GD4705@pflmari>

On Mon, Sep 14, 2020 at 02:19:06PM +0200, Alex Riesen wrote:

> Unfortunately, it only worked for the fetches which didn't use --all or
> --multiple. After a light searching, I failed to find an explanation as to
> why --all|--multiple are handled so inconsistently with single remote fetches
> and added the options (similar to --force or --keep) to the argument list for
> sub-fetches:
> 
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 82ac4be8a5..5e06c07106 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");
>  
>  }
>  
> Am I missing something obvious?

I don't think so. When we're starting fetch sub-processes, some options
will make sense to pass along and some won't. The parent has to either
pass all options and omit some, or explicitly pass ones it knows are
useful. It looks like the code chooses the latter, but these particular
options never got added (and it seems like they should be, as they are
only useful to the child fetch processes that actually touch the
network).

So your patch above looks quite sensible (modulo useful bits like a
signoff and maybe a test, though I guess the impact of those options
is probably hard to cover in our tests).

It is rather unfortunate that anybody adding new fetch options needs to
remember to (maybe) add them to add_options_to_argv() themselves.

Also, regarding these two specific options, it sounds like you'd want
them set for all fetches during the time your IPv6 setup is broken. In
which case I think a config option might have served you better. So that
might be something worth implementing (though either way I think the fix
above is worth doing independently).

-Peff

  reply	other threads:[~2020-09-14 19:49 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-14 12:19 Alex Riesen
2020-09-14 19:49 ` Jeff King [this message]
2020-09-15 11:50   ` Alex Riesen
2020-09-15 11:54     ` [PATCH] Pass --ipv4 and --ipv6 options to sub-fetches when fetching multiple remotes and submodules Alex Riesen
2020-09-15 13:06       ` Jeff King
2020-09-16  4:17         ` Junio C Hamano
2020-09-16  7:27           ` Alex Riesen
2020-09-15 21:19       ` Junio C Hamano
2020-09-16  7:25         ` Alex Riesen
2020-09-15 13:05     ` sub-fetches discard --ipv4|6 option Jeff King
2020-09-15 13:54       ` [PATCH] config: option transfer.ipversion to set transport protocol version for network fetches Alex Riesen
2020-09-16 20:02         ` Jeff King
2020-09-17  8:07           ` Alex Riesen
2020-09-17 13:20           ` [PATCH] Config option to set the " Alex Riesen
2020-09-17 13:26             ` Alex Riesen
2020-09-17 13:31             ` Jeff King
2020-09-17 13:35               ` Alex Riesen
2020-09-17 14:51                 ` Jeff King
2020-09-17 15:17                   ` Alex Riesen
2020-12-22 19:55                     ` Junio C Hamano
2021-01-07 10:06                       ` Alex Riesen
2020-09-17 16:05             ` Alex Riesen
2020-09-16 20:14         ` [PATCH] config: option transfer.ipversion to set " Junio C Hamano
2020-09-16 20:18           ` Jeff King
2020-09-16 22:50             ` Junio C Hamano
2020-09-16 22:52               ` Junio C Hamano
2020-09-17  0:48                 ` Jeff King
2020-09-17  0:57                   ` Junio C Hamano
2020-09-16 21:35           ` Junio C Hamano
2020-09-17 14:02             ` Alex Riesen
2020-09-17 22:31               ` Junio C Hamano
2020-09-18  7:16                 ` Alex Riesen
2020-09-18 16:37                   ` Junio C Hamano
2020-09-21 16:39                     ` Alex Riesen
2020-09-22  5:03                       ` Jeff King
2020-09-17  8:04           ` Alex Riesen
2020-09-17  8:18           ` Alex Riesen
2020-09-15 14:06       ` sub-fetches discard --ipv4|6 option Alex Riesen
2020-09-15 15:27         ` Jeff King
2020-09-15 16:03           ` Alex Riesen
2020-09-16 16:32             ` Jeff King
2020-09-17 14:33               ` Alex Riesen
2020-09-22  5:08                 ` Jeff King
2020-09-15 20:09           ` Junio C Hamano
2020-09-15 21:23             ` Jeff King
2020-09-15 21:32               ` Junio C Hamano
2020-09-16 16:34                 ` Jeff King
2020-09-16 21:20                   ` Junio C Hamano
2020-09-14 20:06 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200914194951.GA2819729@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=alexander.riesen@cetitec.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=normalperson@yhbt.net \
    --subject='Re: sub-fetches discard --ipv4|6 option' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).