From 32269b15e2fc2e5477b8ccfb724cd087c6a41ce7 Mon Sep 17 00:00:00 2001 From: Tyler Church Date: Wed, 18 May 2016 09:33:17 -0700 Subject: [PATCH 01/14] Add retries to win32 p_unlink and p_open. --- src/win32/posix_w32.c | 46 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index fea634b00..e89b065e6 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -93,17 +93,31 @@ int p_unlink(const char *path) { git_win32_path buf; int error; + int unlink_tries; if (git_win32_path_from_utf8(buf, path) < 0) return -1; - error = _wunlink(buf); - - /* If the file could not be deleted because it was - * read-only, clear the bit and try again */ - if (error == -1 && errno == EACCES) { - _wchmod(buf, 0666); + /* wait up to 50ms if file is locked by another thread or process */ + unlink_tries = 0; + while (unlink_tries < 10) { error = _wunlink(buf); + + /* If the file could not be deleted because it was + * read-only, clear the bit and try again */ + if (error == -1 && errno == EACCES) { + _wchmod(buf, 0666); + error = _wunlink(buf); + + if (error == -1 && errno == EACCES) { + Sleep(5); + unlink_tries++; + } else { + break; + } + } else { + break; + } } return error; @@ -284,6 +298,8 @@ int p_open(const char *path, int flags, ...) { git_win32_path buf; mode_t mode = 0; + int open_tries; + int handle; if (git_win32_path_from_utf8(buf, path) < 0) return -1; @@ -296,7 +312,23 @@ int p_open(const char *path, int flags, ...) va_end(arg_list); } - return _wopen(buf, flags | STANDARD_OPEN_FLAGS, mode & WIN32_MODE_MASK); + /* wait up to 50ms if file is locked by another thread or process */ + open_tries = 0; + while (open_tries < 10) { + handle = _wopen(buf, flags | STANDARD_OPEN_FLAGS, mode & WIN32_MODE_MASK); + if (handle != -1) { + break; + } + + if (errno == EACCES) { + Sleep(5); + open_tries++; + } else { + break; + } + } + + return handle; } int p_creat(const char *path, mode_t mode) From dcaa90991f0d14ba62c3749b47f6768bf7cdcfa6 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 29 Mar 2017 23:54:47 +0100 Subject: [PATCH 02/14] win32: map windows error codes to errno Introduce mapping from windows error codes to errno values. This allows us to replace our calls to the Windows posix emulation functions with calls to the Win32 APIs for more fine-grained control over the emulation. These mappings match the Windows CRT's mappings for its posix emulation as they were described to me. --- src/win32/posix_w32.c | 113 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index 7b82d0961..14f30ebe5 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -44,6 +44,119 @@ /* GetFinalPathNameByHandleW signature */ typedef DWORD(WINAPI *PFGetFinalPathNameByHandleW)(HANDLE, LPWSTR, DWORD, DWORD); +GIT_INLINE(void) set_errno(void) +{ + switch (GetLastError()) { + case ERROR_FILE_NOT_FOUND: + case ERROR_PATH_NOT_FOUND: + case ERROR_INVALID_DRIVE: + case ERROR_NO_MORE_FILES: + case ERROR_BAD_NETPATH: + case ERROR_BAD_NET_NAME: + case ERROR_BAD_PATHNAME: + case ERROR_FILENAME_EXCED_RANGE: + errno = ENOENT; + break; + case ERROR_BAD_ENVIRONMENT: + errno = E2BIG; + break; + case ERROR_BAD_FORMAT: + case ERROR_INVALID_STARTING_CODESEG: + case ERROR_INVALID_STACKSEG: + case ERROR_INVALID_MODULETYPE: + case ERROR_INVALID_EXE_SIGNATURE: + case ERROR_EXE_MARKED_INVALID: + case ERROR_BAD_EXE_FORMAT: + case ERROR_ITERATED_DATA_EXCEEDS_64k: + case ERROR_INVALID_MINALLOCSIZE: + case ERROR_DYNLINK_FROM_INVALID_RING: + case ERROR_IOPL_NOT_ENABLED: + case ERROR_INVALID_SEGDPL: + case ERROR_AUTODATASEG_EXCEEDS_64k: + case ERROR_RING2SEG_MUST_BE_MOVABLE: + case ERROR_RELOC_CHAIN_XEEDS_SEGLIM: + case ERROR_INFLOOP_IN_RELOC_CHAIN: + errno = ENOEXEC; + break; + case ERROR_INVALID_HANDLE: + case ERROR_INVALID_TARGET_HANDLE: + case ERROR_DIRECT_ACCESS_HANDLE: + errno = EBADF; + break; + case ERROR_WAIT_NO_CHILDREN: + case ERROR_CHILD_NOT_COMPLETE: + errno = ECHILD; + break; + case ERROR_NO_PROC_SLOTS: + case ERROR_MAX_THRDS_REACHED: + case ERROR_NESTING_NOT_ALLOWED: + errno = EAGAIN; + break; + case ERROR_ARENA_TRASHED: + case ERROR_NOT_ENOUGH_MEMORY: + case ERROR_INVALID_BLOCK: + case ERROR_NOT_ENOUGH_QUOTA: + errno = ENOMEM; + break; + case ERROR_ACCESS_DENIED: + case ERROR_CURRENT_DIRECTORY: + case ERROR_WRITE_PROTECT: + case ERROR_BAD_UNIT: + case ERROR_NOT_READY: + case ERROR_BAD_COMMAND: + case ERROR_CRC: + case ERROR_BAD_LENGTH: + case ERROR_SEEK: + case ERROR_NOT_DOS_DISK: + case ERROR_SECTOR_NOT_FOUND: + case ERROR_OUT_OF_PAPER: + case ERROR_WRITE_FAULT: + case ERROR_READ_FAULT: + case ERROR_GEN_FAILURE: + case ERROR_SHARING_VIOLATION: + case ERROR_LOCK_VIOLATION: + case ERROR_WRONG_DISK: + case ERROR_SHARING_BUFFER_EXCEEDED: + case ERROR_NETWORK_ACCESS_DENIED: + case ERROR_CANNOT_MAKE: + case ERROR_FAIL_I24: + case ERROR_DRIVE_LOCKED: + case ERROR_SEEK_ON_DEVICE: + case ERROR_NOT_LOCKED: + case ERROR_LOCK_FAILED: + errno = EACCES; + break; + case ERROR_FILE_EXISTS: + case ERROR_ALREADY_EXISTS: + errno = EEXIST; + break; + case ERROR_NOT_SAME_DEVICE: + errno = EXDEV; + break; + case ERROR_INVALID_FUNCTION: + case ERROR_INVALID_ACCESS: + case ERROR_INVALID_DATA: + case ERROR_INVALID_PARAMETER: + case ERROR_NEGATIVE_SEEK: + errno = EINVAL; + break; + case ERROR_TOO_MANY_OPEN_FILES: + errno = EMFILE; + break; + case ERROR_DISK_FULL: + errno = ENOSPC; + break; + case ERROR_BROKEN_PIPE: + errno = EPIPE; + break; + case ERROR_DIR_NOT_EMPTY: + errno = ENOTEMPTY; + break; + default: + errno = EINVAL; + } +} + /** * Truncate or extend file. * From cc8d9a29e7cb24a43a10ec86a789efcf12394974 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 1 Apr 2017 10:44:17 +0100 Subject: [PATCH 03/14] win32: introduce `do_with_retries` macro Provide a macro that will allow us to run a function with posix-like return values multiple times in a retry loop, with an optional cleanup function called between invocations. --- include/git2/errors.h | 1 + src/win32/posix_w32.c | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/include/git2/errors.h b/include/git2/errors.h index 3b746b758..71bff0f9d 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -53,6 +53,7 @@ typedef enum { GIT_PASSTHROUGH = -30, /**< Internal only */ GIT_ITEROVER = -31, /**< Signals end of iteration with iterator */ + GIT_RETRY = -32, /**< Internal only */ } git_error_code; /** diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index 14f30ebe5..1dd6bffd7 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -157,6 +157,27 @@ GIT_INLINE(void) set_errno(void) } } +GIT_INLINE(bool) last_error_retryable(void) +{ + int os_error = GetLastError(); + + return (os_error == ERROR_SHARING_VIOLATION || + os_error == ERROR_ACCESS_DENIED); +} + +#define do_with_retries(fn, cleanup) \ + do { \ + int __tries, __ret; \ + for (__tries = 0; __tries < 10; __tries++) { \ + if (__tries && (__ret = (cleanup)) != 0) \ + return __ret; \ + if ((__ret = (fn)) != GIT_RETRY) \ + return __ret; \ + Sleep(5); \ + } \ + return -1; \ + } while (0) \ + /** * Truncate or extend file. * From 8a4e1513010d37f9614e7092c17895a2b79b7f7b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 1 Apr 2017 00:23:03 +0100 Subject: [PATCH 04/14] win32: make p_rename use do_with_retries --- src/win32/posix_w32.c | 63 +++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 39 deletions(-) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index 1dd6bffd7..be8bd4cb2 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -749,62 +749,47 @@ int p_access(const char* path, mode_t mode) return _waccess(buf, mode & WIN32_MODE_MASK); } -static int ensure_writable(wchar_t *fpath) +static int ensure_writable(wchar_t *path) { DWORD attrs; - attrs = GetFileAttributesW(fpath); - if (attrs == INVALID_FILE_ATTRIBUTES) { - if (GetLastError() == ERROR_FILE_NOT_FOUND) - return 0; + if ((attrs = GetFileAttributesW(path)) == INVALID_FILE_ATTRIBUTES) + goto on_error; - giterr_set(GITERR_OS, "failed to get attributes"); - return -1; - } - - if (!(attrs & FILE_ATTRIBUTE_READONLY)) + if ((attrs & FILE_ATTRIBUTE_READONLY) == 0) return 0; - attrs &= ~FILE_ATTRIBUTE_READONLY; - if (!SetFileAttributesW(fpath, attrs)) { - giterr_set(GITERR_OS, "failed to set attributes"); - return -1; - } + if (!SetFileAttributesW(path, (attrs & ~FILE_ATTRIBUTE_READONLY))) + goto on_error; return 0; + +on_error: + set_errno(); + return -1; +} + +GIT_INLINE(int) rename_once(const wchar_t *from, const wchar_t *to) +{ + if (MoveFileExW(from, to, MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED)) + return 0; + + if (last_error_retryable()) + return GIT_RETRY; + + set_errno(); + return -1; } int p_rename(const char *from, const char *to) { - git_win32_path wfrom; - git_win32_path wto; - int rename_tries; - int rename_succeeded; - int error; + git_win32_path wfrom, wto; if (git_win32_path_from_utf8(wfrom, from) < 0 || git_win32_path_from_utf8(wto, to) < 0) return -1; - /* wait up to 50ms if file is locked by another thread or process */ - rename_tries = 0; - rename_succeeded = 0; - while (rename_tries < 10) { - if (ensure_writable(wto) == 0 && - MoveFileExW(wfrom, wto, MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED) != 0) { - rename_succeeded = 1; - break; - } - - error = GetLastError(); - if (error == ERROR_SHARING_VIOLATION || error == ERROR_ACCESS_DENIED) { - Sleep(5); - rename_tries++; - } else - break; - } - - return rename_succeeded ? 0 : -1; + do_with_retries(rename_once(wfrom, wto), ensure_writable(wto)); } int p_recv(GIT_SOCKET socket, void *buffer, size_t length, int flags) From a0f67e4a262cfdc6d430d969a9a257397e4b0406 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 1 Apr 2017 13:19:51 +0100 Subject: [PATCH 05/14] win32: teach p_unlink about do_with_retries --- src/win32/posix_w32.c | 82 ++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 47 deletions(-) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index be8bd4cb2..6f2373410 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -178,6 +178,26 @@ GIT_INLINE(bool) last_error_retryable(void) return -1; \ } while (0) \ +static int ensure_writable(wchar_t *path) +{ + DWORD attrs; + + if ((attrs = GetFileAttributesW(path)) == INVALID_FILE_ATTRIBUTES) + goto on_error; + + if ((attrs & FILE_ATTRIBUTE_READONLY) == 0) + return 0; + + if (!SetFileAttributesW(path, (attrs & ~FILE_ATTRIBUTE_READONLY))) + goto on_error; + + return 0; + +on_error: + set_errno(); + return -1; +} + /** * Truncate or extend file. * @@ -223,38 +243,26 @@ int p_link(const char *old, const char *new) return -1; } +GIT_INLINE(int) unlink_once(const wchar_t *path) +{ + if (DeleteFileW(path)) + return 0; + + if (last_error_retryable()) + return GIT_RETRY; + + set_errno(); + return -1; +} + int p_unlink(const char *path) { - git_win32_path buf; - int error; - int unlink_tries; + git_win32_path wpath; - if (git_win32_path_from_utf8(buf, path) < 0) + if (git_win32_path_from_utf8(wpath, path) < 0) return -1; - /* wait up to 50ms if file is locked by another thread or process */ - unlink_tries = 0; - while (unlink_tries < 10) { - error = _wunlink(buf); - - /* If the file could not be deleted because it was - * read-only, clear the bit and try again */ - if (error == -1 && errno == EACCES) { - _wchmod(buf, 0666); - error = _wunlink(buf); - - if (error == -1 && errno == EACCES) { - Sleep(5); - unlink_tries++; - } else { - break; - } - } else { - break; - } - } - - return error; + do_with_retries(unlink_once(wpath), ensure_writable(wpath)); } int p_fsync(int fd) @@ -749,26 +757,6 @@ int p_access(const char* path, mode_t mode) return _waccess(buf, mode & WIN32_MODE_MASK); } -static int ensure_writable(wchar_t *path) -{ - DWORD attrs; - - if ((attrs = GetFileAttributesW(path)) == INVALID_FILE_ATTRIBUTES) - goto on_error; - - if ((attrs & FILE_ATTRIBUTE_READONLY) == 0) - return 0; - - if (!SetFileAttributesW(path, (attrs & ~FILE_ATTRIBUTE_READONLY))) - goto on_error; - - return 0; - -on_error: - set_errno(); - return -1; -} - GIT_INLINE(int) rename_once(const wchar_t *from, const wchar_t *to) { if (MoveFileExW(from, to, MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED)) From fbc6910f6b087f951ca515d49d48472f55c2c239 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 1 Apr 2017 13:25:14 +0100 Subject: [PATCH 06/14] win32: teach p_open about do_with_retries --- src/win32/posix_w32.c | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index 6f2373410..fd7460604 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -438,14 +438,19 @@ int p_symlink(const char *old, const char *new) return git_futils_fake_symlink(old, new); } +GIT_INLINE(int) open_once(const wchar_t *path, int flags, mode_t mode) +{ + int ret = _wopen(path, flags, mode); + + return (ret < 0 && last_error_retryable()) ? GIT_RETRY : ret; +} + int p_open(const char *path, int flags, ...) { - git_win32_path buf; + git_win32_path wpath; mode_t mode = 0; - int open_tries; - int handle; - if (git_win32_path_from_utf8(buf, path) < 0) + if (git_win32_path_from_utf8(wpath, path) < 0) return -1; if (flags & O_CREAT) { @@ -456,23 +461,9 @@ int p_open(const char *path, int flags, ...) va_end(arg_list); } - /* wait up to 50ms if file is locked by another thread or process */ - open_tries = 0; - while (open_tries < 10) { - handle = _wopen(buf, flags | STANDARD_OPEN_FLAGS, mode & WIN32_MODE_MASK); - if (handle != -1) { - break; - } - - if (errno == EACCES) { - Sleep(5); - open_tries++; - } else { - break; - } - } - - return handle; + do_with_retries( + open_once(wpath, flags | STANDARD_OPEN_FLAGS, mode & WIN32_MODE_MASK), + 0); } int p_creat(const char *path, mode_t mode) From ef5cfcdb4f2a1272813678754496da658807621b Mon Sep 17 00:00:00 2001 From: Sven Strickroth Date: Sat, 14 Jan 2017 18:20:59 +0100 Subject: [PATCH 07/14] win32: use CreateFile in p_open Signed-off-by: Sven Strickroth --- src/win32/posix_w32.c | 60 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index fd7460604..49d4e2b16 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -438,17 +438,33 @@ int p_symlink(const char *old, const char *new) return git_futils_fake_symlink(old, new); } -GIT_INLINE(int) open_once(const wchar_t *path, int flags, mode_t mode) +GIT_INLINE(int) open_once( + const wchar_t *path, + DWORD access, + DWORD sharing, + DWORD creation_disposition, + DWORD attributes, + int osf_flags) { - int ret = _wopen(path, flags, mode); + HANDLE handle = CreateFileW(path, access, sharing, NULL, + creation_disposition, attributes, 0); - return (ret < 0 && last_error_retryable()) ? GIT_RETRY : ret; + if (handle == INVALID_HANDLE_VALUE) { + if (last_error_retryable()) + return GIT_RETRY; + + set_errno(); + return -1; + } + + return _open_osfhandle((intptr_t)handle, osf_flags); } int p_open(const char *path, int flags, ...) { git_win32_path wpath; mode_t mode = 0; + DWORD access, sharing, creation, attributes, osf_flags; if (git_win32_path_from_utf8(wpath, path) < 0) return -1; @@ -461,8 +477,44 @@ int p_open(const char *path, int flags, ...) va_end(arg_list); } + switch (flags & (O_WRONLY | O_RDWR)) { + case O_WRONLY: + access = GENERIC_WRITE; + break; + case O_RDWR: + access = GENERIC_READ | GENERIC_WRITE; + break; + default: + access = GENERIC_READ; + break; + } + + sharing = FILE_SHARE_READ | FILE_SHARE_WRITE; + + switch (flags & (O_CREAT | O_TRUNC | O_EXCL)) { + case O_CREAT | O_EXCL: + case O_CREAT | O_TRUNC | O_EXCL: + creation = CREATE_NEW; + break; + case O_CREAT | O_TRUNC: + creation = CREATE_ALWAYS; + break; + case O_TRUNC: + creation = TRUNCATE_EXISTING; + break; + case O_CREAT: + creation = OPEN_ALWAYS; + break; + default: + creation = OPEN_EXISTING; + break; + } + + attributes = (mode & S_IWRITE) ? FILE_ATTRIBUTE_NORMAL : FILE_ATTRIBUTE_READONLY; + osf_flags = flags & (O_RDONLY | O_APPEND); + do_with_retries( - open_once(wpath, flags | STANDARD_OPEN_FLAGS, mode & WIN32_MODE_MASK), + open_once(wpath, access, sharing, creation, attributes, osf_flags), 0); } From 92d5a6377ad4d77bc51bb596ff7c78e9bcd0f902 Mon Sep 17 00:00:00 2001 From: Sven Strickroth Date: Sat, 14 Jan 2017 17:15:50 +0100 Subject: [PATCH 08/14] win32: deduplicate code: use p_open in p_creat Signed-off-by: Sven Strickroth --- src/win32/posix_w32.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index 49d4e2b16..28cd3e2e9 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -520,14 +520,7 @@ int p_open(const char *path, int flags, ...) int p_creat(const char *path, mode_t mode) { - git_win32_path buf; - - if (git_win32_path_from_utf8(buf, path) < 0) - return -1; - - return _wopen(buf, - _O_WRONLY | _O_CREAT | _O_TRUNC | STANDARD_OPEN_FLAGS, - mode & WIN32_MODE_MASK); + return p_open(path, O_WRONLY | O_CREAT | O_TRUNC, mode); } int p_getcwd(char *buffer_out, size_t size) From d5e6ca1e4a3f21ef9eb94c9e74cc4afb77194751 Mon Sep 17 00:00:00 2001 From: Sven Strickroth Date: Sat, 14 Jan 2017 18:39:32 +0100 Subject: [PATCH 09/14] Allow to configure default file share mode for opening files This can prevent FILE_SHARED_VIOLATIONS when used in tools such as TortoiseGit TGitCache and FILE_SHARE_DELETE, because files can be opened w/o being locked any more. Signed-off-by: Sven Strickroth --- CHANGELOG.md | 4 ++++ include/git2/common.h | 13 +++++++++++++ src/settings.c | 12 ++++++++++++ src/win32/posix.h | 2 ++ src/win32/posix_w32.c | 17 ++++++----------- 5 files changed, 37 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ae75e156..bcf8160c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ v0.25 + 1 ### API additions +* You can now set the default share mode on Windows for opening files using + `GIT_OPT_SET_WINDOWS_SHAREMODE` option with `git_libgit2_opts()`. + You can query the current share mode with `GIT_OPT_GET_WINDOWS_SHAREMODE`. + ### API removals ### Breaking API changes diff --git a/include/git2/common.h b/include/git2/common.h index c909f86ca..6d2092028 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -180,6 +180,8 @@ typedef enum { GIT_OPT_GET_USER_AGENT, GIT_OPT_ENABLE_OFS_DELTA, GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION, + GIT_OPT_GET_WINDOWS_SHAREMODE, + GIT_OPT_SET_WINDOWS_SHAREMODE, } git_libgit2_opt_t; /** @@ -284,6 +286,17 @@ typedef enum { * > - `user_agent` is the value that will be delivered as the * > User-Agent header on HTTP requests. * + * * opts(GIT_OPT_SET_WINDOWS_SHAREMODE, unsigned long value) + * + * > Set the share mode used when opening files on Windows. + * > For more information, see the documentation for CreateFile. + * > The default is: FILE_SHARE_READ | FILE_SHARE_WRITE. This is + * > ignored and unused on non-Windows platforms. + * + * * opts(GIT_OPT_GET_WINDOWS_SHAREMODE, unsigned long *value) + * + * > Get the share mode used when opening files on Windows. + * * * opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, int enabled) * * > Enable strict input validation when creating new objects diff --git a/src/settings.c b/src/settings.c index 24e549ec1..07ac16a8f 100644 --- a/src/settings.c +++ b/src/settings.c @@ -231,6 +231,18 @@ int git_libgit2_opts(int key, ...) git_object__synchronous_writing = (va_arg(ap, int) != 0); break; + case GIT_OPT_GET_WINDOWS_SHAREMODE: +#ifdef GIT_WIN32 + *(va_arg(ap, unsigned long *)) = git_win32__createfile_sharemode; +#endif + break; + + case GIT_OPT_SET_WINDOWS_SHAREMODE: +#ifdef GIT_WIN32 + git_win32__createfile_sharemode = va_arg(ap, unsigned long); +#endif + break; + default: giterr_set(GITERR_INVALID, "invalid option key"); error = -1; diff --git a/src/win32/posix.h b/src/win32/posix.h index 73705fb2b..0ba05a16f 100644 --- a/src/win32/posix.h +++ b/src/win32/posix.h @@ -14,6 +14,8 @@ #include "utf-conv.h" #include "dir.h" +extern unsigned long git_win32__createfile_sharemode; + typedef SOCKET GIT_SOCKET; #define p_lseek(f,n,w) _lseeki64(f, n, w) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index 28cd3e2e9..4943ce202 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -26,15 +26,6 @@ #define IO_REPARSE_TAG_SYMLINK (0xA000000CL) #endif -/* Options which we always provide to _wopen. - * - * _O_BINARY - Raw access; no translation of CR or LF characters - * _O_NOINHERIT - Do not mark the created handle as inheritable by child processes. - * The Windows default is 'not inheritable', but the CRT's default (following - * POSIX convention) is 'inheritable'. We have no desire for our handles to be - * inheritable on Windows, so specify the flag to get default behavior back. */ -#define STANDARD_OPEN_FLAGS (_O_BINARY | _O_NOINHERIT) - /* Allowable mode bits on Win32. Using mode bits that are not supported on * Win32 (eg S_IRWXU) is generally ignored, but Wine warns loudly about it * so we simply remove them. @@ -44,6 +35,9 @@ /* GetFinalPathNameByHandleW signature */ typedef DWORD(WINAPI *PFGetFinalPathNameByHandleW)(HANDLE, LPWSTR, DWORD, DWORD); +unsigned long git_win32__createfile_sharemode = + FILE_SHARE_READ | FILE_SHARE_WRITE; + GIT_INLINE(void) set_errno(void) { switch (GetLastError()) { @@ -489,7 +483,7 @@ int p_open(const char *path, int flags, ...) break; } - sharing = FILE_SHARE_READ | FILE_SHARE_WRITE; + sharing = (DWORD)git_win32__createfile_sharemode; switch (flags & (O_CREAT | O_TRUNC | O_EXCL)) { case O_CREAT | O_EXCL: @@ -510,7 +504,8 @@ int p_open(const char *path, int flags, ...) break; } - attributes = (mode & S_IWRITE) ? FILE_ATTRIBUTE_NORMAL : FILE_ATTRIBUTE_READONLY; + attributes = ((flags & O_CREAT) && !(mode & S_IWRITE)) ? + FILE_ATTRIBUTE_READONLY : FILE_ATTRIBUTE_NORMAL; osf_flags = flags & (O_RDONLY | O_APPEND); do_with_retries( From 1069ad3c54c9aab24f5d5a3c0a84c0b8a599b251 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 3 Apr 2017 23:05:53 +0100 Subject: [PATCH 10/14] win32: do not inherit file descriptors --- src/win32/posix_w32.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index 4943ce202..874892eb6 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -440,7 +440,14 @@ GIT_INLINE(int) open_once( DWORD attributes, int osf_flags) { - HANDLE handle = CreateFileW(path, access, sharing, NULL, + SECURITY_ATTRIBUTES security; + int fd; + + security.nLength = sizeof(SECURITY_ATTRIBUTES); + security.lpSecurityDescriptor = NULL; + security.bInheritHandle = 0; + + HANDLE handle = CreateFileW(path, access, sharing, &security, creation_disposition, attributes, 0); if (handle == INVALID_HANDLE_VALUE) { From 7ece906598fff75ea0d0b73b7158c077e6654698 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 3 Apr 2017 23:07:16 +0100 Subject: [PATCH 11/14] win32: make posix emulation retries configurable POSIX emulation retries should be configurable so that tests can disable them. In particular, maniacally threading tests may end up trying to open locked files and need retries, which will slow continuous integration tests significantly. --- src/win32/posix.h | 1 + src/win32/posix_w32.c | 3 ++- tests/threads/diff.c | 8 ++++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/win32/posix.h b/src/win32/posix.h index 0ba05a16f..64769ecd3 100644 --- a/src/win32/posix.h +++ b/src/win32/posix.h @@ -15,6 +15,7 @@ #include "dir.h" extern unsigned long git_win32__createfile_sharemode; +extern int git_win32__retries; typedef SOCKET GIT_SOCKET; diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index 874892eb6..bf132966d 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -37,6 +37,7 @@ typedef DWORD(WINAPI *PFGetFinalPathNameByHandleW)(HANDLE, LPWSTR, DWORD, DWORD) unsigned long git_win32__createfile_sharemode = FILE_SHARE_READ | FILE_SHARE_WRITE; +int git_win32__retries = 10; GIT_INLINE(void) set_errno(void) { @@ -162,7 +163,7 @@ GIT_INLINE(bool) last_error_retryable(void) #define do_with_retries(fn, cleanup) \ do { \ int __tries, __ret; \ - for (__tries = 0; __tries < 10; __tries++) { \ + for (__tries = 0; __tries < git_win32__retries; __tries++) { \ if (__tries && (__ret = (cleanup)) != 0) \ return __ret; \ if ((__ret = (fn)) != GIT_RETRY) \ diff --git a/tests/threads/diff.c b/tests/threads/diff.c index c32811469..92fd7061b 100644 --- a/tests/threads/diff.c +++ b/tests/threads/diff.c @@ -19,12 +19,20 @@ static git_repository *_repo; static git_tree *_a, *_b; static git_atomic _counts[4]; static int _check_counts; +static int _retries; #define THREADS 20 +void test_threads_diff__initialize(void) +{ + _retries = git_win32__retries; + git_win32__retries = 1; +} + void test_threads_diff__cleanup(void) { cl_git_sandbox_cleanup(); + git_win32__retries = _retries; } static void setup_trees(void) From 89d403cce27eca2ee58d2d5fe7b1bf64b59eaf3c Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 5 Apr 2017 09:50:12 +0100 Subject: [PATCH 12/14] win32: enable `p_utimes` for readonly files Instead of failing to set the timestamp of a read-only file (like any object file), set it writable temporarily to update the timestamp. --- src/posix.h | 4 + src/win32/posix_w32.c | 221 +++++++++++++++++++++++++----------------- tests/odb/freshen.c | 25 +++++ 3 files changed, 161 insertions(+), 89 deletions(-) diff --git a/src/posix.h b/src/posix.h index bd5a98e26..d26371bca 100644 --- a/src/posix.h +++ b/src/posix.h @@ -24,6 +24,10 @@ #define _S_IFLNK S_IFLNK #endif +#ifndef S_IWUSR +#define S_IWUSR 00200 +#endif + #ifndef S_IXUSR #define S_IXUSR 00100 #endif diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index bf132966d..c7094dbb9 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -363,44 +363,6 @@ int p_lstat_posixly(const char *filename, struct stat *buf) return do_lstat(filename, buf, true); } -int p_utimes(const char *filename, const struct p_timeval times[2]) -{ - int fd, error; - - if ((fd = p_open(filename, O_RDWR)) < 0) - return fd; - - error = p_futimes(fd, times); - - close(fd); - return error; -} - -int p_futimes(int fd, const struct p_timeval times[2]) -{ - HANDLE handle; - FILETIME atime = {0}, mtime = {0}; - - if (times == NULL) { - SYSTEMTIME st; - - GetSystemTime(&st); - SystemTimeToFileTime(&st, &atime); - SystemTimeToFileTime(&st, &mtime); - } else { - git_win32__timeval_to_filetime(&atime, times[0]); - git_win32__timeval_to_filetime(&mtime, times[1]); - } - - if ((handle = (HANDLE)_get_osfhandle(fd)) == INVALID_HANDLE_VALUE) - return -1; - - if (SetFileTime(handle, NULL, &atime, &mtime) == 0) - return -1; - - return 0; -} - int p_readlink(const char *path, char *buf, size_t bufsiz) { git_win32_path path_w, target_w; @@ -433,23 +395,69 @@ int p_symlink(const char *old, const char *new) return git_futils_fake_symlink(old, new); } +struct open_opts { + DWORD access; + DWORD sharing; + SECURITY_ATTRIBUTES security; + DWORD creation_disposition; + DWORD attributes; + int osf_flags; +}; + +GIT_INLINE(void) open_opts_from_posix(struct open_opts *opts, int flags, mode_t mode) +{ + memset(opts, 0, sizeof(struct open_opts)); + + switch (flags & (O_WRONLY | O_RDWR)) { + case O_WRONLY: + opts->access = GENERIC_WRITE; + break; + case O_RDWR: + opts->access = GENERIC_READ | GENERIC_WRITE; + break; + default: + opts->access = GENERIC_READ; + break; + } + + opts->sharing = (DWORD)git_win32__createfile_sharemode; + + switch (flags & (O_CREAT | O_TRUNC | O_EXCL)) { + case O_CREAT | O_EXCL: + case O_CREAT | O_TRUNC | O_EXCL: + opts->creation_disposition = CREATE_NEW; + break; + case O_CREAT | O_TRUNC: + opts->creation_disposition = CREATE_ALWAYS; + break; + case O_TRUNC: + opts->creation_disposition = TRUNCATE_EXISTING; + break; + case O_CREAT: + opts->creation_disposition = OPEN_ALWAYS; + break; + default: + opts->creation_disposition = OPEN_EXISTING; + break; + } + + opts->attributes = ((flags & O_CREAT) && !(mode & S_IWRITE)) ? + FILE_ATTRIBUTE_READONLY : FILE_ATTRIBUTE_NORMAL; + opts->osf_flags = flags & (O_RDONLY | O_APPEND); + + opts->security.nLength = sizeof(SECURITY_ATTRIBUTES); + opts->security.lpSecurityDescriptor = NULL; + opts->security.bInheritHandle = 0; +} + GIT_INLINE(int) open_once( const wchar_t *path, - DWORD access, - DWORD sharing, - DWORD creation_disposition, - DWORD attributes, - int osf_flags) + struct open_opts *opts) { - SECURITY_ATTRIBUTES security; int fd; - security.nLength = sizeof(SECURITY_ATTRIBUTES); - security.lpSecurityDescriptor = NULL; - security.bInheritHandle = 0; - - HANDLE handle = CreateFileW(path, access, sharing, &security, - creation_disposition, attributes, 0); + HANDLE handle = CreateFileW(path, opts->access, opts->sharing, + &opts->security, opts->creation_disposition, opts->attributes, 0); if (handle == INVALID_HANDLE_VALUE) { if (last_error_retryable()) @@ -459,14 +467,17 @@ GIT_INLINE(int) open_once( return -1; } - return _open_osfhandle((intptr_t)handle, osf_flags); + if ((fd = _open_osfhandle((intptr_t)handle, opts->osf_flags)) < 0) + CloseHandle(handle); + + return fd; } int p_open(const char *path, int flags, ...) { git_win32_path wpath; mode_t mode = 0; - DWORD access, sharing, creation, attributes, osf_flags; + struct open_opts opts = {0}; if (git_win32_path_from_utf8(wpath, path) < 0) return -1; @@ -479,45 +490,10 @@ int p_open(const char *path, int flags, ...) va_end(arg_list); } - switch (flags & (O_WRONLY | O_RDWR)) { - case O_WRONLY: - access = GENERIC_WRITE; - break; - case O_RDWR: - access = GENERIC_READ | GENERIC_WRITE; - break; - default: - access = GENERIC_READ; - break; - } - - sharing = (DWORD)git_win32__createfile_sharemode; - - switch (flags & (O_CREAT | O_TRUNC | O_EXCL)) { - case O_CREAT | O_EXCL: - case O_CREAT | O_TRUNC | O_EXCL: - creation = CREATE_NEW; - break; - case O_CREAT | O_TRUNC: - creation = CREATE_ALWAYS; - break; - case O_TRUNC: - creation = TRUNCATE_EXISTING; - break; - case O_CREAT: - creation = OPEN_ALWAYS; - break; - default: - creation = OPEN_EXISTING; - break; - } - - attributes = ((flags & O_CREAT) && !(mode & S_IWRITE)) ? - FILE_ATTRIBUTE_READONLY : FILE_ATTRIBUTE_NORMAL; - osf_flags = flags & (O_RDONLY | O_APPEND); + open_opts_from_posix(&opts, flags, mode); do_with_retries( - open_once(wpath, access, sharing, creation, attributes, osf_flags), + open_once(wpath, &opts), 0); } @@ -526,6 +502,73 @@ int p_creat(const char *path, mode_t mode) return p_open(path, O_WRONLY | O_CREAT | O_TRUNC, mode); } +int p_utimes(const char *path, const struct p_timeval times[2]) +{ + git_win32_path wpath; + int fd, error; + DWORD attrs_orig, attrs_new = 0; + struct open_opts opts = { 0 }; + + if (git_win32_path_from_utf8(wpath, path) < 0) + return -1; + + attrs_orig = GetFileAttributesW(wpath); + + if (attrs_orig & FILE_ATTRIBUTE_READONLY) { + attrs_new = attrs_orig & ~FILE_ATTRIBUTE_READONLY; + + if (!SetFileAttributesW(wpath, attrs_new)) { + giterr_set(GITERR_OS, "failed to set attributes"); + return -1; + } + } + + open_opts_from_posix(&opts, O_RDWR, 0); + + if ((fd = open_once(wpath, &opts)) < 0) { + error = -1; + goto done; + } + + error = p_futimes(fd, times); + close(fd); + +done: + if (attrs_orig != attrs_new) { + DWORD os_error = GetLastError(); + SetFileAttributesW(wpath, attrs_orig); + SetLastError(os_error); + } + + return error; +} + +int p_futimes(int fd, const struct p_timeval times[2]) +{ + HANDLE handle; + FILETIME atime = { 0 }, mtime = { 0 }; + + if (times == NULL) { + SYSTEMTIME st; + + GetSystemTime(&st); + SystemTimeToFileTime(&st, &atime); + SystemTimeToFileTime(&st, &mtime); + } + else { + git_win32__timeval_to_filetime(&atime, times[0]); + git_win32__timeval_to_filetime(&mtime, times[1]); + } + + if ((handle = (HANDLE)_get_osfhandle(fd)) == INVALID_HANDLE_VALUE) + return -1; + + if (SetFileTime(handle, NULL, &atime, &mtime) == 0) + return -1; + + return 0; +} + int p_getcwd(char *buffer_out, size_t size) { git_win32_path buf; diff --git a/tests/odb/freshen.c b/tests/odb/freshen.c index f41e4361e..9d3cf51dc 100644 --- a/tests/odb/freshen.c +++ b/tests/odb/freshen.c @@ -55,6 +55,31 @@ void test_odb_freshen__loose_blob(void) cl_assert(before.st_mtime < after.st_mtime); } +#define UNIQUE_STR "doesnt exist in the odb yet\n" +#define UNIQUE_BLOB_ID "78a87d0b8878c5953b9a63015ff4e22a3d898826" +#define UNIQUE_BLOB_FN "78/a87d0b8878c5953b9a63015ff4e22a3d898826" + +void test_odb_freshen__readonly_object(void) +{ + git_oid expected_id, id; + struct stat before, after; + + cl_git_pass(git_oid_fromstr(&expected_id, UNIQUE_BLOB_ID)); + + cl_git_pass(git_blob_create_frombuffer(&id, repo, UNIQUE_STR, CONST_STRLEN(UNIQUE_STR))); + cl_assert_equal_oid(&expected_id, &id); + + set_time_wayback(&before, UNIQUE_BLOB_FN); + cl_assert((before.st_mode & S_IWUSR) == 0); + + cl_git_pass(git_blob_create_frombuffer(&id, repo, UNIQUE_STR, CONST_STRLEN(UNIQUE_STR))); + cl_assert_equal_oid(&expected_id, &id); + cl_must_pass(p_lstat("testrepo.git/objects/" UNIQUE_BLOB_FN, &after)); + + cl_assert(before.st_atime < after.st_atime); + cl_assert(before.st_mtime < after.st_mtime); +} + #define LOOSE_TREE_ID "944c0f6e4dfa41595e6eb3ceecdb14f50fe18162" #define LOOSE_TREE_FN "94/4c0f6e4dfa41595e6eb3ceecdb14f50fe18162" From 48f09c6c475e158588df49cd978553fbb0d1a603 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 5 Apr 2017 11:59:03 +0100 Subject: [PATCH 13/14] win32: only set `git_win32__retries` where it exists --- tests/threads/diff.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/threads/diff.c b/tests/threads/diff.c index 92fd7061b..90d35b15e 100644 --- a/tests/threads/diff.c +++ b/tests/threads/diff.c @@ -25,14 +25,19 @@ static int _retries; void test_threads_diff__initialize(void) { +#ifdef GIT_WIN32 _retries = git_win32__retries; git_win32__retries = 1; +#endif } void test_threads_diff__cleanup(void) { cl_git_sandbox_cleanup(); + +#ifdef GIT_WIN32 git_win32__retries = _retries; +#endif } static void setup_trees(void) From 86536c7e45cce275f4aef187b44eb21f2304489a Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 17 Apr 2017 15:40:03 +0100 Subject: [PATCH 14/14] win32: `remediation` not `cleanup` The `remediation` function is run in the retry loop in order to attempt to fix any problems that the prior run encountered. There is nothing "cleaned up". Clarify the name. --- src/win32/posix_w32.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index c7094dbb9..e4fe4142c 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -160,11 +160,11 @@ GIT_INLINE(bool) last_error_retryable(void) os_error == ERROR_ACCESS_DENIED); } -#define do_with_retries(fn, cleanup) \ +#define do_with_retries(fn, remediation) \ do { \ int __tries, __ret; \ for (__tries = 0; __tries < git_win32__retries; __tries++) { \ - if (__tries && (__ret = (cleanup)) != 0) \ + if (__tries && (__ret = (remediation)) != 0) \ return __ret; \ if ((__ret = (fn)) != GIT_RETRY) \ return __ret; \