Re: [PATCH v2] macos: lazily initialize iconv

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 31, 2012, 20:16 UTC
Message-ID: <7vk3xjit4h.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CA+55aFwE93YeVjZp9VLhRvbxFJNonafmUE6rHzPer5hv-hON5Q@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

Show 16 quoted lines

> On Tue, Jul 31, 2012 at 11:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>  * This is not even compile tested, so it needs testing and
>>    benchmarking, as I do not even know how costly the calls to
>>    open/close are when we do not have to call iconv() itself.
>
> Ok, so it's easily compile-tested: just add
>
> +       COMPAT_OBJS += compat/precompose_utf8.o
> +       BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
>
> to the makefile for Linux too.
>
> Actually testing how well it *works* is hard, since I don't really
> have a mac (well, I do, but it no longer has OS X on it ;), and the
> whole "utf-8-mac" thing does not make sense.

Also the motivation for this change (not the original utf-8-mac one, which is not my code) is about not paying unnecessary iconv_open() overhead when we do not have to, so the measurement has to happen on Mac, not on Linux.

Show 5 quoted lines

> HOWEVER. I actually tested it with the conversion being from Latin1 to
> UTF-8 instead, and it does interesting things, and kind of works. I
> say "kind of", because for the case of the filesystem being in Latin1,
> we actually have to convert things back to the filesystem character
> set ...

Eek.

Not just write_entry() codepath that creates the final paths on the filesystem, you would need to touch lstat() calls that check the existence and freshness of the path, once you go that route. I am sure such a change can be made to work, but I am not sure how much we would gain from one.