Skip to contentSkip to conversation

getcwd(mingw): handle the case when there is no cwd

PATCH In review Johannes Schindelin via GitGitGadget 3 participants 3 messages Jan 19, 2022

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 getcwd(mingw): handle the case when there is no cwd

From: Johannes Schindelin <[email protected]>

A recent upstream topic introduced checks for certain Git commands that prevent them from deleting the current working directory, introducing also a regression test that ensures that commands such as git version _can_ run without a current working directory.

While technically not possible on Windows via the regular Win32 API, we do run the regression tests in an MSYS2 Bash which uses a POSIX emulation layer (the MSYS2/Cygwin runtime) where a really evil hack _does_ allow to delete a directory even if it is the current working directory.

Therefore, Git needs to be prepared for a missing working directory, even on Windows.

This issue was not noticed in upstream Git because there was no caller that tried to discover a Git directory with a deleted current working directory in the test suite. But in the microsoft/git fork, we do want to run pre-command/post-command hooks for every command, even for git version, which means that we make precisely such a call. The bug is not in that pre-command/post-command feature, though, but in mingw_getcwd() and needs to be addressed there.

---
getcwd(mingw): handle the case when there is no current working directory

The bug fixed by this topic was noticed due to test failures while rebasing Microsoft's fork of Git onto v2.35.0-rc1. It may not be desirable to take it into Git v2.35.0 this late in the -rc phase, even though I do plan on integrating it into Git for Windows v2.35.0
[https://github.com/git-for-windows/git/pull/3641].

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1120%2Fdscho%2Fmingw-getcwd-without-cwd-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1120/dscho/mingw-getcwd-without-cwd-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1120

 compat/mingw.c | 4 ++++
 1 file changed, 4 insertions(+)
compat/mingw.c +4 -0
index 640dcb11de0..03af369b2b9 100644
@@ -1127,6 +1127,10 @@ char *mingw_getcwd(char *pointer, int len)
 	}
 	if (!ret || ret >= ARRAY_SIZE(wpointer))
 		return NULL;
+	if (GetFileAttributesW(wpointer) == INVALID_FILE_ATTRIBUTES) {
+		errno = ENOENT;
+		return NULL;
+	}
 	if (xwcstoutf(pointer, wpointer, len) < 0)
 		return NULL;
 	convert_slashes(pointer);

base-commit: af4e5f569bc89f356eb34a9373d7f82aca6faa8a

Signature · 1 line
gitgitgadget
#1 lore raw

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 5 lines · writes:

While technically not possible on Windows via the regular Win32 API, we do run the regression tests in an MSYS2 Bash which uses a POSIX emulation layer (the MSYS2/Cygwin runtime) where a really evil hack _does_ allow to delete a directory even if it is the current working directory.

Wow. I am not sure if I should frown or smile ;-)

Quoted 4 lines

The bug fixed by this topic was noticed due to test failures while rebasing Microsoft's fork of Git onto v2.35.0-rc1. It may not be desirable to take it into Git v2.35.0 this late in the -rc phase, even though I do plan on integrating it into Git for Windows v2.35.0

I think we can and should take it in my tree (even direct to 'master'), as it is very clear it affects nobody other than Windows, and even if this has any unintended negative effect, having that common with "Git for Windows" would only help.

Thanks.

#2 lore raw

Hi Junio,

Quoted 8 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 4 lines · writes:

The bug fixed by this topic was noticed due to test failures while rebasing Microsoft's fork of Git onto v2.35.0-rc1. It may not be desirable to take it into Git v2.35.0 this late in the -rc phase, even though I do plan on integrating it into Git for Windows v2.35.0

Quoted 4 lines

I think we can and should take it in my tree (even direct to 'master'), as it is very clear it affects nobody other than Windows, and even if this has any unintended negative effect, having that common with "Git for Windows" would only help.

Thank you!
Dscho