Skip to content

mingw: Fix unlink for open files #1666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 49 additions & 3 deletions compat/mingw.c
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,50 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
return wbuf;
}

/* rename and unlink a file. This is needed for the ability to recreate a file
* with the same name, for example on checkout which unlinks and recreates the file */
static int rename_and_unlink(const wchar_t *pathname, int (*unlinker)(const wchar_t *))
{
int res;
wchar_t temppath[MAX_LONG_PATH];
*temppath = 0;

/* leave space for the .unlink_XXXXXX extension */
wcsncat(temppath, pathname, sizeof(temppath) - 15);
wcsncat(temppath, L".unlink_XXXXXX", sizeof(temppath));
if (!_wmktemp(temppath) || !MoveFileExW(pathname, temppath, 0))
return 1;
res = unlinker(temppath);
if (res) {
const DWORD err = GetLastError();
MoveFileExW(temppath, pathname, 0);
SetLastError(err);
}
return res;
}

static int needs_rename_before_unlink(const wchar_t *pathname, HANDLE *handle)
{
/* If the file is opened by another process, rename it before unlinking */
*handle = CreateFileW(pathname, GENERIC_WRITE, FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
return *handle == INVALID_HANDLE_VALUE && is_file_in_use_error(GetLastError());
}

static int safe_unlink(const wchar_t *wpathname, int (*unlinker)(const wchar_t *))

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

{
int res;
DWORD err;
HANDLE handle;

if (needs_rename_before_unlink(wpathname, &handle))
return rename_and_unlink(wpathname, unlinker);
res = unlinker(wpathname);
err = GetLastError();
CloseHandle(handle);
SetLastError(err);
return res;
}

int mingw_unlink(const char *pathname)
{
int tries = 0;
Expand All @@ -514,7 +558,7 @@ int mingw_unlink(const char *pathname)
do {
/* read-only files cannot be removed */
_wchmod(wpathname, 0666);
if (!_wunlink(wpathname))
if (!safe_unlink(wpathname, _wunlink))
return 0;
if (!is_file_in_use_error(GetLastError()))
break;
Expand All @@ -523,7 +567,7 @@ int mingw_unlink(const char *pathname)
* ERROR_ACCESS_DENIED (EACCES), so try _wrmdir() as well. This is the
* same error we get if a file is in use (already checked above).
*/
if (!_wrmdir(wpathname))
if (!safe_unlink(wpathname, _wrmdir))
return 0;
} while (retry_ask_yes_no(&tries, "Unlink of file '%s' failed. "
"Should I try again?", pathname));
Expand Down Expand Up @@ -560,7 +604,9 @@ int mingw_rmdir(const char *pathname)
return -1;

do {
if (!_wrmdir(wpathname)) {
int res = tries > 0 ? safe_unlink(wpathname, _wrmdir)
: _wrmdir(wpathname);
if (!res) {
invalidate_lstat_cache();
return 0;
}
Expand Down
63 changes: 63 additions & 0 deletions t/t2035-checkout-locked.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#!/bin/sh

test_description='checkout must be able to overwrite open files'
. ./test-lib.sh

test_expect_success 'setup' '

test_commit hello world &&
git branch other &&
test_commit hello-again world
'

test_expect_success 'checkout overwrites file open for read' '

git checkout -f master &&
exec 8<world &&
git checkout other &&
exec 8<&- &&
git diff-files --raw >output &&
test_must_be_empty output
'

test_expect_success 'checkout overwrites file open for write' '

git checkout -f master &&
exec 8>>world &&
git checkout other &&
exec 8>&- &&
git diff-files --raw >output &&
test_must_be_empty output
'

test_expect_success 'subdir' '

git checkout -f master &&
mkdir -p dear &&
test_commit hello-dear dear/world &&
git branch other-dir &&
git mv dear cruel &&
test_commit goodbye cruel/world
'

test_expect_success 'subdir checkout overwrites file open for read' '

git checkout -f master &&
exec 8<cruel/world &&
git checkout other-dir &&
exec 8<&- &&
git diff-files --raw >output &&
test_must_be_empty output
'

test_expect_success 'subdir checkout overwrites file open for write' '

git checkout -f master &&
exec 8>>cruel/world &&
git checkout other-dir &&
exec 8>&- &&
git diff-files --raw >output &&
test_must_be_empty output
'

test_done