{"thread":{"id":"18553","subject":"[PATCH] git-gui: run post-checkout hook on checkout","startedAt":"2009-03-25T22:48:33Z","lastAt":"2009-03-30T20:00:38Z","messageCount":3,"participants":["Jens Lehmann","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"109439","messageId":"49CAB4C1.6070004@web.de","threadId":"18553","inReplyTo":null,"subject":"[PATCH] git-gui: run post-checkout hook on checkout","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2009-03-25T22:48:33Z","receivedAt":"2009-03-25T22:48:33Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"git-gui is using \"git-read-tree -u\" for checkout which doesn't\ninvoke the post-checkout hook as a plain git-checkout would.\nSo git-gui must call the hook itself.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nIt's been a while since i deeply touched tcl/tk code ... i basically\nadapted the code for calling the post-commit hook here.\n\nI renamed the former local variable \"curHEAD\" from the method\n_start_checkout to a field named \"old_hash\" to be able to remember\nthe hash of the tree *before* the checkout started. This hash has\nto be given as first argument to the post-checkout hook *after* the\ncheckout, where it is not the *current* HEAD anymore.\n\n\n git-gui/lib/checkout_op.tcl |   39 ++++++++++++++++++++++++++++++++++-----\n 1 files changed, 34 insertions(+), 5 deletions(-)\n\ndiff --git a/git-gui/lib/checkout_op.tcl b/git-gui/lib/checkout_op.tcl\nindex caca888..31863a5 100644\n--- a/git-gui/lib/checkout_op.tcl\n+++ b/git-gui/lib/checkout_op.tcl\n@@ -9,6 +9,7 @@ field w_cons   {}; # embedded console window object\n field new_expr   ; # expression the user saw/thinks this is\n field new_hash   ; # commit SHA-1 we are switching to\n field new_ref    ; # ref we are updating/creating\n+field old_hash   ; # commit SHA-1 that was checked when we started\n \n field parent_w      .; # window that started us\n field merge_type none; # type of merge to apply to existing branch\n@@ -280,11 +281,11 @@ method _start_checkout {} {\n \n \t# -- Our in memory state should match the repository.\n \t#\n-\trepository_state curType curHEAD curMERGE_HEAD\n+\trepository_state curType old_hash curMERGE_HEAD\n \tif {[string match amend* $commit_type]\n \t\t&& $curType eq {normal}\n-\t\t&& $curHEAD eq $HEAD} {\n-\t} elseif {$commit_type ne $curType || $HEAD ne $curHEAD} {\n+\t\t&& $old_hash eq $HEAD} {\n+\t} elseif {$commit_type ne $curType || $HEAD ne $old_hash} {\n \t\tinfo_popup [mc \"Last scanned state does not match repository state.\n \n Another Git program has modified this repository since the last scan.  A rescan must be performed before the current branch can be changed.\n@@ -297,7 +298,7 @@ The rescan will be automatically started now.\n \t\treturn\n \t}\n \n-\tif {$curHEAD eq $new_hash} {\n+\tif {$old_hash eq $new_hash} {\n \t\t_after_readtree $this\n \t} elseif {[is_config_true gui.trustmtime]} {\n \t\t_readtree $this\n@@ -471,7 +472,19 @@ If you wanted to be on a branch, create one now starting from 'This Detached Che\n \t\tset PARENT $HEAD\n \t\tui_status [mc \"Checked out '%s'.\" $name]\n \t}\n-\tdelete_this\n+\n+\t# -- Run the post-checkout hook.\n+\t#\n+\tset fd_ph [githook_read post-checkout $old_hash $new_hash 1]\n+\tif {$fd_ph ne {}} {\n+\t\tupvar #0 pch_error pc_err\n+\t\tset pc_err {}\n+\t\tfconfigure $fd_ph -blocking 0 -translation binary -eofchar {}\n+\t\tfileevent $fd_ph readable \\\n+\t\t\t[list checkout_postcheckout_wait $fd_ph $this]\n+\t} else {\n+\t\tdelete_this\n+\t}\n }\n \n git-version proc _detach_HEAD {log new} {\n@@ -608,3 +621,19 @@ $err\"]\n }\n \n }\n+\n+proc checkout_postcheckout_wait {fd_ph t} {\n+\tupvar #0 pch_error pch_error\n+\n+\tappend pch_error [read $fd_ph]\n+\tfconfigure $fd_ph -blocking 1\n+\tif {[eof $fd_ph]} {\n+\t\tif {[catch {close $fd_ph}]} {\n+\t\t\thook_failed_popup post-checkout $pch_error 0\n+\t\t}\n+\t\tunset pch_error\n+\t\tdelete_this $t\n+\t\treturn\n+\t}\n+\tfconfigure $fd_ph -blocking 0\n+}\n-- \n1.6.2.1.307.g91408\n"},{"id":"109908","messageId":"20090330143435.GA23521@spearce.org","threadId":"18553","inReplyTo":"49CAB4C1.6070004@web.de","subject":"Re: [PATCH] git-gui: run post-checkout hook on checkout","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-03-30T14:34:35Z","receivedAt":"2009-03-30T14:34:35Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> wrote:\n> It's been a while since i deeply touched tcl/tk code ... i basically\n> adapted the code for calling the post-commit hook here.\n\nNot bad for not having touched tcl/tk in a long time... :-)\n \n> +\t# -- Run the post-checkout hook.\n> +\t#\n> +\tset fd_ph [githook_read post-checkout $old_hash $new_hash 1]\n> +\tif {$fd_ph ne {}} {\n> +\t\tupvar #0 pch_error pc_err\n\nI'd rather spell this \"global pch_error\".\n\n> +\t\tset pc_err {}\n> +\t\tfconfigure $fd_ph -blocking 0 -translation binary -eofchar {}\n> +\t\tfileevent $fd_ph readable \\\n> +\t\t\t[list checkout_postcheckout_wait $fd_ph $this]\n\nThe callback should be \"[cb _postcheckout_wait $fd_ph]\".  This is\na git-gui macro which returns a handle to invoke a \"method\" named\n\"_postcheckout_wait\", passing in $this as the first parameter.\n\n> +proc checkout_postcheckout_wait {fd_ph t} {\n\nThis should be \"method _postcheckout_wait {fd_ph} {\" to use the\ncb macro above, and have $this automatically carry through.\n\n> +\tupvar #0 pch_error pch_error\n\n\"global pch_error\"\n\n> +\tappend pch_error [read $fd_ph]\n> +\tfconfigure $fd_ph -blocking 1\n> +\tif {[eof $fd_ph]} {\n> +\t\tif {[catch {close $fd_ph}]} {\n> +\t\t\thook_failed_popup post-checkout $pch_error 0\n> +\t\t}\n> +\t\tunset pch_error\n> +\t\tdelete_this $t\n> +\t\treturn\n> +\t}\n> +\tfconfigure $fd_ph -blocking 0\n\nAlso, I'm wondering about the UI state semantics here.  Back on\nline 462 (in the preimage) we unlock_index, which means that the\nuser can start to push buttons again, and then later on around\nline 474 you start up the post-checkout hook.\n\nI think we'd want to defer the unlock_index call until after the\nhook has terminated, if we are going to run a hook at all.  In fact,\nwe probably need to move that entire block 462-474 (the unlock_index\n... delete_this block) to a new method which we invoke if there is\nno hook, or after the hook subprocess terminates.\n\nBy waiting until after the hook to do the rescan, we also ensure\nthat the hook has a chance to update the working directory, perhaps\nby dirtying a file, and the user seeing the result of that dirtying\nimmediately.\n\n-- \nShawn.\n"},{"id":"109940","messageId":"49D124E6.8010206@web.de","threadId":"18553","inReplyTo":"20090330143435.GA23521@spearce.org","subject":"Re: [PATCH] git-gui: run post-checkout hook on checkout","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2009-03-30T20:00:38Z","receivedAt":"2009-03-30T20:00:38Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Shawn O. Pearce schrieb:\n> Jens Lehmann <Jens.Lehmann@web.de> wrote:\n>> +\t# -- Run the post-checkout hook.\n>> +\t#\n>> +\tset fd_ph [githook_read post-checkout $old_hash $new_hash 1]\n>> +\tif {$fd_ph ne {}} {\n>> +\t\tupvar #0 pch_error pc_err\n> \n> I'd rather spell this \"global pch_error\".\n> \n>> +\t\tset pc_err {}\n\nWhat i noticed when fixing this issue is that i copied this upvar\nstatement from the calling of the post-commit hook in commit.tcl\nwith only a minor change (ommitting the \"$cmt_id\" behind\n\"pch_error\").\n\nIt does seem to be incorrect there too, as i couldn't find any use\nof the variable \"pc_err\" or \"pch_error$cmt_id\" anywhere in git-gui.\nSo setting \"pc_err\" to empty seems pretty pointless, as everywhere\nelse in commit.tcl \"pch_error\" is used instead.\n\nOr am i overlooking something? If not, the patch below should correct\nthat.\n\nJens\n\n--------------------- 8>< ---------------------\n\n>From 5ddd7e8c2d52fc99e496ed3bc96358cc07e538f1 Mon Sep 17 00:00:00 2001\nFrom: Jens Lehmann <Jens.Lehmann@web.de>\nDate: Mon, 30 Mar 2009 20:35:57 +0200\nSubject: [PATCH] git-gui: When calling post-commit hook wrong variable was cleared.\n\nBefore calling the post-commit hook, the variable \"pc_err\" is cleared\nwhile later only \"pch_error\" is used. \"pch_error$cmt_id\" only appeared in\n\"upvar\"-Statements (which were changed to \"global\") and was removed.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n git-gui/lib/commit.tcl |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl\nindex 9cc8410..7255efb 100644\n--- a/git-gui/lib/commit.tcl\n+++ b/git-gui/lib/commit.tcl\n@@ -398,8 +398,8 @@ A rescan will be automatically started now.\n \t#\n \tset fd_ph [githook_read post-commit]\n \tif {$fd_ph ne {}} {\n-\t\tupvar #0 pch_error$cmt_id pc_err\n-\t\tset pc_err {}\n+\t\tglobal pch_error\n+\t\tset pch_error {}\n \t\tfconfigure $fd_ph -blocking 0 -translation binary -eofchar {}\n \t\tfileevent $fd_ph readable \\\n \t\t\t[list commit_postcommit_wait $fd_ph $cmt_id]\n@@ -461,7 +461,7 @@ A rescan will be automatically started now.\n }\n \n proc commit_postcommit_wait {fd_ph cmt_id} {\n-\tupvar #0 pch_error$cmt_id pch_error\n+\tglobal pch_error\n \n \tappend pch_error [read $fd_ph]\n \tfconfigure $fd_ph -blocking 1\n-- \n1.6.2.1.414.g2daa3\n"}]}