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

From: Junio C Hamano <[email-masked]>
Date: Mon, 08 Jun 2015 09:16:16 -0700

Max Kirillov <[email-masked]> writes:

> It is an allowed compile-time option to build git without iconv
> support. Resulting build almost always functions correctly, and
> never displays that it is missing anything, but reencode_string_len()
> just never modifies its input.

Correct.

> 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.

> +	if (!same_encoding(in_encoding, out_encoding) && !noiconv_warning_shown) {
> +		warning("Iconv support is disabled at compile time. It is likely that\nincorrect data will be printed or stored in repository.\nConsider using other build for this task.");
> +		noiconv_warning_shown = 1;
> +	}

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.

and this patch is about the latter; I do not think it is reasonable
to give noise only for the latter but not for the former.  The
latter is expected by users who compile out the feature, but the
former is not and deserves the wranings more, so the patch is
backwards in that sense.

Thanks.