From: "Karsten Blees via GitGitGadget" <gitgitgadget@gmail•com>
To: git@vger•kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx•de>,
Karsten Blees <blees@dcon•de>
Subject: [PATCH 07/18] mingw: factor out the retry logic
Date: Wed, 17 Dec 2025 14:08:44 +0000 [thread overview]
Message-ID: <ad74d540f2f6bd19f248606e29ae45d226c264f9.1765980535.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2018.git.1765980535.gitgitgadget@gmail.com>
From: Karsten Blees <blees@dcon•de>
In several places, Git's Windows-specific code follows the pattern where
it tries to perform an operation, and retries several times when that
operation fails, sleeping an increasing amount of time, before finally
giving up and asking the user whether to rety (after, say, closing an
editor that held a handle to a file, preventing the operation from
succeeding).
This logic is a bit hard to use, and inconsistent:
`mingw_unlink()` and `mingw_rmdir()` duplicate the code to retry,
and both of them do so incompletely. They also do not restore `errno` if the
user answers 'no'.
Introduce a `retry_ask_yes_no()` helper function that handles retry with
small delay, asking the user, and restoring `errno`.
Note that in `mingw_unlink()`, we include the `_wchmod()` call in the
retry loop (which may fail if the file is locked exclusively).
In `mingw_rmdir()`, we include special error handling in the retry loop.
Signed-off-by: Karsten Blees <blees@dcon•de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx•de>
---
compat/mingw.c | 104 ++++++++++++++++++++++---------------------------
1 file changed, 46 insertions(+), 58 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index c7571951dc..26e64c6a5a 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -28,8 +28,6 @@
#define HCAST(type, handle) ((type)(intptr_t)handle)
-static const int delay[] = { 0, 1, 10, 20, 40 };
-
void open_in_gdb(void)
{
static struct child_process cp = CHILD_PROCESS_INIT;
@@ -205,15 +203,12 @@ static int read_yes_no_answer(void)
return -1;
}
-static int ask_yes_no_if_possible(const char *format, ...)
+static int ask_yes_no_if_possible(const char *format, va_list args)
{
char question[4096];
const char *retry_hook;
- va_list args;
- va_start(args, format);
vsnprintf(question, sizeof(question), format, args);
- va_end(args);
retry_hook = mingw_getenv("GIT_ASK_YESNO");
if (retry_hook) {
@@ -238,6 +233,31 @@ static int ask_yes_no_if_possible(const char *format, ...)
}
}
+static int retry_ask_yes_no(int *tries, const char *format, ...)
+{
+ static const int delay[] = { 0, 1, 10, 20, 40 };
+ va_list args;
+ int result, saved_errno = errno;
+
+ if ((*tries) < ARRAY_SIZE(delay)) {
+ /*
+ * We assume that some other process had the file open at the wrong
+ * moment and retry. In order to give the other process a higher
+ * chance to complete its operation, we give up our time slice now.
+ * If we have to retry again, we do sleep a bit.
+ */
+ Sleep(delay[*tries]);
+ (*tries)++;
+ return 1;
+ }
+
+ va_start(args, format);
+ result = ask_yes_no_if_possible(format, args);
+ va_end(args);
+ errno = saved_errno;
+ return result;
+}
+
/* Windows only */
enum hide_dotfiles_type {
HIDE_DOTFILES_FALSE = 0,
@@ -298,7 +318,7 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
int mingw_unlink(const char *pathname, int handle_in_use_error)
{
- int ret, tries = 0;
+ int tries = 0;
wchar_t wpathname[MAX_PATH];
if (xutftowcs_path(wpathname, pathname) < 0)
return -1;
@@ -306,29 +326,19 @@ int mingw_unlink(const char *pathname, int handle_in_use_error)
if (DeleteFileW(wpathname))
return 0;
- /* read-only files cannot be removed */
- _wchmod(wpathname, 0666);
- while ((ret = _wunlink(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+ do {
+ /* read-only files cannot be removed */
+ _wchmod(wpathname, 0666);
+ if (!_wunlink(wpathname))
+ return 0;
if (!is_file_in_use_error(GetLastError()))
break;
if (!handle_in_use_error)
- return ret;
+ return -1;
- /*
- * We assume that some other process had the source or
- * destination file open at the wrong moment and retry.
- * In order to give the other process a higher chance to
- * complete its operation, we give up our time slice now.
- * If we have to retry again, we do sleep a bit.
- */
- Sleep(delay[tries]);
- tries++;
- }
- while (ret == -1 && is_file_in_use_error(GetLastError()) &&
- ask_yes_no_if_possible("Unlink of file '%s' failed. "
- "Should I try again?", pathname))
- ret = _wunlink(wpathname);
- return ret;
+ } while (retry_ask_yes_no(&tries, "Unlink of file '%s' failed. "
+ "Should I try again?", pathname));
+ return -1;
}
static int is_dir_empty(const wchar_t *wpath)
@@ -355,7 +365,7 @@ static int is_dir_empty(const wchar_t *wpath)
int mingw_rmdir(const char *pathname)
{
- int ret, tries = 0;
+ int tries = 0;
wchar_t wpathname[MAX_PATH];
struct stat st;
@@ -381,7 +391,11 @@ int mingw_rmdir(const char *pathname)
if (xutftowcs_path(wpathname, pathname) < 0)
return -1;
- while ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+ do {
+ if (!_wrmdir(wpathname)) {
+ invalidate_lstat_cache();
+ return 0;
+ }
if (!is_file_in_use_error(GetLastError()))
errno = err_win_to_posix(GetLastError());
if (errno != EACCES)
@@ -390,23 +404,9 @@ int mingw_rmdir(const char *pathname)
errno = ENOTEMPTY;
break;
}
- /*
- * We assume that some other process had the source or
- * destination file open at the wrong moment and retry.
- * In order to give the other process a higher chance to
- * complete its operation, we give up our time slice now.
- * If we have to retry again, we do sleep a bit.
- */
- Sleep(delay[tries]);
- tries++;
- }
- while (ret == -1 && errno == EACCES && is_file_in_use_error(GetLastError()) &&
- ask_yes_no_if_possible("Deletion of directory '%s' failed. "
- "Should I try again?", pathname))
- ret = _wrmdir(wpathname);
- if (!ret)
- invalidate_lstat_cache();
- return ret;
+ } while (retry_ask_yes_no(&tries, "Deletion of directory '%s' failed. "
+ "Should I try again?", pathname));
+ return -1;
}
static inline int needs_hiding(const char *path)
@@ -2384,20 +2384,8 @@ repeat:
SetFileAttributesW(wpnew, attrs);
}
}
- if (tries < ARRAY_SIZE(delay) && gle == ERROR_ACCESS_DENIED) {
- /*
- * We assume that some other process had the source or
- * destination file open at the wrong moment and retry.
- * In order to give the other process a higher chance to
- * complete its operation, we give up our time slice now.
- * If we have to retry again, we do sleep a bit.
- */
- Sleep(delay[tries]);
- tries++;
- goto repeat;
- }
if (gle == ERROR_ACCESS_DENIED &&
- ask_yes_no_if_possible("Rename from '%s' to '%s' failed. "
+ retry_ask_yes_no(&tries, "Rename from '%s' to '%s' failed. "
"Should I try again?", pold, pnew))
goto repeat;
--
gitgitgadget
next prev parent reply other threads:[~2025-12-17 14:09 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-17 14:08 [PATCH 00/18] Support symbolic links on Windows Johannes Schindelin via GitGitGadget
2025-12-17 14:08 ` [PATCH 01/18] mingw: don't call `GetFileAttributes()` twice in `mingw_lstat()` Karsten Blees via GitGitGadget
2025-12-18 10:34 ` Johannes Sixt
2025-12-17 14:08 ` [PATCH 02/18] mingw: implement `stat()` with symlink support Karsten Blees via GitGitGadget
2025-12-18 10:44 ` Johannes Sixt
2026-01-09 20:04 ` Johannes Schindelin
2025-12-17 14:08 ` [PATCH 03/18] mingw: drop the separate `do_lstat()` function Karsten Blees via GitGitGadget
2025-12-18 10:48 ` Johannes Sixt
2025-12-17 14:08 ` [PATCH 04/18] mingw: let `mingw_lstat()` error early upon problems with reparse points Karsten Blees via GitGitGadget
2025-12-17 14:08 ` [PATCH 05/18] mingw: teach dirent about symlinks Karsten Blees via GitGitGadget
2025-12-17 14:08 ` [PATCH 06/18] mingw: compute the correct size for symlinks in `mingw_lstat()` Bill Zissimopoulos via GitGitGadget
2025-12-17 14:08 ` Karsten Blees via GitGitGadget [this message]
2025-12-17 14:08 ` [PATCH 08/18] mingw: change default of `core.symlinks` to false Karsten Blees via GitGitGadget
2025-12-17 14:08 ` [PATCH 09/18] mingw: add symlink-specific error codes Karsten Blees via GitGitGadget
2025-12-17 14:08 ` [PATCH 10/18] mingw: handle symlinks to directories in `mingw_unlink()` Karsten Blees via GitGitGadget
2025-12-18 2:49 ` Ben Knoble
2026-01-09 20:04 ` Johannes Schindelin
2025-12-17 14:08 ` [PATCH 11/18] mingw: support renaming symlinks Karsten Blees via GitGitGadget
2025-12-18 17:44 ` Johannes Sixt
2026-01-09 20:04 ` Johannes Schindelin
2025-12-17 14:08 ` [PATCH 12/18] mingw: allow `mingw_chdir()` to change to symlink-resolved directories Karsten Blees via GitGitGadget
2025-12-17 14:08 ` [PATCH 13/18] mingw: implement `readlink()` Karsten Blees via GitGitGadget
2025-12-18 18:13 ` Johannes Sixt
2026-01-09 20:04 ` Johannes Schindelin
2025-12-17 14:08 ` [PATCH 14/18] mingw: implement basic `symlink()` functionality (file symlinks only) Karsten Blees via GitGitGadget
2025-12-17 14:08 ` [PATCH 15/18] mingw: add support for symlinks to directories Karsten Blees via GitGitGadget
2025-12-17 14:08 ` [PATCH 16/18] mingw: try to create symlinks without elevated permissions Johannes Schindelin via GitGitGadget
2025-12-17 14:08 ` [PATCH 17/18] mingw: emulate `stat()` a little more faithfully Johannes Schindelin via GitGitGadget
2025-12-17 14:08 ` [PATCH 18/18] mingw: special-case index entries for symlinks with buggy size Johannes Schindelin via GitGitGadget
2025-12-18 0:00 ` [PATCH 00/18] Support symbolic links on Windows Junio C Hamano
2025-12-18 18:51 ` Johannes Sixt
2025-12-18 19:33 ` Karsten Blees
2026-01-09 20:04 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2026-01-09 20:04 ` [PATCH v2 01/18] mingw: don't call `GetFileAttributes()` twice in `mingw_lstat()` Karsten Blees via GitGitGadget
2026-01-09 20:04 ` [PATCH v2 02/18] mingw: implement `stat()` with symlink support Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 03/18] mingw: drop the separate `do_lstat()` function Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 04/18] mingw: let `mingw_lstat()` error early upon problems with reparse points Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 05/18] mingw: teach dirent about symlinks Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 06/18] mingw: compute the correct size for symlinks in `mingw_lstat()` Bill Zissimopoulos via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 07/18] mingw: factor out the retry logic Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 08/18] mingw: change default of `core.symlinks` to false Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 09/18] mingw: add symlink-specific error codes Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 10/18] mingw: handle symlinks to directories in `mingw_unlink()` Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 11/18] mingw: support renaming symlinks Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 12/18] mingw: allow `mingw_chdir()` to change to symlink-resolved directories Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 13/18] mingw: implement `readlink()` Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 14/18] mingw: implement basic `symlink()` functionality (file symlinks only) Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 15/18] mingw: add support for symlinks to directories Karsten Blees via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 16/18] mingw: try to create symlinks without elevated permissions Johannes Schindelin via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 17/18] mingw: emulate `stat()` a little more faithfully Johannes Schindelin via GitGitGadget
2026-01-09 20:05 ` [PATCH v2 18/18] mingw: special-case index entries for symlinks with buggy size Johannes Schindelin via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ad74d540f2f6bd19f248606e29ae45d226c264f9.1765980535.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail$(echo .)com \
--cc=blees@dcon$(echo .)de \
--cc=git@vger$(echo .)kernel.org \
--cc=johannes.schindelin@gmx$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox