Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

dscho
Copy link
Member

@dscho dscho commented Jan 19, 2022

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.

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>
@dscho
Copy link
Member Author

dscho commented Jan 19, 2022

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 19, 2022

Submitted as pull.1120.git.1642618562012.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git pr-1120/dscho/mingw-getcwd-without-cwd-v1

To fetch this version to local tag pr-1120/dscho/mingw-getcwd-without-cwd-v1:

git fetch --no-tags https://github.com/gitgitgadget/git tag pr-1120/dscho/mingw-getcwd-without-cwd-v1

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 19, 2022

On the Git mailing list, Junio C Hamano wrote (reply to this):

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

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 19, 2022

This patch series was integrated into seen via git@e2724c1.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 19, 2022

This patch series was integrated into next via git@fed79ae.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 19, 2022

This patch series was integrated into master via git@e2724c1.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 19, 2022

Closed via e2724c1.

@gitgitgadget gitgitgadget bot closed this Jan 19, 2022
@dscho dscho deleted the mingw-getcwd-without-cwd branch January 19, 2022 22:04
@dscho
Copy link
Member Author

dscho commented Jan 19, 2022

Applied directly to main: git@e2724c1

@dscho dscho self-assigned this Jan 19, 2022
@gitgitgadget
Copy link

gitgitgadget bot commented Jan 19, 2022

On the Git mailing list, Johannes Schindelin wrote (reply to this):

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant