Subject: Re: [PATCH v2 2/2] connect: advertized capability is not a ref

From: Junio C Hamano <[email-masked]>
Date: Wed, 07 Sep 2016 10:10:18 -0700

Jonathan Tan <[email-masked]> writes:

> diff --git a/connect.c b/connect.c
> index 722dc3f..0c2221e 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -165,6 +165,9 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
>  			continue;
>  		}
>  
> +		if (!strcmp(name, "capabilities^{}"))
> +			continue;

While it is true that ignoring this line anywhere in the ref
advertisement is safe, it feels a bit strange to do so, when we know
that it can appear _only_ when there is no other ref advertised.  I
guess you can argue that it is good to be lenient to accept what
others produce, but on the other hand, it can also be argued that
having this among real ref advertisement would be a protocol
violation that we may want to diagnose and prod the other side to
fix their software (but still not fail).

> +
>  		if (!check_ref(name, flags))
>  			continue;
>  		ref = alloc_ref(buffer + GIT_SHA1_HEXSZ + 1);