Re: [PATCH 5/5] index: offer advice for unknown index extensions

[Junio C Hamano <gitster@pobox.com>]

Ben Peart <peartben@gmail.com> writes:

>> This message should say something like "Index uses the mandatory %s
>> extension" to clarify and distinguish it from the below. We don't
>> understand the upper-case one either, but the important distinction is
>> that one is mandatory, and the other can be dropped. The two messages
>> should make this clear.
>>
>> Also, having advice() for that case is even more valuable since we have
>> a hard error at this point. So something like:
>>
>>      "This is likely due to the index having been written by a future
>>      version of Git. All-lowercase index extensions are mandatory, as
>>      opposed to optional all-uppercase ones which we'll drop with a
>>      warning if we see them".
>>
>
> I agree that we should have different messages for mandatory and
> optional extensions.  I don't think we should try and educate the end
> user on the implementation detail that git makes lower cases mandatory
> and upper case optional (ie drop the 'All-lowercase..." part).  They
> will never see the lower vs upper case difference and can't do
> anything about it anyway.

I agree that the "warn and continue" message should say "optional"
(meaning: safe to ignore but you would want to take note) while
"cannot continue" message should say something different.

I do not mind a more verbose error message when we saw unknown but
required extension, but unlike the "warn and continue" case, the
program will stop and die with such an error right there, so I am
not sure if it is worth allowing to tone it down by putting some
part of the verbosity behind the advise() mechanism.

>>>   		trace_printf("ignoring %.4s extension\n", ext);
>>> +		if (advice_unknown_index_extension) {
>>> +			warning(_("ignoring optional %.4s index extension"), ext);

So from that point of view, the distinction between this message and this one

>>>   			return error("index uses %.4s extension, which we do not understand",
>>>   				     ext);

is halfway there.  The message needs to anticipate and answer an
end-user reaction: "we do not understand" so what?

I am still puzzled by the insistence of 3/5 and this step that wants
to kill the coalmine canary.  But I am even more puzzled by the
first two steps that want to disable the two optional extensions.

What's so different this time with the new optional extensions?

The other early optional extensions like cache-tree or resolve-undo
were added unconditionally and by definition appeared much earlier
in git-core than any other Git reimplementations.  verbody who
recorded the fact that s/he resolved merge conflicts got REUC, and
we would have given warning when an older Git did not understand
these extensions [*1*].  We knudged users to more modern Git by
preparing the old Gits to warn when there are unknown extensions,
either by upgrading their Git themselves, or by bugging their
toolsmiths.  Nobody complained to propose to rip the messages like
this round.  This series has a strong smell of pushing back by the
toolsmiths who refuse to promptly upgrade to help their users, and
that is why I do not feel entirely happy with this series.


[Footnote]

 *1* A Git that did not understand TREE would have been silent, as
  it was the first extension and that was the first time we became
  aware of the need to warn unknown extensions.