Re: [PATCH 1/7] pack-protocol.txt: Add warning about protocol inaccuracies

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 1, 2015 at 12:39 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> Hi,
>
> Dave Borowitz wrote:
>
>> --- a/Documentation/technical/pack-protocol.txt
>> +++ b/Documentation/technical/pack-protocol.txt
>> @@ -14,6 +14,17 @@ data.  The protocol functions to have a server tell a client what is
>>  currently on the server, then for the two to negotiate the smallest amount
>>  of data to send in order to fully update one or the other.
>>
>> +Notes to Implementors
>> +---------------------
>> +
>> +WARNING: This document is a work in progress. Some of the protocol
>> +specifications below may be incomplete or inaccurate. When in doubt,
>> +refer to the C code.
>
> If we include this warning, can it also say to contact
> git@xxxxxxxxxxxxxxx for inaccuracies?
>
> Otherwise it is easy to misread as "Some of this document may be
> inaccurate, and we're working on fixing that.  Don't bug me --- I
> already know about the problems --- just be patient!"
>
> I would rather that it would say something like
>
>         Caveat
>         ------
>         This document contains some inaccuracies.  Do not forget to also
>         check against the C code, and please contact git@xxxxxxxxxxxxxxx
>         if you run into any unclear or inaccurate passages in this spec.

Agreed with your rationale and suggested wording, thanks.

>> +
>> +One particular example is that many of the LFs referenced in the
>> +specifications are optional, but may (yet) not be marked as such. If not
>> +explicitly marked one way or the other, double-check with the C code.
>
> The 'Reference Discovery' section says:
>
>         Server SHOULD terminate each non-flush line using LF ("\n") terminator;
>         client MUST NOT complain if there is no terminator.
>
> Unfortunately that's not explained in a section with broader scope.
>
> Isn't that actually the rule everywhere except for in push certs?

It's certainly the rule in more places. I personally have partially
audited send-pack.c, but there are many places that are not yet
audited. I don't feel comfortable making a broader claim without
having done so, and I do not have the time to do so at the moment.

> The documentation will be easier to use if it says so instead of
> asking implementers to check the source of all implementations
> (since interoperability with only one isn't enough).

Unfortunately, the trust I have in this document at this point is less
than zero. A handful of spot fixes, while useful, does not serve as
any sort of assurance that we've gotten all or even most of the
problems. Until such time as this document is actually reliable,
implementors must check the source of all implementations if they want
to be accurate. That sucks for implementors (believe me, I know), but
it's the truth.

> Thanks,
> Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]