{"thread":{"id":"29311","subject":"[PATCH][RFC] git on Mac OS and precomposed unicode","startedAt":"2012-01-07T19:59:18Z","lastAt":"2012-01-09T20:47:07Z","messageCount":5,"participants":["Torsten Bögershausen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"182077","messageId":"201201072059.19103.tboegi@web.de","threadId":"29311","inReplyTo":null,"subject":"[PATCH][RFC] git on Mac OS and precomposed unicode","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-01-07T19:59:18Z","receivedAt":"2012-01-07T19:59:18Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Purpose:\nThis patch is a suggestion to work around the unpleasenties\nwhen Mac OS is decomposing unicode filenames.\n\nThe suggested change:\na) is only used under Mac OS\nb) can be switched off by a configuration variable\nc) is optimized to handle ASCII only filename\nd) will improve the interwork between Mac OS, Linux and Windows*\n   via git push/pull, using USB sticks (technically speaking VFAT)\n   or mounted network shares using samba.\n\n* (Not all Windows versions support UTF-8 yet:\n   Msysgit needs the unicode branch, cygwin supports UTF-8 since 1.7)\n\n\nRuntime configuration:\nA new confguration variable is added: \"core.precomposedunicode\"\nThis variable is only used on Mac OS.\nIf set to false, git behaves exactly as older versions of git.\nWhen a new git version is installed and there is a repository\nwhere the configuration \"core.precomposedunicode\" is not present,\nthe new git is backward compatible.\n\nWhen core.precomposedunicode=true, all filenames are stored in precomposed\nunicode in the index (technically speaking precomposed UTF-8).\nEven when readdir() under Mac OS returns filenames as decomposed.\n\nImplementation:\nTwo files are added to the \"compat\" directory, darwin.h and darwin.c.\nThey implement basically 3 new functions:\ndarwin_opendir(), darwin_readdir() and darwin_closedir().\n\n\nCompile time configuration:\nA new compiler option PRECOMPOSED_UNICODE is introduced in the Makefile,\nso that the patch can be switched off completely at compile time.\n\nNo decomposed file names in a git repository:\nIn order to prevent that ever a file name in decomposed unicode is entering\nthe index, a \"brute force\" attempt is taken:\nall arguments into git (technically argv[1]..argv[n]) are converted into\nprecomposed unicode.\nThis is done in git.c by calling argv_precompose() for all commands\nexcept \"git commit\".\nThis function is actually a #define, and it is only defined under Mac OS.\nNothing is converted on any other OS.\n\nImplementation details:\nThe main work is done in darwin_readdir() and argv_precompose().\nThe conversion into precomposed unicode is done by using iconv,\nwhere decomposed is denoted by \"UTF-8-MAC\" and precomposed is \"UTF-8\".\nWhen already precomposed unicode is precomposed, the string is returned\nunchanged.\n\nThread save:\nSince there is no need for argv_precompose()to be thread-save, one iconv\ninstance is created at the beginning and kept for all conversions.\nEven readdir() is not thread-save, so that darwin_opendir() will call\niconv_open() once and keep the instance for all calls of darwin_readdir()\nuntil darwin_close() is called.\n\nAuto sensing:\nWhen creating a new git repository with \"git init\" or \"git clone\", the\n\"core.precomposedunicode\" will be set automatically to \"true\" or \"false\".\n\nTypically core.precomposedunicode is \"true\" on HFS and VFAT.\nIt is even true for file systems mounted via SAMBA onto a Linux box,\nand \"false\" for drives mounted via NFS onto a Linux box.\n\n\nNew test case:\nThe new t3910-mac-os-precompose.sh is added to check if a filename\ncan be reached either in precomposed or decomposed unicode (NFC or NFD).\n\n\n Documentation/config.txt     |    9 ++\n Makefile                     |    3 +\n builtin/init-db.c            |   22 +++++\n compat/darwin.c              |  200 ++++++++++++++++++++++++++++++++++++++++++\n compat/darwin.h              |   31 +++++++\n git-compat-util.h            |    8 ++\n git.c                        |    1 +\n t/t0050-filesystem.sh        |    1 +\n t/t3910-mac-os-precompose.sh |  104 ++++++++++++++++++++++\n 9 files changed, 379 insertions(+), 0 deletions(-)\n create mode 100644 compat/darwin.c\n create mode 100644 compat/darwin.h\n create mode 100755 t/t3910-mac-os-precompose.sh\n\n-- \n1.7.8.rc0.43.gb49a8\n"},{"id":"182082","messageId":"7vboqehpxm.fsf@alter.siamese.dyndns.org","threadId":"29311","inReplyTo":"201201072059.19103.tboegi@web.de","subject":"Re: [PATCH][RFC] git on Mac OS and precomposed unicode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-08T02:46:13Z","receivedAt":"2012-01-08T02:46:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> Implementation:\n> Two files are added to the \"compat\" directory, darwin.h and darwin.c.\n> They implement basically 3 new functions:\n> darwin_opendir(), darwin_readdir() and darwin_closedir().\n\nI haven't looked at the patch yet but that sounds exactly the right way to\ngo about this. Nice.\n\n> No decomposed file names in a git repository:\n> In order to prevent that ever a file name in decomposed unicode is entering\n> the index, a \"brute force\" attempt is taken:\n> all arguments into git (technically argv[1]..argv[n]) are converted into\n> precomposed unicode.\n\nThat also sounds sensible, but...\n\n> This is done in git.c by calling argv_precompose() for all commands\n> except \"git commit\".\n\n... I think it generally is a bad idea to say \"all except foo\". There may\nbe a reason why \"foo\" happens to be special in today's code, but who says\nthere won't be another command \"bar\" that shares the same reason with\n\"foo\" to be treated specially? Or depending on the options, perhaps some\ncodepath of \"foo\" may not want the special casing and want to go through\nthe argv_precompose(), no?\n\nAfter all, \"git commit -- pathspec\" will have to get the pathspec from the\ncommand line, and match them against the paths in the index, the latter of\nwhich you are keeping in the canonical form, so you would want the argv[]\nalso be in the same form, and applying your argv_precompose() would be a\nsensible way to do so, no?\n\nI would also suspect that the cleanest way to implement it is to replace\nthe main() entry point (see how compat/mingw.h does this).\n"},{"id":"182171","messageId":"4F0B196B.8010904@web.de","threadId":"29311","inReplyTo":"7vboqehpxm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH][RFC] git on Mac OS and precomposed unicode","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-01-09T16:44:27Z","receivedAt":"2012-01-09T16:44:27Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 08.01.12 03:46, Junio C Hamano wrote:\n> Torsten Bögershausen <tboegi@web.de> writes:\n> \n>> Implementation:\n>> Two files are added to the \"compat\" directory, darwin.h and darwin.c.\n>> They implement basically 3 new functions:\n>> darwin_opendir(), darwin_readdir() and darwin_closedir().\n> \n> I haven't looked at the patch yet but that sounds exactly the right way to\n> go about this. Nice.\n> \n>> No decomposed file names in a git repository:\n>> In order to prevent that ever a file name in decomposed unicode is entering\n>> the index, a \"brute force\" attempt is taken:\n>> all arguments into git (technically argv[1]..argv[n]) are converted into\n>> precomposed unicode.\n> \n> That also sounds sensible, but...\n> \n>> This is done in git.c by calling argv_precompose() for all commands\n>> except \"git commit\".\n> \n> ... I think it generally is a bad idea to say \"all except foo\". There may\n> be a reason why \"foo\" happens to be special in today's code, but who says\n> there won't be another command \"bar\" that shares the same reason with\n> \"foo\" to be treated specially? Or depending on the options, perhaps some\n> codepath of \"foo\" may not want the special casing and want to go through\n> the argv_precompose(), no?\n> \n> After all, \"git commit -- pathspec\" will have to get the pathspec from the\n> command line, and match them against the paths in the index, the latter of\n> which you are keeping in the canonical form, so you would want the argv[]\n> also be in the same form, and applying your argv_precompose() would be a\n> sensible way to do so, no?\n\nThanks Junio for catching this.\nI added a new test case as well as fixed the code.\n\n> I would also suspect that the cleanest way to implement it is to replace\n> the main() entry point (see how compat/mingw.h does this).\n\nWe only need to that argv conversion in git.c, (and not in daemon.c), so I sticked\nto the old model for V1.\nI send a new patch soon\n/Torsten\n"},{"id":"182174","messageId":"7vty44eksp.fsf@alter.siamese.dyndns.org","threadId":"29311","inReplyTo":"4F0B196B.8010904@web.de","subject":"Re: [PATCH][RFC] git on Mac OS and precomposed unicode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-09T19:29:58Z","receivedAt":"2012-01-09T19:29:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> On 08.01.12 03:46, Junio C Hamano wrote:\n> ...\n>> That also sounds sensible, but...\n>> \n>>> This is done in git.c by calling argv_precompose() for all commands\n>>> except \"git commit\".\n>> \n>> ... I think it generally is a bad idea to say \"all except foo\". There may\n>> be a reason why \"foo\" happens to be special in today's code, but who says\n>> there won't be another command \"bar\" that shares the same reason with\n>> \"foo\" to be treated specially? Or depending on the options, perhaps some\n>> codepath of \"foo\" may not want the special casing and want to go through\n>> the argv_precompose(), no?\n>> \n>> After all, \"git commit -- pathspec\" will have to get the pathspec from the\n>> command line,...\n>\n> Thanks Junio for catching this.\n> I added a new test case as well as fixed the code.\n\nI think you are sidestepping the real issue I raised, which is:\n\n    What is the reason why you do not want to feed the precompose helper\n    with some arguments to 'git commit', while it is OK to pass all\n    arguments to other commands through precomposition?\n\nI admit it was my fault that I did not spell it out clearly in my\nresponse.\n\nI understand that arguments other than pathspec and revs could be left in\ndecomposed form, but is there any harm in canonicalizing any and all\ncommand line parameters given in decomposed form consistently into\nprecomposed form? What problem are you trying to solve by special casing\n\"git commit\"? That is the real question to be answered, as there may be\nother commands some of whose arguments may not want to be canonicalized\ndue to the same reason, but you simply overlooked them. When other people\nneed to fix that oversight, they need a clearly written criterion what\nkind of arguments should not be fixed and why.\n\nAnd the reason cannot be a desire to pass the value to \"--message\"\nargument intact [*1*]; it is not like osx cannot handle text in\nprecomposed form, right?\n\nIn general, I do not want to see ugly code that says \"this one potentially\nnames a path so we add a call to fix it from decomposed form, but that\nother one is not a path and we take it intact\" sprinkled all over in the\ncodebase, without a good reason.\n\nIt may seem that one alternative to munging argv[] is to have the\nprecomposition [*2*] applied inside get_pathspec() and have it take effect\nonly on the pathspecs, which after all ought to be the only place where\nthis matters, but I doubt it would result in maintainable code. The names\nof branches and tags taken from the command line that are used as revision\nnames will also be compared with results from readdir in $GIT_DIR/refs/\nand need to be canonicalized, for example. So I tend to agree with your\n\"brute force\" approach to canonicalize argv[] before any real part of git\nsees them; that is where my suggestion to wrap main() to do so came from.\n\nAlso some commands (e.g. \"rev-list --stdin\") take pathspecs and revs from\ntheir standard input stream, so you would need to be careful about them.\n\n\n[Footnotes]\n\n*1* Also as other commands like \"git merge\" also take textual message, and\nyou do pass the helper to canonicalize it.  No, I am not suggesting you to\nspecial case \"git merge\".\n\n*2* By the way, this may need a better name if the patch touches anywhere\noutside compat/osx --- it is about \"canonicalize pathname and pathspec\ngiven from the command line into the form used internally by git\", and\nfrom an osx person's point of view, the only difference might be\ndecomposed vs precomposed, but on other odd systems it might be that\npathnames on the filesystem may be using a different encoding from what is\nused for pathnames in the index).\n"},{"id":"182181","messageId":"4F0B524B.8090203@web.de","threadId":"29311","inReplyTo":"7vty44eksp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH][RFC] git on Mac OS and precomposed unicode","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-01-09T20:47:07Z","receivedAt":"2012-01-09T20:47:07Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 01/09/2012 08:29 PM, Junio C Hamano wrote:\n> Torsten Bögershausen<tboegi@web.de>  writes:\n>\n>> On 08.01.12 03:46, Junio C Hamano wrote:\n>> ...\n>>> That also sounds sensible, but...\n>>>\n>>>> This is done in git.c by calling argv_precompose() for all commands\n>>>> except \"git commit\".\n>>>\n>>> ... I think it generally is a bad idea to say \"all except foo\". There may\n>>> be a reason why \"foo\" happens to be special in today's code, but who says\n>>> there won't be another command \"bar\" that shares the same reason with\n>>> \"foo\" to be treated specially? Or depending on the options, perhaps some\n>>> codepath of \"foo\" may not want the special casing and want to go through\n>>> the argv_precompose(), no?\n>>>\n>>> After all, \"git commit -- pathspec\" will have to get the pathspec from the\n>>> command line,...\n>>\n>> Thanks Junio for catching this.\n>> I added a new test case as well as fixed the code.\n>\n> I think you are sidestepping the real issue I raised, which is:\n>\n>      What is the reason why you do not want to feed the precompose helper\n>      with some arguments to 'git commit', while it is OK to pass all\n>      arguments to other commands through precomposition?\n>\n> I admit it was my fault that I did not spell it out clearly in my\n> response.\n>\n> I understand that arguments other than pathspec and revs could be left in\n> decomposed form, but is there any harm in canonicalizing any and all\n> command line parameters given in decomposed form consistently into\n> precomposed form? What problem are you trying to solve by special casing\n> \"git commit\"? That is the real question to be answered, as there may be\n> other commands some of whose arguments may not want to be canonicalized\n> due to the same reason, but you simply overlooked them. When other people\n> need to fix that oversight, they need a clearly written criterion what\n> kind of arguments should not be fixed and why.\n>\n> And the reason cannot be a desire to pass the value to \"--message\"\n> argument intact [*1*]; it is not like osx cannot handle text in\n> precomposed form, right?\n\nThe short answer for treating \"git commit\" special:\n   The test suite didn't pass any more. (t4201-shortlog.sh)\n   This seems more and more to be a bad excuse...\nThe long answer:\n   I have to look into that more deeply.\n\nThanks for your replies.\n/Torsten\n\n     (And yes, Mac OS can handle precomposed unicode (at least the\n      western european code points))\n\n[snip]\n"}]}