[PATCH] compat/mingw: rename the symlink, not the target

[PATCH] compat/mingw: rename the symlink, not the target

[Johannes Schindelin via GitGitGadget]

From: Eliah Kagan <eliah.kagan@gmail.com>

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 <eliah.kagan@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    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

Re: [PATCH] compat/mingw: rename the symlink, not the target

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

>     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.

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

Re: [PATCH] compat/mingw: rename the symlink, not the target

[Patrick Steinhardt]

On Fri, Feb 21, 2025 at 10:26:15AM -0800, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> >     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.
> 
> 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