{"thread":{"id":"56180","subject":"[PATCH] mingw: align symlinks-related rmdir() behavior with Linux","startedAt":"2021-07-29T19:21:39Z","lastAt":"2021-08-02T21:07:36Z","messageCount":7,"participants":["Johannes Schindelin via GitGitGadget","Johannes Schindelin","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"431508","messageId":"pull.1008.git.1627586493659.gitgitgadget@gmail.com","threadId":"56180","inReplyTo":null,"subject":"[PATCH] mingw: align symlinks-related rmdir() behavior with Linux","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-29T19:21:33Z","receivedAt":"2021-07-29T19:21:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen performing a rebase, rmdir() is called on the folder .git/logs. On\nUnix rmdir() exits without deleting anything in case .git/logs is a\nsymbolic link but the equivalent functions on Windows (_rmdir, _wrmdir\nand RemoveDirectoryW) do not behave the same and remove the folder if it\nis symlinked even if it is not empty.\n\nThis creates issues when folders in .git/ are symlinks which is\nespecially the case when git-repo[1] is used.\n\nThis commit updates mingw_rmdir() so that its behavior is the same as\nLinux rmdir() in case of symbolic links.\n\nThis fixes https://github.com/git-for-windows/git/issues/2967\n\n[1]: git-repo is a python tool built on top of Git which helps manage\nmany Git repositories. It stores all the .git/ folders in a central\nplace by taking advantage of symbolic links.\nMore information: https://gerrit.googlesource.com/git-repo/\n\nSigned-off-by: Thomas Bétous <tomspycell@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    mingw: support the git-repo tool better\n    \n    This addresses an issue, originally reported at\n    https://github.com/git-for-windows/git/issues/2967, where the git-repo\n    tool [https://gerrit.googlesource.com/git-repo/] replaces folders in\n    .git/ with symlinks and mingw_rmdir() erroneously removes the symlink\n    target directory's contents.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1008%2Fdscho%2Ffix-rmdir-with-symlinks-on-windows-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1008/dscho/fix-rmdir-with-symlinks-on-windows-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1008\n\n compat/mingw.c    | 15 +++++++++++++++\n t/t3400-rebase.sh | 10 ++++++++++\n t/test-lib.sh     |  6 ++++++\n 3 files changed, 31 insertions(+)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex aa647b367b0..685d3efa3c0 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -341,6 +341,21 @@ int mingw_rmdir(const char *pathname)\n {\n \tint ret, tries = 0;\n \twchar_t wpathname[MAX_PATH];\n+\tstruct stat st;\n+\n+\t/*\n+\t* Contrary to Linux rmdir(), Windows' _wrmdir() and _rmdir()\n+\t* will remove the directory at the path if it is a symbolic link\n+\t* which leads to issues when symlinks are used in the .git folder\n+\t* (in the context of git-repo for instance). So before calling _wrmdir()\n+\t* we first check if the path is a symbolic link. If it is, we exit\n+\t* and return the same error as Linux rmdir() in this case (ENOTDIR).\n+\t*/\n+\tif (!mingw_lstat(pathname, &st) && S_ISLNK(st.st_mode)) {\n+\t\terrno = ENOTDIR;\n+\t\treturn -1;\n+\t}\n+\n \tif (xutftowcs_path(wpathname, pathname) < 0)\n \t\treturn -1;\n \ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 0bb88aa982b..23dbd3c82ed 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -406,4 +406,14 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' '\n \ttest_i18ngrep \"already checked out\" err\n '\n \n+test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '\n+\tgit checkout main &&\n+\tmv .git/logs actual_logs &&\n+\tcmd //c \"mklink /D .git\\logs ..\\actual_logs\" &&\n+\tgit rebase -f HEAD^ &&\n+\ttest -L .git/logs &&\n+\trm .git/logs &&\n+\tmv actual_logs .git/logs\n+'\n+\n test_done\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex adaf03543e8..73f6d645b66 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1513,6 +1513,12 @@ test_lazy_prereq SYMLINKS '\n \tln -s x y && test -h y\n '\n \n+test_lazy_prereq SYMLINKS_WINDOWS '\n+\t# test whether symbolic links are enabled on Windows\n+\ttest_have_prereq MINGW &&\n+\tcmd //c \"mklink y x\" &> /dev/null && test -h y\n+'\n+\n test_lazy_prereq FILEMODE '\n \ttest \"$(git config --bool core.filemode)\" = true\n '\n\nbase-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb\n-- \ngitgitgadget\n"},{"id":"431509","messageId":"nycvar.QRO.7.76.6.2107292122480.55@tvgsbejvaqbjf.bet","threadId":"56180","inReplyTo":"pull.1008.git.1627586493659.gitgitgadget@gmail.com","subject":"Re: [PATCH] mingw: align symlinks-related rmdir() behavior with Linux","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-07-29T19:23:21Z","receivedAt":"2021-07-29T19:23:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 29 Jul 2021, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSorry for missing this, this line should have read\n\n\tFrom: Thomas Bétous <tomspycell@gmail.com>\n\nCiao,\nDscho\n\n>\n> When performing a rebase, rmdir() is called on the folder .git/logs. On\n> Unix rmdir() exits without deleting anything in case .git/logs is a\n> symbolic link but the equivalent functions on Windows (_rmdir, _wrmdir\n> and RemoveDirectoryW) do not behave the same and remove the folder if it\n> is symlinked even if it is not empty.\n>\n> This creates issues when folders in .git/ are symlinks which is\n> especially the case when git-repo[1] is used.\n>\n> This commit updates mingw_rmdir() so that its behavior is the same as\n> Linux rmdir() in case of symbolic links.\n>\n> This fixes https://github.com/git-for-windows/git/issues/2967\n>\n> [1]: git-repo is a python tool built on top of Git which helps manage\n> many Git repositories. It stores all the .git/ folders in a central\n> place by taking advantage of symbolic links.\n> More information: https://gerrit.googlesource.com/git-repo/\n>\n> Signed-off-by: Thomas Bétous <tomspycell@gmail.com>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>     mingw: support the git-repo tool better\n>\n>     This addresses an issue, originally reported at\n>     https://github.com/git-for-windows/git/issues/2967, where the git-repo\n>     tool [https://gerrit.googlesource.com/git-repo/] replaces folders in\n>     .git/ with symlinks and mingw_rmdir() erroneously removes the symlink\n>     target directory's contents.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1008%2Fdscho%2Ffix-rmdir-with-symlinks-on-windows-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1008/dscho/fix-rmdir-with-symlinks-on-windows-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1008\n>\n>  compat/mingw.c    | 15 +++++++++++++++\n>  t/t3400-rebase.sh | 10 ++++++++++\n>  t/test-lib.sh     |  6 ++++++\n>  3 files changed, 31 insertions(+)\n>\n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index aa647b367b0..685d3efa3c0 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -341,6 +341,21 @@ int mingw_rmdir(const char *pathname)\n>  {\n>  \tint ret, tries = 0;\n>  \twchar_t wpathname[MAX_PATH];\n> +\tstruct stat st;\n> +\n> +\t/*\n> +\t* Contrary to Linux rmdir(), Windows' _wrmdir() and _rmdir()\n> +\t* will remove the directory at the path if it is a symbolic link\n> +\t* which leads to issues when symlinks are used in the .git folder\n> +\t* (in the context of git-repo for instance). So before calling _wrmdir()\n> +\t* we first check if the path is a symbolic link. If it is, we exit\n> +\t* and return the same error as Linux rmdir() in this case (ENOTDIR).\n> +\t*/\n> +\tif (!mingw_lstat(pathname, &st) && S_ISLNK(st.st_mode)) {\n> +\t\terrno = ENOTDIR;\n> +\t\treturn -1;\n> +\t}\n> +\n>  \tif (xutftowcs_path(wpathname, pathname) < 0)\n>  \t\treturn -1;\n>\n> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\n> index 0bb88aa982b..23dbd3c82ed 100755\n> --- a/t/t3400-rebase.sh\n> +++ b/t/t3400-rebase.sh\n> @@ -406,4 +406,14 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' '\n>  \ttest_i18ngrep \"already checked out\" err\n>  '\n>\n> +test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '\n> +\tgit checkout main &&\n> +\tmv .git/logs actual_logs &&\n> +\tcmd //c \"mklink /D .git\\logs ..\\actual_logs\" &&\n> +\tgit rebase -f HEAD^ &&\n> +\ttest -L .git/logs &&\n> +\trm .git/logs &&\n> +\tmv actual_logs .git/logs\n> +'\n> +\n>  test_done\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index adaf03543e8..73f6d645b66 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -1513,6 +1513,12 @@ test_lazy_prereq SYMLINKS '\n>  \tln -s x y && test -h y\n>  '\n>\n> +test_lazy_prereq SYMLINKS_WINDOWS '\n> +\t# test whether symbolic links are enabled on Windows\n> +\ttest_have_prereq MINGW &&\n> +\tcmd //c \"mklink y x\" &> /dev/null && test -h y\n> +'\n> +\n>  test_lazy_prereq FILEMODE '\n>  \ttest \"$(git config --bool core.filemode)\" = true\n>  '\n>\n> base-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb\n> --\n> gitgitgadget\n>\n>\n"},{"id":"431518","messageId":"xmqqo8ak51mt.fsf@gitster.g","threadId":"56180","inReplyTo":"pull.1008.git.1627586493659.gitgitgadget@gmail.com","subject":"Re: [PATCH] mingw: align symlinks-related rmdir() behavior with Linux","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-29T20:03:06Z","receivedAt":"2021-07-29T20:03:16Z","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> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> When performing a rebase, rmdir() is called on the folder .git/logs. On\n> Unix rmdir() exits without deleting anything in case .git/logs is a\n> symbolic link but the equivalent functions on Windows (_rmdir, _wrmdir\n> and RemoveDirectoryW) do not behave the same and remove the folder if it\n> is symlinked even if it is not empty.\n\nThe distinction is understandable, and I can see this justifies the\npatch really nicely.\n\nIt is curious why \"rebase\" causes rmdir on the reflog hierarchy,\nthough.  It is also unclear if \"rebase\" is special in having this\nbehaviour (and if so why), or just an example the problem was\nobserved with and other subcommands may benefit from the same fix.\n\nWill queue with authorship corrected (as you reported in the\nfollow-up).\n\nThanks.\n\n\n"},{"id":"431520","messageId":"CAPig+cR9rb+ydc5age+2FzLtTtXhg1t77p5NrA7bqN0iyU6Kyg@mail.gmail.com","threadId":"56180","inReplyTo":"pull.1008.git.1627586493659.gitgitgadget@gmail.com","subject":"Re: [PATCH] mingw: align symlinks-related rmdir() behavior with Linux","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-07-29T20:12:58Z","receivedAt":"2021-07-29T20:13:13Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jul 29, 2021 at 3:21 PM Johannes Schindelin via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> When performing a rebase, rmdir() is called on the folder .git/logs. On\n> Unix rmdir() exits without deleting anything in case .git/logs is a\n> symbolic link but the equivalent functions on Windows (_rmdir, _wrmdir\n> and RemoveDirectoryW) do not behave the same and remove the folder if it\n> is symlinked even if it is not empty.\n>\n> This creates issues when folders in .git/ are symlinks which is\n> especially the case when git-repo[1] is used.\n\n\"issues\" is a rather nebulous word. It doesn't help the reader\nunderstand what actually goes wrong. Presumably some step later in the\nrebase fails? Or is the problem that subsequent interaction with\ngit-repo fails because the link which git-repo (presumably) created\ndisappears out from underneath it?\n\n> This commit updates mingw_rmdir() so that its behavior is the same as\n> Linux rmdir() in case of symbolic links.\n\nOkay, makes sense so far as the above explanation goes. But it also\nfeels like a band-aid fix to support a use-case which only works by\naccident on Unix, if I'm reading between the lines correctly. That is,\npresumably rmdir(\".git/logs\") is an intended cleanup action but the\nfact that the cleanup doesn't happen as intended when it is a symlink\nis not intentional behavior, thus git-repo's reliance upon that\nbehavior is questionable.\n\nI guess this would also help tools which replace .git/worktrees with a\nsymlink since git-worktree -- as a cleanup -- removes its\n.git/worktrees directory when the last worktree is removed...\n\n> Signed-off-by: Thomas Bétous <tomspycell@gmail.com>\n> ---\n>  compat/mingw.c    | 15 +++++++++++++++\n> diff --git a/compat/mingw.c b/compat/mingw.c\n> @@ -341,6 +341,21 @@ int mingw_rmdir(const char *pathname)\n> +       /*\n> +       * Contrary to Linux rmdir(), Windows' _wrmdir() and _rmdir()\n> +       * will remove the directory at the path if it is a symbolic link\n> +       * which leads to issues when symlinks are used in the .git folder\n> +       * (in the context of git-repo for instance). So before calling _wrmdir()\n> +       * we first check if the path is a symbolic link. If it is, we exit\n> +       * and return the same error as Linux rmdir() in this case (ENOTDIR).\n> +       */\n> +       if (!mingw_lstat(pathname, &st) && S_ISLNK(st.st_mode)) {\n> +               errno = ENOTDIR;\n> +               return -1;\n> +       }\n\nUnfortunately, this code comment doesn't help future readers\nunderstand when/how this case can be triggered, which means that\nanyone working on this code in the future will have to consult the\ncommit message anyhow in order to obtain the necessary understanding.\nThis suggests that either the comment should be dropped altogether,\nthus forcing the person to consult the commit message (probably\nundesirable), or improved to give enough information to understand\nwhen/how it can be triggered.\n"},{"id":"431753","messageId":"nycvar.QRO.7.76.6.2108022216590.55@tvgsbejvaqbjf.bet","threadId":"56180","inReplyTo":"xmqqo8ak51mt.fsf@gitster.g","subject":"Re: [PATCH] mingw: align symlinks-related rmdir() behavior with Linux","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-02T20:17:41Z","receivedAt":"2021-08-02T20:17:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 29 Jul 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > When performing a rebase, rmdir() is called on the folder .git/logs. On\n> > Unix rmdir() exits without deleting anything in case .git/logs is a\n> > symbolic link but the equivalent functions on Windows (_rmdir, _wrmdir\n> > and RemoveDirectoryW) do not behave the same and remove the folder if it\n> > is symlinked even if it is not empty.\n>\n> The distinction is understandable, and I can see this justifies the\n> patch really nicely.\n>\n> It is curious why \"rebase\" causes rmdir on the reflog hierarchy,\n> though.  It is also unclear if \"rebase\" is special in having this\n> behaviour (and if so why), or just an example the problem was\n> observed with and other subcommands may benefit from the same fix.\n\nI augmented the commit message in preparation for v2, specifically to\naddress this concern.\n\nCiao,\nDscho\n"},{"id":"431754","messageId":"nycvar.QRO.7.76.6.2108022218120.55@tvgsbejvaqbjf.bet","threadId":"56180","inReplyTo":"CAPig+cR9rb+ydc5age+2FzLtTtXhg1t77p5NrA7bqN0iyU6Kyg@mail.gmail.com","subject":"Re: [PATCH] mingw: align symlinks-related rmdir() behavior with Linux","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-02T20:28:26Z","receivedAt":"2021-08-02T20:28:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Thu, 29 Jul 2021, Eric Sunshine wrote:\n\n> On Thu, Jul 29, 2021 at 3:21 PM Johannes Schindelin via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> > When performing a rebase, rmdir() is called on the folder .git/logs. On\n> > Unix rmdir() exits without deleting anything in case .git/logs is a\n> > symbolic link but the equivalent functions on Windows (_rmdir, _wrmdir\n> > and RemoveDirectoryW) do not behave the same and remove the folder if it\n> > is symlinked even if it is not empty.\n> >\n> > This creates issues when folders in .git/ are symlinks which is\n> > especially the case when git-repo[1] is used.\n>\n> \"issues\" is a rather nebulous word. It doesn't help the reader\n> understand what actually goes wrong. Presumably some step later in the\n> rebase fails? Or is the problem that subsequent interaction with\n> git-repo fails because the link which git-repo (presumably) created\n> disappears out from underneath it?\n\nI added a few lines describing the problem that was observed, what the\ncode specifically does to cause the problem, and then documented also that\nthe regression test specifically verifies that the observed problem won't\nreoccur.\n\n> > This commit updates mingw_rmdir() so that its behavior is the same as\n> > Linux rmdir() in case of symbolic links.\n>\n> Okay, makes sense so far as the above explanation goes. But it also\n> feels like a band-aid fix to support a use-case which only works by\n> accident on Unix, if I'm reading between the lines correctly. That is,\n> presumably rmdir(\".git/logs\") is an intended cleanup action but the\n> fact that the cleanup doesn't happen as intended when it is a symlink\n> is not intentional behavior, thus git-repo's reliance upon that\n> behavior is questionable.\n\nIt is the `remove_path()` function that tries to delete parent directories\nrecursively until things fail.\n\nWhether git-repo's reliance upon that behavior questionable is outside the\npurview of this patch. At this point in time, with so many Git versions\naccommodating that behavior, I would not want to spend much time pondering\nthat question, though: we de facto supported that expectation for too long\nto change the behavior on Git's side, and therefore adjusting the Windows\nside to that expectation feels like the most pragmatic way forward.\n\n> I guess this would also help tools which replace .git/worktrees with a\n> symlink since git-worktree -- as a cleanup -- removes its\n> .git/worktrees directory when the last worktree is removed...\n\nYes, indeed.\n\n> > Signed-off-by: Thomas Bétous <tomspycell@gmail.com>\n> > ---\n> >  compat/mingw.c    | 15 +++++++++++++++\n> > diff --git a/compat/mingw.c b/compat/mingw.c\n> > @@ -341,6 +341,21 @@ int mingw_rmdir(const char *pathname)\n> > +       /*\n> > +       * Contrary to Linux rmdir(), Windows' _wrmdir() and _rmdir()\n> > +       * will remove the directory at the path if it is a symbolic link\n> > +       * which leads to issues when symlinks are used in the .git folder\n> > +       * (in the context of git-repo for instance). So before calling _wrmdir()\n> > +       * we first check if the path is a symbolic link. If it is, we exit\n> > +       * and return the same error as Linux rmdir() in this case (ENOTDIR).\n> > +       */\n> > +       if (!mingw_lstat(pathname, &st) && S_ISLNK(st.st_mode)) {\n> > +               errno = ENOTDIR;\n> > +               return -1;\n> > +       }\n>\n> Unfortunately, this code comment doesn't help future readers\n> understand when/how this case can be triggered, which means that\n> anyone working on this code in the future will have to consult the\n> commit message anyhow in order to obtain the necessary understanding.\n> This suggests that either the comment should be dropped altogether,\n> thus forcing the person to consult the commit message (probably\n> undesirable), or improved to give enough information to understand\n> when/how it can be triggered.\n\nOkay. I reworded the comment extensively, to address your concern.\n\nCiao,\nDscho\n"},{"id":"431760","messageId":"pull.1008.v2.git.1627938451247.gitgitgadget@gmail.com","threadId":"56180","inReplyTo":"pull.1008.git.1627586493659.gitgitgadget@gmail.com","subject":"[PATCH v2] mingw: align symlinks-related rmdir() behavior with Linux","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-02T21:07:30Z","receivedAt":"2021-08-02T21:07:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: =?UTF-8?q?Thomas=20B=C3=A9tous?= <tomspycell@gmail.com>\n\nWhen performing a rebase, rmdir() is called on the folder .git/logs. On\nUnix rmdir() exits without deleting anything in case .git/logs is a\nsymbolic link but the equivalent functions on Windows (_rmdir, _wrmdir\nand RemoveDirectoryW) do not behave the same and remove the folder if it\nis symlinked even if it is not empty.\n\nThis creates issues when folders in .git/ are symlinks which is\nespecially the case when git-repo[1] is used: It replaces `.git/logs/`\nwith a symlink.\n\nOne such issue is that the _target_ of that symlink is removed e.g.\nduring a `git rebase`, where `delete_reflog(\"REBASE_HEAD\")` will not\nonly try to remove `.git/logs/REBASE_HEAD` but then recursively try to\nremove the parent directories until an error occurs, a technique that\nobviously relies on `rmdir()` refusing to remove a symlink.\n\nThis was reported in https://github.com/git-for-windows/git/issues/2967.\n\nThis commit updates mingw_rmdir() so that its behavior is the same as\nLinux rmdir() in case of symbolic links.\n\nTo verify that Git does not regress on the reported issue, this patch\nadds a regression test for the `git rebase` symptom, even if the same\n`rmdir()` behavior is quite likely to cause potential problems in other\nGit commands as well.\n\n[1]: git-repo is a python tool built on top of Git which helps manage\nmany Git repositories. It stores all the .git/ folders in a central\nplace by taking advantage of symbolic links.\nMore information: https://gerrit.googlesource.com/git-repo/\n\nSigned-off-by: Thomas Bétous <tomspycell@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    mingw: support the git-repo tool better\n    \n    This addresses an issue, originally reported at\n    https://github.com/git-for-windows/git/issues/2967, where the git-repo\n    tool [https://gerrit.googlesource.com/git-repo/] replaces folders in\n    .git/ with symlinks and mingw_rmdir() erroneously removes the symlink\n    target directory's contents.\n    \n    Changes since v1:\n    \n     * Fixed the authorship\n     * Augmented the commit message to elaborate on what the issues are,\n       concretely, with the current behavior of mingw_rmdir()\n     * Added an explanation to the commit message as to what happens during\n       a git rebase that would trigger the rmdir() code path to misbehave\n     * Adjusted the code comment to talk specifically about the\n       remove_path() caller and why we want to align with Linux' rmdir()\n       behavior\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1008%2Fdscho%2Ffix-rmdir-with-symlinks-on-windows-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1008/dscho/fix-rmdir-with-symlinks-on-windows-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1008\n\nRange-diff vs v1:\n\n 1:  c5a7fe0007f ! 1:  0a9cb1bcc14 mingw: align symlinks-related rmdir() behavior with Linux\n     @@\n       ## Metadata ##\n     -Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n     +Author: Thomas Bétous <tomspycell@gmail.com>\n      \n       ## Commit message ##\n          mingw: align symlinks-related rmdir() behavior with Linux\n     @@ Commit message\n          is symlinked even if it is not empty.\n      \n          This creates issues when folders in .git/ are symlinks which is\n     -    especially the case when git-repo[1] is used.\n     +    especially the case when git-repo[1] is used: It replaces `.git/logs/`\n     +    with a symlink.\n     +\n     +    One such issue is that the _target_ of that symlink is removed e.g.\n     +    during a `git rebase`, where `delete_reflog(\"REBASE_HEAD\")` will not\n     +    only try to remove `.git/logs/REBASE_HEAD` but then recursively try to\n     +    remove the parent directories until an error occurs, a technique that\n     +    obviously relies on `rmdir()` refusing to remove a symlink.\n     +\n     +    This was reported in https://github.com/git-for-windows/git/issues/2967.\n      \n          This commit updates mingw_rmdir() so that its behavior is the same as\n          Linux rmdir() in case of symbolic links.\n      \n     -    This fixes https://github.com/git-for-windows/git/issues/2967\n     +    To verify that Git does not regress on the reported issue, this patch\n     +    adds a regression test for the `git rebase` symptom, even if the same\n     +    `rmdir()` behavior is quite likely to cause potential problems in other\n     +    Git commands as well.\n      \n          [1]: git-repo is a python tool built on top of Git which helps manage\n          many Git repositories. It stores all the .git/ folders in a central\n     @@ compat/mingw.c: int mingw_rmdir(const char *pathname)\n      +\tstruct stat st;\n      +\n      +\t/*\n     -+\t* Contrary to Linux rmdir(), Windows' _wrmdir() and _rmdir()\n     -+\t* will remove the directory at the path if it is a symbolic link\n     -+\t* which leads to issues when symlinks are used in the .git folder\n     -+\t* (in the context of git-repo for instance). So before calling _wrmdir()\n     -+\t* we first check if the path is a symbolic link. If it is, we exit\n     -+\t* and return the same error as Linux rmdir() in this case (ENOTDIR).\n     -+\t*/\n     ++\t * Contrary to Linux' `rmdir()`, Windows' _wrmdir() and _rmdir()\n     ++\t * (and `RemoveDirectoryW()`) will attempt to remove the target of a\n     ++\t * symbolic link (if it points to a directory).\n     ++\t *\n     ++\t * This behavior breaks the assumption of e.g. `remove_path()` which\n     ++\t * upon successful deletion of a file will attempt to remove its parent\n     ++\t * directories recursively until failure (which usually happens when\n     ++\t * the directory is not empty).\n     ++\t *\n     ++\t * Therefore, before calling `_wrmdir()`, we first check if the path is\n     ++\t * a symbolic link. If it is, we exit and return the same error as\n     ++\t * Linux' `rmdir()` would, i.e. `ENOTDIR`.\n     ++\t */\n      +\tif (!mingw_lstat(pathname, &st) && S_ISLNK(st.st_mode)) {\n      +\t\terrno = ENOTDIR;\n      +\t\treturn -1;\n\n\n compat/mingw.c    | 21 +++++++++++++++++++++\n t/t3400-rebase.sh | 10 ++++++++++\n t/test-lib.sh     |  6 ++++++\n 3 files changed, 37 insertions(+)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex aa647b367b0..9e0cd1e097f 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -341,6 +341,27 @@ int mingw_rmdir(const char *pathname)\n {\n \tint ret, tries = 0;\n \twchar_t wpathname[MAX_PATH];\n+\tstruct stat st;\n+\n+\t/*\n+\t * Contrary to Linux' `rmdir()`, Windows' _wrmdir() and _rmdir()\n+\t * (and `RemoveDirectoryW()`) will attempt to remove the target of a\n+\t * symbolic link (if it points to a directory).\n+\t *\n+\t * This behavior breaks the assumption of e.g. `remove_path()` which\n+\t * upon successful deletion of a file will attempt to remove its parent\n+\t * directories recursively until failure (which usually happens when\n+\t * the directory is not empty).\n+\t *\n+\t * Therefore, before calling `_wrmdir()`, we first check if the path is\n+\t * a symbolic link. If it is, we exit and return the same error as\n+\t * Linux' `rmdir()` would, i.e. `ENOTDIR`.\n+\t */\n+\tif (!mingw_lstat(pathname, &st) && S_ISLNK(st.st_mode)) {\n+\t\terrno = ENOTDIR;\n+\t\treturn -1;\n+\t}\n+\n \tif (xutftowcs_path(wpathname, pathname) < 0)\n \t\treturn -1;\n \ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 0bb88aa982b..23dbd3c82ed 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -406,4 +406,14 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' '\n \ttest_i18ngrep \"already checked out\" err\n '\n \n+test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '\n+\tgit checkout main &&\n+\tmv .git/logs actual_logs &&\n+\tcmd //c \"mklink /D .git\\logs ..\\actual_logs\" &&\n+\tgit rebase -f HEAD^ &&\n+\ttest -L .git/logs &&\n+\trm .git/logs &&\n+\tmv actual_logs .git/logs\n+'\n+\n test_done\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex adaf03543e8..73f6d645b66 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1513,6 +1513,12 @@ test_lazy_prereq SYMLINKS '\n \tln -s x y && test -h y\n '\n \n+test_lazy_prereq SYMLINKS_WINDOWS '\n+\t# test whether symbolic links are enabled on Windows\n+\ttest_have_prereq MINGW &&\n+\tcmd //c \"mklink y x\" &> /dev/null && test -h y\n+'\n+\n test_lazy_prereq FILEMODE '\n \ttest \"$(git config --bool core.filemode)\" = true\n '\n\nbase-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb\n-- \ngitgitgadget\n"}]}