Skip to content

Commit

Permalink
compat/mingw: rename the symlink, not the target
Browse files Browse the repository at this point in the history
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 b30404d [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]: git-for-windows@183ea3e
[2]: git-for-windows@b30404d
[3]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew
[4]: git-for-windows#5436

Signed-off-by: Eliah Kagan <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
  • Loading branch information
EliahKagan authored and dscho committed Feb 21, 2025
1 parent b838bf1 commit 657bfbf
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion compat/mingw.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 657bfbf

Please sign in to comment.