compat/mingw: rename the symlink, not the target #1864
+3
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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]>
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Johannes Schindelin via GitGitGadget" <[email protected]>
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. |
|
This patch series was integrated into seen via git@3c0f5d2. |
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): On Fri, Feb 21, 2025 at 10:26:15AM -0800, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <[email protected]>
> 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 |
|
This patch series was integrated into seen via git@e205ecd. |
|
This patch series was integrated into next via git@8a9f3a5. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Cc: Patrick Steinhardt [email protected]