git/compat, branch v2.48.2

Merge branch 'js/mingw-rename-fix'

2024-12-23T17:32:16Z

Update the way rename() emulation on Windows handle directories to correct an earlier attempt to do the same. * js/mingw-rename-fix: mingw_rename: do support directory renames

mingw_rename: do support directory renames

2024-12-17T20:06:59Z

In 391bceae435 (compat/mingw: support POSIX semantics for atomic renames, 2024-10-27), we taught the `mingw_rename()` function to respect POSIX semantics, but we did so only as a fallback after `_wrename()` fails. This hid a bug in the implementation that was not caught by Git's test suite: The `CreateFileW()` function _can_ open handles to directories, but not when asked to use the `FILE_ATTRIBUTE_NORMAL` flag, as that flag only is allowed for files. Let's fix this by using the common `FILE_FLAG_BACKUP_SEMANTICS` flag that can be used for opening handles to directories, too. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano

global: trivial conversions to fix `-Wsign-compare` warnings

2024-12-06T11:20:04Z

We have a bunch of loops which iterate up to an unsigned boundary using a signed index, which generates warnigs because we compare a signed and unsigned value in the loop condition. Address these sites for trivial cases and enable `-Wsign-compare` warnings for these code units. This patch only adapts those code units where we can drop the `DISABLE_SIGN_COMPARE_WARNINGS` macro in the same step. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

global: mark code units that generate warnings with `-Wsign-compare`

2024-12-06T11:20:02Z

Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

compat/win32: fix -Wsign-compare warning in "wWinMain()"

2024-12-06T11:20:01Z

GCC generates a warning in "headless.c" because we compare `slash` with `size`, where the former is an `int` and the latter is a `size_t`. Fix the warning by storing `slash` as a `size_t`, as well. This commit is being singled out because the file does not include the "git-compat-util.h" header, and consequently, we cannot easily mark it with the `DISABLE_SIGN_COMPARE_WARNING` macro. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

compat/regex: explicitly ignore "-Wsign-compare" warnings

2024-12-06T11:20:01Z

Explicitly ignore "-Wsign-compare" warnings in our bundled copy of the regcomp implementation. We don't use the macro introduced in the preceding commit because this code does not include "git-compat-util.h" in the first place. Note that we already directly use "#pragma GCC diagnostic ignored" in "regcomp.c", so it shouldn't be an issue to use it directly in the new spot, either. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'ps/mingw-rename'

2024-11-12T23:35:34Z

The MinGW compatibility layer has been taught to support POSIX semantics for atomic renames when other process(es) have a file opened at the destination path. * ps/mingw-rename: compat/mingw: support POSIX semantics for atomic renames compat/mingw: allow deletion of most opened files compat/mingw: share file handles created via `CreateFileW()`

compat/mingw: support POSIX semantics for atomic renames

2024-11-06T08:15:25Z

By default, Windows restricts access to files when those files have been opened by another process. As explained in the preceding commits, these restrictions can be loosened such that reads, writes and/or deletes of files with open handles _are_ allowed. While we set up those sharing flags in most relevant code paths now, we still don't properly handle POSIX-style atomic renames in case the target path is open. This is failure demonstrated by t0610, where one of our tests spawns concurrent writes in a reftable-enabled repository and expects all of them to succeed. This test fails most of the time because the process that has acquired the "tables.list" lock is unable to rename it into place while other processes are busy reading that file. Windows 10 has introduced the `FILE_RENAME_FLAG_POSIX_SEMANTICS` flag that allows us to fix this usecase [1]. When set, it is possible to rename a file over a preexisting file even when the target file still has handles open. Those handles must have been opened with the `FILE_SHARE_DELETE` flag, which we have ensured in the preceding commits. Careful readers might have noticed that [1] does not mention the above flag, but instead mentions `FILE_RENAME_POSIX_SEMANTICS`. This flag is not for use with `SetFileInformationByHandle()` though, which is what we use. And while the `FILE_RENAME_FLAG_POSIX_SEMANTICS` flag exists, it is not documented on [2] or anywhere else as far as I can tell. Unfortunately, we still support Windows systems older than Windows 10 that do not yet have this new flag. Our `_WIN32_WINNT` SDK version still targets 0x0600, which is Windows Vista and later. And even though that Windows version is out-of-support, bumping the SDK version all the way to 0x0A00, which is Windows 10 and later, is not an option as it would make it impossible to compile on Windows 8.1, which is still supported. Instead, we have to manually declare the relevant infrastructure to make this feature available and have fallback logic in place in case we run on a Windows version that does not yet have this flag. On another note: `mingw_rename()` has a retry loop that is used in case deleting a file failed because it's still open in another process. One might be pressed to not use this loop anymore when we can use POSIX semantics. But unfortunately, we have to keep it around due to our dependence on the `FILE_SHARE_DELETE` flag. While we know to set that sharing flag now, other applications may not do so and may thus still cause sharing violations when we try to rename a file. This fixes concurrent writes in the reftable backend as demonstrated in t0610, but may also end up fixing other usecases where Git wants to perform renames. [1]: https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/ns-ntifs-_file_rename_information [2]: https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_rename_info Signed-off-by: Patrick Steinhardt Signed-off-by: Taylor Blau Reviewed-by: Johannes Sixt Signed-off-by: Junio C Hamano

compat/mingw: allow deletion of most opened files

2024-10-27T23:51:35Z

On Windows, we emulate open(3p) via `mingw_open()`. This function implements handling of some platform-specific quirks that are required to make it behave as closely as possible like open(3p) would, but for most cases we just call the Windows-specific `_wopen()` function. This function has a major downside though: it does not allow us to specify the sharing mode. While there is `_wsopen()` that allows us to pass sharing flags, those sharing flags are not the same `FILE_SHARE_*` flags as `CreateFileW()` accepts. Instead, `_wsopen()` only allows concurrent read- and write-access, but does not allow for concurrent deletions. Unfortunately though, we have to allow concurrent deletions if we want to have POSIX-style atomic renames on top of an existing file that has open file handles. Implement a new function that emulates open(3p) for existing files via `CreateFileW()` such that we can set the required sharing flags. While we have the same issue when calling open(3p) with `O_CREAT`, implementing that mode would be more complex due to the required permission handling. Furthermore, atomic updates via renames typically write to exclusive lockfile and then perform the rename, and thus we don't have to handle the case where the locked path has been created with `O_CREATE`. So while it would be nice to have proper POSIX semantics in all paths, we instead aim for a minimum viable fix here. Signed-off-by: Patrick Steinhardt Signed-off-by: Taylor Blau

compat/mingw: share file handles created via `CreateFileW()`

2024-10-27T23:51:35Z

Unless told otherwise, Windows will keep other processes from reading, writing and deleting files when one has an open handle that was created via `CreateFileW()`. This behaviour can be altered via `FILE_SHARE_*` flags: - `FILE_SHARE_READ` allows a concurrent process to open the file for reading. - `FILE_SHARE_WRITE` allows a concurrent process to open the file for writing. - `FILE_SHARE_DELETE` allows a concurrent process to delete the file or to replace it via an atomic rename. This sharing mechanism is quite important in the context of Git, as we assume POSIX semantics all over the place. But there are two callsites where we don't pass all three of these flags: - We don't set `FILE_SHARE_DELETE` when creating a file for appending via `mingw_open_append()`. This makes it impossible to delete the file from another process or to replace it via an atomic rename. The function was introduced via d641097589 (mingw: enable atomic O_APPEND, 2018-08-13) and has been using `FILE_SHARE_READ | FILE_SHARE_WRITE` since the inception. There aren't any indicators that the omission of `FILE_SHARE_DELETE` was intentional. - We don't set any sharing flags in `mingw_utime()`, which changes the access and modification of a file. This makes it impossible to perform any kind of operation on this file at all from another process. While we only open the file for a short amount of time to update its timestamps, this still opens us up for a race condition with another process. `mingw_utime()` was originally implemented via `_wopen()`, which doesn't give you full control over the sharing mode. Instead, it calls `_wsopen()` with `_SH_DENYNO`, which ultimately translates to `FILE_SHARE_READ | FILE_SHARE_WRITE`. It was then refactored via 090a3085bc (t/helper/test-chmtime: update mingw to support chmtime on directories, 2022-03-02) to use `CreateFileW()`, but we stopped setting any sharing flags at all, which seems like an unintentional side effect. By restoring `FILE_SHARE_READ | FILE_SHARE_WRITE` we thus fix this and get back the old behaviour of `_wopen()`. The fact that we didn't set the equivalent of `FILE_SHARE_DELETE` can be explained, as well: neither `_wopen()` nor `_wsopen()` allow you to do so. So overall, it doesn't seem intentional that we didn't allow deletions here, either. Adapt both of these callsites to pass all three sharing flags. Signed-off-by: Patrick Steinhardt Signed-off-by: Taylor Blau