From 657bfbf0ed11880f5bab1c488e2d916ccd57996a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 21 Feb 2025 01:18:59 -0500 Subject: [PATCH] compat/mingw: rename the symlink, not the target Since 183ea3e [1], a new technique is used on Windows to rename files, where supported. The first step of this technique is to open the file with `CreateFileW`. At that time, `FILE_ATTRIBUTE_NORMAL` was passed as the value of the `dwFlagsAndAttributes` argument. In b30404df [2], this was improved by passing `FILE_FLAG_BACKUP_SEMANTICS`, to support directories as well as regular files. However, neither value of `dwFlagsAndAttributes` is sufficient to open a symbolic link with the correct semantics to rename it. Symlinks on Windows are reparse points. Attempting to open a reparse point with `CreateFileW` dereferences the reparse point and opens the target instead, unless `FILE_FLAG_OPEN_REPARSE_POINT` is included in `dwFlagsAndAttributes`. This is documented for that flag and in the "Symbolic Link Behavior" section of the `CreateFileW` docs [3]. This produces a regression where attempting to rename a symlink on Windows renames its target to the intended new name and location of the symlink. For example, if `symlink` points to `file`, then running git mv symlink symlink-renamed leaves `symlink` in place and unchanged, but renames `file` to `symlink-renamed` [4]. This regression is detectable by existing tests in `t7001-mv.sh`, but the tests must be run by a Windows user with the ability to create symlinks, and the `ln -s` command used to create the initial symlink must also be able to create a real symlink (such as by setting the `MSYS` environment variable to `winsymlinks:nativestrict`). Then these two tests fail if the regression is present, and pass otherwise: 38 - git mv should overwrite file with a symlink 39 - check moved symlink Let's fix this, so that renaming a symlink again renames the symlink itself and leaves the target unchanged, by passing FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT as the `dwFlagsAndAttributes` argument. This is sufficient (and safe) because including `FILE_FLAG_OPEN_REPARSE_POINT` causes no harm even when used to open a file or directory that is not a reparse point. In that case, as noted in [3], this flag is simply ignored. [1]: https://github.com/git-for-windows/git/commit/183ea3eabf81822506d2cd3aa1dc0727099ebccd [2]: https://github.com/git-for-windows/git/commit/b30404dfc04a4b087b630aea4ab88a51cd3a7459 [3]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew [4]: https://github.com/git-for-windows/git/issues/5436 Signed-off-by: Eliah Kagan Signed-off-by: Johannes Schindelin --- compat/mingw.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compat/mingw.c b/compat/mingw.c index 1d5b211b548dab..f524c54d06d965 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2278,7 +2278,9 @@ int mingw_rename(const char *pold, const char *pnew) old_handle = CreateFileW(wpold, DELETE, FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE, - NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); + NULL, OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, + NULL); if (old_handle == INVALID_HANDLE_VALUE) { errno = err_win_to_posix(GetLastError()); return -1;