Re: [PATCH] utf8.c: print warning about disabled iconv

From: Max Kirillov <max@max630.net>
Date: Jun 8, 2015, 21:07 UTC
Message-ID: <20150608210705.GA22850@wheezy.local>
In-Reply-To: <xmqqfv62ch0v.fsf@gitster.dls.corp.google.com>

Hi.

On Mon, Jun 08, 2015 at 09:16:16AM -0700, Junio C Hamano wrote:

> Max Kirillov <max@max630.net> writes:

Show 9 quoted lines

>> This gives undesirable result that returned data or even
>> data written into repository is incorrect and user is not
>> aware about it.
> 
> I do not necessarily agree with that.  The user knows what
> s/he is doing, data written to or shown from the
> repository is correct as far as the user is concerned, and
> the user takes the full respoinsibility when compiling out
> certain features.

User, in theory, can be not the same person who builds, or can be not aware that the case needs recoding. It actually started when I compiled git without iconv support and got about 10 failed tests, and only 2 of them mentioned i18n in their name.

Compiling out other features is not exactly the same. If user compiles out curl, for example, git will not be able to push or fetch through http, but it is not going to pretend to be working, it will fail visibly.

Show 6 quoted lines

> I actually am OK if the user gets exactly the same warning between
> the two cases:
> 
>  - iconv failed to convert in the real reencode_string_len()
> 
>  - we compiled out iconv() and real conversion was asked.

Does 'exactly the same' mean the same text? Shouldn't it describe the reason? I can see 2 possible failures in case of real iconv: unknown or unsupported encoding and invalid input. Wouldn't them better to be detailed in warning?

-- 
Max