@matheusbsbs / matheus-bagliano / commits / 3262a53c12

reftable: ignore file-in-use errors when unlink(3p) fails on Windows

Unlinking a file may fail on Windows systems when the file is still held open by another process. This is incompatible with POSIX semantics and by extension with Git's assumed semantics when unlinking files, which is that files can be unlinked regardless of whether they are still open or not. To counteract this incompatibility, we have some custom error handling in the `mingw_unlink()` wrapper that first retries the deletion with some delay, and then asks the user whether we should continue to retry. While this logic might be sensible in many callsites throughout Git, it is less when used in the reftable library. We only use unlink(3) there to delete tables which aren't referenced anymore, and the code is very aware of the limitations on Windows. As such, all calls to unlink(3p) don't perform any error checking at all and are fine with the call failing. Instead, the library provides the `reftable_stack_clean()` function, which Git knows to execute in git-pack-refs(1) after compacting a stack. The effect of this function is that all stale tables will eventually get deleted once they aren't kept open anymore. So while we're fine with unlink(3p) failing, the Windows-emulation of that function will still perform several sleeps and ultimately end up asking the user: $ git pack-refs Unlink of file 'C:/temp/jgittest/jgit/.git/reftable/0x000000000002-0x000000000004-50486d0e.ref' failed. Should I try again? (y/n) n Unlink of file 'C:/temp/jgittest/jgit/.git/reftable/0x000000000002-0x000000000004-50486d0e.ref' failed. Should I try again? (y/n) n Unlink of file 'C:/temp/jgittest/jgit/.git/reftable/0x000000000002-0x000000000004-50486d0e.ref' failed. Should I try again? (y/n) n It even asks multiple times, which is doubly annoying and puzzling to the user: 1. It asks when trying to delete the old file after having written the compacted stack. 2. It asks when reloading the stack, where it will try to unlink now-unreferenced tables. 3. It asks when calling `reftable_stack_clean()`, where it will try to unlink now-stale tables. Fix the issue by making it possible to disable this behaviour with a preprocessor define. As "git-compat-util.h" is only included from "system.h", and given that "system.h" is only ever included by headers and code that are internal to the reftable library, we can set that macro in this header without impacting anything else but the reftable library. Reported-by: Christian Reich <Zottelbart@t-online.de> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 6, 2025 at 08:53 UTC 3262a53c123844b3b9140f4e35b86f1444555aad

3 files changed +11 -3

compat/mingw-posix.h

+6 -2

@@ -201,8 +201,12 @@ int uname(struct utsname *buf);

201 * replacements of existing functions

202 */

203

204 -int mingw_unlink(const char *pathname);

205 -#define unlink mingw_unlink

204 +int mingw_unlink(const char *pathname, int handle_in_use_error);

205 +#ifdef MINGW_DONT_HANDLE_IN_USE_ERROR

206 +# define unlink(path) mingw_unlink(path, 0)

207 +#else

208 +# define unlink(path) mingw_unlink(path, 1)

209 +#endif

210

211 int mingw_rmdir(const char *path);

212 #define rmdir mingw_rmdir

compat/mingw.c

+4 -1

@@ -302,7 +302,7 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)

302 return wbuf;

303 }

304

305 -int mingw_unlink(const char *pathname)

305 +int mingw_unlink(const char *pathname, int handle_in_use_error)

306 {

307 int ret, tries = 0;

308 wchar_t wpathname[MAX_PATH];

@@ -317,6 +317,9 @@ int mingw_unlink(const char *pathname)

317 while ((ret = _wunlink(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {

318 if (!is_file_in_use_error(GetLastError()))

319 break;

320 + if (!handle_in_use_error)

321 + return ret;

322 +

323 /*

324 * We assume that some other process had the source or

325 * destination file open at the wrong moment and retry.

reftable/system.h

@@ -11,6 +11,7 @@ https://developers.google.com/open-source/licenses/bsd

12 /* This header glues the reftable library to the rest of Git */

14 +#define MINGW_DONT_HANDLE_IN_USE_ERROR

15 #include "compat/posix.h"

16 #include "compat/zlib-compat.h"