Re: sub-fetches discard --ipv4|6 option

From: Jeff King <peff@peff.net>
Date: Sep 15, 2020, 21:23 UTC
Message-ID: <20200915212338.GA2868700@coredump.intra.peff.net>
In-Reply-To: <xmqq4kny2461.fsf@gitster.c.googlers.com>

On Tue, Sep 15, 2020 at 01:09:10PM -0700, Junio C Hamano wrote:

Show 13 quoted lines

> > If you go that route, we have some "_F" macros that take flags. Probably
> > would make sense to add it more consistently, which lets you convert:
> >
> >   OPT_BOOL('f', "foo", &foo, "the foo option");
> >
> > into:
> >
> >   OPT_BOOL_F('f', "foo", &foo, "the foo option", PARSE_OPT_RECURSIVE);
> >
> > but could also be used for other flags.
> 
> What is this "recursive" about?  Does it have much in common with
> "passthru", or are they orthogonal?

I agree the name is not super-descriptive. And it's a bit odd that the parse-options code itself would not care about it. It's simply a convenient bit for the calling code to use (rather than try to manage a separate array whose values correspond). It could be PARSE_OPT_USER1, but that is probably too inscrutable. :)

It's sort-of similar to passthru, but not quite. The problem with passthru is that it _always_ applies to the option. We never parse it as an option itself, but rather always stick its canonicalized form into an array of strings. But for the case we're talking about here, we don't know ahead of time whether we want the passthru behavior or not. It depends on whether we see an option like "--all" (which might even come after us).

So I think the best you could do is:

  1. Keep two separate option lists, "parent" and "child". The parent
     list has "--all" in it. The child list has stuff like "--ipv6".

  2. Parse using the parent list with PARSE_OPT_KEEP_UNKNOWN. That lets
     you decide whether we're in a mode that is spawning child fetch
     processes.

  3. If we are spawning, then everything in the "child" option list
     becomes passthru. We could either mark them as such, or really, I
     guess we could just pass the remainder of argv on as-is (though it
     might be nice to diagnose a bogus config option once in the parent
     rather than in each child).

That would work OK. One downside is that PARSE_OPT_KEEP_UNKNOWN is inherently flaky. It doesn't know if "--foo --bar" is two options, or the option "--foo" with the value "--bar". You could solve that by leaving dummy passthru options in the "parent" list, but now we're back to having two copies.

I guess parse-options could provide a MAYBE_PASSTHRU flag. On the first parse_options() call, it would skip over any such options, leaving them in argv. On the second, the caller would tell it to actually parse them.

-Peff