Skip to content

Commit 0238c63

Browse files
committed
mingw: Fix unlink for open files
Files that were opened by a (possibly another) process either for read or for failed to be replaced by Git on checkout. If a file is opened with FILE_SHARE_DELETE share mode, it is possible to delete or rename the file, but it is *not possible* to create a new file until the file's handle is closed. To overcome this, unlink now renames files that are not writable before actually unlinking them. If rename fails, then the file either doesn't exist, or it is open without share permissions. On this case, it is still possible to unlink the file. The file will effectively be deleted when it is closed. On this case, preserve the existing behavior. If rename succeeds, unlink the temporary file, making it possible for the real file name to be reused. Fixes git#1653. Signed-off-by: Orgad Shaneh <orgads@gmail.com>
1 parent 21f188e commit 0238c63

File tree

3 files changed

+115
-3
lines changed

3 files changed

+115
-3
lines changed

compat/mingw.c

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,50 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
531531
return wbuf;
532532
}
533533

534+
/* rename and unlink a file. This is needed for the ability to recreate a file
535+
* with the same name, for example on checkout which unlinks and recreates the file */
536+
static int rename_and_unlink(const wchar_t *pathname, int (*unlinker)(const wchar_t *))
537+
{
538+
int res;
539+
wchar_t temppath[MAX_LONG_PATH];
540+
*temppath = 0;
541+
542+
/* leave space for the .unlink_XXXXXX extension */
543+
wcsncat(temppath, pathname, sizeof(temppath) - 15);
544+
wcsncat(temppath, L".unlink_XXXXXX", sizeof(temppath));
545+
if (!_wmktemp(temppath) || !MoveFileExW(pathname, temppath, 0))
546+
return 1;
547+
res = unlinker(temppath);
548+
if (res) {
549+
const DWORD err = GetLastError();
550+
MoveFileExW(temppath, pathname, 0);
551+
SetLastError(err);
552+
}
553+
return res;
554+
}
555+
556+
static int needs_rename_before_unlink(const wchar_t *pathname, HANDLE *handle)
557+
{
558+
/* If the file is opened by another process, rename it before unlinking */
559+
*handle = CreateFileW(pathname, GENERIC_WRITE, FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
560+
return *handle == INVALID_HANDLE_VALUE && is_file_in_use_error(GetLastError());
561+
}
562+
563+
static int safe_unlink(const wchar_t *wpathname, int (*unlinker)(const wchar_t *))
564+
{
565+
int res;
566+
DWORD err;
567+
HANDLE handle;
568+
569+
if (needs_rename_before_unlink(wpathname, &handle))
570+
return rename_and_unlink(wpathname, unlinker);
571+
res = unlinker(wpathname);
572+
err = GetLastError();
573+
CloseHandle(handle);
574+
SetLastError(err);
575+
return res;
576+
}
577+
534578
int mingw_unlink(const char *pathname, int handle_in_use_error)
535579
{
536580
int tries = 0;
@@ -544,7 +588,7 @@ int mingw_unlink(const char *pathname, int handle_in_use_error)
544588
do {
545589
/* read-only files cannot be removed */
546590
_wchmod(wpathname, 0666);
547-
if (!_wunlink(wpathname))
591+
if (!safe_unlink(wpathname, _wunlink))
548592
return 0;
549593
if (!is_file_in_use_error(GetLastError()))
550594
break;
@@ -553,7 +597,7 @@ int mingw_unlink(const char *pathname, int handle_in_use_error)
553597
* ERROR_ACCESS_DENIED (EACCES), so try _wrmdir() as well. This is the
554598
* same error we get if a file is in use (already checked above).
555599
*/
556-
if (!_wrmdir(wpathname))
600+
if (!safe_unlink(wpathname, _wrmdir))
557601
return 0;
558602

559603
if (!handle_in_use_error)
@@ -614,7 +658,9 @@ int mingw_rmdir(const char *pathname)
614658
return -1;
615659

616660
do {
617-
if (!_wrmdir(wpathname)) {
661+
int res = tries > 0 ? safe_unlink(wpathname, _wrmdir)
662+
: _wrmdir(wpathname);
663+
if (!res) {
618664
invalidate_lstat_cache();
619665
return 0;
620666
}

t/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ integration_tests = [
268268
't2027-checkout-track.sh',
269269
't2030-unresolve-info.sh',
270270
't2031-checkout-long-paths.sh',
271+
't2035-checkout-locked.sh',
271272
't2040-checkout-symlink-attr.sh',
272273
't2050-git-dir-relative.sh',
273274
't2060-switch.sh',

t/t2035-checkout-locked.sh

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
#!/bin/sh
2+
3+
test_description='checkout must be able to overwrite open files'
4+
5+
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6+
. ./test-lib.sh
7+
8+
test_expect_success 'setup' '
9+
10+
test_commit hello world &&
11+
git branch other &&
12+
test_commit hello-again world
13+
'
14+
15+
test_expect_success 'checkout overwrites file open for read' '
16+
17+
git checkout -f main &&
18+
exec 8<world &&
19+
git checkout other &&
20+
exec 8<&- &&
21+
git diff-files --raw >output &&
22+
test_must_be_empty output
23+
'
24+
25+
test_expect_success 'checkout overwrites file open for write' '
26+
27+
git checkout -f main &&
28+
exec 8>>world &&
29+
git checkout other &&
30+
exec 8>&- &&
31+
git diff-files --raw >output &&
32+
test_must_be_empty output
33+
'
34+
35+
test_expect_success 'subdir' '
36+
37+
git checkout -f main &&
38+
mkdir -p dear &&
39+
test_commit hello-dear dear/world &&
40+
git branch other-dir &&
41+
git mv dear cruel &&
42+
test_commit goodbye cruel/world
43+
'
44+
45+
test_expect_success 'subdir checkout overwrites file open for read' '
46+
47+
git checkout -f main &&
48+
exec 8<cruel/world &&
49+
git checkout other-dir &&
50+
exec 8<&- &&
51+
git diff-files --raw >output &&
52+
test_must_be_empty output
53+
'
54+
55+
test_expect_success 'subdir checkout overwrites file open for write' '
56+
57+
git checkout -f main &&
58+
exec 8>>cruel/world &&
59+
git checkout other-dir &&
60+
exec 8>&- &&
61+
git diff-files --raw >output &&
62+
test_must_be_empty output
63+
'
64+
65+
test_done

0 commit comments

Comments
 (0)