Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 8, 2016, 18:39 UTC
Message-ID: <xmqqshvkkm86.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160708175821.GA29326@google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 15 quoted lines

> Hi,
>
> Junio C Hamano wrote:
>> Stefan Beller <sbeller@google.com> writes:
>
>>>> More importantly, if we plan to make this configurable and not make
>>>> the limit a hardwired constant of the wire protocol, it may be
>>>> better to advertise push-options capability with the limit, e.g.
>>>> "push-options=32" (or even "push-options=1024/32"), so that the
>>>> client side can count and abort early?
>
> Sorry to butt into the conversation late, but: I am not yet convinced.
>
> Is the idea that if the push options were very large, this would save
> the client from the cost of sending them?

Not really. I have no strong opinion on the benefit of limiting number/size. Stefan limited the number/size at the receiving end and made receiving end die with its message. I was merely trying to tweak the arrangement so that the sending end can complain with its own message, possibly in its own language, especially because it was unclear to me if the die() message on the receiving end would always go back to the sending end correctly.

Show 7 quoted lines

> But this comes with a
> downside: the server doesn't get to send an error message about
> where 
> the maximum number of push options can come from (e.g., with a link to
> a page where the limit can be adjusted, or with an explanation of when
> clients tend to run into this problem and what they should do
> instead).

Hmm, interesting point. That would be better told by the receiving end, as the way to configure it (if offered) would be different from installation to installation.

Show 11 quoted lines

> So I'd like to propose an alternative. What if the client tells the
> server the number of push options early on (and possibly also a cap on
> the length of those push options)?  That way, the client doesn't have
> to waste bit sending the push options but the server gets an
> opportunity to send a helpful error message on sideband 3.
>
> 	server> HEAD\0push-options ...
> 	client> ... commands ...
> 	client> push-options 2
> 	client> my-first-option
> 	client> my-second-option