Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 1, 2015, 20:49 UTC
Message-ID: <xmqqzj3f7gde.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAD0k6qSN9=afCe3RumJPfP9JERy1w+tAYdjq01MsQnsOjdKu3A@mail.gmail.com>

Dave Borowitz <dborowitz@google.com> writes:

Show 6 quoted lines

>> I am moderately negative about this; wouldn't it make the end result
>> cleaner to fix the implementation?
>
> I'm not sure I understand your suggestion. Are you saying, you would
> prefer to make LFs optional in the push cert, for consistency with LFs
> being optional elsewhere?

Absolutely. It is not "make" it optional, but "even though it is optional, the receiver has not been following the spec, and it is not too late to fix it".

The earliest these documentation updates can hit the public is 2.6; by that time I'd expect the deployed receivers would be fixed with 2.5.1 and 2.4.7 maintenance releases.

If some third-party reimplemented their client not to terminate with LF, they wouldn't be working correctly with the deployed servers right now *anyway*. And with the more lenient receive-pack in 2.5.1 or 2.4.7, they will start working.

And we will not change our client to drop LF termination. So overall I do not see that it is too much a price to pay for consistency across the protocol.

> If LF is optional, then with that approach you might end up with a
> section of that buffer like:

I think I touched on this in my previous message. You cannot send an empty line anywhere, and this is not limited to push-cert section of the protocol. Strictly speaking, the wire level allows it, but I do not think the deployed client APIs easily lets you deal with it.

So you must follow the "SHOULD terminate with LF" for an empty line, even when you choose to ignore the "SHOULD" in most other places.

I do not think it is such a big loss, as long as it is properly documented.