threads / patch / 63899

patch, 4 partsmingw: rename and open fixes

Subject: [PATCH 0/4] mingw: rename and open fixes

## tl;dr

7 messages between Aug 3, 2025 and Aug 4, 2025. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Johannes Schindelin via GitGitGadget· Aug 3, 2025, 21:25 UTC · lore

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.

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
-- 
gitgitgadget
Matthias Aßhauer via GitGitGadget· Aug 3, 2025, 21:25 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 1/4] mingw_open_existing: handle directories better

From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>

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 https://github.com/git-for-windows/git/issues/5068
Signed-off-by: Matthias Aßhauer <mha1993@live.de>
---
 compat/mingw.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)
Show changes to compat/mingw.c +16 −5
diff --git a/compat/mingw.c b/compat/mingw.c
index 5d69ae32f4b9..2dd5cbcaee0d 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -588,13 +588,24 @@ static int mingw_open_existing(const wchar_t *filename, int oflags, ...)
 			     &security_attributes, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
 	if (handle == INVALID_HANDLE_VALUE) {
 		DWORD err = GetLastError();
+		if (err == ERROR_ACCESS_DENIED) {
+			DWORD attrs = GetFileAttributesW(filename);
+			if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY))
+				handle = CreateFileW(filename, access,
+							FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE,
+							&security_attributes, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL| FILE_FLAG_BACKUP_SEMANTICS, NULL);
+		}
 
-		/* See `mingw_open_append()` for why we have this conversion. */
-		if (err == ERROR_INVALID_PARAMETER)
-			err = ERROR_PATH_NOT_FOUND;
+		if (handle == INVALID_HANDLE_VALUE) {
+			err = GetLastError();
 
-		errno = err_win_to_posix(err);
-		return -1;
+			/* See `mingw_open_append()` for why we have this conversion. */
+			if (err == ERROR_INVALID_PARAMETER)
+				err = ERROR_PATH_NOT_FOUND;
+
+			errno = err_win_to_posix(err);
+			return -1;
+		}
 	}
 
 	fd = _open_osfhandle((intptr_t)handle, oflags | O_BINARY);
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Aug 3, 2025, 21:25 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 2/4] mingw: drop Windows 7-specific work-around

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

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. 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 <johannes.schindelin@gmx.de>
---
 Documentation/config/core.adoc |  6 ---
 compat/mingw.c                 | 68 ++--------------------------------
 2 files changed, 4 insertions(+), 70 deletions(-)
Show changes to 2 files +4 −70

Documentation/config/core.adoc, compat/mingw.c

diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc
index 9fde1ab63a70..3fbe83eef161 100644
--- a/Documentation/config/core.adoc
+++ b/Documentation/config/core.adoc
@@ -696,12 +696,6 @@ core.unsetenvvars::
 	Defaults to `PERL5LIB` to account for the fact that Git for
 	Windows insists on using its own Perl interpreter.
 
-core.restrictinheritedhandles::
-	Windows-only: override whether spawned processes inherit only standard
-	file handles (`stdin`, `stdout` and `stderr`) or all handles. Can be
-	`auto`, `true` or `false`. Defaults to `auto`, which means `true` on
-	Windows 7 and later, and `false` on older Windows versions.
-
 core.createObject::
 	You can set this to 'link', in which case a hardlink followed by
 	a delete of the source are used to make sure that object creation
diff --git a/compat/mingw.c b/compat/mingw.c
index 2dd5cbcaee0d..c331c3ac32a8 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -244,7 +244,6 @@ enum hide_dotfiles_type {
 	HIDE_DOTFILES_DOTGITONLY
 };
 
-static int core_restrict_inherited_handles = -1;
 static enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
 static char *unset_environment_variables;
 
@@ -268,15 +267,6 @@ int mingw_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.restrictinheritedhandles")) {
-		if (value && !strcasecmp(value, "auto"))
-			core_restrict_inherited_handles = -1;
-		else
-			core_restrict_inherited_handles =
-				git_config_bool(var, value);
-		return 0;
-	}
-
 	return 0;
 }
 
@@ -1667,7 +1657,6 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
 			      const char *dir,
 			      int prepend_cmd, int fhin, int fhout, int fherr)
 {
-	static int restrict_handle_inheritance = -1;
 	STARTUPINFOEXW si;
 	PROCESS_INFORMATION pi;
 	LPPROC_THREAD_ATTRIBUTE_LIST attr_list = NULL;
@@ -1687,16 +1676,6 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
 	/* Make sure to override previous errors, if any */
 	errno = 0;
 
-	if (restrict_handle_inheritance < 0)
-		restrict_handle_inheritance = core_restrict_inherited_handles;
-	/*
-	 * The following code to restrict which handles are inherited seems
-	 * to work properly only on Windows 7 and later, so let's disable it
-	 * on Windows Vista and 2008.
-	 */
-	if (restrict_handle_inheritance < 0)
-		restrict_handle_inheritance = GetVersion() >> 16 >= 7601;
-
 	do_unset_environment_variables();
 
 	/* Determine whether or not we are associated to a console */
@@ -1798,7 +1777,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
 	wenvblk = make_environment_block(deltaenv);
 
 	memset(&pi, 0, sizeof(pi));
-	if (restrict_handle_inheritance && stdhandles_count &&
+	if (stdhandles_count &&
 	    (InitializeProcThreadAttributeList(NULL, 1, 0, &size) ||
 	     GetLastError() == ERROR_INSUFFICIENT_BUFFER) &&
 	    (attr_list = (LPPROC_THREAD_ATTRIBUTE_LIST)
@@ -1819,52 +1798,13 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
 			     &si.StartupInfo, &pi);
 
 	/*
-	 * On Windows 2008 R2, it seems that specifying certain types of handles
-	 * (such as FILE_TYPE_CHAR or FILE_TYPE_PIPE) will always produce an
-	 * error. Rather than playing finicky and fragile games, let's just try
-	 * to detect this situation and simply try again without restricting any
-	 * handle inheritance. This is still better than failing to create
-	 * processes.
+	 * On the off-chance that something with the file handle restriction
+	 * went wrong, silently fall back to trying without it.
 	 */
-	if (!ret && restrict_handle_inheritance && stdhandles_count) {
+	if (!ret && stdhandles_count) {
 		DWORD err = GetLastError();
 		struct strbuf buf = STRBUF_INIT;
 
-		if (err != ERROR_NO_SYSTEM_RESOURCES &&
-		    /*
-		     * On Windows 7 and earlier, handles on pipes and character
-		     * devices are inherited automatically, and cannot be
-		     * specified in the thread handle list. Rather than trying
-		     * to catch each and every corner case (and running the
-		     * chance of *still* forgetting a few), let's just fall
-		     * back to creating the process without trying to limit the
-		     * handle inheritance.
-		     */
-		    !(err == ERROR_INVALID_PARAMETER &&
-		      GetVersion() >> 16 < 9200) &&
-		    !getenv("SUPPRESS_HANDLE_INHERITANCE_WARNING")) {
-			DWORD fl = 0;
-			int i;
-
-			setenv("SUPPRESS_HANDLE_INHERITANCE_WARNING", "1", 1);
-
-			for (i = 0; i < stdhandles_count; i++) {
-				HANDLE h = stdhandles[i];
-				strbuf_addf(&buf, "handle #%d: %p (type %lx, "
-					    "handle info (%d) %lx\n", i, h,
-					    GetFileType(h),
-					    GetHandleInformation(h, &fl),
-					    fl);
-			}
-			strbuf_addstr(&buf, "\nThis is a bug; please report it "
-				      "at\nhttps://github.com/git-for-windows/"
-				      "git/issues/new\n\n"
-				      "To suppress this warning, please set "
-				      "the environment variable\n\n"
-				      "\tSUPPRESS_HANDLE_INHERITANCE_WARNING=1"
-				      "\n");
-		}
-		restrict_handle_inheritance = 0;
 		flags &= ~EXTENDED_STARTUPINFO_PRESENT;
 		ret = CreateProcessW(*wcmd ? wcmd : NULL, wargs, NULL, NULL,
 				     TRUE, flags, wenvblk, dir ? wdir : NULL,
-- 
gitgitgadget
Oswald Buddenhagen· Aug 4, 2025, 08:54 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 2/4] mingw: drop Windows 7-specific work-around

On Sun, Aug 03, 2025 at 09:25:16PM +0000, Johannes Schindelin via GitGitGadget wrote:
Show 10 quoted lines
>From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
>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.

Show 5 quoted lines
>+	 * 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?"

Johannes Schindelin via GitGitGadget· Aug 3, 2025, 21:25 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 3/4] mingw_rename: support ReFS on Windows 2022

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

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 391bceae4350 (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 https://github.com/git-for-windows/git/issues/5427
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to compat/mingw.c +1 −1
diff --git a/compat/mingw.c b/compat/mingw.c
index c331c3ac32a8..d53ce38b7f82 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2277,7 +2277,7 @@ repeat:
 		 * current system doesn't support FileRenameInfoEx. Keep us
 		 * from using it in future calls and retry.
 		 */
-		if (gle == ERROR_INVALID_PARAMETER) {
+		if (gle == ERROR_INVALID_PARAMETER || gle == ERROR_NOT_SUPPORTED) {
 			supports_file_rename_info_ex = 0;
 			goto repeat;
 		}
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Aug 3, 2025, 21:25 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 4/4] mingw: support Windows Server 2016 again

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

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 https://github.com/git-for-windows/git/issues/5695.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Show changes to compat/mingw.c +3 −1
diff --git a/compat/mingw.c b/compat/mingw.c
index d53ce38b7f82..8538e3d1729d 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2277,7 +2277,9 @@ repeat:
 		 * current system doesn't support FileRenameInfoEx. Keep us
 		 * from using it in future calls and retry.
 		 */
-		if (gle == ERROR_INVALID_PARAMETER || gle == ERROR_NOT_SUPPORTED) {
+		if (gle == ERROR_INVALID_PARAMETER ||
+		    gle == ERROR_NOT_SUPPORTED ||
+		    gle == ERROR_INVALID_FUNCTION) {
 			supports_file_rename_info_ex = 0;
 			goto repeat;
 		}
-- 
gitgitgadget
Junio C Hamano· Aug 4, 2025, 01:29 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 0/4] mingw: rename and open fixes

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 6 quoted lines
> 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'.
Show 17 quoted lines
> 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

← back to recent threads