From: Alex Riesen <firstname.lastname@example.org> To: Jeff King <email@example.com> Cc: firstname.lastname@example.org, Eric Wong <email@example.com>, Junio C Hamano <firstname.lastname@example.org> Subject: Re: sub-fetches discard --ipv4|6 option Date: Tue, 15 Sep 2020 13:50:25 +0200 [thread overview] Message-ID: <20200915115025.GA18984@pflmari> (raw) In-Reply-To: <20200914194951.GA2819729@coredump.intra.peff.net> Jeff King, Mon, Sep 14, 2020 21:49:51 +0200: > 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: ... > > > > 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). I tried to come up with one, but (aside from rather pointless checking of option presence in the trace output) failed to. Or may be precisely this could be the point of the test: just do a fetch with all options we intend to pass down to sub-fetches and check that they are indeed present in the invocation of fetch --all/--multiple/--recurse-submodules? > It is rather unfortunate that anybody adding new fetch options needs to > remember to (maybe) add them to add_options_to_argv() themselves. Maybe make add_options_to_argv to go through builtin_fetch_options and copy the options with a special marker if they were provided? And use the word "recursive" in help text as the marker :) > 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). Sure! Thinking about it, I actually would have preferred to have both: a config option and a command-line option. So that I can set --ipv4 in, say, ~/.config/git/config file, but still have the option to try --ipv6 from time to time to check if the network setup magically fixed itself. What would the preferred name for that config option be? fetch.ipv? I'll be sending the first change reformatted as patch shortly. Just in case.
next prev parent reply other threads:[~2020-09-15 11:53 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 2020-09-15 11:50 ` Alex Riesen [this message] 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=20200915115025.GA18984@pflmari \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --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).