[PATCH] compat/mingw: rename the symlink, not the target
From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2025-02-21 12:01:40
Subsystem:
the rest · Maintainer:
Linus Torvalds
From: Eliah Kagan <redacted>
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 <redacted>
Signed-off-by: Johannes Schindelin <redacted>
---
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(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 1d5b211b548..f524c54d06d 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c@@ -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