{"thread":{"id":"26069","subject":"[PATCH v3 0/5] make open/unlink failures user friendly on windows using retry/abort","startedAt":"2010-12-14T22:06:04Z","lastAt":"2011-02-17T23:18:54Z","messageCount":34,"participants":["Heiko Voigt","Erik Faye-Lund","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":3,"patchTotal":5},"messages":[{"id":"158095","messageId":"20101214220604.GA4084@sandbox","threadId":"26069","inReplyTo":null,"subject":"[PATCH v3 0/5] make open/unlink failures user friendly on windows using retry/abort","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-14T22:06:04Z","receivedAt":"2010-12-14T22:06:04Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Wed, Nov 10, 2010 at 09:32:00PM +0100, Johannes Sixt wrote:\n> On Dienstag, 9. November 2010, Heiko Voigt wrote:\n> > So it seems that unlink also has the problem of getting an\n> > ERROR_ACCESS_DENIED (Code 5) sometimes. Johannes would you agree that\n> > including this code into the is_file_in_use_error() function and thus\n> > having the potential risk of a 71ms delay for real access denials for\n> > these calls makes sense?\n> \n> Of course, it matches my own observations.\n\nSorry for the delay. Here is finally a new iteration of the patch\nseries. I hope I have addressed all raised issues. An extra pair of eyes\nis appreciated.\n\nThis series has been ported to Junios tree.\n\nI also added one patch from Johannes which I think should be part of\nthis series.\n\nCheers Heiko\n\nHeiko Voigt (4):\n  mingw: move unlink wrapper to mingw.c\n  mingw: work around irregular failures of unlink on windows\n  mingw: make failures to unlink or move raise a question\n  mingw: add fallback for rmdir in case directory is in use\n\nJohannes Schindelin (1):\n  mingw_rmdir: set errno=ENOTEMPTY when appropriate\n\n compat/mingw.c |  172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n compat/mingw.h |   14 ++---\n 2 files changed, 177 insertions(+), 9 deletions(-)\n\n-- \n1.7.3.3.566.gf422f\n"},{"id":"158096","messageId":"20101214220948.GB4084@sandbox","threadId":"26069","inReplyTo":"20101214220604.GA4084@sandbox","subject":"[PATCH v3 1/8] mingw: move unlink wrapper to mingw.c","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-14T22:09:49Z","receivedAt":"2010-12-14T22:09:49Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"The next patch implements a workaround in case unlink fails on Windows.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n compat/mingw.c |    8 ++++++++\n compat/mingw.h |   11 +++--------\n 2 files changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex bee6054..a7e1c6b 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -116,6 +116,14 @@ int err_win_to_posix(DWORD winerr)\n \treturn error;\n }\n \n+#undef unlink\n+int mingw_unlink(const char *pathname)\n+{\n+\t/* read-only files cannot be removed */\n+\tchmod(pathname, 0666);\n+\treturn unlink(pathname);\n+}\n+\n #undef open\n int mingw_open (const char *filename, int oflags, ...)\n {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 2283071..8316938 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -125,14 +125,6 @@ static inline int mingw_mkdir(const char *path, int mode)\n }\n #define mkdir mingw_mkdir\n \n-static inline int mingw_unlink(const char *pathname)\n-{\n-\t/* read-only files cannot be removed */\n-\tchmod(pathname, 0666);\n-\treturn unlink(pathname);\n-}\n-#define unlink mingw_unlink\n-\n #define WNOHANG 1\n pid_t waitpid(pid_t pid, int *status, unsigned options);\n \n@@ -180,6 +172,9 @@ int link(const char *oldpath, const char *newpath);\n  * replacements of existing functions\n  */\n \n+int mingw_unlink(const char *pathname);\n+#define unlink mingw_unlink\n+\n int mingw_open (const char *filename, int oflags, ...);\n #define open mingw_open\n \n-- \n1.7.3.3.566.gf422f\n"},{"id":"158097","messageId":"20101214221134.GC4084@sandbox","threadId":"26069","inReplyTo":"20101214220604.GA4084@sandbox","subject":"[PATCH v3 2/8] mingw: work around irregular failures of unlink on windows","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-14T22:11:34Z","receivedAt":"2010-12-14T22:11:34Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"If a file is opened by another process (e.g. indexing of an IDE) for\nreading it is not allowed to be deleted. So in case unlink fails retry\nafter waiting for some time. This extends the workaround from 6ac6f878.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n compat/mingw.c |   31 +++++++++++++++++++++++++++++--\n 1 files changed, 29 insertions(+), 2 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex a7e1c6b..52183a7 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -3,6 +3,8 @@\n #include <conio.h>\n #include \"../strbuf.h\"\n \n+static const int delay[] = { 0, 1, 10, 20, 40 };\n+\n int err_win_to_posix(DWORD winerr)\n {\n \tint error = ENOSYS;\n@@ -116,12 +118,38 @@ int err_win_to_posix(DWORD winerr)\n \treturn error;\n }\n \n+static inline int is_file_in_use_error(DWORD errcode)\n+{\n+\tswitch(GetLastError()) {\n+\tcase ERROR_SHARING_VIOLATION:\n+\tcase ERROR_ACCESS_DENIED:\n+\t\treturn 1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n #undef unlink\n int mingw_unlink(const char *pathname)\n {\n+\tint ret, tries = 0;\n+\n \t/* read-only files cannot be removed */\n \tchmod(pathname, 0666);\n-\treturn unlink(pathname);\n+\twhile ((ret = unlink(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {\n+\t\tif (!is_file_in_use_error(GetLastError()))\n+\t\t\tbreak;\n+\t\t/*\n+\t\t * We assume that some other process had the source or\n+\t\t * destination file open at the wrong moment and retry.\n+\t\t * In order to give the other process a higher chance to\n+\t\t * complete its operation, we give up our time slice now.\n+\t\t * If we have to retry again, we do sleep a bit.\n+\t\t */\n+\t\tSleep(delay[tries]);\n+\t\ttries++;\n+\t}\n+\treturn ret;\n }\n \n #undef open\n@@ -1257,7 +1285,6 @@ int mingw_rename(const char *pold, const char *pnew)\n {\n \tDWORD attrs, gle;\n \tint tries = 0;\n-\tstatic const int delay[] = { 0, 1, 10, 20, 40 };\n \n \t/*\n \t * Try native rename() first to get errno right.\n-- \n1.7.3.3.566.gf422f\n"},{"id":"158098","messageId":"AANLkTiksyVETuR8ftTojf=T2AhKGFtUAmqyj5u-00kW0@mail.gmail.com","threadId":"26069","inReplyTo":"20101214221134.GC4084@sandbox","subject":"Re: [PATCH v3 2/8] mingw: work around irregular failures of unlink on windows","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-12-14T22:14:13Z","receivedAt":"2010-12-14T22:14:13Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Dec 14, 2010 at 11:11 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> If a file is opened by another process (e.g. indexing of an IDE) for\n> reading it is not allowed to be deleted. So in case unlink fails retry\n> after waiting for some time. This extends the workaround from 6ac6f878.\n>\n> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n> ---\n>  compat/mingw.c |   31 +++++++++++++++++++++++++++++--\n>  1 files changed, 29 insertions(+), 2 deletions(-)\n>\n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index a7e1c6b..52183a7 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -3,6 +3,8 @@\n>  #include <conio.h>\n>  #include \"../strbuf.h\"\n>\n> +static const int delay[] = { 0, 1, 10, 20, 40 };\n> +\n>  int err_win_to_posix(DWORD winerr)\n>  {\n>        int error = ENOSYS;\n> @@ -116,12 +118,38 @@ int err_win_to_posix(DWORD winerr)\n>        return error;\n>  }\n>\n> +static inline int is_file_in_use_error(DWORD errcode)\n> +{\n> +       switch(GetLastError()) {\n\nWhy pass the error code in, just to ignore it? Shouldn't this switch\non \"errcode\" instead?\n"},{"id":"158099","messageId":"20101214222122.GD4084@sandbox","threadId":"26069","inReplyTo":"20101214220604.GA4084@sandbox","subject":"[PATCH v3 3/8] mingw: make failures to unlink or move raise a question","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-14T22:21:23Z","receivedAt":"2010-12-14T22:21:23Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Windows in case a program is accessing a file unlink or\nmove operations may fail. To give the user a chance to correct\nthis we simply wait until the user asks us to retry or fail.\n\nThis is useful because of the following use case which seem\nto happen rarely but when it does it is a mess:\n\nAfter making some changes the user realizes that he was on the\nincorrect branch. When trying to change the branch some file\nis still in use by some other process and git stops in the\nmiddle of changing branches. Now the user has lots of files\nwith changes mixed with his own. This is especially confusing\non repositories that contain lots of files.\n\nAlthough the recent implementation of automatic retry makes\nthis scenario much more unlikely lets provide a fallback as\na last resort.\n\nThanks to Albert Dvornik for disabling the question if users can't see it.\n\nIf the stdout of the command is connected to a terminal but the stderr\nhas been redirected, the odds are good that the user can't see any\nquestion we print out to stderr.  This will result in a \"mysterious\nhang\" while the app is waiting for user input.\n\nIt seems better to be conservative, and avoid asking for input\nwhenever the stderr is not a terminal, just like we do for stdin.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\nSigned-off-by: Albert Dvornik <dvornik+git@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\nI have added the sign-off from the squashed commit of Albert and\nJohannes. I hope its ok this way.\n\n compat/mingw.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 82 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 52183a7..ac9fb4a 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -2,6 +2,7 @@\n #include \"win32.h\"\n #include <conio.h>\n #include \"../strbuf.h\"\n+#include \"../run-command.h\"\n \n static const int delay[] = { 0, 1, 10, 20, 40 };\n \n@@ -129,6 +130,78 @@ static inline int is_file_in_use_error(DWORD errcode)\n \treturn 0;\n }\n \n+static int read_yes_no_answer()\n+{\n+\tchar answer[1024];\n+\n+\tif (fgets(answer, sizeof(answer), stdin)) {\n+\t\tsize_t answer_len = strlen(answer);\n+\t\tint got_full_line = 0, c;\n+\n+\t\t/* remove the newline */\n+\t\tif (answer_len >= 2 && answer[answer_len-2] == '\\r') {\n+\t\t\tanswer[answer_len-2] = '\\0';\n+\t\t\tgot_full_line = 1;\n+\t\t}\n+\t\telse if (answer_len >= 1 && answer[answer_len-1] == '\\n') {\n+\t\t\tanswer[answer_len-1] = '\\0';\n+\t\t\tgot_full_line = 1;\n+\t\t}\n+\t\t/* flush the buffer in case we did not get the full line */\n+\t\tif (!got_full_line)\n+\t\t\twhile((c = getchar()) != EOF && c != '\\n');\n+\t} else\n+\t\t/* we could not read, return the\n+\t\t * default answer which is no */\n+\t\treturn 0;\n+\n+\tif (answer[0] == 'y' && strlen(answer) == 1)\n+\t\treturn 1;\n+\tif (!strncasecmp(answer, \"yes\", sizeof(answer)))\n+\t\treturn 1;\n+\tif (answer[0] == 'n' && strlen(answer) == 1)\n+\t\treturn 0;\n+\tif (!strncasecmp(answer, \"no\", sizeof(answer)))\n+\t\treturn 0;\n+\n+\t/* did not find an answer we understand */\n+\treturn -1;\n+}\n+\n+static int ask_user_yes_no(const char *format, ...)\n+{\n+\tchar question[4096];\n+\tconst char *retry_hook[] = { NULL, NULL, NULL };\n+\tva_list args;\n+\n+\tif ((retry_hook[0] = getenv(\"GIT_ASK_YESNO\"))) {\n+\n+\t\tva_start(args, format);\n+\t\tvsnprintf(question, sizeof(question), format, args);\n+\t\tva_end(args);\n+\n+\t\tretry_hook[1] = question;\n+\t\treturn !run_command_v_opt(retry_hook, 0);\n+\t}\n+\n+\tif (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))\n+\t\treturn 0;\n+\n+\twhile (1) {\n+\t\tint answer;\n+\t\tva_start(args, format);\n+\t\tvfprintf(stderr, format, args);\n+\t\tva_end(args);\n+\t\tfprintf(stderr, \" (y/n)? \");\n+\n+\t\tif ((answer = read_yes_no_answer()) >= 0)\n+\t\t\treturn answer;\n+\n+\t\tfprintf(stderr, \"Sorry, I did not understand your answer. \"\n+\t\t\t\t\"Please type 'y' or 'n'\\n\");\n+\t}\n+}\n+\n #undef unlink\n int mingw_unlink(const char *pathname)\n {\n@@ -149,6 +222,10 @@ int mingw_unlink(const char *pathname)\n \t\tSleep(delay[tries]);\n \t\ttries++;\n \t}\n+\twhile (ret == -1 && is_file_in_use_error(GetLastError()) &&\n+\t       ask_user_yes_no(\"Unlink of file '%s' failed. \"\n+\t\t\t\"Should I try again?\", pathname))\n+\t       ret = unlink(pathname);\n \treturn ret;\n }\n \n@@ -1326,6 +1403,11 @@ repeat:\n \t\ttries++;\n \t\tgoto repeat;\n \t}\n+\tif (gle == ERROR_ACCESS_DENIED &&\n+\t       ask_user_yes_no(\"Rename from '%s' to '%s' failed. \"\n+\t\t       \"Should I try again?\", pold, pnew))\n+\t\tgoto repeat;\n+\n \terrno = EACCES;\n \treturn -1;\n }\n-- \n1.7.3.3.566.gf422f\n"},{"id":"158101","messageId":"20101214222528.GE4084@sandbox","threadId":"26069","inReplyTo":"20101214220604.GA4084@sandbox","subject":"[PATCH v3 4/5] mingw: add fallback for rmdir in case directory is in use","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-14T22:25:29Z","receivedAt":"2010-12-14T22:25:29Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"The same logic as for unlink and rename also applies to rmdir. For\nexample in case you have a shell open in a git controlled folder. This\nwill easily fail. So lets be nice for such cases as well.\n\nSigned-off-by: Heiko Voigt <heiko.voigt@mahr.de>\n---\nI just realized that there was a wrong patch count in the subject. It\nshould have been 5 instead of 8 all the time.\n\n compat/mingw.c |   25 +++++++++++++++++++++++++\n compat/mingw.h |    3 +++\n 2 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex ac9fb4a..b920644 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -229,6 +229,31 @@ int mingw_unlink(const char *pathname)\n \treturn ret;\n }\n \n+#undef rmdir\n+int mingw_rmdir(const char *pathname)\n+{\n+\tint ret, tries = 0;\n+\n+\twhile ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {\n+\t\tif (!is_file_in_use_error(GetLastError()))\n+\t\t\tbreak;\n+\t\t/*\n+\t\t * We assume that some other process had the source or\n+\t\t * destination file open at the wrong moment and retry.\n+\t\t * In order to give the other process a higher chance to\n+\t\t * complete its operation, we give up our time slice now.\n+\t\t * If we have to retry again, we do sleep a bit.\n+\t\t */\n+\t\tSleep(delay[tries]);\n+\t\ttries++;\n+\t}\n+\twhile (ret == -1 && is_file_in_use_error(GetLastError()) &&\n+\t       ask_user_yes_no(\"Deletion of directory '%s' failed. \"\n+\t\t\t\"Should I try again?\", pathname))\n+\t       ret = rmdir(pathname);\n+\treturn ret;\n+}\n+\n #undef open\n int mingw_open (const char *filename, int oflags, ...)\n {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 8316938..8b159c4 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -175,6 +175,9 @@ int link(const char *oldpath, const char *newpath);\n int mingw_unlink(const char *pathname);\n #define unlink mingw_unlink\n \n+int mingw_rmdir(const char *path);\n+#define rmdir mingw_rmdir\n+\n int mingw_open (const char *filename, int oflags, ...);\n #define open mingw_open\n \n-- \n1.7.3.3.566.gf422f\n"},{"id":"158102","messageId":"20101214222830.GF4084@sandbox","threadId":"26069","inReplyTo":"20101214220604.GA4084@sandbox","subject":"[PATCH v3 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-14T22:28:30Z","receivedAt":"2010-12-14T22:28:30Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nOn Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the\ndirectory is busy, we only want to retry deleting the directory if it\nis empty, so test specifically for that case and set ENOTEMPTY rather\nthan EACCES.\n\nNoticed by Greg Hazel.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n compat/mingw.c |   28 ++++++++++++++++++++++++++++\n 1 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex b920644..3e013c6 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -229,6 +229,30 @@ int mingw_unlink(const char *pathname)\n \treturn ret;\n }\n \n+static int is_dir_empty(const char *path)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tWIN32_FIND_DATAA findbuf;\n+\tHANDLE handle;\n+\n+\tstrbuf_addf(&buf, \"%s\\\\*\", path);\n+\thandle = FindFirstFileA(buf.buf, &findbuf);\n+\tif (handle == INVALID_HANDLE_VALUE) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn GetLastError() == ERROR_NO_MORE_FILES;\n+\t}\n+\n+\twhile (!strcmp(findbuf.cFileName, \".\") ||\n+\t\t\t!strcmp(findbuf.cFileName, \"..\"))\n+\t\tif (!FindNextFile(handle, &findbuf)) {\n+\t\t\tstrbuf_release(&buf);\n+\t\t\treturn GetLastError() == ERROR_NO_MORE_FILES;\n+\t\t}\n+\tFindClose(handle);\n+\tstrbuf_release(&buf);\n+\treturn 0;\n+}\n+\n #undef rmdir\n int mingw_rmdir(const char *pathname)\n {\n@@ -237,6 +261,10 @@ int mingw_rmdir(const char *pathname)\n \twhile ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {\n \t\tif (!is_file_in_use_error(GetLastError()))\n \t\t\tbreak;\n+\t\tif (!is_dir_empty(pathname)) {\n+\t\t\terrno = ENOTEMPTY;\n+\t\t\tbreak;\n+\t\t}\n \t\t/*\n \t\t * We assume that some other process had the source or\n \t\t * destination file open at the wrong moment and retry.\n-- \n1.7.3.3.566.gf422f\n"},{"id":"158103","messageId":"20101214223145.GG4084@sandbox","threadId":"26069","inReplyTo":"AANLkTiksyVETuR8ftTojf=T2AhKGFtUAmqyj5u-00kW0@mail.gmail.com","subject":"Re: Re: [PATCH v3 2/8] mingw: work around irregular failures of unlink on windows","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-14T22:31:45Z","receivedAt":"2010-12-14T22:31:45Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Tue, Dec 14, 2010 at 11:14:13PM +0100, Erik Faye-Lund wrote:\n> On Tue, Dec 14, 2010 at 11:11 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> > +static inline int is_file_in_use_error(DWORD errcode)\n> > +{\n> > +       switch(GetLastError()) {\n> \n> Why pass the error code in, just to ignore it? Shouldn't this switch\n> on \"errcode\" instead?\n\nDefinitely. Thanks for spotting that.\n\nCheers Heiko\n"},{"id":"158104","messageId":"AANLkTi=cHb2kV2MaYu72nXVOksO7O9HhJLEo-fU0sV5N@mail.gmail.com","threadId":"26069","inReplyTo":"20101214222122.GD4084@sandbox","subject":"Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-12-14T22:35:26Z","receivedAt":"2010-12-14T22:35:26Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Dec 14, 2010 at 11:21 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> On Windows in case a program is accessing a file unlink or\n> move operations may fail. To give the user a chance to correct\n> this we simply wait until the user asks us to retry or fail.\n>\n> This is useful because of the following use case which seem\n> to happen rarely but when it does it is a mess:\n>\n> After making some changes the user realizes that he was on the\n> incorrect branch. When trying to change the branch some file\n> is still in use by some other process and git stops in the\n> middle of changing branches. Now the user has lots of files\n> with changes mixed with his own. This is especially confusing\n> on repositories that contain lots of files.\n>\n> Although the recent implementation of automatic retry makes\n> this scenario much more unlikely lets provide a fallback as\n> a last resort.\n>\n> Thanks to Albert Dvornik for disabling the question if users can't see it.\n>\n> If the stdout of the command is connected to a terminal but the stderr\n> has been redirected, the odds are good that the user can't see any\n> question we print out to stderr.  This will result in a \"mysterious\n> hang\" while the app is waiting for user input.\n>\n> It seems better to be conservative, and avoid asking for input\n> whenever the stderr is not a terminal, just like we do for stdin.\n>\n> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n> Signed-off-by: Albert Dvornik <dvornik+git@gmail.com>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n> I have added the sign-off from the squashed commit of Albert and\n> Johannes. I hope its ok this way.\n>\n>  compat/mingw.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>  1 files changed, 82 insertions(+), 0 deletions(-)\n>\n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index 52183a7..ac9fb4a 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -2,6 +2,7 @@\n>  #include \"win32.h\"\n>  #include <conio.h>\n>  #include \"../strbuf.h\"\n> +#include \"../run-command.h\"\n>\n>  static const int delay[] = { 0, 1, 10, 20, 40 };\n>\n> @@ -129,6 +130,78 @@ static inline int is_file_in_use_error(DWORD errcode)\n>        return 0;\n>  }\n>\n> +static int read_yes_no_answer()\n\nPerhaps \"static int read_yes_no_answer(void)\" for portability?\n\n> +{\n> +       char answer[1024];\n> +\n> +       if (fgets(answer, sizeof(answer), stdin)) {\n> +               size_t answer_len = strlen(answer);\n> +               int got_full_line = 0, c;\n> +\n> +               /* remove the newline */\n> +               if (answer_len >= 2 && answer[answer_len-2] == '\\r') {\n> +                       answer[answer_len-2] = '\\0';\n> +                       got_full_line = 1;\n> +               }\n> +               else if (answer_len >= 1 && answer[answer_len-1] == '\\n') {\n> +                       answer[answer_len-1] = '\\0';\n> +                       got_full_line = 1;\n> +               }\n> +               /* flush the buffer in case we did not get the full line */\n> +               if (!got_full_line)\n> +                       while((c = getchar()) != EOF && c != '\\n');\n> +       } else\n> +               /* we could not read, return the\n> +                * default answer which is no */\n> +               return 0;\n> +\n> +       if (answer[0] == 'y' && strlen(answer) == 1)\n> +               return 1;\n> +       if (!strncasecmp(answer, \"yes\", sizeof(answer)))\n> +               return 1;\n> +       if (answer[0] == 'n' && strlen(answer) == 1)\n> +               return 0;\n> +       if (!strncasecmp(answer, \"no\", sizeof(answer)))\n> +               return 0;\n\nSince you're doing case insensitive checks for \"yes\" and \"no\", perhaps\nit'd make sense to allow upper case 'Y' and 'N' also? Something like:\n\n-       if (answer[0] == 'n' && strlen(answer) == 1)\n+       if (tolower(answer[0]) == 'n' && strlen(answer) == 1)\n\nhm?\n\n\n> +static int ask_user_yes_no(const char *format, ...)\n> +{\n> +       char question[4096];\n> +       const char *retry_hook[] = { NULL, NULL, NULL };\n> +       va_list args;\n> +\n> +       if ((retry_hook[0] = getenv(\"GIT_ASK_YESNO\"))) {\n> +\n> +               va_start(args, format);\n> +               vsnprintf(question, sizeof(question), format, args);\n> +               va_end(args);\n> +\n> +               retry_hook[1] = question;\n> +               return !run_command_v_opt(retry_hook, 0);\n> +       }\n> +\n> +       if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))\n> +               return 0;\n\nI'm wondering, doesn't this make the semantics a bit wrong? The\nfunction is called \"ask_user_yes_no\", but it might end up not asking\nafter all. Perhaps it should be called something that reflects this?\n\"maybe_ask_yes_no\", \"ask_yes_no_if_tty\", \"should_retry\"? I don't have\na non-ugly suggestion, but I suspect something like that might leave\nother people less puzzled when reading the code.\n"},{"id":"158106","messageId":"20101214224413.GH4084@sandbox","threadId":"26069","inReplyTo":"20101214223145.GG4084@sandbox","subject":"[PATCH v4 2/5] mingw: work around irregular failures of unlink on windows","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-14T22:44:14Z","receivedAt":"2010-12-14T22:44:14Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"If a file is opened by another process (e.g. indexing of an IDE) for\nreading it is not allowed to be deleted. So in case unlink fails retry\nafter waiting for some time. This extends the workaround from 6ac6f878.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\nHere is a replacement for the previous patch.\n\n compat/mingw.c |   31 +++++++++++++++++++++++++++++--\n 1 files changed, 29 insertions(+), 2 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex a7e1c6b..4a1c218 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -3,6 +3,8 @@\n #include <conio.h>\n #include \"../strbuf.h\"\n \n+static const int delay[] = { 0, 1, 10, 20, 40 };\n+\n int err_win_to_posix(DWORD winerr)\n {\n \tint error = ENOSYS;\n@@ -116,12 +118,38 @@ int err_win_to_posix(DWORD winerr)\n \treturn error;\n }\n \n+static inline int is_file_in_use_error(DWORD errcode)\n+{\n+\tswitch(errcode) {\n+\tcase ERROR_SHARING_VIOLATION:\n+\tcase ERROR_ACCESS_DENIED:\n+\t\treturn 1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n #undef unlink\n int mingw_unlink(const char *pathname)\n {\n+\tint ret, tries = 0;\n+\n \t/* read-only files cannot be removed */\n \tchmod(pathname, 0666);\n-\treturn unlink(pathname);\n+\twhile ((ret = unlink(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {\n+\t\tif (!is_file_in_use_error(GetLastError()))\n+\t\t\tbreak;\n+\t\t/*\n+\t\t * We assume that some other process had the source or\n+\t\t * destination file open at the wrong moment and retry.\n+\t\t * In order to give the other process a higher chance to\n+\t\t * complete its operation, we give up our time slice now.\n+\t\t * If we have to retry again, we do sleep a bit.\n+\t\t */\n+\t\tSleep(delay[tries]);\n+\t\ttries++;\n+\t}\n+\treturn ret;\n }\n \n #undef open\n@@ -1257,7 +1285,6 @@ int mingw_rename(const char *pold, const char *pnew)\n {\n \tDWORD attrs, gle;\n \tint tries = 0;\n-\tstatic const int delay[] = { 0, 1, 10, 20, 40 };\n \n \t/*\n \t * Try native rename() first to get errno right.\n-- \n1.7.3.3.566.g98577\n"},{"id":"158107","messageId":"AANLkTi=12WqYn5GJpqPYqKE_2L51Em7Ek5szTZYryHd9@mail.gmail.com","threadId":"26069","inReplyTo":"20101214222830.GF4084@sandbox","subject":"Re: [PATCH v3 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-12-14T22:49:17Z","receivedAt":"2010-12-14T22:49:17Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Dec 14, 2010 at 11:28 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the\n> directory is busy, we only want to retry deleting the directory if it\n> is empty, so test specifically for that case and set ENOTEMPTY rather\n> than EACCES.\n>\n\nHmm... According to MSDN, rmdir(*) should already handle ENOTEMPTY.\nIsn't the problem rather the structure of that loop? Shouldn't it be\nsufficient to do something like this (note: untested, but the concept\nshould work, no)?\n\n---8<---\n#undef rmdir\nint mingw_rmdir(const char *pathname)\n{\n       int ret, tries;\n\n       for (tries = 0; tries < ARRAY_SIZE(delay); ++tries) {\n               if (!(ret = rmdir(pathname)) || errno == ENOTEMPTY ||\n                   !is_file_in_use_error(GetLastError()))\n                      return ret;\n\n               /*\n                * We assume that some other process had the source or\n                * destination file open at the wrong moment and retry.\n                * In order to give the other process a higher chance to\n                * complete its operation, we give up our time slice now.\n                * If we have to retry again, we do sleep a bit.\n                */\n               Sleep(delay[tries]);\n       }\n       while (is_file_in_use_error(GetLastError()) &&\n              ask_user_yes_no(\"Deletion of directory '%s' failed. \"\n                       \"Should I try again?\", pathname))\n              ret = rmdir(pathname);\n       return ret;\n}\n---8<---\n\n(*) http://msdn.microsoft.com/en-us/library/wt8es881(v=VS.80).aspx\n"},{"id":"158125","messageId":"7vk4jbrjxn.fsf@alter.siamese.dyndns.org","threadId":"26069","inReplyTo":"AANLkTi=cHb2kV2MaYu72nXVOksO7O9HhJLEo-fU0sV5N@mail.gmail.com","subject":"Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-14T23:52:52Z","receivedAt":"2010-12-14T23:52:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n> Since you're doing case insensitive checks for \"yes\" and \"no\", perhaps\n> it'd make sense to allow upper case 'Y' and 'N' also? Something like:\n>\n> -       if (answer[0] == 'n' && strlen(answer) == 1)\n> +       if (tolower(answer[0]) == 'n' && strlen(answer) == 1)\n>\n> hm?\n\nWhy not\n\n\tif (tolower(answer[0]) == 'n' && !answer[1])\n\nthink of the case answer[] is very long ;-)\n\n>> +       if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))\n>> +               return 0;\n>\n> I'm wondering, doesn't this make the semantics a bit wrong? The\n> function is called \"ask_user_yes_no\", but it might end up not asking\n> after all.\n\nI don't think that is such a big issue.\n\nImagine you had only getenv(\"GIT_ASK_YESNO\") codepath, and no fallback\n\"tty\" codepath.  And you ship with a separate program as a default\n\"asker\".\n\nThe implementation of that asker happens to read yes/no from the tty, but\nit defauts to \"no\" if there is no tty interaction available.\n\nIf you view it that way, the code we see above is just an optimization to\navoid spawning that default \"asker\" as a separate process.\n\nI was more puzzled by the code to formulate question[]; why doesn't it\nbuild the same question for both codepaths and spit that out to stderr\nwith fputs() in the fallvack asker?\n"},{"id":"158126","messageId":"alpine.DEB.1.00.1012150109340.1461@bonsai2","threadId":"26069","inReplyTo":"AANLkTi=cHb2kV2MaYu72nXVOksO7O9HhJLEo-fU0sV5N@mail.gmail.com","subject":"Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-12-15T00:11:03Z","receivedAt":"2010-12-15T00:11:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 14 Dec 2010, Erik Faye-Lund wrote:\n\n> On Tue, Dec 14, 2010 at 11:21 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> > On Windows in case a program is accessing a file unlink or\n> > move operations may fail. To give the user a chance to correct\n> > this we simply wait until the user asks us to retry or fail.\n> >\n> > This is useful because of the following use case which seem\n> > to happen rarely but when it does it is a mess:\n> >\n> > After making some changes the user realizes that he was on the\n> > incorrect branch. When trying to change the branch some file\n> > is still in use by some other process and git stops in the\n> > middle of changing branches. Now the user has lots of files\n> > with changes mixed with his own. This is especially confusing\n> > on repositories that contain lots of files.\n> >\n> > Although the recent implementation of automatic retry makes\n> > this scenario much more unlikely lets provide a fallback as\n> > a last resort.\n> >\n> > Thanks to Albert Dvornik for disabling the question if users can't see it.\n> >\n> > If the stdout of the command is connected to a terminal but the stderr\n> > has been redirected, the odds are good that the user can't see any\n> > question we print out to stderr.  This will result in a \"mysterious\n> > hang\" while the app is waiting for user input.\n> >\n> > It seems better to be conservative, and avoid asking for input\n> > whenever the stderr is not a terminal, just like we do for stdin.\n> >\n> > Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n> > Signed-off-by: Albert Dvornik <dvornik+git@gmail.com>\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> > I have added the sign-off from the squashed commit of Albert and\n> > Johannes. I hope its ok this way.\n\nI'm fine with it.\n\n> >  compat/mingw.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> >  1 files changed, 82 insertions(+), 0 deletions(-)\n> >\n> > diff --git a/compat/mingw.c b/compat/mingw.c\n> > index 52183a7..ac9fb4a 100644\n> > --- a/compat/mingw.c\n> > +++ b/compat/mingw.c\n> > @@ -2,6 +2,7 @@\n> >  #include \"win32.h\"\n> >  #include <conio.h>\n> >  #include \"../strbuf.h\"\n> > +#include \"../run-command.h\"\n> >\n> >  static const int delay[] = { 0, 1, 10, 20, 40 };\n> >\n> > @@ -129,6 +130,78 @@ static inline int is_file_in_use_error(DWORD errcode)\n> >        return 0;\n> >  }\n> >\n> > +static int read_yes_no_answer()\n> \n> Perhaps \"static int read_yes_no_answer(void)\" for portability?\n\nLOL. This file is called compat/mingw.c... :-)\n\nBut I have no objection to stay with the convention of the rest of Git. \nNobody needs to convince me that consistency is good.\n\n> > +{\n> > +       char answer[1024];\n> > +\n> > +       if (fgets(answer, sizeof(answer), stdin)) {\n> > +               size_t answer_len = strlen(answer);\n> > +               int got_full_line = 0, c;\n> > +\n> > +               /* remove the newline */\n> > +               if (answer_len >= 2 && answer[answer_len-2] == '\\r') {\n> > +                       answer[answer_len-2] = '\\0';\n> > +                       got_full_line = 1;\n> > +               }\n> > +               else if (answer_len >= 1 && answer[answer_len-1] == '\\n') {\n> > +                       answer[answer_len-1] = '\\0';\n> > +                       got_full_line = 1;\n> > +               }\n> > +               /* flush the buffer in case we did not get the full line */\n> > +               if (!got_full_line)\n> > +                       while((c = getchar()) != EOF && c != '\\n');\n> > +       } else\n> > +               /* we could not read, return the\n> > +                * default answer which is no */\n> > +               return 0;\n> > +\n> > +       if (answer[0] == 'y' && strlen(answer) == 1)\n> > +               return 1;\n> > +       if (!strncasecmp(answer, \"yes\", sizeof(answer)))\n> > +               return 1;\n> > +       if (answer[0] == 'n' && strlen(answer) == 1)\n> > +               return 0;\n> > +       if (!strncasecmp(answer, \"no\", sizeof(answer)))\n> > +               return 0;\n> \n> Since you're doing case insensitive checks for \"yes\" and \"no\", perhaps\n> it'd make sense to allow upper case 'Y' and 'N' also? Something like:\n> \n> -       if (answer[0] == 'n' && strlen(answer) == 1)\n> +       if (tolower(answer[0]) == 'n' && strlen(answer) == 1)\n> \n> hm?\n\nMakes sense to me.\n\n> > +static int ask_user_yes_no(const char *format, ...)\n> > +{\n> > +       char question[4096];\n> > +       const char *retry_hook[] = { NULL, NULL, NULL };\n> > +       va_list args;\n> > +\n> > +       if ((retry_hook[0] = getenv(\"GIT_ASK_YESNO\"))) {\n> > +\n> > +               va_start(args, format);\n> > +               vsnprintf(question, sizeof(question), format, args);\n> > +               va_end(args);\n> > +\n> > +               retry_hook[1] = question;\n> > +               return !run_command_v_opt(retry_hook, 0);\n> > +       }\n> > +\n> > +       if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))\n> > +               return 0;\n> \n> I'm wondering, doesn't this make the semantics a bit wrong? The\n> function is called \"ask_user_yes_no\", but it might end up not asking\n> after all. Perhaps it should be called something that reflects this?\n> \"maybe_ask_yes_no\", \"ask_yes_no_if_tty\", \"should_retry\"? I don't have\n> a non-ugly suggestion, but I suspect something like that might leave\n> other people less puzzled when reading the code.\n\nI like ask_yes_no_if_tty.\n\nCiao,\nDscho\n"},{"id":"158128","messageId":"alpine.DEB.1.00.1012150114010.1461@bonsai2","threadId":"26069","inReplyTo":"AANLkTi=12WqYn5GJpqPYqKE_2L51Em7Ek5szTZYryHd9@mail.gmail.com","subject":"Re: [PATCH v3 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-12-15T00:21:00Z","receivedAt":"2010-12-15T00:21:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 14 Dec 2010, Erik Faye-Lund wrote:\n\n> On Tue, Dec 14, 2010 at 11:28 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the\n> > directory is busy, we only want to retry deleting the directory if it\n> > is empty, so test specifically for that case and set ENOTEMPTY rather\n> > than EACCES.\n> >\n> \n> Hmm... According to MSDN, rmdir(*) should already handle ENOTEMPTY. \n> Isn't the problem rather the structure of that loop? Shouldn't it be \n> sufficient to do something like this (note: untested, but the concept \n> should work, no)?\n\nThis is how the patch looks in current 4msysgit.git's devel branch:\n\n-- snip --\n #undef rmdir\n int mingw_rmdir(const char *pathname)\n {\n-    int ret, tries = 0;\n+       int ret, tries = 0;\n \n        while ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) \n{\n                if (errno != EACCES)\n                        break;\n+               if (!is_dir_empty(pathname)) {\n+                       errno = ENOTEMPTY;\n+                       break;\n+               }\n                /*\n                 * We assume that some other process had the source or\n                 * destination file open at the wrong moment and retry.\n-- snap --\n\nOf course, with so much water running down the Elbe between me writing \nthat patch and me answering you, I cannot really say whether rmdir() \nset errno to ENOTEMPTY.\n\nBut as the patch looked the same when I wrote it originally (you can see \nit in the history, since I introduced rebasing merges prior to making this \npatch) I would assume that in my tests, errno was set to EACCESS rather \nthan ENOTEMPTY.\n\nHowever, since I am distrusted on the Git mailing list it would be good if \nyou tried your version and verified whether what I say is true.\n\nThanks,\nDscho\n"},{"id":"158133","messageId":"7v1v5jrb1f.fsf@alter.siamese.dyndns.org","threadId":"26069","inReplyTo":"alpine.DEB.1.00.1012150109340.1461@bonsai2","subject":"Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-15T03:05:00Z","receivedAt":"2010-12-15T03:05:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> > @@ -129,6 +130,78 @@ static inline int is_file_in_use_error(DWORD errcode)\n>> >        return 0;\n>> >  }\n>> >\n>> > +static int read_yes_no_answer()\n>> \n>> Perhaps \"static int read_yes_no_answer(void)\" for portability?\n>\n> LOL. This file is called compat/mingw.c... :-)\n\nI had the same reaction.  Maybe MinGW will get a different compiler\nsomeday ;-)\n\n> But I have no objection to stay with the convention of the rest of Git. \n> Nobody needs to convince me that consistency is good.\n\nI recall there are a few old-style declaration in compat/ directory,\nespecially in borrowed code like nedmalloc/ and possibly regex/, and\nI am not so sure if we want to touch them.\n\nI'll leave this up to msysgit folks.\n\nThanks.\n"},{"id":"158140","messageId":"20101215072803.GA14646@book.hvoigt.net","threadId":"26069","inReplyTo":"7v1v5jrb1f.fsf@alter.siamese.dyndns.org","subject":"Re: Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-15T07:28:04Z","receivedAt":"2010-12-15T07:28:04Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Tue, Dec 14, 2010 at 07:05:00PM -0800, Junio C Hamano wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >> > @@ -129,6 +130,78 @@ static inline int is_file_in_use_error(DWORD errcode)\n> >> >        return 0;\n> >> >  }\n> >> >\n> >> > +static int read_yes_no_answer()\n> >> \n> >> Perhaps \"static int read_yes_no_answer(void)\" for portability?\n> >\n> > LOL. This file is called compat/mingw.c... :-)\n> \n> I had the same reaction.  Maybe MinGW will get a different compiler\n> someday ;-)\n> \n> > But I have no objection to stay with the convention of the rest of Git. \n> > Nobody needs to convince me that consistency is good.\n> \n> I recall there are a few old-style declaration in compat/ directory,\n> especially in borrowed code like nedmalloc/ and possibly regex/, and\n> I am not so sure if we want to touch them.\n> \n> I'll leave this up to msysgit folks.\n\nI do not mind changing this sincce I need to change this patch anyway.\nWill do in the next iteration.\n\nCheers Heiko\n"},{"id":"158141","messageId":"20101215073639.GB14646@book.hvoigt.net","threadId":"26069","inReplyTo":"alpine.DEB.1.00.1012150109340.1461@bonsai2","subject":"Re: Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-15T07:36:40Z","receivedAt":"2010-12-15T07:36:40Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Wed, Dec 15, 2010 at 01:11:03AM +0100, Johannes Schindelin wrote:\n> On Tue, 14 Dec 2010, Erik Faye-Lund wrote:\n> \n> > On Tue, Dec 14, 2010 at 11:21 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n[...]\n> > > +       if (answer[0] == 'y' && strlen(answer) == 1)\n> > > +               return 1;\n> > > +       if (!strncasecmp(answer, \"yes\", sizeof(answer)))\n> > > +               return 1;\n> > > +       if (answer[0] == 'n' && strlen(answer) == 1)\n> > > +               return 0;\n> > > +       if (!strncasecmp(answer, \"no\", sizeof(answer)))\n> > > +               return 0;\n> > \n> > Since you're doing case insensitive checks for \"yes\" and \"no\", perhaps\n> > it'd make sense to allow upper case 'Y' and 'N' also? Something like:\n> > \n> > -       if (answer[0] == 'n' && strlen(answer) == 1)\n> > +       if (tolower(answer[0]) == 'n' && strlen(answer) == 1)\n> > \n> > hm?\n> \n> Makes sense to me.\n\nWill do in the next iteration.\n\n> > > +static int ask_user_yes_no(const char *format, ...)\n[...]\n> > \n> > I'm wondering, doesn't this make the semantics a bit wrong? The\n> > function is called \"ask_user_yes_no\", but it might end up not asking\n> > after all. Perhaps it should be called something that reflects this?\n> > \"maybe_ask_yes_no\", \"ask_yes_no_if_tty\", \"should_retry\"? I don't have\n> > a non-ugly suggestion, but I suspect something like that might leave\n> > other people less puzzled when reading the code.\n> \n> I like ask_yes_no_if_tty.\n\nHow about ask_yes_no_if_possible() ? Since we do not just rely on tty\nbut still have GIT_ASK_YESNO to get to the user I think this would be\na closer description of what happens.\n\nCheers Heiko\n"},{"id":"158142","messageId":"20101215074832.GC14646@book.hvoigt.net","threadId":"26069","inReplyTo":"7vk4jbrjxn.fsf@alter.siamese.dyndns.org","subject":"Re: Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-12-15T07:48:32Z","receivedAt":"2010-12-15T07:48:32Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Tue, Dec 14, 2010 at 03:52:52PM -0800, Junio C Hamano wrote:\n> Erik Faye-Lund <kusmabite@gmail.com> writes:\n> \n> > Since you're doing case insensitive checks for \"yes\" and \"no\", perhaps\n> > it'd make sense to allow upper case 'Y' and 'N' also? Something like:\n> >\n> > -       if (answer[0] == 'n' && strlen(answer) == 1)\n> > +       if (tolower(answer[0]) == 'n' && strlen(answer) == 1)\n> >\n> > hm?\n> \n> Why not\n> \n> \tif (tolower(answer[0]) == 'n' && !answer[1])\n> \n> think of the case answer[] is very long ;-)\n\nWill change as stated in the previous email but of course using this\ncode for efficiency ;) Can anybody estimate how fast the user would need\nto type to actually make this noticeable and how much heat that would\nproduce on the keyboard?\n\n> >> +       if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))\n> >> +               return 0;\n> >\n> > I'm wondering, doesn't this make the semantics a bit wrong? The\n> > function is called \"ask_user_yes_no\", but it might end up not asking\n> > after all.\n> \n> I don't think that is such a big issue.\n> \n> Imagine you had only getenv(\"GIT_ASK_YESNO\") codepath, and no fallback\n> \"tty\" codepath.  And you ship with a separate program as a default\n> \"asker\".\n> \n> The implementation of that asker happens to read yes/no from the tty, but\n> it defauts to \"no\" if there is no tty interaction available.\n> \n> If you view it that way, the code we see above is just an optimization to\n> avoid spawning that default \"asker\" as a separate process.\n\nI do not mind to change this function name to make it more match what\nits doing. Since code is read way more often than written I think this\nmakes sense. See my other email about the suggestion.\n\n> I was more puzzled by the code to formulate question[]; why doesn't it\n> build the same question for both codepaths and spit that out to stderr\n> with fputs() in the fallvack asker?\n\nDo you mean that I append \" (y/n)? \" ? For the (y/n) you can think of it\nas the tui implementation of the yes/no button. I can see that the ?\nmight need to go into the question string itself. Is it that what you\nmeant?\n\nCheers Heiko\n"},{"id":"158150","messageId":"AANLkTi=vkkHmzmmGshn6v5E0e5mVkt0PMCkdhk=Mvn4u@mail.gmail.com","threadId":"26069","inReplyTo":"7v1v5jrb1f.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-12-15T09:09:28Z","receivedAt":"2010-12-15T09:09:28Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Dec 15, 2010 at 4:05 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n>>> > @@ -129,6 +130,78 @@ static inline int is_file_in_use_error(DWORD errcode)\n>>> >        return 0;\n>>> >  }\n>>> >\n>>> > +static int read_yes_no_answer()\n>>>\n>>> Perhaps \"static int read_yes_no_answer(void)\" for portability?\n>>\n>> LOL. This file is called compat/mingw.c... :-)\n>\n> I had the same reaction.  Maybe MinGW will get a different compiler\n> someday ;-)\n>\n\nWe already have; compat/msvc.c includes compat/mingw.c. mingw.c is\ncalled mingw.c because it was the first native windows port, not\nbecause it will always be compiled with MinGW. So this file is REALLY\nmore about the OS than the compiler.\n\nI don't think MSVC has a problem with this declaration either, but\nwouldn't it be nicer if we had Windows-code that was as portable as\npossible across compilers? I've also been playing around with the idea\nof using LLVM's clang for Git on Windows, because it's support for\ncross compiling between 32bit and 64bit is a bit less nasty than\nMinGW's. This might never come happen, and I don't know if clang\nsupports this or not. And then there's Intel's ICC that some times\noutperforms GCC. I don't think it would hurt fixing it in case people\nwill port - one less trip-wire in the code.\n\nBut I of course only suggest this because this is new code. It's easy\nto change it to be slightly more portable (and more consistent with\nthe rest of the code base), so why not?\n"},{"id":"158189","messageId":"AANLkTiks1drfpu9eR6S7KQ9X2MgVy91QAfKs-SRF_voG@mail.gmail.com","threadId":"26069","inReplyTo":"20101214220604.GA4084@sandbox","subject":"Re: [PATCH v3 0/5] make open/unlink failures user friendly on windows using retry/abort","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-12-15T15:52:17Z","receivedAt":"2010-12-15T15:52:17Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Dec 14, 2010 at 11:06 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> Hi,\n>\n> On Wed, Nov 10, 2010 at 09:32:00PM +0100, Johannes Sixt wrote:\n>> On Dienstag, 9. November 2010, Heiko Voigt wrote:\n>> > So it seems that unlink also has the problem of getting an\n>> > ERROR_ACCESS_DENIED (Code 5) sometimes. Johannes would you agree that\n>> > including this code into the is_file_in_use_error() function and thus\n>> > having the potential risk of a 71ms delay for real access denials for\n>> > these calls makes sense?\n>>\n>> Of course, it matches my own observations.\n>\n> Sorry for the delay. Here is finally a new iteration of the patch\n> series. I hope I have addressed all raised issues. An extra pair of eyes\n> is appreciated.\n>\n> This series has been ported to Junios tree.\n>\n> I also added one patch from Johannes which I think should be part of\n> this series.\n>\n> Cheers Heiko\n>\n> Heiko Voigt (4):\n>  mingw: move unlink wrapper to mingw.c\n>  mingw: work around irregular failures of unlink on windows\n>  mingw: make failures to unlink or move raise a question\n>  mingw: add fallback for rmdir in case directory is in use\n>\n> Johannes Schindelin (1):\n>  mingw_rmdir: set errno=ENOTEMPTY when appropriate\n>\n>  compat/mingw.c |  172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n>  compat/mingw.h |   14 ++---\n>  2 files changed, 177 insertions(+), 9 deletions(-)\n>\n\nI like the goal of the series (and enjoy the end-result on mingw), but\nthe more I think of this the more I wonder if this is solving the\nproblem on the wrong abstraction level.\n\nPOSIX says that unlink et al can set errno to EBUSY if a file is in\nuse and the implementation considers that an error. I suspect they had\na reason for adding that, and I doubt that reason was Windows. POSIX\nis already pretty incompatible with Windows. Some Googling have\nsuggested (but I haven't found definitive proof) that some flavors of\nQNX (depending on the file system used, it seems) and Solaris might be\naffected. Even Linux seems to be affected if the NTFS 3G driver is\nused. Perhaps we've only seen this on Windows so far because\nanti-virus is uncommon on these other systems? Perhaps something like\nDropbox-syncing on a mounted NTFS drive under Linux can cause this in\nreal life as well?\n\nPerhaps instead we would be better of with something like an xunlink\n(etc) in wrapper.c that deals with this on all platforms (and make\nsure that unlink sets errno to EBUSY correctly if it doesn't already)?\n"},{"id":"158193","messageId":"7vmxo6pxyi.fsf@alter.siamese.dyndns.org","threadId":"26069","inReplyTo":"AANLkTiks1drfpu9eR6S7KQ9X2MgVy91QAfKs-SRF_voG@mail.gmail.com","subject":"Re: [PATCH v3 0/5] make open/unlink failures user friendly on windows using retry/abort","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-15T20:45:09Z","receivedAt":"2010-12-15T20:45:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n> Perhaps instead we would be better of with something like an xunlink\n> (etc) in wrapper.c that deals with this on all platforms (and make\n> sure that unlink sets errno to EBUSY correctly if it doesn't already)?\n\nThat is superficially similar to the way we let xread() silently handle\nshort read to give us an easier to use API.  Especially, the part to\nsilently retry for a few times is similar to xread() recovering by\nrepeating short reads.\n\nI do not think \"ask the user one last time\" part belongs to such a\nwrapper, though.\n"},{"id":"160627","messageId":"20110207204818.GA63976@book.hvoigt.net","threadId":"26069","inReplyTo":"7vmxo6pxyi.fsf@alter.siamese.dyndns.org","subject":"[PATCH v4 0/5] make open/unlink failures user friendly on windows using retry/abort","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-07T20:48:18Z","receivedAt":"2011-02-07T20:48:18Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"This is the fourth iteration of my patch series with hopefully all the\nmentioned issues addressed.\n\nOn Wed, Dec 15, 2010 at 12:45:09PM -0800, Junio C Hamano wrote:\n> Erik Faye-Lund <kusmabite@gmail.com> writes:\n> \n> > Perhaps instead we would be better of with something like an xunlink\n> > (etc) in wrapper.c that deals with this on all platforms (and make\n> > sure that unlink sets errno to EBUSY correctly if it doesn't already)?\n> \n> That is superficially similar to the way we let xread() silently handle\n> short read to give us an easier to use API.  Especially, the part to\n> silently retry for a few times is similar to xread() recovering by\n> repeating short reads.\n> \n> I do not think \"ask the user one last time\" part belongs to such a\n> wrapper, though.\n\nI think we should currently keep this Windows specific because this is the\nonly system where such problems occur in practise. If we find later on\nthat other systems are affected with the same problem we can still move\nthe functions to another more public place. But without proper testing\nwhether it helps on other setups as well putting this into a more\nprominent place does not make sense, IMO.\n\nCheers Heiko\n\nHeiko Voigt (4):\n  mingw: move unlink wrapper to mingw.c\n  mingw: work around irregular failures of unlink on windows\n  mingw: make failures to unlink or move raise a question\n  mingw: add fallback for rmdir in case directory is in use\n\nJohannes Schindelin (1):\n  mingw_rmdir: set errno=ENOTEMPTY when appropriate\n\n compat/mingw.c |  168 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n compat/mingw.h |   14 ++---\n 2 files changed, 173 insertions(+), 9 deletions(-)\n\n-- \n1.7.4.34.gd2cb1\n"},{"id":"160629","messageId":"20110207204932.GB63976@book.hvoigt.net","threadId":"26069","inReplyTo":"20110207204818.GA63976@book.hvoigt.net","subject":"[PATCH v4 1/5] mingw: move unlink wrapper to mingw.c","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-07T20:49:33Z","receivedAt":"2011-02-07T20:49:33Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"The next patch implements a workaround in case unlink fails on Windows.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n compat/mingw.c |    8 ++++++++\n compat/mingw.h |   11 +++--------\n 2 files changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex bee6054..a7e1c6b 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -116,6 +116,14 @@ int err_win_to_posix(DWORD winerr)\n \treturn error;\n }\n \n+#undef unlink\n+int mingw_unlink(const char *pathname)\n+{\n+\t/* read-only files cannot be removed */\n+\tchmod(pathname, 0666);\n+\treturn unlink(pathname);\n+}\n+\n #undef open\n int mingw_open (const char *filename, int oflags, ...)\n {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex cafc1eb..970d743 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -119,14 +119,6 @@ static inline int mingw_mkdir(const char *path, int mode)\n }\n #define mkdir mingw_mkdir\n \n-static inline int mingw_unlink(const char *pathname)\n-{\n-\t/* read-only files cannot be removed */\n-\tchmod(pathname, 0666);\n-\treturn unlink(pathname);\n-}\n-#define unlink mingw_unlink\n-\n #define WNOHANG 1\n pid_t waitpid(pid_t pid, int *status, unsigned options);\n \n@@ -174,6 +166,9 @@ int link(const char *oldpath, const char *newpath);\n  * replacements of existing functions\n  */\n \n+int mingw_unlink(const char *pathname);\n+#define unlink mingw_unlink\n+\n int mingw_open (const char *filename, int oflags, ...);\n #define open mingw_open\n \n-- \n1.7.4.34.gd2cb1\n"},{"id":"160630","messageId":"20110207205026.GC63976@book.hvoigt.net","threadId":"26069","inReplyTo":"20110207204818.GA63976@book.hvoigt.net","subject":"[PATCH v4 2/5] mingw: work around irregular failures of unlink on windows","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-07T20:50:26Z","receivedAt":"2011-02-07T20:50:26Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"If a file is opened by another process (e.g. indexing of an IDE) for\nreading it is not allowed to be deleted. So in case unlink fails retry\nafter waiting for some time. This extends the workaround from 6ac6f878.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n compat/mingw.c |   31 +++++++++++++++++++++++++++++--\n 1 files changed, 29 insertions(+), 2 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex a7e1c6b..4a1c218 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -3,6 +3,8 @@\n #include <conio.h>\n #include \"../strbuf.h\"\n \n+static const int delay[] = { 0, 1, 10, 20, 40 };\n+\n int err_win_to_posix(DWORD winerr)\n {\n \tint error = ENOSYS;\n@@ -116,12 +118,38 @@ int err_win_to_posix(DWORD winerr)\n \treturn error;\n }\n \n+static inline int is_file_in_use_error(DWORD errcode)\n+{\n+\tswitch(errcode) {\n+\tcase ERROR_SHARING_VIOLATION:\n+\tcase ERROR_ACCESS_DENIED:\n+\t\treturn 1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n #undef unlink\n int mingw_unlink(const char *pathname)\n {\n+\tint ret, tries = 0;\n+\n \t/* read-only files cannot be removed */\n \tchmod(pathname, 0666);\n-\treturn unlink(pathname);\n+\twhile ((ret = unlink(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {\n+\t\tif (!is_file_in_use_error(GetLastError()))\n+\t\t\tbreak;\n+\t\t/*\n+\t\t * We assume that some other process had the source or\n+\t\t * destination file open at the wrong moment and retry.\n+\t\t * In order to give the other process a higher chance to\n+\t\t * complete its operation, we give up our time slice now.\n+\t\t * If we have to retry again, we do sleep a bit.\n+\t\t */\n+\t\tSleep(delay[tries]);\n+\t\ttries++;\n+\t}\n+\treturn ret;\n }\n \n #undef open\n@@ -1257,7 +1285,6 @@ int mingw_rename(const char *pold, const char *pnew)\n {\n \tDWORD attrs, gle;\n \tint tries = 0;\n-\tstatic const int delay[] = { 0, 1, 10, 20, 40 };\n \n \t/*\n \t * Try native rename() first to get errno right.\n-- \n1.7.4.34.gd2cb1\n"},{"id":"160631","messageId":"20110207205120.GD63976@book.hvoigt.net","threadId":"26069","inReplyTo":"20110207204818.GA63976@book.hvoigt.net","subject":"[PATCH v4 3/5] mingw: make failures to unlink or move raise a question","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-07T20:51:21Z","receivedAt":"2011-02-07T20:51:21Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Windows in case a program is accessing a file unlink or\nmove operations may fail. To give the user a chance to correct\nthis we simply wait until the user asks us to retry or fail.\n\nThis is useful because of the following use case which seem\nto happen rarely but when it does it is a mess:\n\nAfter making some changes the user realizes that he was on the\nincorrect branch. When trying to change the branch some file\nis still in use by some other process and git stops in the\nmiddle of changing branches. Now the user has lots of files\nwith changes mixed with his own. This is especially confusing\non repositories that contain lots of files.\n\nAlthough the recent implementation of automatic retry makes\nthis scenario much more unlikely lets provide a fallback as\na last resort.\n\nThanks to Albert Dvornik for disabling the question if users can't see it.\n\nIf the stdout of the command is connected to a terminal but the stderr\nhas been redirected, the odds are good that the user can't see any\nquestion we print out to stderr.  This will result in a \"mysterious\nhang\" while the app is waiting for user input.\n\nIt seems better to be conservative, and avoid asking for input\nwhenever the stderr is not a terminal, just like we do for stdin.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\nSigned-off-by: Albert Dvornik <dvornik+git@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/mingw.c |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 78 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 4a1c218..1354b8e 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -2,6 +2,7 @@\n #include \"win32.h\"\n #include <conio.h>\n #include \"../strbuf.h\"\n+#include \"../run-command.h\"\n \n static const int delay[] = { 0, 1, 10, 20, 40 };\n \n@@ -129,6 +130,74 @@ static inline int is_file_in_use_error(DWORD errcode)\n \treturn 0;\n }\n \n+static int read_yes_no_answer(void)\n+{\n+\tchar answer[1024];\n+\n+\tif (fgets(answer, sizeof(answer), stdin)) {\n+\t\tsize_t answer_len = strlen(answer);\n+\t\tint got_full_line = 0, c;\n+\n+\t\t/* remove the newline */\n+\t\tif (answer_len >= 2 && answer[answer_len-2] == '\\r') {\n+\t\t\tanswer[answer_len-2] = '\\0';\n+\t\t\tgot_full_line = 1;\n+\t\t}\n+\t\telse if (answer_len >= 1 && answer[answer_len-1] == '\\n') {\n+\t\t\tanswer[answer_len-1] = '\\0';\n+\t\t\tgot_full_line = 1;\n+\t\t}\n+\t\t/* flush the buffer in case we did not get the full line */\n+\t\tif (!got_full_line)\n+\t\t\twhile((c = getchar()) != EOF && c != '\\n');\n+\t} else\n+\t\t/* we could not read, return the\n+\t\t * default answer which is no */\n+\t\treturn 0;\n+\n+\tif (tolower(answer[0]) == 'y' && !answer[1])\n+\t\treturn 1;\n+\tif (!strncasecmp(answer, \"yes\", sizeof(answer)))\n+\t\treturn 1;\n+\tif (tolower(answer[0]) == 'n' && !answer[1])\n+\t\treturn 0;\n+\tif (!strncasecmp(answer, \"no\", sizeof(answer)))\n+\t\treturn 0;\n+\n+\t/* did not find an answer we understand */\n+\treturn -1;\n+}\n+\n+static int ask_yes_no_if_possible(const char *format, ...)\n+{\n+\tchar question[4096];\n+\tconst char *retry_hook[] = { NULL, NULL, NULL };\n+\tva_list args;\n+\n+\tva_start(args, format);\n+\tvsnprintf(question, sizeof(question), format, args);\n+\tva_end(args);\n+\n+\tif ((retry_hook[0] = getenv(\"GIT_ASK_YESNO\"))) {\n+\t\tretry_hook[1] = question;\n+\t\treturn !run_command_v_opt(retry_hook, 0);\n+\t}\n+\n+\tif (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))\n+\t\treturn 0;\n+\n+\twhile (1) {\n+\t\tint answer;\n+\t\tfprintf(stderr, \"%s (y/n) \", question);\n+\n+\t\tif ((answer = read_yes_no_answer()) >= 0)\n+\t\t\treturn answer;\n+\n+\t\tfprintf(stderr, \"Sorry, I did not understand your answer. \"\n+\t\t\t\t\"Please type 'y' or 'n'\\n\");\n+\t}\n+}\n+\n #undef unlink\n int mingw_unlink(const char *pathname)\n {\n@@ -149,6 +218,10 @@ int mingw_unlink(const char *pathname)\n \t\tSleep(delay[tries]);\n \t\ttries++;\n \t}\n+\twhile (ret == -1 && is_file_in_use_error(GetLastError()) &&\n+\t       ask_yes_no_if_possible(\"Unlink of file '%s' failed. \"\n+\t\t\t\"Should I try again?\", pathname))\n+\t       ret = unlink(pathname);\n \treturn ret;\n }\n \n@@ -1326,6 +1399,11 @@ repeat:\n \t\ttries++;\n \t\tgoto repeat;\n \t}\n+\tif (gle == ERROR_ACCESS_DENIED &&\n+\t       ask_yes_no_if_possible(\"Rename from '%s' to '%s' failed. \"\n+\t\t       \"Should I try again?\", pold, pnew))\n+\t\tgoto repeat;\n+\n \terrno = EACCES;\n \treturn -1;\n }\n-- \n1.7.4.34.gd2cb1\n"},{"id":"160633","messageId":"20110207205233.GE63976@book.hvoigt.net","threadId":"26069","inReplyTo":"20110207204818.GA63976@book.hvoigt.net","subject":"[PATCH v4 4/5] mingw: add fallback for rmdir in case directory is in use","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-07T20:52:34Z","receivedAt":"2011-02-07T20:52:34Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"The same logic as for unlink and rename also applies to rmdir. For\nexample in case you have a shell open in a git controlled folder. This\nwill easily fail. So lets be nice for such cases as well.\n\nSigned-off-by: Heiko Voigt <heiko.voigt@mahr.de>\n---\n compat/mingw.c |   25 +++++++++++++++++++++++++\n compat/mingw.h |    3 +++\n 2 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 1354b8e..68cc79f 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -225,6 +225,31 @@ int mingw_unlink(const char *pathname)\n \treturn ret;\n }\n \n+#undef rmdir\n+int mingw_rmdir(const char *pathname)\n+{\n+\tint ret, tries = 0;\n+\n+\twhile ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {\n+\t\tif (!is_file_in_use_error(GetLastError()))\n+\t\t\tbreak;\n+\t\t/*\n+\t\t * We assume that some other process had the source or\n+\t\t * destination file open at the wrong moment and retry.\n+\t\t * In order to give the other process a higher chance to\n+\t\t * complete its operation, we give up our time slice now.\n+\t\t * If we have to retry again, we do sleep a bit.\n+\t\t */\n+\t\tSleep(delay[tries]);\n+\t\ttries++;\n+\t}\n+\twhile (ret == -1 && 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+\treturn ret;\n+}\n+\n #undef open\n int mingw_open (const char *filename, int oflags, ...)\n {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 970d743..fe6ba34 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -169,6 +169,9 @@ int link(const char *oldpath, const char *newpath);\n int mingw_unlink(const char *pathname);\n #define unlink mingw_unlink\n \n+int mingw_rmdir(const char *path);\n+#define rmdir mingw_rmdir\n+\n int mingw_open (const char *filename, int oflags, ...);\n #define open mingw_open\n \n-- \n1.7.4.34.gd2cb1\n"},{"id":"160635","messageId":"20110207205400.GF63976@book.hvoigt.net","threadId":"26069","inReplyTo":"20110207204818.GA63976@book.hvoigt.net","subject":"[PATCH v4 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-07T20:54:01Z","receivedAt":"2011-02-07T20:54:01Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nOn Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the\ndirectory is busy, we only want to retry deleting the directory if it\nis empty, so test specifically for that case and set ENOTEMPTY rather\nthan EACCES.\n\nNoticed by Greg Hazel.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n compat/mingw.c |   28 ++++++++++++++++++++++++++++\n 1 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 68cc79f..368edf2 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -225,6 +225,30 @@ int mingw_unlink(const char *pathname)\n \treturn ret;\n }\n \n+static int is_dir_empty(const char *path)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tWIN32_FIND_DATAA findbuf;\n+\tHANDLE handle;\n+\n+\tstrbuf_addf(&buf, \"%s\\\\*\", path);\n+\thandle = FindFirstFileA(buf.buf, &findbuf);\n+\tif (handle == INVALID_HANDLE_VALUE) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn GetLastError() == ERROR_NO_MORE_FILES;\n+\t}\n+\n+\twhile (!strcmp(findbuf.cFileName, \".\") ||\n+\t\t\t!strcmp(findbuf.cFileName, \"..\"))\n+\t\tif (!FindNextFile(handle, &findbuf)) {\n+\t\t\tstrbuf_release(&buf);\n+\t\t\treturn GetLastError() == ERROR_NO_MORE_FILES;\n+\t\t}\n+\tFindClose(handle);\n+\tstrbuf_release(&buf);\n+\treturn 0;\n+}\n+\n #undef rmdir\n int mingw_rmdir(const char *pathname)\n {\n@@ -233,6 +257,10 @@ int mingw_rmdir(const char *pathname)\n \twhile ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {\n \t\tif (!is_file_in_use_error(GetLastError()))\n \t\t\tbreak;\n+\t\tif (!is_dir_empty(pathname)) {\n+\t\t\terrno = ENOTEMPTY;\n+\t\t\tbreak;\n+\t\t}\n \t\t/*\n \t\t * We assume that some other process had the source or\n \t\t * destination file open at the wrong moment and retry.\n-- \n1.7.4.34.gd2cb1\n"},{"id":"160640","messageId":"AANLkTin6A-HVKvM9_5ggMezpM--tt1qUwSXF+CEaXg7J@mail.gmail.com","threadId":"26069","inReplyTo":"20110207205400.GF63976@book.hvoigt.net","subject":"Re: [PATCH v4 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-02-07T21:07:10Z","receivedAt":"2011-02-07T21:07:10Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Feb 7, 2011 at 9:54 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the\n> directory is busy, we only want to retry deleting the directory if it\n> is empty, so test specifically for that case and set ENOTEMPTY rather\n> than EACCES.\n>\n\nI'm sorry, I don't quite understand. rmdir on Windows/MinGW fails with\nerrno=ENOTEMPTY if a directory isn't empty:\n---8<---\n#include <errno.h>\n#include <stdlib.h>\n#include <stdio.h>\n\nint main()\n{\n\tif (mkdir(\"foo\") < 0 && errno != EEXIST) {\n\t\tperror(\"mkdir\");\n\t\texit(-1);\n\t}\n\n\tif (!fopen(\"foo/bar\", \"w\")) {\n\t\tperror(\"fopen\");\n\t\texit(-1);\n\t}\n\n\tif (rmdir(\"foo\") < 0)\n\t\tperror(\"rmdir\");\n\tprintf(\"errno: %d (expected %d)\\n\", errno, ENOTEMPTY);\n}\n---8<---\n$ ./a.exe\nrmdir: Directory not empty\nerrno: 41 (expected 41)\n"},{"id":"160643","messageId":"20110207211844.GH63976@book.hvoigt.net","threadId":"26069","inReplyTo":"AANLkTin6A-HVKvM9_5ggMezpM--tt1qUwSXF+CEaXg7J@mail.gmail.com","subject":"Re: [msysGit] Re: [PATCH v4 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-07T21:18:44Z","receivedAt":"2011-02-07T21:18:44Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Mon, Feb 07, 2011 at 10:07:10PM +0100, Erik Faye-Lund wrote:\n> On Mon, Feb 7, 2011 at 9:54 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the\n> > directory is busy, we only want to retry deleting the directory if it\n> > is empty, so test specifically for that case and set ENOTEMPTY rather\n> > than EACCES.\n> >\n> \n> I'm sorry, I don't quite understand. rmdir on Windows/MinGW fails with\n> errno=ENOTEMPTY if a directory isn't empty:\n\nI think Johannes was referring to the case when a directory is busy.\nE.g. a process is running that has its working directory inside that\ndirectory. In that case ENOTEMPTY was not returned even though the\ndirectory is not empty. Thats what I read from the patch.\n\nCheers Heiko\n"},{"id":"160644","messageId":"AANLkTi=ntgYOW58pPnt-azMw6rFZfJjDW3OaCgLsvJp6@mail.gmail.com","threadId":"26069","inReplyTo":"20110207211844.GH63976@book.hvoigt.net","subject":"Re: [msysGit] Re: [PATCH v4 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-02-07T21:23:24Z","receivedAt":"2011-02-07T21:23:24Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Feb 7, 2011 at 10:18 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> On Mon, Feb 07, 2011 at 10:07:10PM +0100, Erik Faye-Lund wrote:\n>> On Mon, Feb 7, 2011 at 9:54 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n>> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>> >\n>> > On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the\n>> > directory is busy, we only want to retry deleting the directory if it\n>> > is empty, so test specifically for that case and set ENOTEMPTY rather\n>> > than EACCES.\n>> >\n>>\n>> I'm sorry, I don't quite understand. rmdir on Windows/MinGW fails with\n>> errno=ENOTEMPTY if a directory isn't empty:\n>\n> I think Johannes was referring to the case when a directory is busy.\n> E.g. a process is running that has its working directory inside that\n> directory. In that case ENOTEMPTY was not returned even though the\n> directory is not empty. Thats what I read from the patch.\n>\n\nI don't think that's the case either:\n$ echo \"int main() { while (1); }\" | gcc -x c - -o foo/bin.exe\n[kusma@HUE-PC:/git@work/xgetenv]\n$ foo/bin.exe &\n[2] 3188\n[kusma@HUE-PC:/git@work/xgetenv]\n$ ./a.exe\nrmdir: Directory not empty\nerrno: 41 (expected 41)\n"},{"id":"160649","messageId":"7vk4hbtsro.fsf@alter.siamese.dyndns.org","threadId":"26069","inReplyTo":"AANLkTi=ntgYOW58pPnt-azMw6rFZfJjDW3OaCgLsvJp6@mail.gmail.com","subject":"Re: [msysGit] Re: [PATCH v4 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-07T21:54:35Z","receivedAt":"2011-02-07T21:54:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n> On Mon, Feb 7, 2011 at 10:18 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> ...\n>> I think Johannes was referring to the case when a directory is busy.\n>> E.g. a process is running that has its working directory inside that\n>> directory. In that case ENOTEMPTY was not returned even though the\n>> directory is not empty. Thats what I read from the patch.\n>\n> I don't think that's the case either:\n> $ echo \"int main() { while (1); }\" | gcc -x c - -o foo/bin.exe\n> [kusma@HUE-PC:/git@work/xgetenv]\n> $ foo/bin.exe &\n> [2] 3188\n> [kusma@HUE-PC:/git@work/xgetenv]\n> $ ./a.exe\n> rmdir: Directory not empty\n> errno: 41 (expected 41)\n\nI don't do windows, but I think Heiko is talking about running some\nprocess inside the directory that is getting removed.  Assuming that your\na.exe is to remove the 'foo' directory, you would need to run ./bin.exe\nafter going into 'foo' directory, perhaps like this:\n\n    $ (cd foo && ./bin.exe) &\n    $ ./a.exe\n"},{"id":"160650","messageId":"AANLkTi=r1Ujz7RXTTepiVUCMNRWUmydTeOmUkyd1+AC4@mail.gmail.com","threadId":"26069","inReplyTo":"7vk4hbtsro.fsf@alter.siamese.dyndns.org","subject":"Re: [msysGit] Re: [PATCH v4 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-02-07T21:57:37Z","receivedAt":"2011-02-07T21:57:37Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Feb 7, 2011 at 10:54 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Erik Faye-Lund <kusmabite@gmail.com> writes:\n>\n>> On Mon, Feb 7, 2011 at 10:18 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n>> ...\n>>> I think Johannes was referring to the case when a directory is busy.\n>>> E.g. a process is running that has its working directory inside that\n>>> directory. In that case ENOTEMPTY was not returned even though the\n>>> directory is not empty. Thats what I read from the patch.\n>>\n>> I don't think that's the case either:\n>> $ echo \"int main() { while (1); }\" | gcc -x c - -o foo/bin.exe\n>> [kusma@HUE-PC:/git@work/xgetenv]\n>> $ foo/bin.exe &\n>> [2] 3188\n>> [kusma@HUE-PC:/git@work/xgetenv]\n>> $ ./a.exe\n>> rmdir: Directory not empty\n>> errno: 41 (expected 41)\n>\n> I don't do windows, but I think Heiko is talking about running some\n> process inside the directory that is getting removed.  Assuming that your\n> a.exe is to remove the 'foo' directory, you would need to run ./bin.exe\n> after going into 'foo' directory, perhaps like this:\n>\n>    $ (cd foo && ./bin.exe) &\n>    $ ./a.exe\n\nOf course, silly me:\n\n$ ./a.exe\nrmdir: Permission denied\nerrno: 13 (expected 41)\n\nThanks for clearing that up, now I'm on board :)\n"},{"id":"160678","messageId":"7vfwrzqh4j.fsf@alter.siamese.dyndns.org","threadId":"26069","inReplyTo":"20110207204818.GA63976@book.hvoigt.net","subject":"Re: [PATCH v4 0/5] make open/unlink failures user friendly on windows using retry/abort","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-08T04:34:20Z","receivedAt":"2011-02-08T04:34:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will queue (with a handful of minor style fixes).\n"},{"id":"161469","messageId":"alpine.DEB.1.00.1102180018250.1541@bonsai2","threadId":"26069","inReplyTo":"20110207204932.GB63976@book.hvoigt.net","subject":"Re: [PATCH v4 1/5] mingw: move unlink wrapper to mingw.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2011-02-17T23:18:54Z","receivedAt":"2011-02-17T23:18:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 7 Feb 2011, Heiko Voigt wrote:\n\n> The next patch implements a workaround in case unlink fails on Windows.\n> \n> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n\nI merged the current 'next', which has them all.\n\nThanks!\nDscho\n"}]}