Re: [PATCH] transport: report refs only if transport does

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 31, 2018, 21:38 UTC
Message-ID: <xmqqa7q79jcf.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180731192415.GC3372@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 16 quoted lines

> On Mon, Jul 30, 2018 at 03:56:01PM -0700, Jonathan Tan wrote:
>
>> Commit 989b8c4452 ("fetch-pack: put shallow info in output parameter",
>> 2018-06-28) allows transports to report the refs that they have fetched
>> in a new out-parameter "fetched_refs". If they do so,
>> transport_fetch_refs() makes this information available to its caller.
>> 
>> Because transport_fetch_refs() filters the refs sent to the transport,
>> it cannot just report the transport's result directly, but first needs
>> to readd the excluded refs, pretending that they are fetched. However,
>> this results in a wrong result if the transport did not report the refs
>> that they have fetched in "fetched_refs" - the excluded refs would be
>> added and reported, presenting an incomplete picture to the caller.
>
> This part leaves me confused. If we are not fetching them, then why do
> we need to pretend that they are fetched?

What leaves me even more confused is that the entire log message does not make it clear what the end-user observable problem the patch is trying to solve.

Is this "we sometimes follow and sometimes fail to follow refs while fetching"? Does it affect all protocol versions and transports, or only just selected few (and if so which ones)?

In minds of those who reported an issue and wrote the fix, the issue may be fresh, but let's write the commit log message for ourselves 6 months down the road.

Thanks.