git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] getcwd(mingw): handle the case when there is no cwd
@ 2022-01-19 18:56 Johannes Schindelin via GitGitGadget
  2022-01-19 19:27 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2022-01-19 18:56 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    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(+)

diff --git a/compat/mingw.c b/compat/mingw.c
index 640dcb11de0..03af369b2b9 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -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
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] getcwd(mingw): handle the case when there is no cwd
  2022-01-19 18:56 [PATCH] getcwd(mingw): handle the case when there is no cwd Johannes Schindelin via GitGitGadget
@ 2022-01-19 19:27 ` Junio C Hamano
  2022-01-19 22:39   ` Johannes Schindelin
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2022-01-19 19:27 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
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 ;-)

>     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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] getcwd(mingw): handle the case when there is no cwd
  2022-01-19 19:27 ` Junio C Hamano
@ 2022-01-19 22:39   ` Johannes Schindelin
  0 siblings, 0 replies; 3+ messages in thread
From: Johannes Schindelin @ 2022-01-19 22:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git

Hi Junio,

On Wed, 19 Jan 2022, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> 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
>
> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-01-19 22:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-19 18:56 [PATCH] getcwd(mingw): handle the case when there is no cwd Johannes Schindelin via GitGitGadget
2022-01-19 19:27 ` Junio C Hamano
2022-01-19 22:39   ` Johannes Schindelin

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).