# [PATCH 0/1] mingw: handle non-ASCII PATH components correctly 3 messages from 2019-08-24 to 2019-08-26. Participants: Johannes Schindelin via GitGitGadget, Adam Roben via GitGitGadget, Junio C Hamano. Thread: https://gitlist.dev/t/51724 ## Johannes Schindelin via GitGitGadget, 2019-08-24 22:38 Subject: [PATCH 0/1] mingw: handle non-ASCII PATH components correctly Message-ID: URL: https://gitlist.dev/e/pull.135.git.gitgitgadget%40gmail.com ``` We need to be careful on Windows: there are "ANSI" versions of the API functions that take char *, and "Unicode" versions that take "wchar_t ` strings as parameters. The ANSI versions are subject to the current codepage, i.e. almost guaranteed to *not handle UTF-8. Internally, we do want to use UTF-8, though, at least in compat/mingw.c, so we really have to use the Unicode versions of the Win32 API. Adam Roben (1): mingw: fix launching of externals from Unicode paths compat/mingw.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) base-commit: 8104ec994ea3849a968b4667d072fedd1e688642 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-135%2Fdscho%2Ffix-externals-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-135/dscho/fix-externals-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/135 -- gitgitgadget ``` ## Adam Roben via GitGitGadget, 2019-08-24 22:38 Subject: [PATCH 1/1] mingw: fix launching of externals from Unicode paths Message-ID: <8f2d64a88518d05579701b7093ecbc197ebca2c7.1566686335.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/8f2d64a88518d05579701b7093ecbc197ebca2c7.1566686335.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Adam Roben If Git were installed in a path containing non-ASCII characters, commands such as `git am` and `git submodule`, which are implemented as externals, would fail to launch with the following error: > fatal: 'am' appears to be a git command, but we were not > able to execute it. Maybe git-am is broken? This was due to lookup_prog not being Unicode-aware. It was somehow missed in 85faec9d3a (Win32: Unicode file name support (except dirent), 2012-03-15). Note that the only problem in this function was calling `GetFileAttributes()` instead of `GetFileAttributesW()`. The calls to `access()` were fine because `access()` is a macro which resolves to `mingw_access()`, which already handles Unicode correctly. But `lookup_prog()` was changed to use `_waccess()` directly so that we only convert the path to UTF-16 once. To make things work correctly, we have to maintain UTF-8 and UTF-16 versions in tandem in `lookup_prog()`. Signed-off-by: Adam Roben Signed-off-by: Johannes Schindelin --- compat/mingw.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 8141f77189..9f02403ebf 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1161,14 +1161,21 @@ static char *lookup_prog(const char *dir, int dirlen, const char *cmd, int isexe, int exe_only) { char path[MAX_PATH]; + wchar_t wpath[MAX_PATH]; snprintf(path, sizeof(path), "%.*s\\%s.exe", dirlen, dir, cmd); - if (!isexe && access(path, F_OK) == 0) + if (xutftowcs_path(wpath, path) < 0) + return NULL; + + if (!isexe && _waccess(wpath, F_OK) == 0) return xstrdup(path); - path[strlen(path)-4] = '\0'; - if ((!exe_only || isexe) && access(path, F_OK) == 0) - if (!(GetFileAttributes(path) & FILE_ATTRIBUTE_DIRECTORY)) + wpath[wcslen(wpath)-4] = '\0'; + if ((!exe_only || isexe) && _waccess(wpath, F_OK) == 0) { + if (!(GetFileAttributesW(wpath) & FILE_ATTRIBUTE_DIRECTORY)) { + path[strlen(path)-4] = '\0'; return xstrdup(path); + } + } return NULL; } -- gitgitgadget ``` ## Junio C Hamano, 2019-08-26 17:09 Subject: Re: [PATCH 1/1] mingw: fix launching of externals from Unicode paths Message-ID: URL: https://gitlist.dev/e/xmqqv9ujhn07.fsf%40gitster-ct.c.googlers.com In-Reply-To: <8f2d64a88518d05579701b7093ecbc197ebca2c7.1566686335.git.gitgitgadget@gmail.com> ``` "Adam Roben via GitGitGadget" writes: > Note that the only problem in this function was calling > `GetFileAttributes()` instead of `GetFileAttributesW()`. The calls to > `access()` were fine because `access()` is a macro which resolves to > `mingw_access()`, which already handles Unicode correctly. But > `lookup_prog()` was changed to use `_waccess()` directly so that we only > convert the path to UTF-16 once. Nicely explained. Thanks. > > To make things work correctly, we have to maintain UTF-8 and UTF-16 > versions in tandem in `lookup_prog()`. > > Signed-off-by: Adam Roben > Signed-off-by: Johannes Schindelin > --- > compat/mingw.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/compat/mingw.c b/compat/mingw.c > index 8141f77189..9f02403ebf 100644 > --- a/compat/mingw.c > +++ b/compat/mingw.c > @@ -1161,14 +1161,21 @@ static char *lookup_prog(const char *dir, int dirlen, const char *cmd, > int isexe, int exe_only) > { > char path[MAX_PATH]; > + wchar_t wpath[MAX_PATH]; > snprintf(path, sizeof(path), "%.*s\\%s.exe", dirlen, dir, cmd); > > - if (!isexe && access(path, F_OK) == 0) > + if (xutftowcs_path(wpath, path) < 0) > + return NULL; > + > + if (!isexe && _waccess(wpath, F_OK) == 0) > return xstrdup(path); > - path[strlen(path)-4] = '\0'; > - if ((!exe_only || isexe) && access(path, F_OK) == 0) > - if (!(GetFileAttributes(path) & FILE_ATTRIBUTE_DIRECTORY)) > + wpath[wcslen(wpath)-4] = '\0'; > + if ((!exe_only || isexe) && _waccess(wpath, F_OK) == 0) { > + if (!(GetFileAttributesW(wpath) & FILE_ATTRIBUTE_DIRECTORY)) { > + path[strlen(path)-4] = '\0'; > return xstrdup(path); > + } > + } > return NULL; > } ```