Re: [PATCH 5/6] fetch: teach independent negotiation (no packfile)

[Derrick Stolee <stolee@gmail.com>]

On 5/3/21 11:25 AM, Derrick Stolee wrote:
> On 4/8/21 9:10 PM, Jonathan Tan wrote:
>> Currently, the packfile negotiation step within a Git fetch cannot be
>> done independent of sending the packfile, even though there is at least
>> one application wherein this is useful. Therefore, make it possible for
>> this negotiation step to be done independently. A subsequent commit will
>> use this for one such application - push negotiation.
> ...
>> diff --git a/transport-helper.c b/transport-helper.c
>> index 4cd76366fa..4be035edb8 100644
>> --- a/transport-helper.c
>> +++ b/transport-helper.c
>> @@ -684,6 +684,16 @@ static int fetch(struct transport *transport,
>>  		return transport->vtable->fetch(transport, nr_heads, to_fetch);
>>  	}
>>  
>> +	/*
>> +	 * If we reach here, then the server, the client, and/or the transport
>> +	 * helper does not support protocol v2. --negotiate-only requires
>> +	 * protocol v2.
>> +	 */
>> +	if (data->transport_options.acked_commits) {
>> +		warning(_("--negotiate-only requires protocol v2"));
>> +		return -1;
>> +	}
>> +
> 
> This method continues to do a lot that doesn't seem specific to
> --negotiate-only. The warning message seems incorrect to me, but
> is also seems like this would break several cases when using
> protocol v0. It is equally possible that I'm misunderstanding
> what is going on here.

I didn't see this cause any failures when running the test suite
with GIT_TEST_PROTOCOL_VERSION=0, but I _do_ see failures in your
tests added in PATCH 6 with that environment variable set. You
might want to set it manually in the important test cases.

Thanks,
-Stolee