{"thread":{"id":"51724","subject":"[PATCH 0/1] mingw: handle non-ASCII PATH components correctly","startedAt":"2019-08-24T22:38:59Z","lastAt":"2019-08-26T17:09:37Z","messageCount":3,"participants":["Johannes Schindelin via GitGitGadget","Adam Roben via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"381081","messageId":"pull.135.git.gitgitgadget@gmail.com","threadId":"51724","inReplyTo":null,"subject":"[PATCH 0/1] mingw: handle non-ASCII PATH components correctly","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-24T22:38:55Z","receivedAt":"2019-08-24T22:38:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"We need to be careful on Windows: there are \"ANSI\" versions of the API\nfunctions that take char *, and \"Unicode\" versions that take \"wchar_t `\nstrings as parameters. The ANSI versions are subject to the current\ncodepage, i.e. almost guaranteed to *not handle UTF-8. Internally, we do\nwant to use UTF-8, though, at least in compat/mingw.c, so we really have to\nuse the Unicode versions of the Win32 API.\n\nAdam Roben (1):\n mingw: fix launching of externals from Unicode paths\n\n compat/mingw.c | 15 +++++++++++----\n 1 file changed, 11 insertions(+), 4 deletions(-)\n\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-135%2Fdscho%2Ffix-externals-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-135/dscho/fix-externals-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/135\n-- \ngitgitgadget\n"},{"id":"381082","messageId":"8f2d64a88518d05579701b7093ecbc197ebca2c7.1566686335.git.gitgitgadget@gmail.com","threadId":"51724","inReplyTo":"pull.135.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] mingw: fix launching of externals from Unicode paths","fromName":"Adam Roben via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-24T22:38:56Z","receivedAt":"2019-08-24T22:39:00Z","isPatch":true,"sender":{"key":"aroben@apple.com","avatar":"https://gravatar.com/avatar/9d3697e1de53890adf241331f4b970bdd2b18962b2ff0b8028ebb00e085807f8?d=mp&s=160"},"body":"From: Adam Roben \n\nIf Git were installed in a path containing non-ASCII characters,\ncommands such as `git am` and `git submodule`, which are implemented as\nexternals, would fail to launch with the following error:\n\n> fatal: 'am' appears to be a git command, but we were not\n> able to execute it. Maybe git-am is broken?\n\nThis was due to lookup_prog not being Unicode-aware. It was somehow\nmissed in 85faec9d3a (Win32: Unicode file name support (except dirent),\n2012-03-15).\n\nNote that the only problem in this function was calling\n`GetFileAttributes()` instead of `GetFileAttributesW()`. The calls to\n`access()` were fine because `access()` is a macro which resolves to\n`mingw_access()`, which already handles Unicode correctly. But\n`lookup_prog()` was changed to use `_waccess()` directly so that we only\nconvert the path to UTF-16 once.\n\nTo make things work correctly, we have to maintain UTF-8 and UTF-16\nversions in tandem in `lookup_prog()`.\n\nSigned-off-by: Adam Roben \nSigned-off-by: Johannes Schindelin \n---\n compat/mingw.c | 15 +++++++++++----\n 1 file changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 8141f77189..9f02403ebf 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1161,14 +1161,21 @@ static char *lookup_prog(const char *dir, int dirlen, const char *cmd,\n \t\t\t int isexe, int exe_only)\n {\n \tchar path[MAX_PATH];\n+\twchar_t wpath[MAX_PATH];\n \tsnprintf(path, sizeof(path), \"%.*s\\\\%s.exe\", dirlen, dir, cmd);\n \n-\tif (!isexe && access(path, F_OK) == 0)\n+\tif (xutftowcs_path(wpath, path) < 0)\n+\t\treturn NULL;\n+\n+\tif (!isexe && _waccess(wpath, F_OK) == 0)\n \t\treturn xstrdup(path);\n-\tpath[strlen(path)-4] = '\\0';\n-\tif ((!exe_only || isexe) && access(path, F_OK) == 0)\n-\t\tif (!(GetFileAttributes(path) & FILE_ATTRIBUTE_DIRECTORY))\n+\twpath[wcslen(wpath)-4] = '\\0';\n+\tif ((!exe_only || isexe) && _waccess(wpath, F_OK) == 0) {\n+\t\tif (!(GetFileAttributesW(wpath) & FILE_ATTRIBUTE_DIRECTORY)) {\n+\t\t\tpath[strlen(path)-4] = '\\0';\n \t\t\treturn xstrdup(path);\n+\t\t}\n+\t}\n \treturn NULL;\n }\n \n-- \ngitgitgadget\n"},{"id":"381217","messageId":"xmqqv9ujhn07.fsf@gitster-ct.c.googlers.com","threadId":"51724","inReplyTo":"8f2d64a88518d05579701b7093ecbc197ebca2c7.1566686335.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] mingw: fix launching of externals from Unicode paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-26T17:09:28Z","receivedAt":"2019-08-26T17:09:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Adam Roben via GitGitGadget\" writes:\n\n> Note that the only problem in this function was calling\n> `GetFileAttributes()` instead of `GetFileAttributesW()`. The calls to\n> `access()` were fine because `access()` is a macro which resolves to\n> `mingw_access()`, which already handles Unicode correctly. But\n> `lookup_prog()` was changed to use `_waccess()` directly so that we only\n> convert the path to UTF-16 once.\n\nNicely explained. Thanks.\n\n>\n> To make things work correctly, we have to maintain UTF-8 and UTF-16\n> versions in tandem in `lookup_prog()`.\n>\n> Signed-off-by: Adam Roben \n> Signed-off-by: Johannes Schindelin \n> ---\n> compat/mingw.c | 15 +++++++++++----\n> 1 file changed, 11 insertions(+), 4 deletions(-)\n>\n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index 8141f77189..9f02403ebf 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -1161,14 +1161,21 @@ static char *lookup_prog(const char *dir, int dirlen, const char *cmd,\n> \t\t\t int isexe, int exe_only)\n> {\n> \tchar path[MAX_PATH];\n> +\twchar_t wpath[MAX_PATH];\n> \tsnprintf(path, sizeof(path), \"%.*s\\\\%s.exe\", dirlen, dir, cmd);\n> \n> -\tif (!isexe && access(path, F_OK) == 0)\n> +\tif (xutftowcs_path(wpath, path) < 0)\n> +\t\treturn NULL;\n> +\n> +\tif (!isexe && _waccess(wpath, F_OK) == 0)\n> \t\treturn xstrdup(path);\n> -\tpath[strlen(path)-4] = '\\0';\n> -\tif ((!exe_only || isexe) && access(path, F_OK) == 0)\n> -\t\tif (!(GetFileAttributes(path) & FILE_ATTRIBUTE_DIRECTORY))\n> +\twpath[wcslen(wpath)-4] = '\\0';\n> +\tif ((!exe_only || isexe) && _waccess(wpath, F_OK) == 0) {\n> +\t\tif (!(GetFileAttributesW(wpath) & FILE_ATTRIBUTE_DIRECTORY)) {\n> +\t\t\tpath[strlen(path)-4] = '\\0';\n> \t\t\treturn xstrdup(path);\n> +\t\t}\n> +\t}\n> \treturn NULL;\n> }\n"}]}