{"thread":{"id":"32306","subject":"[PATCHv2] mingw_rmdir: do not prompt for retry when non-empty","startedAt":"2012-12-10T14:42:27Z","lastAt":"2012-12-10T17:26:26Z","messageCount":5,"participants":["Erik Faye-Lund","Junio C Hamano","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"204647","messageId":"1355150547-8212-1-git-send-email-kusmabite@gmail.com","threadId":"32306","inReplyTo":null,"subject":"[PATCHv2] mingw_rmdir: do not prompt for retry when non-empty","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-12-10T14:42:27Z","receivedAt":"2012-12-10T14:42:27Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"in ab1a11be (\"mingw_rmdir: set errno=ENOTEMPTY when appropriate\"),\na check was added to prevent us from retrying to delete a directory\nthat is both in use and non-empty.\n\nHowever, this logic was slightly flawed; since we didn't return\nimmediately, we end up falling out of the retry-loop, but right into\nthe prompting-loop.\n\nFix this by setting errno, and guarding the prompting-loop with an\nerrno-check.\n\nSigned-off-by: Erik Faye-Lund \n---\n\nHere's the second version of this patch, sorry for the slight delay.\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 1eb974f..440224c 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -260,6 +260,8 @@ int mingw_rmdir(const char *pathname)\n \n \twhile ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {\n \t\tif (!is_file_in_use_error(GetLastError()))\n+\t\t\terrno = err_win_to_posix(GetLastError());\n+\t\tif (errno != EACCES)\n \t\t\tbreak;\n \t\tif (!is_dir_empty(wpathname)) {\n \t\t\terrno = ENOTEMPTY;\n@@ -275,7 +277,7 @@ int mingw_rmdir(const char *pathname)\n \t\tSleep(delay[tries]);\n \t\ttries++;\n \t}\n-\twhile (ret == -1 && is_file_in_use_error(GetLastError()) &&\n+\twhile (ret == -1 && errno == EACCES && is_file_in_use_error(GetLastError()) &&\n \t ask_yes_no_if_possible(\"Deletion of directory '%s' failed. \"\n \t\t\t\"Should I try again?\", pathname))\n \t ret = _wrmdir(wpathname);\n-- \n1.8.0.msysgit.0.3.gafa53b0.dirty\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204648","messageId":"7vr4mxc1rd.fsf@alter.siamese.dyndns.org","threadId":"32306","inReplyTo":"1355150547-8212-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCHv2] mingw_rmdir: do not prompt for retry when non-empty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-10T16:25:58Z","receivedAt":"2012-12-10T16:25:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund writes:\n\n> in ab1a11be (\"mingw_rmdir: set errno=ENOTEMPTY when appropriate\"),\n> a check was added to prevent us from retrying to delete a directory\n> that is both in use and non-empty.\n>\n> However, this logic was slightly flawed; since we didn't return\n> immediately, we end up falling out of the retry-loop, but right into\n> the prompting-loop.\n>\n> Fix this by setting errno, and guarding the prompting-loop with an\n> errno-check.\n>\n> Signed-off-by: Erik Faye-Lund \n> ---\n>\n> Here's the second version of this patch, sorry for the slight delay.\n\nIs this meant for me, or is it to be applied to msysgit and later\nsomehow fed to me in different form?\n\nI can s/_wrmdir/rmdir/;s/wpathname/pathname/ if that is what you\nwant me to do, but otherwise this patch does not apply.\n\n>\n> compat/mingw.c | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index 1eb974f..440224c 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -260,6 +260,8 @@ int mingw_rmdir(const char *pathname)\n> \n> \twhile ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {\n> \t\tif (!is_file_in_use_error(GetLastError()))\n> +\t\t\terrno = err_win_to_posix(GetLastError());\n> +\t\tif (errno != EACCES)\n> \t\t\tbreak;\n> \t\tif (!is_dir_empty(wpathname)) {\n> \t\t\terrno = ENOTEMPTY;\n> @@ -275,7 +277,7 @@ int mingw_rmdir(const char *pathname)\n> \t\tSleep(delay[tries]);\n> \t\ttries++;\n> \t}\n> -\twhile (ret == -1 && is_file_in_use_error(GetLastError()) &&\n> +\twhile (ret == -1 && errno == EACCES && is_file_in_use_error(GetLastError()) &&\n> \t ask_yes_no_if_possible(\"Deletion of directory '%s' failed. \"\n> \t\t\t\"Should I try again?\", pathname))\n> \t ret = _wrmdir(wpathname);\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204650","messageId":"CABPQNSZL-edn4izfMuss1-3KbLBSrGm8J08wn0TbETtsn2nn+A@mail.gmail.com","threadId":"32306","inReplyTo":"7vr4mxc1rd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2] mingw_rmdir: do not prompt for retry when non-empty","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-12-10T16:50:21Z","receivedAt":"2012-12-10T16:50:21Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Dec 10, 2012 at 5:25 PM, Junio C Hamano wrote:\n> Erik Faye-Lund writes:\n>\n>> in ab1a11be (\"mingw_rmdir: set errno=ENOTEMPTY when appropriate\"),\n>> a check was added to prevent us from retrying to delete a directory\n>> that is both in use and non-empty.\n>>\n>> However, this logic was slightly flawed; since we didn't return\n>> immediately, we end up falling out of the retry-loop, but right into\n>> the prompting-loop.\n>>\n>> Fix this by setting errno, and guarding the prompting-loop with an\n>> errno-check.\n>>\n>> Signed-off-by: Erik Faye-Lund \n>> ---\n>>\n>> Here's the second version of this patch, sorry for the slight delay.\n>\n> Is this meant for me, or is it to be applied to msysgit and later\n> somehow fed to me in different form?\n>\n> I can s/_wrmdir/rmdir/;s/wpathname/pathname/ if that is what you\n> want me to do, but otherwise this patch does not apply.\n>\n\nUgh, you are right. I intended for you to apply it, but I didn't\nrealize that my patch was based on the msysGit-master, where Karsten's\nUTF-8 patches has been applied.\n\nI'm not entirely sure what the best approach would be. The issue is\npresent in both branches, but we only build installers from the\nmsysGit-branch. But I think there are other people who builds Git from\nthe upstream source code, so it would be slightly less annoying for\nthem if the patch was fixed up and applied. But on the other hand,\nthat causes some annoyance when (if?) Karsten's UTF-8 patches gets\nupstreamed.\n\nThoughts?\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204652","messageId":"alpine.DEB.1.00.1212101804290.32206@s15462909.onlinehome-server.info","threadId":"32306","inReplyTo":"CABPQNSZL-edn4izfMuss1-3KbLBSrGm8J08wn0TbETtsn2nn+A@mail.gmail.com","subject":"Re: [PATCHv2] mingw_rmdir: do not prompt for retry when non-empty","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2012-12-10T17:05:30Z","receivedAt":"2012-12-10T17:05:30Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi kusma,\n\nOn Mon, 10 Dec 2012, Erik Faye-Lund wrote:\n\n> On Mon, Dec 10, 2012 at 5:25 PM, Junio C Hamano wrote:\n> > Erik Faye-Lund writes:\n> >\n> >> in ab1a11be (\"mingw_rmdir: set errno=ENOTEMPTY when appropriate\"), a\n> >> check was added to prevent us from retrying to delete a directory\n> >> that is both in use and non-empty.\n> >>\n> >> However, this logic was slightly flawed; since we didn't return\n> >> immediately, we end up falling out of the retry-loop, but right into\n> >> the prompting-loop.\n> >>\n> >> Fix this by setting errno, and guarding the prompting-loop with an\n> >> errno-check.\n> >>\n> >> Signed-off-by: Erik Faye-Lund \n> >> ---\n> >>\n> >> Here's the second version of this patch, sorry for the slight delay.\n> >\n> > Is this meant for me, or is it to be applied to msysgit and later\n> > somehow fed to me in different form?\n> >\n> > I can s/_wrmdir/rmdir/;s/wpathname/pathname/ if that is what you\n> > want me to do, but otherwise this patch does not apply.\n> >\n> \n> Ugh, you are right. I intended for you to apply it, but I didn't realize\n> that my patch was based on the msysGit-master, where Karsten's UTF-8\n> patches has been applied.\n> \n> I'm not entirely sure what the best approach would be. The issue is\n> present in both branches, but we only build installers from the\n> msysGit-branch. But I think there are other people who builds Git from\n> the upstream source code, so it would be slightly less annoying for them\n> if the patch was fixed up and applied. But on the other hand, that\n> causes some annoyance when (if?) Karsten's UTF-8 patches gets\n> upstreamed.\n> \n> Thoughts?\n\nMy preference would be to fix it in both branches. I will fix the merge\nconflicts when rebasing onto Junio's master branch next time.\n\nCiao,\nDscho\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"},{"id":"204654","messageId":"7vehixbyyl.fsf@alter.siamese.dyndns.org","threadId":"32306","inReplyTo":"alpine.DEB.1.00.1212101804290.32206@s15462909.onlinehome-server.info","subject":"Re: [PATCHv2] mingw_rmdir: do not prompt for retry when non-empty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-10T17:26:26Z","receivedAt":"2012-12-10T17:26:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> My preference would be to fix it in both branches. I will fix the merge\n> conflicts when rebasing onto Junio's master branch next time.\n\nOK, then I'll queue the following to my tree.\n\nThanks for a quick turnaround.\n\n-- >8 --\nFrom: Erik Faye-Lund \nDate: Mon, 10 Dec 2012 15:42:27 +0100\nSubject: [PATCH] mingw_rmdir: do not prompt for retry when non-empty\n\nin ab1a11be (\"mingw_rmdir: set errno=ENOTEMPTY when appropriate\"),\na check was added to prevent us from retrying to delete a directory\nthat is both in use and non-empty.\n\nHowever, this logic was slightly flawed; since we didn't return\nimmediately, we end up falling out of the retry-loop, but right into\nthe prompting-loop.\n\nFix this by setting errno, and guarding the prompting-loop with an\nerrno-check.\n\nSigned-off-by: Erik Faye-Lund \nSigned-off-by: Junio C Hamano \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 4e63838..28527ab 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -256,6 +256,8 @@ int mingw_rmdir(const char *pathname)\n \n \twhile ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {\n \t\tif (!is_file_in_use_error(GetLastError()))\n+\t\t\terrno = err_win_to_posix(GetLastError());\n+\t\tif (errno != EACCES)\n \t\t\tbreak;\n \t\tif (!is_dir_empty(pathname)) {\n \t\t\terrno = ENOTEMPTY;\n@@ -271,7 +273,7 @@ int mingw_rmdir(const char *pathname)\n \t\tSleep(delay[tries]);\n \t\ttries++;\n \t}\n-\twhile (ret == -1 && is_file_in_use_error(GetLastError()) &&\n+\twhile (ret == -1 && errno == EACCES && is_file_in_use_error(GetLastError()) &&\n \t ask_yes_no_if_possible(\"Deletion of directory '%s' failed. \"\n \t\t\t\"Should I try again?\", pathname))\n \t ret = rmdir(pathname);\n-- \n1.8.1.rc1.123.gf61cb86\n\n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n"}]}