[PATCH 0/3] mingw: make workaround for unlink failures interactive
Heiko Voigt
operation occurs the user ends up in between branches, which is not a
nice place to be. Lets make this really robust so git and user can stay
friends.
sarcasm_flag := 1
With this series git and git gui on Windows finally behave like a "real"
Windows program in case an instable file operation fails.
sarcasm_flag := 0
cheers Heiko
P.S.: Where should I document the new environment variable?
Heiko Voigt (3):
mingw: make failures to unlink or move raise a question
mingw: add fallback for rmdir in case directory is in use
git-gui: provide question helper for retry fallback on Windows
compat/mingw.c | 83 +++++++++++++++++++++++++++++++++++++++++++++
compat/mingw.h | 3 ++
git-gui/Makefile | 2 +
git-gui/git-gui--askyesno | 59 ++++++++++++++++++++++++++++++++
git-gui/git-gui.sh | 3 ++
5 files changed, 150 insertions(+), 0 deletions(-)
create mode 100755 git-gui/git-gui--askyesno
Heiko Voigt
move operations may fail. To give the user a chance to correct
this we simply wait until the user asks us to retry or fail.
This is useful because of the following use case, which seems
to happen rarely but when it does it is a mess:
After making some changes the user realizes that he was on the
incorrect branch. When trying to change the branch some file
is still in use by some other process and git stops in the
middle of changing branches. Now the user has lots of files
with changes mixed with his own. This is especially confusing
on repositories that contain lots of files.
Although the recent implementation of automatic retry makes
this scenario much more unlikely lets provide a fallback as
a last resort.
Signed-off-by: Heiko Voigt <hvo...@hvoigt.net>
---
compat/mingw.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index e637209..5326a1b 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -5,6 +5,7 @@
#include <shellapi.h>
#include "../strbuf.h"
#include "../cache.h"
+#include "../run-command.h"
unsigned int _CRT_fmode = _O_BINARY;
static const int delay[] = { 0, 1, 10, 20, 40 };
@@ -158,6 +159,54 @@ int mingw_mkdir(const char *path, int mode)
return ret;
}
+static int ask_user_yes_no(const char *format, ...)
+{
+ char answer[5];
+ char question[4096];
+ const char *retry_hook[] = { NULL, NULL, NULL };
+ va_list args;
+
+ if ((retry_hook[0] = getenv("GIT_ASK_YESNO"))) {
+
+ va_start(args, format);
+ vsnprintf(question, sizeof(question), format, args);
+ va_end(args);
+
+ retry_hook[1] = question;
+ return !run_command_v_opt(retry_hook, 0);
+ }
+
+ if (!isatty(_fileno(stdin)))
+ return 0;
+
+ while (1) {
+ va_start(args, format);
+ vfprintf(stderr, format, args);
+ va_end(args);
+ fprintf(stderr, " (y/n)? ");
+
+ if (fgets(answer, sizeof(answer), stdin)) {
+ /* remove the newline */
+ if (answer[strlen(answer)-2] == '\r')
+ answer[strlen(answer)-2] = '\0';
+ if (answer[strlen(answer)-1] == '\n')
+ answer[strlen(answer)-1] = '\0';
+ } else
+ return 0;
+
+ if (answer[0] == 'y' && strlen(answer) == 1)
+ return 1;
+ if (!strncasecmp(answer, "yes", sizeof(answer)))
+ return 1;
+ if (answer[0] == 'n' && strlen(answer) == 1)
+ return 0;
+ if (!strncasecmp(answer, "no", sizeof(answer)))
+ return 0;
+ fprintf(stderr, "I did not understand your answer: '%s'\n",
+ answer);
+ }
+}
+
#undef unlink
int mingw_unlink(const char *pathname)
{
@@ -178,6 +227,10 @@ int mingw_unlink(const char *pathname)
Sleep(delay[tries]);
tries++;
}
+ while (ret == -1 && errno == EACCES &&
+ ask_user_yes_no("Unlink of file '%s' failed. "
+ "Should I try again?", pathname))
+ ret = unlink(pathname);
return ret;
}
@@ -1277,6 +1330,11 @@ repeat:
tries++;
goto repeat;
}
+ if (gle == ERROR_ACCESS_DENIED &&
+ ask_user_yes_no("Rename from '%s' to '%s' failed. "
+ "Should I try again?", pold, pnew))
+ goto repeat;
+
errno = EACCES;
return -1;
}
--
1.7.0.m5.rc3.5.g38df2
Heiko Voigt
example in case you have a shell open in a git controlled folder. This
will easily fail. So lets be nice for such cases as well.
Signed-off-by: Heiko Voigt <heiko...@mahr.de>
---
compat/mingw.c | 25 +++++++++++++++++++++++++
compat/mingw.h | 3 +++
2 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 5326a1b..d7d8970 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -234,6 +234,31 @@ int mingw_unlink(const char *pathname)
return ret;
}
+#undef rmdir
+int mingw_rmdir(const char *pathname)
+{
+ int ret, tries = 0;
+
+ while ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+ if (errno != EACCES)
+ break;
+ /*
+ * We assume that some other process had the source or
+ * destination file open at the wrong moment and retry.
+ * In order to give the other process a higher chance to
+ * complete its operation, we give up our time slice now.
+ * If we have to retry again, we do sleep a bit.
+ */
+ Sleep(delay[tries]);
+ tries++;
+ }
+ while (ret == -1 && errno == EACCES &&
+ ask_user_yes_no("Deletion of directory '%s' failed. "
+ "Should I try again?", pathname))
+ ret = rmdir(pathname);
+ return ret;
+}
+
#undef open
int mingw_open (const char *filename, int oflags, ...)
{
diff --git a/compat/mingw.h b/compat/mingw.h
index f66c831..4650d8a 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -170,6 +170,9 @@ int mingw_mkdir(const char *path, int mode);
int mingw_unlink(const char *pathname);
#define unlink mingw_unlink
+int mingw_rmdir(const char *path);
+#define rmdir mingw_rmdir
+
int mingw_open (const char *filename, int oflags, ...);
#define open mingw_open
--
1.7.0.m5.rc3.5.g38df2
Heiko Voigt
recently implemented fallback in case unlink, rename or rmdir fail for
files in use on Windows. The added dialog will present a yes/no question
to the the user which will currently be used by the windows compat layer
to let the user retry a failed file operation.
Signed-off-by: Heiko Voigt <hvo...@hvoigt.net>
---
git-gui/Makefile | 2 +
git-gui/git-gui--askyesno | 59 +++++++++++++++++++++++++++++++++++++++++++++
git-gui/git-gui.sh | 3 ++
3 files changed, 64 insertions(+), 0 deletions(-)
create mode 100755 git-gui/git-gui--askyesno
diff --git a/git-gui/Makefile b/git-gui/Makefile
index 197b55e..7d8b541 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -289,6 +289,7 @@ install: all
$(QUIET)$(INSTALL_D0)'$(DESTDIR_SQ)$(gitexecdir_SQ)' $(INSTALL_D1)
$(QUIET)$(INSTALL_X0)git-gui $(INSTALL_X1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
$(QUIET)$(INSTALL_X0)git-gui--askpass $(INSTALL_X1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+ $(QUIET)$(INSTALL_X0)git-gui--askyesno $(INSTALL_X1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
$(QUIET)$(foreach p,$(GITGUI_BUILT_INS), $(INSTALL_L0)'$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' $(INSTALL_L1)'$(DESTDIR_SQ)$(gitexecdir_SQ)/git-gui' $(INSTALL_L2)'$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' $(INSTALL_L3) &&) true
ifdef GITGUI_WINDOWS_WRAPPER
$(QUIET)$(INSTALL_R0)git-gui.tcl $(INSTALL_R1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
@@ -307,6 +308,7 @@ uninstall:
$(QUIET)$(CLEAN_DST) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
$(QUIET)$(REMOVE_F0)'$(DESTDIR_SQ)$(gitexecdir_SQ)'/git-gui $(REMOVE_F1)
$(QUIET)$(REMOVE_F0)'$(DESTDIR_SQ)$(gitexecdir_SQ)'/git-gui--askpass $(REMOVE_F1)
+ $(QUIET)$(REMOVE_F0)'$(DESTDIR_SQ)$(gitexecdir_SQ)'/git-gui--askyesno $(REMOVE_F1)
$(QUIET)$(foreach p,$(GITGUI_BUILT_INS), $(REMOVE_F0)'$(DESTDIR_SQ)$(gitexecdir_SQ)'/$p $(REMOVE_F1) &&) true
ifdef GITGUI_WINDOWS_WRAPPER
$(QUIET)$(REMOVE_F0)'$(DESTDIR_SQ)$(gitexecdir_SQ)'/git-gui.tcl $(REMOVE_F1)
diff --git a/git-gui/git-gui--askyesno b/git-gui/git-gui--askyesno
new file mode 100755
index 0000000..5d00ded
--- /dev/null
+++ b/git-gui/git-gui--askyesno
@@ -0,0 +1,59 @@
+#!/bin/sh
+# Tcl ignores the next line -*- tcl -*- \
+exec wish "$0" -- "$@"
+
+# This is an implementation of a simple yes no dialog
+# which is injected into the git commandline by git gui
+# in case a yesno question needs to be answered.
+
+set NS {}
+set use_ttk [package vsatisfies [package provide Tk] 8.5]
+if {$use_ttk} {
+ set NS ttk
+}
+
+if {$argc < 1} {
+ puts stderr "Usage: $argv0 <question>"
+ exit 1
+} else {
+ set prompt [join $argv " "]
+}
+
+${NS}::frame .t
+${NS}::label .t.m -text $prompt -justify center -width 40
+.t.m configure -wraplength 400
+pack .t.m -side top -fill x -padx 20 -pady 20 -expand 1
+pack .t -side top -fill x -ipadx 20 -ipady 20 -expand 1
+
+${NS}::frame .b
+${NS}::frame .b.left -width 200
+${NS}::button .b.yes -text Yes -command yes
+${NS}::button .b.no -text No -command no
+
+
+pack .b.left -side left -expand 1 -fill x
+pack .b.yes -side left -expand 1
+pack .b.no -side right -expand 1 -ipadx 5
+pack .b -side bottom -fill x -ipadx 20 -ipady 15
+
+bind . <Key-Return> {exit 0}
+bind . <Key-Escape> {exit 1}
+
+proc handle_connection {sock addr port} {
+ gets $sock line
+ .t.m configure -text $line
+ close $sock
+}
+socket -server handle_connection 12345
+
+
+proc no {} {
+ exit 1
+}
+
+proc yes {} {
+ exit 0
+}
+
+wm title . "Question?"
+tk::PlaceWindow .
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index cd8da37..2f1d076 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1115,6 +1115,9 @@ set have_tk85 [expr {[package vcompare $tk_version "8.5"] >= 0}]
if {![info exists env(SSH_ASKPASS)]} {
set env(SSH_ASKPASS) [gitexec git-gui--askpass]
}
+if {![info exists env(GIT_ASK_YESNO)]} {
+ set env(GIT_ASK_YESNO) [gitexec git-gui--askyesno]
+}
######################################################################
##
--
1.7.0.m5.rc3.5.g38df2
Pat Thoyts
This would be better done using tk_messageBox which will use a native
windows message box (or a Mac one on the Mac or something reasonable
on unix):
tk_messageBox -type yesno -icon question -title $title -message $prompt
which will return 'yes' or 'no'.
What is the [socket] for? I don't see anything else in your patch set
create a client socket. But also, sockets can fail (consider if two
git-gui's end up in the same condition) and it should only create a
local socket (use "-myaddr localhost"). It should probably use the
async mechanism too if this will be running with a gui otherwise you
jam the gui when you block waiting for [gets]. That is a client that
connects and never sends anything will freeze your program. The
fileevent command is used to avoid that.
Pat Thoyts
Heiko Voigt
> On 27 February 2010 20:23, Heiko Voigt <hvo...@hvoigt.net> wrote:
> > Make use of the new environment variable GIT_ASK_YESNO to support the
> > recently implemented fallback in case unlink, rename or rmdir fail for
> > files in use on Windows. The added dialog will present a yes/no question
> > to the the user which will currently be used by the windows compat layer
> > to let the user retry a failed file operation.
> > --- a/git-gui/git-gui.sh
> > +++ b/git-gui/git-gui.sh
> > @@ -1115,6 +1115,9 @@ set have_tk85 [expr {[package vcompare $tk_version "8.5"] >= 0}]
> > �if {![info exists env(SSH_ASKPASS)]} {
> > � � � �set env(SSH_ASKPASS) [gitexec git-gui--askpass]
> > �}
> > +if {![info exists env(GIT_ASK_YESNO)]} {
> > + � � � set env(GIT_ASK_YESNO) [gitexec git-gui--askyesno]
> > +}
>
> This would be better done using tk_messageBox which will use a native
> windows message box (or a Mac one on the Mac or something reasonable
> on unix):
> tk_messageBox -type yesno -icon question -title $title -message $prompt
> which will return 'yes' or 'no'.
Yes today I realized that tk actually has a ready made messagebox for
this. I took the code for this one from the ssh--askpass which was
easiest while developing.
> What is the [socket] for? I don't see anything else in your patch set
> create a client socket. But also, sockets can fail (consider if two
> git-gui's end up in the same condition) and it should only create a
> local socket (use "-myaddr localhost"). It should probably use the
> async mechanism too if this will be running with a gui otherwise you
> jam the gui when you block waiting for [gets]. That is a client that
> connects and never sends anything will freeze your program. The
> fileevent command is used to avoid that.
That socket is for some other feature in git gui I played with while
developing this. Sorry this should not have stayed in there. I will send
another patch for the git gui part soonish.
cheers Heiko
Heiko Voigt
I started working on this but I could not figure out how to disable the
default tk root window that always opens when you start wish. So in my
new implementation[1] there is always a second empty window shown.
Is there a way to place the tk_messageBox inside this window or another
workaround?
cheers Heiko
[1] ---8<---
#!/bin/sh
# Tcl ignores the next line -*- tcl -*- \
exec wish "$0" -- "$@"
# This is an implementation of a simple yes no dialog
# which is injected into the git commandline by git gui
# in case a yesno question needs to be answered.
if {$argc < 1} {
puts stderr "Usage: $argv0 <question>"
exit 1
} else {
set prompt [join $argv " "]
}
set answer [tk_messageBox -message $prompt \
-type yesno \
-icon question]
bind . <Key-Return> {exit 0}
bind . <Key-Escape> {exit 1}
switch -- $answer {
yes {exit 0}
no {exit 1}
}
Pat Thoyts
> I started working on this but I could not figure out how to disable the
> default tk root window that always opens when you start wish. So in my
> new implementation[1] there is always a second empty window shown.
wm withdraw .
(notice the dot at the end there)
Heiko Voigt
> On 26 March 2010 13:28, Heiko Voigt <hvo...@hvoigt.net> wrote:
> > I started working on this but I could not figure out how to disable the
> > default tk root window that always opens when you start wish. So in my
> > new implementation[1] there is always a second empty window shown.
>
> wm withdraw .
>
> (notice the dot at the end there)
Nice, thanks a lot. I knew that there must be an easy solution. Find
attached an updated patch. Do you think this is ready for upstream?
cheers Heiko
---8<--
From: Heiko Voigt <hvo...@hvoigt.net>
Subject: [PATCH] git-gui: provide question helper for retry fallback on Windows
Make use of the new environment variable GIT_ASK_YESNO to support the
recently implemented fallback in case unlink, rename or rmdir fail for
files in use on Windows. The added dialog will present a yes/no question
to the the user which will currently be used by the windows compat layer
to let the user retry a failed file operation.
Signed-off-by: Heiko Voigt <hvo...@hvoigt.net>
---
git-gui/Makefile | 2 ++
git-gui/git-gui--askyesno | 28 ++++++++++++++++++++++++++++
git-gui/git-gui.sh | 3 +++
3 files changed, 33 insertions(+), 0 deletions(-)
create mode 100755 git-gui/git-gui--askyesno
diff --git a/git-gui/Makefile b/git-gui/Makefile
index 197b55e..7d8b541 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -289,6 +289,7 @@ install: all
$(QUIET)$(INSTALL_D0)'$(DESTDIR_SQ)$(gitexecdir_SQ)' $(INSTALL_D1)
$(QUIET)$(INSTALL_X0)git-gui $(INSTALL_X1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
$(QUIET)$(INSTALL_X0)git-gui--askpass $(INSTALL_X1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+ $(QUIET)$(INSTALL_X0)git-gui--askyesno $(INSTALL_X1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
$(QUIET)$(foreach p,$(GITGUI_BUILT_INS), $(INSTALL_L0)'$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' $(INSTALL_L1)'$(DESTDIR_SQ)$(gitexecdir_SQ)/git-gui' $(INSTALL_L2)'$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' $(INSTALL_L3) &&) true
ifdef GITGUI_WINDOWS_WRAPPER
$(QUIET)$(INSTALL_R0)git-gui.tcl $(INSTALL_R1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
@@ -307,6 +308,7 @@ uninstall:
$(QUIET)$(CLEAN_DST) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
$(QUIET)$(REMOVE_F0)'$(DESTDIR_SQ)$(gitexecdir_SQ)'/git-gui $(REMOVE_F1)
$(QUIET)$(REMOVE_F0)'$(DESTDIR_SQ)$(gitexecdir_SQ)'/git-gui--askpass $(REMOVE_F1)
+ $(QUIET)$(REMOVE_F0)'$(DESTDIR_SQ)$(gitexecdir_SQ)'/git-gui--askyesno $(REMOVE_F1)
$(QUIET)$(foreach p,$(GITGUI_BUILT_INS), $(REMOVE_F0)'$(DESTDIR_SQ)$(gitexecdir_SQ)'/$p $(REMOVE_F1) &&) true
ifdef GITGUI_WINDOWS_WRAPPER
$(QUIET)$(REMOVE_F0)'$(DESTDIR_SQ)$(gitexecdir_SQ)'/git-gui.tcl $(REMOVE_F1)
diff --git a/git-gui/git-gui--askyesno b/git-gui/git-gui--askyesno
new file mode 100755
index 0000000..3588b36
--- /dev/null
+++ b/git-gui/git-gui--askyesno
@@ -0,0 +1,28 @@
+#!/bin/sh
+# Tcl ignores the next line -*- tcl -*- \
+exec wish "$0" -- "$@"
+
+wm withdraw .
+
+# This is an implementation of a simple yes no dialog
+# which is injected into the git commandline by git gui
+# in case a yesno question needs to be answered.
+
+if {$argc < 1} {
+ puts stderr "Usage: $argv0 <question>"
+ exit 1
+} else {
+ set prompt [join $argv " "]
+}
+
+set answer [tk_messageBox -message $prompt \
+ -type yesno \
+ -icon question]
+
+bind . <Key-Return> {exit 0}
+bind . <Key-Escape> {exit 1}
+
+switch -- $answer {
+ yes {exit 0}
+ no {exit 1}
+}
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index cd8da37..2f1d076 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1115,6 +1115,9 @@ set have_tk85 [expr {[package vcompare $tk_version "8.5"] >= 0}]
if {![info exists env(SSH_ASKPASS)]} {
set env(SSH_ASKPASS) [gitexec git-gui--askpass]
}
+if {![info exists env(GIT_ASK_YESNO)]} {
+ set env(GIT_ASK_YESNO) [gitexec git-gui--askyesno]
+}
######################################################################
##
--
1.7.0.3.294.gd9776.dirty