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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 8, 2015, 21:14 UTC
Message-ID: <xmqqoakp9a2f.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20150608210705.GA22850@wheezy.local>

Max Kirillov <max@max630.net> writes:

> User, in theory, can be not the same person who builds, or
> can be not aware that the case needs recoding.

Because you can pretty much say the same for build with iconv enabled, I think that line of argument is futile. The users do not have control over platform's iconv (and sysadmin's choice of locale packages) what charset/encoding can be converted to what other ones.

Show 8 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?

No, I was trying to point out the total lack of corresponding warnings in the iconv-enabled build.

After all, if you had to convert between UTF-8 and ISO-2022-JP, the latter of which your system does not support, whether you use iconv-disabled build of Git or iconv-enabled build of Git, we pass the bytestream through, right? Your patch gives warning for the former (which is a good starting point if we want to warn "user expected them to be converted, we didn't" case) but does not do anything to the latter, even though users of the iconv-disabled build is more likely to be aware of the potential issue (and are likely to be willing to accept that) than the ones with iconv-enabled build that runs on a sysmet that cannot convert the specific encoding.