Re: [PATCHv5 1/2] clone: respect additional configured fetch refspecs during initial fetch

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 22, 2017, 22:23 UTC
Message-ID: <xmqqmv8zhdap.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170617112228.vugswym4o4owf6wj@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 12 quoted lines

> I'd still prefer this to have:
>
>   if (!remote->fetch && remote->fetch_refspec_nr)
> 	BUG("attempt to add refspec to uninitialized list");
>
> at the top, as otherwise this case writes garbage into remote->fetch[0].
>
> I see you have another series dealing with the lazy parsing, but I
> haven't looked at it yet (hopefully this danger would just go away after
> that).
>
> Other than that, the patch looks fine to me.

SZEDER? As long as the end result together with two series are safe, I do not have a strong preference, but given that the other one is a lot more invasive change [*1*], I think it is nicer to have this two-patch series already safe without the other one.

What's your take on Peff's point?

[Footnote]

*1* Especially the other branch does not merge cleanly into 'pu' and
    I haven't managed to include it in my tree yet.