Re: [PATCH] protocol: treat unrecognized protocol.version setting as 0

Hi,

Junio C Hamano wrote:

Jonathan Nieder [off-list ref] writes:

quoted

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