From 3f944424ac899fb6705e7463d937c5ed581da207 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 1 Nov 2017 18:10:25 +0100 Subject: [PATCH 1/3] mingw: add experimental feature to redirect standard handles Particularly when calling Git from applications, such as Visual Studio's Team Explorer, it is important that stdin/stdout/stderr are closed properly. However, when spawning processes on Windows, those handles must be marked as inheritable if we want to use them, but that flag is a global flag and may very well be used by other spawned processes which then do not know to close those handles. Let's introduce a set of environment variables (GIT_REDIRECT_STDIN and friends) that specify paths to files, or even better, named pipes (which are similar to Unix sockets) and that are used by the spawned Git process. This helps work around above-mentioned issue: those named pipes will be opened in a non-inheritable way upon startup, and no handles are passed around (and therefore no inherited handles need to be closed by any spawned child). This feature shipped with Git for Windows (marked as experimental) since v2.11.0(2), so it has seen some serious testing in the meantime. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- compat/mingw.c | 43 +++++++++++++++++++++++++++++++++++++++++++ t/t0001-init.sh | 6 ++++++ 2 files changed, 49 insertions(+) diff --git a/compat/mingw.c b/compat/mingw.c index 8b6fa0db44..6c6c7795a7 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2139,6 +2139,47 @@ static char *wcstoutfdup_startup(char *buffer, const wchar_t *wcs, size_t len) return memcpy(malloc_startup(len), buffer, len); } +static void maybe_redirect_std_handle(const wchar_t *key, DWORD std_id, int fd, + DWORD desired_access, DWORD flags) +{ + DWORD create_flag = fd ? OPEN_ALWAYS : OPEN_EXISTING; + wchar_t buf[MAX_PATH]; + DWORD max = ARRAY_SIZE(buf); + HANDLE handle; + DWORD ret = GetEnvironmentVariableW(key, buf, max); + + if (!ret || ret >= max) + return; + + /* make sure this does not leak into child processes */ + SetEnvironmentVariableW(key, NULL); + if (!wcscmp(buf, L"off")) { + close(fd); + handle = GetStdHandle(std_id); + if (handle != INVALID_HANDLE_VALUE) + CloseHandle(handle); + return; + } + handle = CreateFileW(buf, desired_access, 0, NULL, create_flag, + flags, NULL); + if (handle != INVALID_HANDLE_VALUE) { + int new_fd = _open_osfhandle((intptr_t)handle, O_BINARY); + SetStdHandle(std_id, handle); + dup2(new_fd, fd); + close(new_fd); + } +} + +static void maybe_redirect_std_handles(void) +{ + maybe_redirect_std_handle(L"GIT_REDIRECT_STDIN", STD_INPUT_HANDLE, 0, + GENERIC_READ, FILE_ATTRIBUTE_NORMAL); + maybe_redirect_std_handle(L"GIT_REDIRECT_STDOUT", STD_OUTPUT_HANDLE, 1, + GENERIC_WRITE, FILE_ATTRIBUTE_NORMAL); + maybe_redirect_std_handle(L"GIT_REDIRECT_STDERR", STD_ERROR_HANDLE, 2, + GENERIC_WRITE, FILE_FLAG_NO_BUFFERING); +} + void mingw_startup(void) { int i, maxlen, argc; @@ -2146,6 +2187,8 @@ void mingw_startup(void) wchar_t **wenv, **wargv; _startupinfo si; + maybe_redirect_std_handles(); + /* get wide char arguments and environment */ si.newmode = 0; if (__wgetmainargs(&argc, &wargv, &wenv, _CRT_glob, &si) < 0) diff --git a/t/t0001-init.sh b/t/t0001-init.sh index 86c1a51654..0fd2fc4538 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -453,4 +453,10 @@ test_expect_success 're-init from a linked worktree' ' ) ' +test_expect_success MINGW 'redirect std handles' ' + GIT_REDIRECT_STDOUT=output.txt git rev-parse --git-dir && + test .git = "$(cat output.txt)" && + test -z "$(GIT_REDIRECT_STDOUT=off git rev-parse --git-dir)" +' + test_done From 1a172e4ac1719a068c76384bd077ee65d915ebea Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 1 Nov 2017 18:10:30 +0100 Subject: [PATCH 2/3] mingw: optionally redirect stderr/stdout via the same handle The "2>&1" notation in Powershell and in Unix shells implies that stderr is redirected to the same handle into which stdout is already written. Let's use this special value to allow the same trick with GIT_REDIRECT_STDERR and GIT_REDIRECT_STDOUT: if the former's value is `2>&1`, then stderr will simply be written to the same handle as stdout. The functionality was suggested by Jeff Hostetler. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- compat/mingw.c | 15 +++++++++++++++ t/t0001-init.sh | 8 +++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/compat/mingw.c b/compat/mingw.c index 6c6c7795a7..2d44d21aca 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2160,6 +2160,21 @@ static void maybe_redirect_std_handle(const wchar_t *key, DWORD std_id, int fd, CloseHandle(handle); return; } + if (std_id == STD_ERROR_HANDLE && !wcscmp(buf, L"2>&1")) { + handle = GetStdHandle(STD_OUTPUT_HANDLE); + if (handle == INVALID_HANDLE_VALUE) { + close(fd); + handle = GetStdHandle(std_id); + if (handle != INVALID_HANDLE_VALUE) + CloseHandle(handle); + } else { + int new_fd = _open_osfhandle((intptr_t)handle, O_BINARY); + SetStdHandle(std_id, handle); + dup2(new_fd, fd); + /* do *not* close the new_fd: that would close stdout */ + } + return; + } handle = CreateFileW(buf, desired_access, 0, NULL, create_flag, flags, NULL); if (handle != INVALID_HANDLE_VALUE) { diff --git a/t/t0001-init.sh b/t/t0001-init.sh index 0fd2fc4538..c413bff9cf 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -456,7 +456,13 @@ test_expect_success 're-init from a linked worktree' ' test_expect_success MINGW 'redirect std handles' ' GIT_REDIRECT_STDOUT=output.txt git rev-parse --git-dir && test .git = "$(cat output.txt)" && - test -z "$(GIT_REDIRECT_STDOUT=off git rev-parse --git-dir)" + test -z "$(GIT_REDIRECT_STDOUT=off git rev-parse --git-dir)" && + test_must_fail env \ + GIT_REDIRECT_STDOUT=output.txt \ + GIT_REDIRECT_STDERR="2>&1" \ + git rev-parse --git-dir --verify refs/invalid && + printf ".git\nfatal: Needed a single revision\n" >expect && + test_cmp expect output.txt ' test_done From b2f55717c7f9b335b7ac2e3358b0498116b94a5d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 1 Nov 2017 18:10:33 +0100 Subject: [PATCH 3/3] mingw: document the standard handle redirection This feature has been in Git for Windows since v2.11.0(2), as an experimental option. Now it is considered mature, and it is high time to document it properly. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/git.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 7a1d629ca0..463b0eb0f5 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -709,6 +709,24 @@ of clones and fetches. the background which do not want to cause lock contention with other operations on the repository. Defaults to `1`. +`GIT_REDIRECT_STDIN`:: +`GIT_REDIRECT_STDOUT`:: +`GIT_REDIRECT_STDERR`:: + Windows-only: allow redirecting the standard input/output/error + handles to paths specified by the environment variables. This is + particularly useful in multi-threaded applications where the + canonical way to pass standard handles via `CreateProcess()` is + not an option because it would require the handles to be marked + inheritable (and consequently *every* spawned process would + inherit them, possibly blocking regular Git operations). The + primary intended use case is to use named pipes for communication + (e.g. `\\.\pipe\my-git-stdin-123`). ++ +Two special values are supported: `off` will simply close the +corresponding standard handle, and if `GIT_REDIRECT_STDERR` is +`2>&1`, standard error will be redirected to the same handle as +standard output. + Discussion[[Discussion]] ------------------------