compat/mingw: rename the symlink, not the target

From: Eliah Kagan <[email protected]>

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

---
compat/mingw: rename the symlink, not the target

This contribution just came in as a Git for Windows Pull Request.

Granted, I have not yet managed to find time to upstream support for symbolic links (it is in the pipeline:
https://github.com/dscho/git/tree/support-symlinks-on-windows), but this patch still should be in upstream Git because there are other ways to create symbolic links than by using Git.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1864%2Fdscho%2Ffix-renaming-symlinks-on-windows-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1864/dscho/fix-renaming-symlinks-on-windows-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1864

 compat/mingw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index 1d5b211b548..f524c54d06d 100644
@@ -2278,7 +2278,9 @@ repeat:
 
 		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;

base-commit: b838bf1938926a7a900166136d995d86f8a00e24

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Thanks, let me mark it for 'next' immediately.

The Windows API continues to be quite esoteric to me, so thanks for plugging the gaps in my knowledge and fixing this edge case.

Patrick