mingw: rename and open fixes #1948
Closed
+23
−76
Conversation
This file contains hidden or 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
CreateFileW() requires FILE_FLAG_BACKUP_SEMANTICS to create a directory handle [1] and errors out with ERROR_ACCESS_DENIED without this flag. Fall back to accessing Directory handles this way. [1] https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew#directories This fixes git-for-windows#5068 Signed-off-by: Matthias Aßhauer <[email protected]>
In ac33519 (mingw: restrict file handle inheritance only on Windows 7 and later, 2019-11-22), I introduced code to safe-guard the defense-in-depth handling that restricts handles' inheritance so that it would work with Windows 7, too. Let's revert this patch: Git for Windows dropped supporting Windows 7 (and Windows 8) directly after Git for Windows v2.46.2. For full details, see https://gitforwindows.org/requirements#windows-version. Actually, on second thought: revert only the part that makes this handle inheritance restriction logic optional and that suggests to open a bug report if it fails, but keep the fall-back to try again without said logic: There have been a few false positives over the past few years (where the warning was triggered e.g. because Defender was still accessing a file that Git wanted to overwrite), and the fall-back logic seems to have helped occasionally in such situations. Signed-off-by: Johannes Schindelin <[email protected]>
ReFS is an alternative filesystem to NTFS. On Windows 2022, it seems not to support the rename operation using POSIX semantics that Git uses on Windows as of 391bcea (compat/mingw: support POSIX semantics for atomic renames, 2024-10-27). However, Windows 2022 reports `ERROR_NOT_SUPPORTED` in this instance. This is in contrast to `ERROR_INVALID_PARAMETER` (as previous Windows versions would report that do not support POSIX semantics in renames at all). Let's handle both errors the same: by falling back to the best-effort option, namely to rename without POSIX semantics. This fixes git-for-windows#5427 Signed-off-by: Johannes Schindelin <[email protected]>
It was reported to the Git for Windows project that a simple `git init` fails on Windows Server 2016: D:\Dev\test> git init error: could not write config file D:/Dev/test/.git/config: Function not implemented fatal: could not set 'core.repositoryformatversion' to '0' According to https://endoflife.date/windows-server, Windows Server 2016 is officially supported for another one-and-a-half years as of time of writing, so this is not good. The culprit is the `mingw_rename()` changes that try to use POSIX semantics when available, but fail to fall back properly on Windows Server 2016. This fixes git-for-windows#5695. Signed-off-by: Johannes Schindelin <[email protected]>
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Johannes Schindelin via GitGitGadget" <[email protected]>
writes:
> The recent change of mingw_rename() to use POSIX semantics had quite a bit
> of fall-out, breaking in pre-Windows 11 setups that use ReFS, and in a
> different way on Windows Server 2016.
>
> While at it, this patch series also upstreams two related patches that
> matured in Git for Windows for long enough already.
Thanks. What a great timing, just before the -rc0 preview release
;-)
Will apply directly to 'master'.
> Johannes Schindelin (3):
> mingw: drop Windows 7-specific work-around
> mingw_rename: support ReFS on Windows 2022
> mingw: support Windows Server 2016 again
>
> Matthias Aßhauer (1):
> mingw_open_existing: handle directories better
>
> Documentation/config/core.adoc | 6 ---
> compat/mingw.c | 93 +++++++++-------------------------
> 2 files changed, 23 insertions(+), 76 deletions(-)
>
>
> base-commit: 866e6a391f466baeeb98bc585845ea638322c04b
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1948%2Fdscho%2Fmingw-rename-and-open-fixes-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1948/dscho/mingw-rename-and-open-fixes-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1948 |
|
This patch series was integrated into seen via git@569111a. |
|
This patch series was integrated into next via git@a222fda. |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| core.unsetenvvars:: | ||
| Windows-only: comma-separated list of environment variables' | ||
| names that need to be unset before spawning any other process. | ||
| Defaults to `PERL5LIB` to account for the fact that Git for |
There was a problem hiding this comment.
On the Git mailing list, Oswald Buddenhagen wrote (reply to this):
On Sun, Aug 03, 2025 at 09:25:16PM +0000, Johannes Schindelin via GitGitGadget wrote:
>From: Johannes Schindelin <[email protected]>
>
>In ac33519ddfa8 (mingw: restrict file handle inheritance only on Windows
>7 and later, 2019-11-22), I introduced code to safe-guard the
>defense-in-depth handling that restricts handles' inheritance so that it
>would work with Windows 7, too.
>
>Let's revert this patch: Git for Windows dropped supporting Windows 7 (and
>Windows 8) directly after Git for Windows v2.46.2.
>
it doesn't follow from this why it's apparently ok to remove this for even newer versions.
>+ * On the off-chance that something with the file handle restriction
>+ * went wrong, silently fall back to trying without it.
> */
>+ if (!ret && stdhandles_count) {
>
the comment should really spell out what that off chance is, so one doesn't have to check the log.
it may also make sense to elaborate why just dropping the restrictions isn't a problem - my first thought is "huh, doesn't this open the door for security holes, at least theoretically?"
|
User |
|
This branch is now known as |
|
This patch series was integrated into seen via git@817d661. |
|
This patch series was integrated into master via git@817d661. |
|
This patch series was integrated into next via git@817d661. |
|
Closed via 817d661. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The recent change of
mingw_rename()to use POSIX semantics had quite a bit of fall-out, breaking in pre-Windows 11 setups that use ReFS, and in a different way on Windows Server 2016.While at it, this patch series also upstreams two related patches that matured in Git for Windows for long enough already.
cc: Oswald Buddenhagen [email protected]