mingw: handle non-ASCII PATH components correctly #135
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 85faec9 (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 <adam@roben.org> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
35 tasks
|
/submit |
|
Submitted as pull.135.git.gitgitgadget@gmail.com |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -1161,14 +1161,21 @@ static char *lookup_prog(const char *dir, int dirlen, const char *cmd, | |||
| int isexe, int exe_only) | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Adam Roben via GitGitGadget" <gitgitgadget@gmail.com> 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 <adam@roben.org>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 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;
> }
|
This branch is now known as |
|
This patch series was integrated into pu via git@dddb330. |
|
This patch series was integrated into pu via git@90b217d. |
|
This patch series was integrated into pu via git@d1a68cd. |
|
This patch series was integrated into pu via git@5dadc96. |
|
This patch series was integrated into pu via git@0b18aac. |
|
This patch series was integrated into pu via git@6d6706b. |
|
This patch series was integrated into next via git@1973826. |
|
This patch series was integrated into pu via git@a616121. |
|
This patch series was integrated into pu via git@f8a13c3. |
|
This patch series was integrated into pu via git@b38f69d. |
|
This patch series was integrated into pu via git@ae43462. |
|
This patch series was integrated into pu via git@6f21347. |
|
This patch series was integrated into master via git@6f21347. |
|
Closed via 6f21347. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 incompat/mingw.c`, so we really have to use the Unicode versions of the Win32 API.