From 431bb406d813abe2db413a7c53f16ea2377b4364 Mon Sep 17 00:00:00 2001 From: Orgad Shaneh Date: Wed, 9 May 2018 00:35:15 +0300 Subject: [PATCH] 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 #1653. Signed-off-by: Orgad Shaneh --- compat/mingw.c | 52 +++++++++++++++++++++++++++++-- t/t2035-checkout-locked.sh | 63 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 3 deletions(-) create mode 100755 t/t2035-checkout-locked.sh diff --git a/compat/mingw.c b/compat/mingw.c index 24bde577486115..73c20b05fdea0f 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -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 *)) +{ + 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; @@ -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; @@ -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)); @@ -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; } diff --git a/t/t2035-checkout-locked.sh b/t/t2035-checkout-locked.sh new file mode 100755 index 00000000000000..bd9eb312362a2f --- /dev/null +++ b/t/t2035-checkout-locked.sh @@ -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 8output && + 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 8output && + 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