git-gui: Consolidate hook execution code into a single function
The code we use to test if a hook is executable or not differs on Cygwin from the normal POSIX case. Rather then repeating that for all three hooks we call in our commit code path we can place the common logic into a global procedure and invoke it when necessary. This also lets us get rid of the ugly "|& cat" we were using before as we can now rely on the Tcl 8.4 feature of "2>@1" or fallback to the "|& cat" when necessary. The post-commit hook is now run through the same API, but its outcome does not influence the commit status. As a result we now show any of the errors from the post-commit hook in a dialog window, instead of on the user's tty that was used to launch git-gui. This resolves a long standing bug related to not getting errors out of the post-commit hook when launched under git-gui. Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Shawn O. Pearce committed
Jan 20, 2008 at 14:46 UTC
ed76cb70f47225fc1a2ba4209b38b89be71adeb6
3 files changed
+63
-46
git-gui.sh
+28
@@ -438,6 +438,34 @@ proc git_write {args} {
438
return [open [concat $opt $cmdp $args] w]
439
}
440
441
+proc githook_read {hook_name args} {
442
+ set pchook [gitdir hooks $hook_name]
443
+ lappend args 2>@1
444
+
445
+ # On Cygwin [file executable] might lie so we need to ask
446
+ # the shell if the hook is executable. Yes that's annoying.
447
+ #
448
+ if {[is_Cygwin]} {
449
+ upvar #0 _sh interp
450
+ if {![info exists interp]} {
451
+ set interp [_which sh]
452
+ }
453
+ if {$interp eq {}} {
454
+ error "hook execution requires sh (not in PATH)"
455
+ }
456
+
457
+ set scr {if test -x "$1";then exec "$@";fi}
458
+ set sh_c [list | $interp -c $scr $interp $pchook]
459
+ return [_open_stdout_stderr [concat $sh_c $args]]
460
+ }
461
+
462
+ if {[file executable $pchook]} {
463
+ return [_open_stdout_stderr [concat [list | $pchook] $args]]
464
+ }
465
+
466
+ return {}
467
+}
468
+
469
proc sq {value} {
470
regsub -all ' $value "'\\''" value
471
return "'$value'"
lib/commit.tcl
+26
-39
@@ -212,26 +212,14 @@ A good commit message has the following format:
212
213
# -- Run the pre-commit hook.
214
#
215
- set pchook [gitdir hooks pre-commit]
216
-
217
- # On Cygwin [file executable] might lie so we need to ask
218
- # the shell if the hook is executable. Yes that's annoying.
219
- #
220
- if {[is_Cygwin] && [file isfile $pchook]} {
221
- set pchook [list sh -c [concat \
222
- "if test -x \"$pchook\";" \
223
- "then exec \"$pchook\" 2>&1;" \
224
- "fi"]]
225
- } elseif {[file executable $pchook]} {
226
- set pchook [list $pchook |& cat]
227
- } else {
215
+ set fd_ph [githook_read pre-commit]
216
+ if {$fd_ph eq {}} {
217
commit_commitmsg $curHEAD $msg_p
218
return
219
}
220
221
ui_status {Calling pre-commit hook...}
222
set pch_error {}
234
- set fd_ph [open "| $pchook" r]
223
fconfigure $fd_ph -blocking 0 -translation binary -eofchar {}
224
fileevent $fd_ph readable \
225
[list commit_prehook_wait $fd_ph $curHEAD $msg_p]
@@ -262,26 +250,14 @@ proc commit_commitmsg {curHEAD msg_p} {
250
251
# -- Run the commit-msg hook.
252
#
265
- set pchook [gitdir hooks commit-msg]
266
-
267
- # On Cygwin [file executable] might lie so we need to ask
268
- # the shell if the hook is executable. Yes that's annoying.
269
- #
270
- if {[is_Cygwin] && [file isfile $pchook]} {
271
- set pchook [list sh -c [concat \
272
- "if test -x \"$pchook\";" \
273
- "then exec \"$pchook\" \"$msg_p\" 2>&1;" \
274
- "fi"]]
275
- } elseif {[file executable $pchook]} {
276
- set pchook [list $pchook $msg_p |& cat]
277
- } else {
253
+ set fd_ph [githook_read commit-msg $msg_p]
254
+ if {$fd_ph eq {}} {
255
commit_writetree $curHEAD $msg_p
256
return
257
}
258
259
ui_status {Calling commit-msg hook...}
260
set pch_error {}
284
- set fd_ph [open "| $pchook" r]
261
fconfigure $fd_ph -blocking 0 -translation binary -eofchar {}
262
fileevent $fd_ph readable \
263
[list commit_commitmsg_wait $fd_ph $curHEAD $msg_p]
@@ -415,17 +391,13 @@ A rescan will be automatically started now.
391
392
# -- Run the post-commit hook.
393
#
418
- set pchook [gitdir hooks post-commit]
419
- if {[is_Cygwin] && [file isfile $pchook]} {
420
- set pchook [list sh -c [concat \
421
- "if test -x \"$pchook\";" \
422
- "then exec \"$pchook\";" \
423
- "fi"]]
424
- } elseif {![file executable $pchook]} {
425
- set pchook {}
426
- }
427
- if {$pchook ne {}} {
428
- catch {exec $pchook &}
394
+ set fd_ph [githook_read post-commit]
395
+ if {$fd_ph ne {}} {
396
+ upvar #0 pch_error$cmt_id pc_err
397
+ set pc_err {}
398
+ fconfigure $fd_ph -blocking 0 -translation binary -eofchar {}
399
+ fileevent $fd_ph readable \
400
+ [list commit_postcommit_wait $fd_ph $cmt_id]
401
}
402
403
$ui_comm delete 0.0 end
@@ -481,3 +453,18 @@ A rescan will be automatically started now.
453
reshow_diff
454
ui_status [mc "Created commit %s: %s" [string range $cmt_id 0 7] $subject]
455
}
456
+
457
+proc commit_postcommit_wait {fd_ph cmt_id} {
458
+ upvar #0 pch_error$cmt_id pch_error
459
+
460
+ append pch_error [read $fd_ph]
461
+ fconfigure $fd_ph -blocking 1
462
+ if {[eof $fd_ph]} {
463
+ if {[catch {close $fd_ph}]} {
464
+ hook_failed_popup post-commit $pch_error 0
465
+ }
466
+ unset pch_error
467
+ return
468
+ }
469
+ fconfigure $fd_ph -blocking 0
470
+}
lib/error.tcl
+9
-7
@@ -62,7 +62,7 @@ proc ask_popup {msg} {
62
eval $cmd
63
}
64
65
-proc hook_failed_popup {hook msg} {
65
+proc hook_failed_popup {hook msg {is_fatal 1}} {
66
set w .hookfail
67
toplevel $w
68
@@ -77,14 +77,16 @@ proc hook_failed_popup {hook msg} {
77
-width 80 -height 10 \
78
-font font_diff \
79
-yscrollcommand [list $w.m.sby set]
80
- label $w.m.l2 \
81
- -text [mc "You must correct the above errors before committing."] \
82
- -anchor w \
83
- -justify left \
84
- -font font_uibold
80
scrollbar $w.m.sby -command [list $w.m.t yview]
81
pack $w.m.l1 -side top -fill x
87
- pack $w.m.l2 -side bottom -fill x
82
+ if {$is_fatal} {
83
+ label $w.m.l2 \
84
+ -text [mc "You must correct the above errors before committing."] \
85
+ -anchor w \
86
+ -justify left \
87
+ -font font_uibold
88
+ pack $w.m.l2 -side bottom -fill x
89
+ }
90
pack $w.m.sby -side right -fill y
91
pack $w.m.t -side left -fill both -expand 1
92
pack $w.m -side top -fill both -expand 1 -padx 5 -pady 10