Re: [RFC 1/8] UTF helpers

From: Junio C Hamano <gitster@pobox.com>
Date: May 14, 2009, 04:38 UTC
Message-ID: <7viqk4z4cv.fsf@alter.siamese.dyndns.org>
In-Reply-To: <200905130724.44634.robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> writes:

Show 9 quoted lines

>> This is ugly.
>
> I told you so. No news.
>
>> Okay, I'll stop here.  You might want to clean up your patch series before 
>> resending.
>
> I also told you why why I stopped working on the patches. The patches are not part of
> a beauty contest and not meant for inclusion as such.

It is rather sad; I suspect that the core of the series is buried in too much cruft deep enough to discourage many potential reviewers. I think the entire series look incoherent because attacking two largely unrelated things at once.

 (1) Normalizing pathnames internally to UTF-8 and possibly convert it
     back to native upon use (e.g. creat(), lstat(), unlink()) and output.
     As Linus analyzed, this shouldn't be done too early in the callchain
     for performance reasons, but I think your patch would give us a good
     set of starting points to follow where the result from readdir(),
     user input and other things that are pathnames come from and go.

     This part of the patch series was inspiring.  You have to worry about
     gitignore, gitattributes and readlink() vs contents of a blob object
     that records a symbolic link values, which I think either escaped
     analysis people have done so far or being ignored as a small detail,
     but they are important;

 (2) Passing cat-file output through iconv to convert it.

     I think this is unwarranted, even if the object given to cat-file
     happens to be a commit or a tag object and you want to convert their
     messages in native encoding.

     I am not sure what should happen to "cat-file tree", "ls-files" and
     "ls-tree".  The output from these plumbing does show pathnames, but I
     tend to think it is Porcelain's job to turn them into whatever
     encoding they want to use.  So are input to "update-index --stdin",
     but I am still just thinking out loud.