{"thread":{"id":"62986","subject":"[PATCH] compat/mingw: rename the symlink, not the target","startedAt":"2025-02-21T12:01:40Z","lastAt":"2025-02-24T11:12:39Z","messageCount":3,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"512823","messageId":"pull.1864.git.1740139296483.gitgitgadget@gmail.com","threadId":"62986","inReplyTo":null,"subject":"[PATCH] compat/mingw: rename the symlink, not the target","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-02-21T12:01:36Z","receivedAt":"2025-02-21T12:01:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Eliah Kagan <eliah.kagan@gmail.com>\n\nSince 183ea3e [1], a new technique is used on Windows to rename files,\nwhere supported. The first step of this technique is to open the file\nwith `CreateFileW`. At that time, `FILE_ATTRIBUTE_NORMAL` was passed as\nthe value of the `dwFlagsAndAttributes` argument. In b30404df [2], this\nwas improved by passing `FILE_FLAG_BACKUP_SEMANTICS`, to support\ndirectories as well as regular files.\n\nHowever, neither value of `dwFlagsAndAttributes` is sufficient to open\na symbolic link with the correct semantics to rename it. Symlinks on\nWindows are reparse points. Attempting to open a reparse point with\n`CreateFileW` dereferences the reparse point and opens the target\ninstead, unless `FILE_FLAG_OPEN_REPARSE_POINT` is included in\n`dwFlagsAndAttributes`. This is documented for that flag and in the\n\"Symbolic Link Behavior\" section of the `CreateFileW` docs [3].\n\nThis produces a regression where attempting to rename a symlink on\nWindows renames its target to the intended new name and location of the\nsymlink. For example, if `symlink` points to `file`, then running\n\n    git mv symlink symlink-renamed\n\nleaves `symlink` in place and unchanged, but renames `file` to\n`symlink-renamed` [4].\n\nThis regression is detectable by existing tests in `t7001-mv.sh`, but\nthe tests must be run by a Windows user with the ability to create\nsymlinks, and the `ln -s` command used to create the initial symlink\nmust also be able to create a real symlink (such as by setting the\n`MSYS` environment variable to `winsymlinks:nativestrict`). Then\nthese two tests fail if the regression is present, and pass otherwise:\n\n    38 - git mv should overwrite file with a symlink\n    39 - check moved symlink\n\nLet's fix this, so that renaming a symlink again renames the symlink\nitself and leaves the target unchanged, by passing\n\n    FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT\n\nas the `dwFlagsAndAttributes` argument. This is sufficient (and safe)\nbecause including `FILE_FLAG_OPEN_REPARSE_POINT` causes no harm even\nwhen used to open a file or directory that is not a reparse point. In\nthat case, as noted in [3], this flag is simply ignored.\n\n[1]: https://github.com/git-for-windows/git/commit/183ea3eabf81822506d2cd3aa1dc0727099ebccd\n[2]: https://github.com/git-for-windows/git/commit/b30404dfc04a4b087b630aea4ab88a51cd3a7459\n[3]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew\n[4]: https://github.com/git-for-windows/git/issues/5436\n\nSigned-off-by: Eliah Kagan <eliah.kagan@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    compat/mingw: rename the symlink, not the target\n    \n    This contribution just came in as a Git for Windows Pull Request.\n    \n    Granted, I have not yet managed to find time to upstream support for\n    symbolic links (it is in the pipeline:\n    https://github.com/dscho/git/tree/support-symlinks-on-windows), but this\n    patch still should be in upstream Git because there are other ways to\n    create symbolic links than by using Git.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1864%2Fdscho%2Ffix-renaming-symlinks-on-windows-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1864/dscho/fix-renaming-symlinks-on-windows-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1864\n\n compat/mingw.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 1d5b211b548..f524c54d06d 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -2278,7 +2278,9 @@ repeat:\n \n \t\told_handle = CreateFileW(wpold, DELETE,\n \t\t\t\t\t FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE,\n-\t\t\t\t\t NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);\n+\t\t\t\t\t NULL, OPEN_EXISTING,\n+\t\t\t\t\t FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT,\n+\t\t\t\t\t NULL);\n \t\tif (old_handle == INVALID_HANDLE_VALUE) {\n \t\t\terrno = err_win_to_posix(GetLastError());\n \t\t\treturn -1;\n\nbase-commit: b838bf1938926a7a900166136d995d86f8a00e24\n-- \ngitgitgadget\n"},{"id":"512841","messageId":"xmqqtt8n9mt4.fsf@gitster.g","threadId":"62986","inReplyTo":"pull.1864.git.1740139296483.gitgitgadget@gmail.com","subject":"Re: [PATCH] compat/mingw: rename the symlink, not the target","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-21T18:26:15Z","receivedAt":"2025-02-21T18:26:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n>     This contribution just came in as a Git for Windows Pull Request.\n>     \n>     Granted, I have not yet managed to find time to upstream support for\n>     symbolic links (it is in the pipeline:\n>     https://github.com/dscho/git/tree/support-symlinks-on-windows), but this\n>     patch still should be in upstream Git because there are other ways to\n>     create symbolic links than by using Git.\n\nThanks, let me mark it for 'next' immediately.\n"},{"id":"512891","messageId":"Z7xUIjRVmmYbwRpF@pks.im","threadId":"62986","inReplyTo":"xmqqtt8n9mt4.fsf@gitster.g","subject":"Re: [PATCH] compat/mingw: rename the symlink, not the target","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-24T11:12:34Z","receivedAt":"2025-02-24T11:12:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Feb 21, 2025 at 10:26:15AM -0800, Junio C Hamano wrote:\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n> \n> >     This contribution just came in as a Git for Windows Pull Request.\n> >     \n> >     Granted, I have not yet managed to find time to upstream support for\n> >     symbolic links (it is in the pipeline:\n> >     https://github.com/dscho/git/tree/support-symlinks-on-windows), but this\n> >     patch still should be in upstream Git because there are other ways to\n> >     create symbolic links than by using Git.\n> \n> Thanks, let me mark it for 'next' immediately.\n\nThe Windows API continues to be quite esoteric to me, so thanks for\nplugging the gaps in my knowledge and fixing this edge case.\n\nPatrick\n"}]}