Hi,
Junio C Hamano wrote:
> Jonathan Nieder <[email protected]> writes:
>> If I share my .gitconfig or .git/config file between multiple machines
>> (or between multiple Git versions on a single machine) and set
>>
>> [protocol]
>> version = 2
>>
>> then running "git fetch" with a Git version that does not support
>> protocol v2 errors out with
>>
>> fatal: unknown value for config 'protocol.version': 2
>>
>> In the spirit of v1.7.6-rc0~77^2~1 (Improve error handling when
>> parsing dirstat parameters, 2011-04-29), it is better to (perhaps
>> after warning the user) ignore the unrecognized protocol version.
>
> I do not agree with the analogy at all.
>
> Showing dirstat with different tweaks than the user expected to see
> is a local and read-only thing. Talking to the other side over a
> protocol the user explicitly wanted to avoid (e.g. imagine the case
> where your upstream's protocol version 1 implementation is
> critically buggy and you want to use version 2 if you talk with
> them) by accident is a more grave error, possibly affecting the
> other side that you may not have enough power to recover from
> (e.g. damaging the remote repository to which you only have push
> access and not interactive shell).
Fair enough about the analogy being a poor one.
I disagree with characterizing using protocol v0 as a grave error,
because the scenario seems far-fetched to me. I can imagine the
opposite scenario, wanting to use protocol v0 because a server's
implementation of v2 is buggy (for example, producing wrong
negotiation results and wasting bandwidth, or erroring out for a
request that should not be an error). I am having trouble imagining a
broken v0 implementation doing anything worse than being slow or
erroring out.
That said, erroring out to catch spelling errors is nice and helpful,
so I would be okay with continuing to apply this as a local patch and
it not landing upstream.
The following third way would also be possible, but I'm pretty sure I
don't like it:
- As Duy suggested, allowing multiple 'protocol.version' settings.
The last setting that the Git implementation recognizes wins.
- If there is at least one 'protocol.version' setting but the Git
implementation doesn't recognize any of them, error out.
The reason I don't like it is that it doesn't make your example case
work significantly better. If I have
[protocol]
version = 1
in my ~/.gitconfig and
[protocol]
version = v2
in .git/config, then that means I intended to use protocol v2 and
misspelled it, but this heuristic would ignore the v2 and fall back to
version 1.
As a side note, the exact same problem would happen if I make a typo
in the config key:
[protocol]
vesion = 2
Treating unrecognized values from the growing set of values as an
error while not treating unrecognized keys from the growing set of
keys as an error feels odd to me. I think it would be useful to
document whatever we decide about this subject in
Documentation/CodingGuidelines.
Thanks,
Jonathan
