Re: sub-fetches discard --ipv4|6 option
From: Jeff King <hidden>
Date: 2020-09-14 19:49:55
On Mon, Sep 14, 2020 at 02:19:06PM +0200, Alex Riesen wrote:
quoted hunk ↗ jump to hunk
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