{"thread":{"id":"61429","subject":"[PATCH v1 2/2] strbuf_getcwd() needs precompse_strbuf_if_needed()","startedAt":"2024-05-07T08:44:45Z","lastAt":"2024-06-04T01:05:19Z","messageCount":24,"participants":["tboegi@web.de","Junio C Hamano","brian m. carlson","Torsten Bögershausen","Jun. T","Jun T"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"494263","messageId":"20240507084431.19797-1-tboegi@web.de","threadId":"61429","inReplyTo":"20240430032717281.IXLP.121462.mail.biglobe.ne.jp@biglobe.ne.jp","subject":"[PATCH v1 2/2] strbuf_getcwd() needs precompse_strbuf_if_needed()","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2024-05-07T08:44:31Z","receivedAt":"2024-05-07T08:44:45Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen <tboegi@web.de>\n\nWhen running under macOs a call to strbuf_getcwd() may return\ndecomposed unicode.\nThis could make `git ls-files` fail, see previous commit of t0050\n\nThe solution is to precompose the result of getcwd() if needed.\nOne possible implementation would be to re-define getcwd() similar\nto opendir(), readdir() and closedir().\nSince there is already a strbuf wrapper around getcwd(), and only this\nwrapper is used inside the whole codebase, equip strbuf_getcwd() with\na call to the newly created function precompse_strbuf_if_needed().\nNote that precompse_strbuf_if_needed() is a function under macOs,\nand is a \"nop\" on all other systems.\n\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n compat/precompose_utf8.c | 11 +++++++++++\n compat/precompose_utf8.h |  1 +\n git-compat-util.h        |  2 ++\n strbuf.c                 |  1 +\n 4 files changed, 15 insertions(+)\n\ndiff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c\nindex 0bd5c24250..82ec2a1c5b 100644\n--- a/compat/precompose_utf8.c\n+++ b/compat/precompose_utf8.c\n@@ -94,6 +94,17 @@ const char *precompose_string_if_needed(const char *in)\n \treturn in;\n }\n\n+void precompse_strbuf_if_needed(struct strbuf *sb)\n+{\n+\tchar *buf_prec = (char *)precompose_string_if_needed(sb->buf);\n+\tif (buf_prec != sb->buf) {\n+\t\tsize_t buf_prec_len = strlen(buf_prec);\n+\t\tfree(strbuf_detach(sb, NULL));\n+\t\tstrbuf_attach(sb, buf_prec, buf_prec_len, buf_prec_len + 1);\n+\t}\n+\n+}\n+\n const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)\n {\n \tint i = 0;\ndiff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h\nindex fea06cf28a..fb17b1bd4a 100644\n--- a/compat/precompose_utf8.h\n+++ b/compat/precompose_utf8.h\n@@ -30,6 +30,7 @@ typedef struct {\n\n const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix);\n const char *precompose_string_if_needed(const char *in);\n+void precompse_strbuf_if_needed(struct strbuf *sb);\n void probe_utf8_pathname_composition(void);\n\n PREC_DIR *precompose_utf8_opendir(const char *dirname);\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ca7678a379..80ae463410 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -344,6 +344,8 @@ static inline const char *precompose_string_if_needed(const char *in)\n \treturn in;\n }\n\n+#define precompse_strbuf_if_needed(a)\n+\n #define probe_utf8_pathname_composition()\n #endif\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 0d929e4e19..cefea6b75f 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -591,6 +591,7 @@ int strbuf_getcwd(struct strbuf *sb)\n \tfor (;; guessed_len *= 2) {\n \t\tstrbuf_grow(sb, guessed_len);\n \t\tif (getcwd(sb->buf, sb->alloc)) {\n+\t\t\tprecompse_strbuf_if_needed(sb);\n \t\t\tstrbuf_setlen(sb, strlen(sb->buf));\n \t\t\treturn 0;\n \t\t}\n--\n2.41.0.394.ge43f4fd0bd\n\n"},{"id":"494264","messageId":"20240507084429.19781-1-tboegi@web.de","threadId":"61429","inReplyTo":"20240430032717281.IXLP.121462.mail.biglobe.ne.jp@biglobe.ne.jp","subject":"[PATCH v1 1/2] t0050: ls-files path fails if path of workdir is NFD","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2024-05-07T08:44:29Z","receivedAt":"2024-05-07T08:44:45Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen <tboegi@web.de>\n\nAdd a test case for this bug report, slightly edited and shortened:\n\nls-files path' fails if absolute path of workdir contains NFD (macOS)\nOn macOS, 'git ls-files path' does not work (gives an error)\nif the absolute 'path' contains characters in NFD (decomposed).\nI guess this is a (minor) bug of git.\n\n$ cd /somewhere         # some safe place, /tmp or ~/tmp etc.\n$ mkdir $'u\\xcc\\x88'    # ü in NFD\n$ cd ü                  # or cd $'u\\xcc\\x88' or cd $'\\xc3\\xbc'\n$ git init\n$ git ls-files $'/somewhere/u\\xcc\\x88'   # NFD\n  fatal: /somewhere/ü: '/somewhere/ü' is outside repository at '/somewhere/ü'\n$ git ls-files $'/somewhere/\\xc3\\xbc'    # NFC\n(the same error as above)\n\nIn the 'fatal:' error message, there are three ü;\nthe 1st and 2nd are in NFC, the 3rd is in NFD.\n\nThe added test case here follows the error description,\nwith the exception that the 'ü' is replaced by an 'ä',\nwhich we already have as NFD and NFC in t0050.\nA fix will be done in the next commit.\n\nReported-by: Jun T <takimoto-j@kba.biglobe.ne.jp>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n t/t0050-filesystem.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh\nindex 325eb1c3cd..bb85ec38cb 100755\n--- a/t/t0050-filesystem.sh\n+++ b/t/t0050-filesystem.sh\n@@ -156,4 +156,16 @@ test_expect_success CASE_INSENSITIVE_FS 'checkout with no pathspec and a case in\n \t)\n '\n\n+test_expect_success 'git ls-files under NFD' '\n+\t(\n+\t\tmkdir somewhere &&\n+\t\tmkdir somewhere/$aumlcdiar &&\n+\t\tmypwd=$PWD &&\n+\t\tcd somewhere/$aumlcdiar &&\n+\t\tgit init &&\n+\t\tgit ls-files \"$mypwd/somewhere/$aumlcdiar\"  2>err &&\n+\t\t>expected &&\n+\t\ttest_cmp expected err\n+\t)\n+'\n test_done\n--\n2.41.0.394.ge43f4fd0bd\n\n"},{"id":"494282","messageId":"xmqqa5l1pmf9.fsf@gitster.g","threadId":"61429","inReplyTo":"20240507084431.19797-1-tboegi@web.de","subject":"Re: [PATCH v1 2/2] strbuf_getcwd() needs precompse_strbuf_if_needed()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-07T17:22:02Z","receivedAt":"2024-05-07T17:22:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> +void precompse_strbuf_if_needed(struct strbuf *sb)\n> +{\n> +\tchar *buf_prec = (char *)precompose_string_if_needed(sb->buf);\n> +\tif (buf_prec != sb->buf) {\n\nCute.  This matches with the !PRECOMPSE_UNICODE case in git-compat-util.h\nwhere we do\n\n    static inline const char *precompose_string_if_needed(const char *in)\n    {\n            return in;\n    }\n\nto make it a no-op.  I was wondering how you are avoiding an\ninevitable crash from trying to free an unfreeable piece of memory,\nbut this should do just fine.\n\nYou'd want to fix the typo in the name of the new function, I\npresume?  \"precompse\" -> \"precompose\"\n\n> +\t\tsize_t buf_prec_len = strlen(buf_prec);\n> +\t\tfree(strbuf_detach(sb, NULL));\n> +\t\tstrbuf_attach(sb, buf_prec, buf_prec_len, buf_prec_len + 1);\n> +\t}\n> +\n> +}\n\n> diff --git a/strbuf.c b/strbuf.c\n> index 0d929e4e19..cefea6b75f 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -591,6 +591,7 @@ int strbuf_getcwd(struct strbuf *sb)\n>  \tfor (;; guessed_len *= 2) {\n>  \t\tstrbuf_grow(sb, guessed_len);\n>  \t\tif (getcwd(sb->buf, sb->alloc)) {\n> +\t\t\tprecompse_strbuf_if_needed(sb);\n>  \t\t\tstrbuf_setlen(sb, strlen(sb->buf));\n\nThe need for strbuf_setlen() stems from the use of getcwd() that may\nand will place a string that is much shorter than sb->alloc, so they\nlogically belong together.  It will make more sense to call the\nprecompose _after_ arranging the members of strbuf in a consistent\nstate with the call to strbuf_setlen().\n\n>  \t\t\treturn 0;\n>  \t\t}\n> --\n> 2.41.0.394.ge43f4fd0bd\n"},{"id":"494284","messageId":"xmqq1q6dpm1j.fsf@gitster.g","threadId":"61429","inReplyTo":"20240507084429.19781-1-tboegi@web.de","subject":"Re: [PATCH v1 1/2] t0050: ls-files path fails if path of workdir is NFD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-07T17:30:16Z","receivedAt":"2024-05-07T17:30:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> From: Torsten Bögershausen <tboegi@web.de>\n>\n> Add a test case for this bug report, slightly edited and shortened:\n>\n> ls-files path' fails if absolute path of workdir contains NFD (macOS)\n> On macOS, 'git ls-files path' does not work (gives an error)\n> if the absolute 'path' contains characters in NFD (decomposed).\n> I guess this is a (minor) bug of git.\n>\n> $ cd /somewhere         # some safe place, /tmp or ~/tmp etc.\n> $ mkdir $'u\\xcc\\x88'    # ü in NFD\n> $ cd ü                  # or cd $'u\\xcc\\x88' or cd $'\\xc3\\xbc'\n> $ git init\n> $ git ls-files $'/somewhere/u\\xcc\\x88'   # NFD\n>   fatal: /somewhere/ü: '/somewhere/ü' is outside repository at '/somewhere/ü'\n> $ git ls-files $'/somewhere/\\xc3\\xbc'    # NFC\n> (the same error as above)\n>\n> In the 'fatal:' error message, there are three ü;\n> the 1st and 2nd are in NFC, the 3rd is in NFD.\n>\n> The added test case here follows the error description,\n> with the exception that the 'ü' is replaced by an 'ä',\n> which we already have as NFD and NFC in t0050.\n> A fix will be done in the next commit.\n\nThat will break bisection.  I think combining the two commits into\none would make sense for a small change like this, consisting a\nfocused and straight-forward fix plus a clean and concise test.\n\n> Reported-by: Jun T <takimoto-j@kba.biglobe.ne.jp>\n> Signed-off-by: Torsten Bögershausen <tboegi@web.de>\n> ---\n>  t/t0050-filesystem.sh | 12 ++++++++++++\n>  1 file changed, 12 insertions(+)\n>\n> diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh\n> index 325eb1c3cd..bb85ec38cb 100755\n> --- a/t/t0050-filesystem.sh\n> +++ b/t/t0050-filesystem.sh\n> @@ -156,4 +156,16 @@ test_expect_success CASE_INSENSITIVE_FS 'checkout with no pathspec and a case in\n>  \t)\n>  '\n>\n> +test_expect_success 'git ls-files under NFD' '\n> +\t(\n> +\t\tmkdir somewhere &&\n> +\t\tmkdir somewhere/$aumlcdiar &&\n\nWould a single \"mkdir -p\" suffice?\n\n\t\tmkdir -p \"somewhere/$aumlcdiar\" &&\n\n> +\t\tmypwd=$PWD &&\n> +\t\tcd somewhere/$aumlcdiar &&\n> +\t\tgit init &&\n> +\t\tgit ls-files \"$mypwd/somewhere/$aumlcdiar\"  2>err &&\n\nWe do not control what is in \"$mypwd\".  Can it have funny characters\nthat can confuse Git?  Quoting the path with a pair of double quotes\nprotects the shell from getting confused with $IFS whitespaces, but\nwe may want to protect the pathspec handing in Git with something\nlike\n\n\tgit --literal-pathspecs ls-files \"...\" \n\nhere.\n\n> +\t\t>expected &&\n> +\t\ttest_cmp expected err\n> +\t)\n> +'\n>  test_done\n> --\n> 2.41.0.394.ge43f4fd0bd\n"},{"id":"494285","messageId":"xmqqv83po6o2.fsf@gitster.g","threadId":"61429","inReplyTo":"20240507084431.19797-1-tboegi@web.de","subject":"Re: [PATCH v1 2/2] strbuf_getcwd() needs precompse_strbuf_if_needed()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-07T17:47:41Z","receivedAt":"2024-05-07T17:47:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> From: Torsten Bögershausen <tboegi@web.de>\n>\n> When running under macOs a call to strbuf_getcwd() may return\n\nYou spelled it as \"macOS\" in [1/2].  The hits from\n\n    $ git grep -i 'mac *os' \\*.[ch]\n\ntell me that we seem to say \"macOS\", \"MacOS X\", \"Mac OSX\" and \"Mac\nOS X\" pretty much interchangeably.  We may want to eventually\nconsolidate them to whatever the official name Apple uses, but in\nthe meantime let's make sure we do not add even more.\n"},{"id":"494292","messageId":"ZjrIAEq54EVS6yXR@tapette.crustytoothpaste.net","threadId":"61429","inReplyTo":"xmqqv83po6o2.fsf@gitster.g","subject":"Re: [PATCH v1 2/2] strbuf_getcwd() needs precompse_strbuf_if_needed()","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-05-08T00:32:00Z","receivedAt":"2024-05-08T00:32:08Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2024-05-07 at 17:47:41, Junio C Hamano wrote:\n> tboegi@web.de writes:\n> \n> > From: Torsten Bögershausen <tboegi@web.de>\n> >\n> > When running under macOs a call to strbuf_getcwd() may return\n> \n> You spelled it as \"macOS\" in [1/2].  The hits from\n> \n>     $ git grep -i 'mac *os' \\*.[ch]\n> \n> tell me that we seem to say \"macOS\", \"MacOS X\", \"Mac OSX\" and \"Mac\n> OS X\" pretty much interchangeably.  We may want to eventually\n> consolidate them to whatever the official name Apple uses, but in\n> the meantime let's make sure we do not add even more.\n\nI believe the current preferred form is \"macOS\".  That's what I see on\nApple's website, and that's consistent with my understanding as well.\n\nIt was previously \"Mac OS X\", which is why we probably have that in our\ncode base, but it's my understanding Apple has moved away from that.\n\nWikipedia's opening sentence states, \"macOS, originally Mac OS X,\npreviously shortened as OS X, is an operating system developed and\nmarketed by Apple since 2001,\" so I think Wikipedia agrees, too.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"494349","messageId":"xmqqh6f7vwiy.fsf@gitster.g","threadId":"61429","inReplyTo":"xmqqa5l1pmf9.fsf@gitster.g","subject":"Re: [PATCH v1 2/2] strbuf_getcwd() needs precompse_strbuf_if_needed()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-09T15:24:05Z","receivedAt":"2024-05-09T15:24:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> diff --git a/strbuf.c b/strbuf.c\n>> index 0d929e4e19..cefea6b75f 100644\n>> --- a/strbuf.c\n>> +++ b/strbuf.c\n>> @@ -591,6 +591,7 @@ int strbuf_getcwd(struct strbuf *sb)\n>>  \tfor (;; guessed_len *= 2) {\n>>  \t\tstrbuf_grow(sb, guessed_len);\n>>  \t\tif (getcwd(sb->buf, sb->alloc)) {\n>> +\t\t\tprecompse_strbuf_if_needed(sb);\n>>  \t\t\tstrbuf_setlen(sb, strlen(sb->buf));\n>\n> The need for strbuf_setlen() stems from the use of getcwd() that may\n> and will place a string that is much shorter than sb->alloc, so they\n> logically belong together.  It will make more sense to call the\n> precompose _after_ arranging the members of strbuf in a consistent\n> state with the call to strbuf_setlen().\n\nOf course, we need to make sure precompose_string_if_needed() will\nleave the strbuf in an consistent state.  I think the implementation\nof that helper function in this patch already does so, so\n\n\t\tstrbuf_grow(sb, guessed_len);\n\t\tif (getcwd(sb->buf, sb->alloc)) {\n\t\t\tstrbuf_setlen(sb, strlen(sb->buf));\n\t\t\tprecompse_strbuf_if_needed(sb);\n\nwould be what we would want.\n\nThanks.\n"},{"id":"494350","messageId":"20240509152935.GA31752@tb-raspi4","threadId":"61429","inReplyTo":"xmqqh6f7vwiy.fsf@gitster.g","subject":"Re: [PATCH v1 2/2] strbuf_getcwd() needs precompse_strbuf_if_needed()","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2024-05-09T15:29:35Z","receivedAt":"2024-05-09T15:29:54Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Thu, May 09, 2024 at 08:24:05AM -0700, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n> Of course, we need to make sure precompose_string_if_needed() will\n> leave the strbuf in an consistent state.  I think the implementation\n> of that helper function in this patch already does so, so\n>\n> \t\tstrbuf_grow(sb, guessed_len);\n> \t\tif (getcwd(sb->buf, sb->alloc)) {\n> \t\t\tstrbuf_setlen(sb, strlen(sb->buf));\n> \t\t\tprecompse_strbuf_if_needed(sb);\n>\n\nI think that is what I have in mind as well.\nThanks for the review, a V2 should come the next days.\n"},{"id":"494354","messageId":"20240509161110.12121-1-tboegi@web.de","threadId":"61429","inReplyTo":"20240430032717281.IXLP.121462.mail.biglobe.ne.jp@biglobe.ne.jp","subject":"[PATCH v2 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2024-05-09T16:11:10Z","receivedAt":"2024-05-09T16:11:29Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen <tboegi@web.de>\n\nUnder macOS, `git ls-files path` does not work (gives an error)\nif the absolute 'path' contains characters in NFD (decomposed).\nThis happens when core.precomposeunicode is true, which is the\nmost common case. The bug report says:\n\n$ cd somewhere          # some safe place, /tmp or ~/tmp etc.\n$ mkdir $'u\\xcc\\x88'    # ü in NFD\n$ cd ü                  # or cd $'u\\xcc\\x88' or cd $'\\xc3\\xbc'\n$ git init\n$ git ls-files $'/somewhere/u\\xcc\\x88'   # NFD\n  fatal: /somewhere/ü: '/somewhere/ü' is outside repository at '/somewhere/ü'\n$ git ls-files $'/somewhere/\\xc3\\xbc'    # NFC\n(the same error as above)\n\nIn the 'fatal:' error message, there are three ü;\nthe 1st and 2nd are in NFC, the 3rd is in NFD.\n\nThis commit adds a test case that follows the bug report,\nwith the simplification that the 'ü' is replaced by an 'ä',\nwhich is already used as NFD and NFC in t0050.\n\nThe solution is to precompose the result of getcwd(), if needed.\n\nOne possible implementation would be to re-define getcwd() similar\nto opendir(), readdir() and closedir().\nSince there is already a strbuf wrapper around getcwd(), and only this\nwrapper is used inside the whole codebase, equip strbuf_getcwd() with\na call to the newly created function precompose_strbuf_if_needed().\nNote that precompose_strbuf_if_needed() is a function under macOS,\nand is a \"no-op\" on all other systems.\n\nReported-by: Jun T <takimoto-j@kba.biglobe.ne.jp>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n compat/precompose_utf8.c | 10 ++++++++++\n compat/precompose_utf8.h |  1 +\n git-compat-util.h        |  1 +\n strbuf.c                 |  1 +\n t/t0050-filesystem.sh    | 11 +++++++++++\n 5 files changed, 24 insertions(+)\n\nThanks everybody for the review, which makes V2 much better.\n\n\ndiff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c\nindex 0bd5c24250..5a7c90c90d 100644\n--- a/compat/precompose_utf8.c\n+++ b/compat/precompose_utf8.c\n@@ -94,6 +94,16 @@ const char *precompose_string_if_needed(const char *in)\n \treturn in;\n }\n\n+void precompose_strbuf_if_needed(struct strbuf *sb)\n+{\n+\tchar *buf_prec = (char *)precompose_string_if_needed(sb->buf);\n+\tif (buf_prec != sb->buf) {\n+\t\tsize_t buf_prec_len = strlen(buf_prec);\n+\t\tfree(strbuf_detach(sb, NULL));\n+\t\tstrbuf_attach(sb, buf_prec, buf_prec_len, buf_prec_len + 1);\n+\t}\n+}\n+\n const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)\n {\n \tint i = 0;\ndiff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h\nindex fea06cf28a..7c3cfcadb0 100644\n--- a/compat/precompose_utf8.h\n+++ b/compat/precompose_utf8.h\n@@ -30,6 +30,7 @@ typedef struct {\n\n const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix);\n const char *precompose_string_if_needed(const char *in);\n+void precompose_strbuf_if_needed(struct strbuf *sb);\n void probe_utf8_pathname_composition(void);\n\n PREC_DIR *precompose_utf8_opendir(const char *dirname);\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ca7678a379..892e1f9067 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -344,6 +344,7 @@ static inline const char *precompose_string_if_needed(const char *in)\n \treturn in;\n }\n\n+#define precompose_strbuf_if_needed(a)\n #define probe_utf8_pathname_composition()\n #endif\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 0d929e4e19..d5b4b3903a 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -592,6 +592,7 @@ int strbuf_getcwd(struct strbuf *sb)\n \t\tstrbuf_grow(sb, guessed_len);\n \t\tif (getcwd(sb->buf, sb->alloc)) {\n \t\t\tstrbuf_setlen(sb, strlen(sb->buf));\n+\t\t\tprecompose_strbuf_if_needed(sb);\n \t\t\treturn 0;\n \t\t}\n\ndiff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh\nindex 325eb1c3cd..a24ec866d1 100755\n--- a/t/t0050-filesystem.sh\n+++ b/t/t0050-filesystem.sh\n@@ -156,4 +156,15 @@ test_expect_success CASE_INSENSITIVE_FS 'checkout with no pathspec and a case in\n \t)\n '\n\n+test_expect_success 'git ls-files under NFD' '\n+\t(\n+\t\tmkdir -p somewhere/$aumlcdiar &&\n+\t\tmypwd=$PWD &&\n+\t\tcd somewhere/$aumlcdiar &&\n+\t\tgit init &&\n+\t\tgit --literal-pathspecs ls-files \"$mypwd/somewhere/$aumlcdiar\"  2>err &&\n+\t\t>expected &&\n+\t\ttest_cmp expected err\n+\t)\n+'\n test_done\n--\n2.41.0.394.ge43f4fd0bd\n\n"},{"id":"494363","messageId":"xmqqikznueju.fsf@gitster.g","threadId":"61429","inReplyTo":"20240509161110.12121-1-tboegi@web.de","subject":"Re: [PATCH v2 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-09T16:37:41Z","receivedAt":"2024-05-09T16:37:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> From: Torsten Bögershausen <tboegi@web.de>\n>\n> Under macOS, `git ls-files path` does not work (gives an error)\n> if the absolute 'path' contains characters in NFD (decomposed).\n> ...\n>\n> Reported-by: Jun T <takimoto-j@kba.biglobe.ne.jp>\n> Signed-off-by: Torsten Bögershausen <tboegi@web.de>\n> ---\n\nLooks good.  I've queued with a slight rewording to the proposed log\nmessage, and a bit of extra quoting in the test.  Any string that\ncontains \"$aumlcdiar\" are enclosed in a pair of double-quotes in the\nscript, so not just the one given to ls-files, other two references\nto it are also quoted now.\n\nThanks.\n\n1:  a00cec23cf ! 1:  ee6ba4053d macOS: ls-files path fails if path of workdir is NFD\n    @@ Commit message\n         In the 'fatal:' error message, there are three ü;\n         the 1st and 2nd are in NFC, the 3rd is in NFD.\n     \n    -    This commit adds a test case that follows the bug report,\n    -    with the simplification that the 'ü' is replaced by an 'ä',\n    -    which is already used as NFD and NFC in t0050.\n    +    Add a test case that follows the bug report, with the simplification\n    +    that the 'ü' is replaced by an 'ä', which is already used as NFD and\n    +    NFC in t0050.\n     \n    -    The solution is to precompose the result of getcwd(), if needed.\n    +    Precompose the result of getcwd(), if needed, just like all other\n    +    paths we use internally.  That way, paths comparisons are all done\n    +    in NFC and we would correctly notice that the early part of the\n    +    path given as an absolute path matches the current directory.\n     \n         One possible implementation would be to re-define getcwd() similar\n    -    to opendir(), readdir() and closedir().\n    -    Since there is already a strbuf wrapper around getcwd(), and only this\n    -    wrapper is used inside the whole codebase, equip strbuf_getcwd() with\n    -    a call to the newly created function precompose_strbuf_if_needed().\n    +    to opendir(), readdir() and closedir(), but since there is already a\n    +    strbuf wrapper around getcwd(), and only this wrapper is used inside\n    +    the whole codebase, equip strbuf_getcwd() with a call to the newly\n    +    created function precompose_strbuf_if_needed().\n    +\n         Note that precompose_strbuf_if_needed() is a function under macOS,\n         and is a \"no-op\" on all other systems.\n     \n    @@ t/t0050-filesystem.sh: test_expect_success CASE_INSENSITIVE_FS 'checkout with no\n      \n     +test_expect_success 'git ls-files under NFD' '\n     +\t(\n    -+\t\tmkdir -p somewhere/$aumlcdiar &&\n    ++\t\tmkdir -p \"somewhere/$aumlcdiar\" &&\n     +\t\tmypwd=$PWD &&\n    -+\t\tcd somewhere/$aumlcdiar &&\n    ++\t\tcd \"somewhere/$aumlcdiar\" &&\n     +\t\tgit init &&\n    -+\t\tgit --literal-pathspecs ls-files \"$mypwd/somewhere/$aumlcdiar\"  2>err &&\n    ++\t\tgit --literal-pathspecs ls-files \"$mypwd/somewhere/$aumlcdiar\" 2>err &&\n     +\t\t>expected &&\n     +\t\ttest_cmp expected err\n     +\t)\n"},{"id":"495054","messageId":"98AD4B35-ECE2-4349-AEA9-86F5CA52EA9B@kba.biglobe.ne.jp","threadId":"61429","inReplyTo":"20240509161110.12121-1-tboegi@web.de","subject":"Re: [PATCH v2 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Jun. T","fromEmail":"takimoto-j@kba.biglobe.ne.jp","sentAt":"2024-05-19T07:03:03Z","receivedAt":"2024-05-19T07:08:06Z","isPatch":true,"sender":{"key":"takimoto-j@kba.biglobe.ne.jp","avatar":null},"body":"Sorry for not responding quickly.\n\nThank you for the patch, but it seems the problem still remains.\n\nAlthough\n% git ls-files NFD\n(apparently) works,\n% git ls-files NFC\nstill gives the error\n(if core.precomposeunicode is not set in global config).\n\nThe following is some info I got (hope it is correct and useful),\nbut I have no idea how to fix the problem.\n\nprecompose_string_if_needed() works only if:\n  precomposed_unicode is already set to 1, or\n  git_config_get_bool(\"core.precomposeunicode\") sets it to 1.\n\nBut git_config_get_bool() reads the file .git/config only if:\n  the_repository->commondir is already set to \".git\".\n\nBack trace when the strbuf_getcwd() is called for the\n3rd time is (frame #4 is set_git_work_tree()):\n\n  * frame #0: git`strbuf_getcwd(sb=0x00007ff7bfeff0a8) at strbuf.c:588:20\n    frame #1: git`strbuf_realpath_1(resolved=0x00007ff7bfeff0a8, path=\".\", flags=2) at abspath.c:101:7\n    frame #2: git`strbuf_realpath(resolved=0x00007ff7bfeff0a8, path=\".\", die_on_error=1) at abspath.c:219:9\n    frame #3: git`real_pathdup(path=\".\", die_on_error=1) at abspath.c:240:6\n    frame #4: git`repo_set_worktree(repo=0x000000010044eb98, path=\".\") at repository.c:145:19\n    frame #5: git`set_git_work_tree(new_work_tree=\".\") at environment.c:278:2\n    frame #6: git`setup_discovered_git_dir(gitdir=\".git\", cwd=0x0000000100435238, offset=16, repo_fmt=0x00007ff7bfeff1d8, nongit_ok=0x0000000000000000) at setup.c:1119:2\n    frame #7: git`setup_git_directory_gently(nongit_ok=0x0000000000000000) at setup.c:1606:12\n    frame #8: git`setup_git_directory at setup.c:1815:9\n    frame #9: git`run_builtin(p=0x0000000100424d58, argc=2, argv=0x00007ff7bfeff6d8) at git.c:448:12\n    frame #10: git`handle_builtin(argc=2, argv=0x00007ff7bfeff6d8) at git.c:729:3        \n    frame #11: git`run_argv(argcp=0x00007ff7bfeff54c, argv=0x00007ff7bfeff540) at git.c:793:4                                            \n    frame #12: git`cmd_main(argc=2, argv=0x00007ff7bfeff6d8) at git.c:928:19                                   \n    frame #13: git`main(argc=3, argv=0x00007ff7bfeff6d0) at common-main.c:62:11\n\nAt this point, precomposed_unicode is still -1 and\nthe_repository->commondir is still NULL.\nThis means strbuf_getcwd() retuns NFD, and                                      the_repository->worktree is set to NFD.\n                \nMoreover, precompose_string_if_needed() calls    \ngit_config_get_bool(\"core.precomposeunicode\"), and\nthis function indirecly sets  \nthe_repository->config->hash_initialized = 1\n\nLater setup_git_directory_gently() (frame #7) calls\nsetup_git_env() --> repo_set_gitdir() --> repo_set_commondir()\nand the_repository->commondir is now set to \".git\".\n\nThen run_builtin() (frame #10) calls precompose_argv_prefix()\n --> precompose_string_if_needed(). Here we have\n  precomposed_unicode = -1\n  the_repository->config->hash_initialized = 1\nThis means git_config_check_init() does not read\n.git/config (does not call repo_read_config()) even if\nthe_repository->commondir is set to \".git\",\nand precomposed_unicode is not set to 1.\nSo the NFD in argv is not converted to NFC,\nand\n% git ls-files NFD\napparently works.\n\n\n"},{"id":"495082","messageId":"20240520160601.GA29154@tb-raspi4","threadId":"61429","inReplyTo":"98AD4B35-ECE2-4349-AEA9-86F5CA52EA9B@kba.biglobe.ne.jp","subject":"Re: [PATCH v2 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2024-05-20T16:06:01Z","receivedAt":"2024-05-20T16:06:16Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Sun, May 19, 2024 at 04:03:03PM +0900, Jun. T wrote:\n> Sorry for not responding quickly.\n>\n> Thank you for the patch, but it seems the problem still remains.\n>\n> Although\n> % git ls-files NFD\n> (apparently) works,\n> % git ls-files NFC\n> still gives the error\n> (if core.precomposeunicode is not set in global config).\n>\n> The following is some info I got (hope it is correct and useful),\n> but I have no idea how to fix the problem.\n>\n> precompose_string_if_needed() works only if:\n>   precomposed_unicode is already set to 1, or\n>   git_config_get_bool(\"core.precomposeunicode\") sets it to 1.\n>\n> But git_config_get_bool() reads the file .git/config only if:\n>   the_repository->commondir is already set to \".git\".\n>\n> Back trace when the strbuf_getcwd() is called for the\n> 3rd time is (frame #4 is set_git_work_tree()):\n>\n>   * frame #0: git`strbuf_getcwd(sb=0x00007ff7bfeff0a8) at strbuf.c:588:20\n>     frame #1: git`strbuf_realpath_1(resolved=0x00007ff7bfeff0a8, path=\".\", flags=2) at abspath.c:101:7\n>     frame #2: git`strbuf_realpath(resolved=0x00007ff7bfeff0a8, path=\".\", die_on_error=1) at abspath.c:219:9\n>     frame #3: git`real_pathdup(path=\".\", die_on_error=1) at abspath.c:240:6\n>     frame #4: git`repo_set_worktree(repo=0x000000010044eb98, path=\".\") at repository.c:145:19\n>     frame #5: git`set_git_work_tree(new_work_tree=\".\") at environment.c:278:2\n>     frame #6: git`setup_discovered_git_dir(gitdir=\".git\", cwd=0x0000000100435238, offset=16, repo_fmt=0x00007ff7bfeff1d8, nongit_ok=0x0000000000000000) at setup.c:1119:2\n>     frame #7: git`setup_git_directory_gently(nongit_ok=0x0000000000000000) at setup.c:1606:12\n>     frame #8: git`setup_git_directory at setup.c:1815:9\n>     frame #9: git`run_builtin(p=0x0000000100424d58, argc=2, argv=0x00007ff7bfeff6d8) at git.c:448:12\n>     frame #10: git`handle_builtin(argc=2, argv=0x00007ff7bfeff6d8) at git.c:729:3\n>     frame #11: git`run_argv(argcp=0x00007ff7bfeff54c, argv=0x00007ff7bfeff540) at git.c:793:4\n>     frame #12: git`cmd_main(argc=2, argv=0x00007ff7bfeff6d8) at git.c:928:19\n>     frame #13: git`main(argc=3, argv=0x00007ff7bfeff6d0) at common-main.c:62:11\n>\n> At this point, precomposed_unicode is still -1 and\n> the_repository->commondir is still NULL.\n> This means strbuf_getcwd() retuns NFD, and                                      the_repository->worktree is set to NFD.\n>\n> Moreover, precompose_string_if_needed() calls\n> git_config_get_bool(\"core.precomposeunicode\"), and\n> this function indirecly sets\n> the_repository->config->hash_initialized = 1\n>\n> Later setup_git_directory_gently() (frame #7) calls\n> setup_git_env() --> repo_set_gitdir() --> repo_set_commondir()\n> and the_repository->commondir is now set to \".git\".\n>\n> Then run_builtin() (frame #10) calls precompose_argv_prefix()\n>  --> precompose_string_if_needed(). Here we have\n>   precomposed_unicode = -1\n>   the_repository->config->hash_initialized = 1\n> This means git_config_check_init() does not read\n> .git/config (does not call repo_read_config()) even if\n> the_repository->commondir is set to \".git\",\n> and precomposed_unicode is not set to 1.\n> So the NFD in argv is not converted to NFC,\n> and\n> % git ls-files NFD\n> apparently works.\n>\n\nThanks so much for the detailed analysis, that is appreciated.\nTo be honest, I have set core.precomposeunicode true globally,\ncore.quotepath=false, together with settings for\npull.rebase and init.defaultbranch\n\nBecause of that, the new testcase passed, and the patch was in\nimprovement.\nHowever, it would be nice to have the case fixed as well,\nwhere core.precomposeunicode is not set at a global level.\n\nI am happy to provide a patch (a new testcase is already there),\nbut for a change in the codebase I would need some help from an expert,\nto get the config-reading right both for hash_initialized\n(that is may be not about the hash-algorithn at all ?)\nand precompose.\n\n\n\n\n\n"},{"id":"495095","messageId":"xmqqikz8gxuc.fsf@gitster.g","threadId":"61429","inReplyTo":"20240520160601.GA29154@tb-raspi4","subject":"Re: [PATCH v2 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-20T18:08:43Z","receivedAt":"2024-05-20T18:08:46Z","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> Thanks so much for the detailed analysis, that is appreciated.\n> To be honest, I have set core.precomposeunicode true globally,\n> ...\n> ...\n> I am happy to provide a patch (a new testcase is already there),\n> but for a change in the codebase I would need some help from an expert,\n> to get the config-reading right both for hash_initialized\n> (that is may be not about the hash-algorithn at all ?)\n> and precompose.\n\nIt does not sound like an issue with the hash algorithm.\n\nWhy isn't the local config (presumably set with auto-probing when\nthe repository was initialized) being read?  Are we reading the\ncore.precomposeunicode in some funny ways?  Is per-worktree config\ninvolved that is trying to read from one but the auto-probing code\nis setting it to another, or something silly like that?\n\nThanks for working well together, both of you.\n\n"},{"id":"495104","messageId":"20240520192144.GA4111@tb-raspi4","threadId":"61429","inReplyTo":"xmqqikz8gxuc.fsf@gitster.g","subject":"Re: [PATCH v2 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2024-05-20T19:21:44Z","receivedAt":"2024-05-20T19:21:57Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Mon, May 20, 2024 at 11:08:43AM -0700, Junio C Hamano wrote:\n> Torsten Bögershausen <tboegi@web.de> writes:\n>\n> > Thanks so much for the detailed analysis, that is appreciated.\n> > To be honest, I have set core.precomposeunicode true globally,\n> > ...\n> > ...\n> > I am happy to provide a patch (a new testcase is already there),\n> > but for a change in the codebase I would need some help from an expert,\n> > to get the config-reading right both for hash_initialized\n> > (that is may be not about the hash-algorithn at all ?)\n> > and precompose.\n>\n> It does not sound like an issue with the hash algorithm.\n>\n> Why isn't the local config (presumably set with auto-probing when\n> the repository was initialized) being read?\n\nI have the same question, kind of. The callstack provided does give some\nhints, but I was lost...\n\n\n> Are we reading the core.precomposeunicode in some funny ways?\nNot what I am aware of. But the order of initialization, when a git command\nis executed, some need a worktree or .git directory, some not, is somewhat\nbeyond my yet expertise.\n\n>Is per-worktree config  involved that is trying to read from one but the auto-probing code\n> is setting it to another, or something silly like that?\nNo, not to my understanding.\nWe have a \"local\" config (per repo), that is there and has the right value.\nHowever, for some reason we miss to read the repo-config here,\nwhen argv[] needs to be precomposed.\nReading the global config does work, but if core.precomposeunicode is\nnot set here, but set in the repo-config, we miss that.\n\n>\n> Thanks for working well together, both of you.\nYes, please let someone join the force, reading the callstack(s) and\ntry to find what is wrong here. I will try to do the same.\n"},{"id":"495163","messageId":"20240521141452.26210-1-tboegi@web.de","threadId":"61429","inReplyTo":"20240430032717281.IXLP.121462.mail.biglobe.ne.jp@biglobe.ne.jp","subject":"[PATCH v3 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2024-05-21T14:14:52Z","receivedAt":"2024-05-21T14:15:11Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen <tboegi@web.de>\n\nUnder macOS, `git ls-files path` does not work (gives an error)\nif the absolute 'path' contains characters in NFD (decomposed).\nThis happens when core.precomposeunicode is true, which is the\nmost common case. The bug report says:\n\n$ cd somewhere          # some safe place, /tmp or ~/tmp etc.\n$ mkdir $'u\\xcc\\x88'    # ü in NFD\n$ cd ü                  # or cd $'u\\xcc\\x88' or cd $'\\xc3\\xbc'\n$ git init\n$ git ls-files $'/somewhere/u\\xcc\\x88'   # NFD\n  fatal: /somewhere/ü: '/somewhere/ü' is outside repository at '/somewhere/ü'\n$ git ls-files $'/somewhere/\\xc3\\xbc'    # NFC\n(the same error as above)\n\nIn the 'fatal:' error message, there are three ü;\nthe 1st and 2nd are in NFC, the 3rd is in NFD.\n\nAdd a test case that follows the bug report, with the simplification\nthat the 'ü' is replaced by an 'ä', which is already used as NFD and\nNFC in t0050.\n\nPrecompose the result of getcwd(), if needed, just like all other\npaths we use internally.  That way, paths comparisons are all done\nin NFC and we would correctly notice that the early part of the\npath given as an absolute path matches the current directory.\n\nOne possible implementation would be to re-define getcwd() similar\nto opendir(), readdir() and closedir(), but since there is already a\nstrbuf wrapper around getcwd(), and only this wrapper is used inside\nthe whole codebase, equip strbuf_getcwd() with a call to the newly\ncreated function precompose_strbuf_if_needed().\n\nNote that precompose_strbuf_if_needed() is a function under macOS,\nand is a \"no-op\" on all other systems.\n\nAdd a missing call to precompose_string_if_needed() to this code\nin setup.c :\n`work_tree = precompose_string_if_needed(get_git_work_tree());`\n\nReported-by: Jun T <takimoto-j@kba.biglobe.ne.jp>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n compat/precompose_utf8.c | 10 ++++++++++\n compat/precompose_utf8.h |  1 +\n git-compat-util.h        |  1 +\n setup.c                  |  2 +-\n strbuf.c                 |  1 +\n t/t0050-filesystem.sh    | 26 ++++++++++++++++++++++++++\n 6 files changed, 40 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c\nindex 0bd5c24250..5a7c90c90d 100644\n--- a/compat/precompose_utf8.c\n+++ b/compat/precompose_utf8.c\n@@ -94,6 +94,16 @@ const char *precompose_string_if_needed(const char *in)\n \treturn in;\n }\n\n+void precompose_strbuf_if_needed(struct strbuf *sb)\n+{\n+\tchar *buf_prec = (char *)precompose_string_if_needed(sb->buf);\n+\tif (buf_prec != sb->buf) {\n+\t\tsize_t buf_prec_len = strlen(buf_prec);\n+\t\tfree(strbuf_detach(sb, NULL));\n+\t\tstrbuf_attach(sb, buf_prec, buf_prec_len, buf_prec_len + 1);\n+\t}\n+}\n+\n const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)\n {\n \tint i = 0;\ndiff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h\nindex fea06cf28a..7c3cfcadb0 100644\n--- a/compat/precompose_utf8.h\n+++ b/compat/precompose_utf8.h\n@@ -30,6 +30,7 @@ typedef struct {\n\n const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix);\n const char *precompose_string_if_needed(const char *in);\n+void precompose_strbuf_if_needed(struct strbuf *sb);\n void probe_utf8_pathname_composition(void);\n\n PREC_DIR *precompose_utf8_opendir(const char *dirname);\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 3e7a59b5ff..8b63108f16 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -331,6 +331,7 @@ static inline const char *precompose_string_if_needed(const char *in)\n \treturn in;\n }\n\n+#define precompose_strbuf_if_needed(a)\n #define probe_utf8_pathname_composition()\n #endif\n\ndiff --git a/setup.c b/setup.c\nindex 2e607632db..61f61496ec 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -48,7 +48,7 @@ static int abspath_part_inside_repo(char *path)\n \tsize_t wtlen;\n \tchar *path0;\n \tint off;\n-\tconst char *work_tree = get_git_work_tree();\n+\tconst char *work_tree = precompose_string_if_needed(get_git_work_tree());\n \tstruct strbuf realpath = STRBUF_INIT;\n\n \tif (!work_tree)\ndiff --git a/strbuf.c b/strbuf.c\nindex 4c9ac6dc5e..b05581d8e7 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -569,6 +569,7 @@ int strbuf_getcwd(struct strbuf *sb)\n \t\tstrbuf_grow(sb, guessed_len);\n \t\tif (getcwd(sb->buf, sb->alloc)) {\n \t\t\tstrbuf_setlen(sb, strlen(sb->buf));\n+\t\t\tprecompose_strbuf_if_needed(sb);\n \t\t\treturn 0;\n \t\t}\n\ndiff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh\nindex 325eb1c3cd..5a9ee5be92 100755\n--- a/t/t0050-filesystem.sh\n+++ b/t/t0050-filesystem.sh\n@@ -156,4 +156,30 @@ test_expect_success CASE_INSENSITIVE_FS 'checkout with no pathspec and a case in\n \t)\n '\n\n+test_expect_success 'git ls-files under NFD' '\n+\t(\n+\t\tmkdir -p \"somewhere/$aumlcdiar\" &&\n+\t\tmypwd=$PWD &&\n+\t\tcd \"somewhere/$aumlcdiar\" &&\n+\t\tgit init &&\n+\t\tgit --literal-pathspecs ls-files \"$mypwd/somewhere/$aumlcdiar\" 2>err &&\n+\t\t>expected &&\n+\t\ttest_cmp expected err\n+\t)\n+'\n+\n+# Re-do the same test. Note: global core.precomposeunicode is changed\n+test_expect_success 'git ls-files under NFD. global precompose false' '\n+\ttest_when_finished \"git config --global --unset core.precomposeunicode\" &&\n+\t(\n+\t\tmypwd=$PWD &&\n+\t\tcd \"somewhere/$aumlcdiar\" &&\n+\t\tgit config --global core.precomposeunicode false &&\n+\t\tgit config core.precomposeunicode true &&\n+\t\tgit --literal-pathspecs ls-files \"$mypwd/somewhere/$aumlcdiar\" 2>err &&\n+\t\t>expected &&\n+\t\ttest_cmp expected err\n+\t)\n+'\n+\n test_done\n--\n2.41.0.394.ge43f4fd0bd\n\n"},{"id":"495180","messageId":"xmqqttir9hr2.fsf@gitster.g","threadId":"61429","inReplyTo":"20240521141452.26210-1-tboegi@web.de","subject":"Re: [PATCH v3 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-21T17:50:25Z","receivedAt":"2024-05-21T17:50:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> Add a missing call to precompose_string_if_needed() to this code\n> in setup.c :\n> `work_tree = precompose_string_if_needed(get_git_work_tree());`\n\nThis is new in this iteration, I presume?  The old one did the\nprecompose only in strbuf_getcwd().  We now precompose also the\nresult of get_git_work_tree().\n\nTwo questions.\n\n * It is unclear to me why this makes a difference only when the\n   precompuse configuration is set only in the local configuration.\n\n * As the leading part of the value placed in get_git_work_tree()\n   comes from strbuf_getcwd() called by abspath.c:real_pathdup()\n   that is called by repository.c:repo_set_worktree(), doesn't this\n   potentially call precompse twice on the already precomposed early\n   parth of the get_git_work_tree() result?\n\nI suspect that with the arrangement in your test, the argument given\nto set_git_work_tree() from setup.c:setup_discovered_git_dir() is\nalways \".\", and that dot is passed to repository.c:repo_set_worktree()\nwhich calls abspath.c:real_pathdup() to turn it into an absolute,\nwhere it has a call to strbuf_getcwd().\n\nSo with the provided test, I suspect there is no difference between\nthe previous and this iteration in behaviour, as what is fed to\nprecompose should be identical?\n\nWhat this iteration does differently is that inside real_pathdup(),\nif the string given to repo_set_worktree() is more than the trivial\n\".\", it is appended to the result of strbuf_getcwd(), and the new\ncode precomposes after such appending in real_pathdup() happens.  It\nwill convert the leading part twice [*] and more importantly the\nappended part is now converted, unlike the previous one?\n\n\tSide note: [*] hopefully precompose is idempotent?  Relying\n\ton that property somewhat feels yucky, though.\n\nPuzzled...\n\nWill replace and queue, but I couldn't figure out what is going on\nwith the help by the proposed log message, so...\n\nThanks.\n\n"},{"id":"495236","messageId":"20240521205749.GA8165@tb-raspi4","threadId":"61429","inReplyTo":"xmqqttir9hr2.fsf@gitster.g","subject":"Re: [PATCH v3 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2024-05-21T20:57:49Z","receivedAt":"2024-05-21T20:58:01Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Tue, May 21, 2024 at 10:50:25AM -0700, Junio C Hamano wrote:\n> tboegi@web.de writes:\n>\n> > Add a missing call to precompose_string_if_needed() to this code\n> > in setup.c :\n> > `work_tree = precompose_string_if_needed(get_git_work_tree());`\n>\n> This is new in this iteration, I presume?  The old one did the\n> precompose only in strbuf_getcwd().  We now precompose also the\n> result of get_git_work_tree().\n>\n> Two questions.\n>\n>  * It is unclear to me why this makes a difference only when the\n>    precompuse configuration is set only in the local configuration.\n>\n>  * As the leading part of the value placed in get_git_work_tree()\n>    comes from strbuf_getcwd() called by abspath.c:real_pathdup()\n>    that is called by repository.c:repo_set_worktree(), doesn't this\n>    potentially call precompse twice on the already precomposed early\n>    parth of the get_git_work_tree() result?\n>\n> I suspect that with the arrangement in your test, the argument given\n> to set_git_work_tree() from setup.c:setup_discovered_git_dir() is\n> always \".\", and that dot is passed to repository.c:repo_set_worktree()\n> which calls abspath.c:real_pathdup() to turn it into an absolute,\n> where it has a call to strbuf_getcwd().\n>\n> So with the provided test, I suspect there is no difference between\n> the previous and this iteration in behaviour, as what is fed to\n> precompose should be identical?\n>\n> What this iteration does differently is that inside real_pathdup(),\n> if the string given to repo_set_worktree() is more than the trivial\n> \".\", it is appended to the result of strbuf_getcwd(), and the new\n> code precomposes after such appending in real_pathdup() happens.  It\n> will convert the leading part twice [*] and more importantly the\n> appended part is now converted, unlike the previous one?\n>\n> \tSide note: [*] hopefully precompose is idempotent?  Relying\n> \ton that property somewhat feels yucky, though.\n>\n> Puzzled...\n>\n> Will replace and queue, but I couldn't figure out what is going on\n> with the help by the proposed log message, so...\n\nAcknowledge.\nThe commit message deserves an update, for sure.\nMy suggestion would be too keep it in seen, until I have managed\nto write a better commit message.\nAt the same time, I would ask Jun-ichi Takimoto to do a re-test\nof the new version.\n"},{"id":"495243","messageId":"xmqqa5ki95i1.fsf@gitster.g","threadId":"61429","inReplyTo":"20240521205749.GA8165@tb-raspi4","subject":"Re: [PATCH v3 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-21T22:15:02Z","receivedAt":"2024-05-21T22:15:06Z","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> The commit message deserves an update, for sure.\n> My suggestion would be too keep it in seen, until I have managed\n> to write a better commit message.\n> At the same time, I would ask Jun-ichi Takimoto to do a re-test\n> of the new version.\n\nYup, that sounds extremely sensible.  Thanks for working on this.\n"},{"id":"495412","messageId":"C5E35F2C-2423-4571-B737-411F4D4B13B5@kba.biglobe.ne.jp","threadId":"61429","inReplyTo":"xmqqa5ki95i1.fsf@gitster.g","subject":"Re: [PATCH v3 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Jun. T","fromEmail":"takimoto-j@kba.biglobe.ne.jp","sentAt":"2024-05-23T15:33:08Z","receivedAt":"2024-05-23T15:40:50Z","isPatch":true,"sender":{"key":"takimoto-j@kba.biglobe.ne.jp","avatar":null},"body":"\nUnfortunately v3 still doesn't work.\n'git ls-files NFD' works but 'git ls-files NFC' does not.\n\nI think it better to test both \"ls-config NFD\" and \"ls-config NFC\".\n\nThe reason of the failure seems to be the same as v2, but\nI describe it here in more detail (or too detailed).\n\n(one of?) The problem is the_repository->config->hash_initialized\nis set to 1 before the_repository->commondir is set to \".git\".\nDue to this, .git/config is never read, and precomposed_unicode\nis never set to 1 (remains -1).\n\nrun_builtin() {\n    setup_git_directory() {\n        strbuf_getcwd() {   # setup.c:1542\n            precompose_{strbuf,string}_if_needed() {\n                # precomposed_unicode is still -1\n                git_congig_get_bool(\"core.precomposeunicode\") {\n                    git_config_check_init() {\n                        repo_read_config() {\n                            git_config_init() {\n                                # !!!\n                                the_repository->config->hash_initialized=1\n                                # !!!\n                            }\n                            # does not read .git/config since\n                            # the_repository->commondir is still NULL\n                        }\n                    }\n                }\n                returns without converting to NFC\n            }\n            returns cwd in NFD\n        }\n\n        setup_discovered_git_dir() {\n            set_git_work_tree(\".\") {\n                repo_set_worktree() {\n                    # this function indirectly calls strbuf_getcwd()\n                    # --> precompose_{strbuf,string}_if_needed() -->\n                    # {git,repo}_config_get_bool(\"core.precomposeunicode\"),\n                    # but does not try to read .git/config since\n                    # the_repository->config->hash_initialized\n                    # is already set to 1 above. And it will not read\n                    # .git/config even if hash_initialized is 0\n                    # since the_repository->commondir is still NULL.\n\n                    the_repository->worktree = NFD\n                }\n            }\n        }\n\n        setup_git_env() {\n            repo_setup_gitdir() {\n                repo_set_commondir() {\n                    # finally commondir is set here\n                    the_repository->commondir = \".git\"\n                }\n            }\n        }\n\n    } // END setup_git_directory\n\n    precompose_argv_prefix() {\n        # since the_repository->config->hash_initialized is still 1\n        # .git/config is not read and precomposed_unicode remains -1,\n        # and argv (if in NFD) is not converted to NFC\n    }\n\n    cmd_ls_files() {\n        parse_pathspec(.., argv /* may be in NFD, see above */) {\n            init_path_spec_item() {\n                prefix_path_gently() {\n                    abspath_part_inside_repo() {\n                        work_tree = precomose_string_if_needed(\n                                        get_git_work_gree())\n                            # get_git_work_tree() returns NFD, and\n                            # precompose_string_if_needed() does not\n                            # convert it to NFC since \n                            # the_repository->config->hash_initialized is 1\n                        worktree = NFD\n\n                        returns 0 for \"ls-files NFD\" since both argv\n                        and work_tree are in NFD, but returns -1 for\n                        \"ls-files NFC\" since argv is in NFC.\n                    }\n                    returns NULL for \"ls-files NFC\"\n                }\n                die() at pathspec.c:499 for \"ls-files NFC\"\n            }\n        }\n    }\n} END run_builtin\n\nI don't know how to fix the problem, but I think it better to avoid\ncalling precompose_{strbuf,string}_if_needed() before commondir\nis set to \".git\" and .git/config is successfully read.\n\nOr reset the_repository->config->hash_initialized at some point?\n\n\n"},{"id":"495637","messageId":"20240525200113.GA14951@tb-raspi4","threadId":"61429","inReplyTo":"C5E35F2C-2423-4571-B737-411F4D4B13B5@kba.biglobe.ne.jp","subject":"Re: [PATCH v3 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2024-05-25T20:01:13Z","receivedAt":"2024-05-25T20:01:34Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Fri, May 24, 2024 at 12:33:08AM +0900, Jun. T wrote:\n>\n> Unfortunately v3 still doesn't work.\n> 'git ls-files NFD' works but 'git ls-files NFC' does not.\n>\n> I think it better to test both \"ls-config NFD\" and \"ls-config NFC\".\n>\n> The reason of the failure seems to be the same as v2, but\n> I describe it here in more detail (or too detailed).\n\nThanks for testing - I was fully convinced that the new test case\ndid cover all problems - but proved to be wrong.\n\n[snip the nice analyses]\n\n> I don't know how to fix the problem, but I think it better to avoid\n> calling precompose_{strbuf,string}_if_needed() before commondir\n> is set to \".git\" and .git/config is successfully read.\n>\n> Or reset the_repository->config->hash_initialized at some point?\n>\n\nI think that I may be able to offer a v4 patch, which has better test cases.\nFrom my understanding, the reading of the local/repo .git/config does\nnot work as we need it, since the .git/ directroy is determined after\nthe config has been read. And so it is never read.\nHaving said that, a patch relying on the global .gitconfig may still\nbe an improvement on it's own.\n"},{"id":"496027","messageId":"20240531193156.28046-1-tboegi@web.de","threadId":"61429","inReplyTo":"20240430032717281.IXLP.121462.mail.biglobe.ne.jp@biglobe.ne.jp","subject":"[PATCH v4 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2024-05-31T19:31:56Z","receivedAt":"2024-05-31T19:32:14Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen <tboegi@web.de>\n\nUnder macOS, `git ls-files path` does not work (gives an error)\nif the absolute 'path' contains characters in NFD (decomposed).\nThis happens when core.precomposeunicode is true, which is the\nmost common case. The bug report says:\n\n$ cd somewhere          # some safe place, /tmp or ~/tmp etc.\n$ mkdir $'u\\xcc\\x88'    # ü in NFD\n$ cd ü                  # or cd $'u\\xcc\\x88' or cd $'\\xc3\\xbc'\n$ git init\n$ git ls-files $'/somewhere/u\\xcc\\x88'   # NFD\n  fatal: /somewhere/ü: '/somewhere/ü' is outside repository at '/somewhere/ü'\n$ git ls-files $'/somewhere/\\xc3\\xbc'    # NFC\n(the same error as above)\n\nIn the 'fatal:' error message, there are three ü;\nthe 1st and 2nd are in NFC, the 3rd is in NFD.\n\nAdd test cases that follows the bug report, with the simplification\nthat the 'ü' is replaced by an 'ä', which is already used as NFD and\nNFC in t3910.\n\nThe solution is to add a call to precompose_string_if_needed()\nto this code in setup.c :\n`work_tree = precompose_string_if_needed(get_git_work_tree());`\n\nThere is, however, a limitation with this very usage of Git:\nThe (repo) local .gitconfig file is not used, only the global\n\"core.precomposeunicode\" is taken into account, if it is set (or not).\nTo set it to true is a good recommendation anyway, and here is the\nanalyzes from Jun T :\n\nThe problem is the_repository->config->hash_initialized\nis set to 1 before the_repository->commondir is set to \".git\".\nDue to this, .git/config is never read, and precomposed_unicode\nis never set to 1 (remains -1).\n\nrun_builtin() {\n    setup_git_directory() {\n        strbuf_getcwd() {   # setup.c:1542\n            precompose_{strbuf,string}_if_needed() {\n                # precomposed_unicode is still -1\n                git_congig_get_bool(\"core.precomposeunicode\") {\n                    git_config_check_init() {\n                        repo_read_config() {\n                            git_config_init() {\n                                # !!!\n                                the_repository->config->hash_initialized=1\n                                # !!!\n                            }\n                            # does not read .git/config since\n                            # the_repository->commondir is still NULL\n                        }\n                    }\n                }\n                returns without converting to NFC\n            }\n            returns cwd in NFD\n        }\n\n        setup_discovered_git_dir() {\n            set_git_work_tree(\".\") {\n                repo_set_worktree() {\n                    # this function indirectly calls strbuf_getcwd()\n                    # --> precompose_{strbuf,string}_if_needed() -->\n                    # {git,repo}_config_get_bool(\"core.precomposeunicode\"),\n                    # but does not try to read .git/config since\n                    # the_repository->config->hash_initialized\n                    # is already set to 1 above. And it will not read\n                    # .git/config even if hash_initialized is 0\n                    # since the_repository->commondir is still NULL.\n\n                    the_repository->worktree = NFD\n                }\n            }\n        }\n\n        setup_git_env() {\n            repo_setup_gitdir() {\n                repo_set_commondir() {\n                    # finally commondir is set here\n                    the_repository->commondir = \".git\"\n                }\n            }\n        }\n\n    } // END setup_git_directory\n\nReported-by: Jun T <takimoto-j@kba.biglobe.ne.jp>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n setup.c                      |  2 +-\n t/t3910-mac-os-precompose.sh | 39 +++++++++++++++++++++++++++++++++++-\n 2 files changed, 39 insertions(+), 2 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 2e607632db..61f61496ec 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -48,7 +48,7 @@ static int abspath_part_inside_repo(char *path)\n \tsize_t wtlen;\n \tchar *path0;\n \tint off;\n-\tconst char *work_tree = get_git_work_tree();\n+\tconst char *work_tree = precompose_string_if_needed(get_git_work_tree());\n \tstruct strbuf realpath = STRBUF_INIT;\n\n \tif (!work_tree)\ndiff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh\nindex 898267a6bd..6d5918c8fe 100755\n--- a/t/t3910-mac-os-precompose.sh\n+++ b/t/t3910-mac-os-precompose.sh\n@@ -37,6 +37,27 @@ Alongc=$Alongc$Alongc$Alongc$Alongc$Alongc           #50 Byte\n Alongc=$Alongc$Alongc$Alongc$Alongc$Alongc           #250 Byte\n Alongc=$Alongc$AEligatu$AEligatu                     #254 Byte\n\n+\n+ls_files_nfc_nfd () {\n+\ttest_when_finished \"git config --global --unset core.precomposeunicode\" &&\n+\tprglbl=$1\n+\tprlocl=$2\n+\taumlcreat=$3\n+\taumllist=$4\n+\tgit config --global core.precomposeunicode $prglbl &&\n+\t(\n+\t\trm -rf .git &&\n+\t\tmkdir -p \"somewhere/$prglbl/$prlocl/$aumlcreat\" &&\n+\t\tmypwd=$PWD &&\n+\t\tcd \"somewhere/$prglbl/$prlocl/$aumlcreat\" &&\n+\t\tgit init &&\n+\t\tgit config core.precomposeunicode $prlocl &&\n+\t\tgit --literal-pathspecs ls-files \"$mypwd/somewhere/$prglbl/$prlocl/$aumllist\" 2>err &&\n+\t\t>expected &&\n+\t\ttest_cmp expected err\n+\t)\n+}\n+\n test_expect_success \"detect if nfd needed\" '\n \tprecomposeunicode=$(git config core.precomposeunicode) &&\n \ttest \"$precomposeunicode\" = true &&\n@@ -211,8 +232,8 @@ test_expect_success \"unicode decomposed: git restore -p . \" '\n '\n\n # Test if the global core.precomposeunicode stops autosensing\n-# Must be the last test case\n test_expect_success \"respect git config --global core.precomposeunicode\" '\n+\ttest_when_finished \"git config --global --unset core.precomposeunicode\" &&\n \tgit config --global core.precomposeunicode true &&\n \trm -rf .git &&\n \tgit init &&\n@@ -220,4 +241,20 @@ test_expect_success \"respect git config --global core.precomposeunicode\" '\n \ttest \"$precomposeunicode\" = \"true\"\n '\n\n+test_expect_success \"ls-files false false nfd nfd\" '\n+\tls_files_nfc_nfd false false $Adiarnfd $Adiarnfd\n+'\n+\n+test_expect_success \"ls-files false true nfd nfd\" '\n+\tls_files_nfc_nfd false true $Adiarnfd $Adiarnfd\n+'\n+\n+test_expect_success \"ls-files true false nfd nfd\" '\n+\tls_files_nfc_nfd true false $Adiarnfd $Adiarnfd\n+'\n+\n+test_expect_success \"ls-files true true nfd nfd\" '\n+\tls_files_nfc_nfd true true $Adiarnfd $Adiarnfd\n+'\n+\n test_done\n--\n2.41.0.394.ge43f4fd0bd\n\n"},{"id":"496039","messageId":"xmqqh6ecbqug.fsf@gitster.g","threadId":"61429","inReplyTo":"20240531193156.28046-1-tboegi@web.de","subject":"Re: [PATCH v4 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-01T15:55:03Z","receivedAt":"2024-06-01T15:55:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> The problem is the_repository->config->hash_initialized\n> is set to 1 before the_repository->commondir is set to \".git\".\n> Due to this, .git/config is never read, and precomposed_unicode\n> is never set to 1 (remains -1).\n\nThe \"is never read\" part is a bit confusing and misleading.  If it\nwere\n\n    At the point of code flow where we would want to learn the value\n    of precompose configuration, the local configuration has not\n    been read.\n\nthen I would understand it, though.\n\nIs the analysis telling us that we need to rethink the order of\nthings setup_git_directory() does?  Or is this inherently unsolvable\nbecause we need to discover the .git/ directory and path to it\nbefore we can read configuration to learn from it, but we need the\nvalue of precompose setting to compute the \"path to it\"?\n\nPresumably chdir() done in setup_discovered_git_dir() can be done\nwith either NFC or NFD if the filesystem is squashing the\ndifferences between them, so perhaps doing the repo_set_worktree()\ndone in setup_discovered_git_dir() is wrong, and we could delay\npopulating the .worktree member until much later?  For reading the\nlocal config, it does matter we know where the git-dir and\ncommon-dir are, but the location of worktree is immaterial.\n\nAnyway, thanks for a patch.  Will queue.\n\n> run_builtin() {\n>     setup_git_directory() {\n>         strbuf_getcwd() {   # setup.c:1542\n>             precompose_{strbuf,string}_if_needed() {\n>                 # precomposed_unicode is still -1\n>                 git_congig_get_bool(\"core.precomposeunicode\") {\n>                     git_config_check_init() {\n>                         repo_read_config() {\n>                             git_config_init() {\n>                                 # !!!\n>                                 the_repository->config->hash_initialized=1\n>                                 # !!!\n>                             }\n>                             # does not read .git/config since\n>                             # the_repository->commondir is still NULL\n>                         }\n>                     }\n>                 }\n>                 returns without converting to NFC\n>             }\n>             returns cwd in NFD\n>         }\n>\n>         setup_discovered_git_dir() {\n>             set_git_work_tree(\".\") {\n>                 repo_set_worktree() {\n>                     # this function indirectly calls strbuf_getcwd()\n>                     # --> precompose_{strbuf,string}_if_needed() -->\n>                     # {git,repo}_config_get_bool(\"core.precomposeunicode\"),\n>                     # but does not try to read .git/config since\n>                     # the_repository->config->hash_initialized\n>                     # is already set to 1 above. And it will not read\n>                     # .git/config even if hash_initialized is 0\n>                     # since the_repository->commondir is still NULL.\n>\n>                     the_repository->worktree = NFD\n>                 }\n>             }\n>         }\n>\n>         setup_git_env() {\n>             repo_setup_gitdir() {\n>                 repo_set_commondir() {\n>                     # finally commondir is set here\n>                     the_repository->commondir = \".git\"\n>                 }\n>             }\n>         }\n>\n>     } // END setup_git_directory\n>\n> Reported-by: Jun T <takimoto-j@kba.biglobe.ne.jp>\n> Signed-off-by: Torsten Bögershausen <tboegi@web.de>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  setup.c                      |  2 +-\n>  t/t3910-mac-os-precompose.sh | 39 +++++++++++++++++++++++++++++++++++-\n>  2 files changed, 39 insertions(+), 2 deletions(-)\n>\n> diff --git a/setup.c b/setup.c\n> index 2e607632db..61f61496ec 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -48,7 +48,7 @@ static int abspath_part_inside_repo(char *path)\n>  \tsize_t wtlen;\n>  \tchar *path0;\n>  \tint off;\n> -\tconst char *work_tree = get_git_work_tree();\n> +\tconst char *work_tree = precompose_string_if_needed(get_git_work_tree());\n>  \tstruct strbuf realpath = STRBUF_INIT;\n>\n>  \tif (!work_tree)\n> diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh\n> index 898267a6bd..6d5918c8fe 100755\n> --- a/t/t3910-mac-os-precompose.sh\n> +++ b/t/t3910-mac-os-precompose.sh\n> @@ -37,6 +37,27 @@ Alongc=$Alongc$Alongc$Alongc$Alongc$Alongc           #50 Byte\n>  Alongc=$Alongc$Alongc$Alongc$Alongc$Alongc           #250 Byte\n>  Alongc=$Alongc$AEligatu$AEligatu                     #254 Byte\n>\n> +\n> +ls_files_nfc_nfd () {\n> +\ttest_when_finished \"git config --global --unset core.precomposeunicode\" &&\n> +\tprglbl=$1\n> +\tprlocl=$2\n> +\taumlcreat=$3\n> +\taumllist=$4\n> +\tgit config --global core.precomposeunicode $prglbl &&\n> +\t(\n> +\t\trm -rf .git &&\n> +\t\tmkdir -p \"somewhere/$prglbl/$prlocl/$aumlcreat\" &&\n> +\t\tmypwd=$PWD &&\n> +\t\tcd \"somewhere/$prglbl/$prlocl/$aumlcreat\" &&\n> +\t\tgit init &&\n> +\t\tgit config core.precomposeunicode $prlocl &&\n> +\t\tgit --literal-pathspecs ls-files \"$mypwd/somewhere/$prglbl/$prlocl/$aumllist\" 2>err &&\n> +\t\t>expected &&\n> +\t\ttest_cmp expected err\n> +\t)\n> +}\n> +\n>  test_expect_success \"detect if nfd needed\" '\n>  \tprecomposeunicode=$(git config core.precomposeunicode) &&\n>  \ttest \"$precomposeunicode\" = true &&\n> @@ -211,8 +232,8 @@ test_expect_success \"unicode decomposed: git restore -p . \" '\n>  '\n>\n>  # Test if the global core.precomposeunicode stops autosensing\n> -# Must be the last test case\n>  test_expect_success \"respect git config --global core.precomposeunicode\" '\n> +\ttest_when_finished \"git config --global --unset core.precomposeunicode\" &&\n>  \tgit config --global core.precomposeunicode true &&\n>  \trm -rf .git &&\n>  \tgit init &&\n> @@ -220,4 +241,20 @@ test_expect_success \"respect git config --global core.precomposeunicode\" '\n>  \ttest \"$precomposeunicode\" = \"true\"\n>  '\n>\n> +test_expect_success \"ls-files false false nfd nfd\" '\n> +\tls_files_nfc_nfd false false $Adiarnfd $Adiarnfd\n> +'\n> +\n> +test_expect_success \"ls-files false true nfd nfd\" '\n> +\tls_files_nfc_nfd false true $Adiarnfd $Adiarnfd\n> +'\n> +\n> +test_expect_success \"ls-files true false nfd nfd\" '\n> +\tls_files_nfc_nfd true false $Adiarnfd $Adiarnfd\n> +'\n> +\n> +test_expect_success \"ls-files true true nfd nfd\" '\n> +\tls_files_nfc_nfd true true $Adiarnfd $Adiarnfd\n> +'\n> +\n>  test_done\n> --\n> 2.41.0.394.ge43f4fd0bd\n"},{"id":"496058","messageId":"20240602194008.GA27539@tb-raspi4","threadId":"61429","inReplyTo":"xmqqh6ecbqug.fsf@gitster.g","subject":"Re: [PATCH v4 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2024-06-02T19:40:08Z","receivedAt":"2024-06-02T19:53:36Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Sat, Jun 01, 2024 at 08:55:03AM -0700, Junio C Hamano wrote:\n> tboegi@web.de writes:\n>\n> > The problem is the_repository->config->hash_initialized\n> > is set to 1 before the_repository->commondir is set to \".git\".\n> > Due to this, .git/config is never read, and precomposed_unicode\n> > is never set to 1 (remains -1).\n>\n> The \"is never read\" part is a bit confusing and misleading.  If it\n> were\n>\n>     At the point of code flow where we would want to learn the value\n>     of precompose configuration, the local configuration has not\n>     been read.\n>\n> then I would understand it, though.\n\nYes, the thing is that it has not been read, and will never been read,\nin this very very usage of Git.\n\n>\n> Is the analysis telling us that we need to rethink the order of\n> things setup_git_directory() does?  Or is this inherently unsolvable\n> because we need to discover the .git/ directory and path to it\n> before we can read configuration to learn from it, but we need the\n> value of precompose setting to compute the \"path to it\"?\nI think it is solvable, see below.\n>\n> Presumably chdir() done in setup_discovered_git_dir() can be done\n> with either NFC or NFD if the filesystem is squashing the\n> differences between them, so perhaps doing the repo_set_worktree()\n> done in setup_discovered_git_dir() is wrong, and we could delay\n> populating the .worktree member until much later?  For reading the\n> local config, it does matter we know where the git-dir and\n> common-dir are, but the location of worktree is immaterial.\n>\n> Anyway, thanks for a patch.  Will queue.\n\nMy understanding is that detecting the .git/ dir and then reading\nthe config file from here does not work here.\nThat may be better documented in this very commit message,\nplease feel free to ammend it.\nThe root cause may be fixed in a different commit later.\n\n[snip]\n\nThe best info that I have at the moment is the call stack analysis\ndone by Jun. T, where the global core.precomposeunicode is not set,\nand reading the local one \"fails\".\nI hope this is a useful repetition:\n\n\n  The following is some info I got (hope it is correct and useful),\n  but I have no idea how to fix the problem.\n\n  precompose_string_if_needed() works only if:\n    precomposed_unicode is already set to 1, or\n    git_config_get_bool(\"core.precomposeunicode\") sets it to 1.\n\n  But git_config_get_bool() reads the file .git/config only if:\n    the_repository->commondir is already set to \".git\".\n\n  Back trace when the strbuf_getcwd() is called for the\n  3rd time is (frame #4 is set_git_work_tree()):\n\n    * frame #0: git`strbuf_getcwd(sb=0x00007ff7bfeff0a8) at strbuf.c:588:20\n      frame #1: git`strbuf_realpath_1(resolved=0x00007ff7bfeff0a8, path=\".\", flags=2) at abspath.c:101:7\n      frame #2: git`strbuf_realpath(resolved=0x00007ff7bfeff0a8, path=\".\", die_on_error=1) at abspath.c:219:9\n      frame #3: git`real_pathdup(path=\".\", die_on_error=1) at abspath.c:240:6\n      frame #4: git`repo_set_worktree(repo=0x000000010044eb98, path=\".\") at repository.c:145:19\n      frame #5: git`set_git_work_tree(new_work_tree=\".\") at environment.c:278:2\n      frame #6: git`setup_discovered_git_dir(gitdir=\".git\", cwd=0x0000000100435238, offset=16, repo_fmt=0x00007ff7bfeff1d8, nongit_ok=0x0000000000000000) at setup.c:1119:2\n      frame #7: git`setup_git_directory_gently(nongit_ok=0x0000000000000000) at setup.c:1606:12\n      frame #8: git`setup_git_directory at setup.c:1815:9\n      frame #9: git`run_builtin(p=0x0000000100424d58, argc=2, argv=0x00007ff7bfeff6d8) at git.c:448:12\n      frame #10: git`handle_builtin(argc=2, argv=0x00007ff7bfeff6d8) at git.c:729:3\n      frame #11: git`run_argv(argcp=0x00007ff7bfeff54c, argv=0x00007ff7bfeff540) at git.c:793:4\n      frame #12: git`cmd_main(argc=2, argv=0x00007ff7bfeff6d8) at git.c:928:19\n      frame #13: git`main(argc=3, argv=0x00007ff7bfeff6d0) at common-main.c:62:11\n\n  At this point, precomposed_unicode is still -1 and\n  the_repository->commondir is still NULL.\n  This means strbuf_getcwd() retuns NFD, and the_repository->worktree is set to NFD.\n\n  Moreover, precompose_string_if_needed() calls\n  git_config_get_bool(\"core.precomposeunicode\"), and\n  this function indirecly sets\n  the_repository->config->hash_initialized = 1\n\n  Later setup_git_directory_gently() (frame #7) calls\n  setup_git_env() --> repo_set_gitdir() --> repo_set_commondir()\n  and the_repository->commondir is now set to \".git\".\n\n  Then run_builtin() (frame #10) calls precompose_argv_prefix()\n   --> precompose_string_if_needed(). Here we have\n    precomposed_unicode = -1\n    the_repository->config->hash_initialized = 1\n  This means git_config_check_init() does not read\n  .git/config (does not call repo_read_config()) even if\n  the_repository->commondir is set to \".git\",\n  and precomposed_unicode is not set to 1.\n  [snip]\n\n"},{"id":"496195","messageId":"BD63F8A7-986B-424B-B67C-B78887E8CA00@kba.biglobe.ne.jp","threadId":"61429","inReplyTo":"20240531193156.28046-1-tboegi@web.de","subject":"Re: [PATCH v4 1/1] macOS: ls-files path fails if path of workdir is NFD","fromName":"Jun T","fromEmail":"takimoto-j@kba.biglobe.ne.jp","sentAt":"2024-06-04T00:56:50Z","receivedAt":"2024-06-04T01:05:19Z","isPatch":true,"sender":{"key":"takimoto-j@kba.biglobe.ne.jp","avatar":null},"body":"\n> 2024/06/01 4:3, tboegi@web.de <mailto:tboegi@web.de> wrote:\n> \n> The solution is to add a call to precompose_string_if_needed()\n> to this code in setup.c :\n> `work_tree = precompose_string_if_needed(get_git_work_tree());`\n\nThis simple patch works for both 'ls-files NFD' and 'ls-files NFC'.\n> \n> There is, however, a limitation with this very usage of Git:\n> The (repo) local .gitconfig file is not used,\n\ncore.precomposeunicode in .git/config is read, in function\nprecompose_argv_prefix(), and NFD in argv is converted to NFC.\n\nBut, as you know, the variable the_repository->worktree or\nthe return value of get_git_work_tree() is in NFD.\n\n> 2024/06/03 4:40, Torsten Bögershausen <tboegi@web.de> wrote:\n> \n> The root cause may be fixed in a different commit later.\n\n\nOf course fixing the 'root cause' is better, but I don't\nknow whether it's easy or not.\n\nAnyway, thank you for woking on this problem.\n\n--\nJun"}]}