{"thread":{"id":"47402","subject":"[PATCH] worktree: invoke post-checkout hook (unless --no-checkout)","startedAt":"2017-12-07T21:21:37Z","lastAt":"2017-12-07T21:21:37Z","messageCount":1,"participants":["Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"334374","messageId":"20171207212017.53774-1-sunshine@sunshineco.com","threadId":"47402","inReplyTo":null,"subject":"[PATCH] worktree: invoke post-checkout hook (unless --no-checkout)","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-12-07T21:20:17Z","receivedAt":"2017-12-07T21:21:37Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"git-clone and git-checkout both invoke the post-checkout hook following\na successful checkout, yet git-worktree neglects to do so even though it\ntoo \"checks out\" the worktree. Fix this oversight.\n\nImplementation note: The newly-created worktree may reference a branch\nor be detached. In the latter case, a commit lookup is performed, though\nthe result is used only in a boolean sense to (a) determine if the\ncommit actually exists, and (b) assign either the branch name or commit\nID to HEAD. Since the post-commit hook needs to know the ID of the\nchecked-out commit, the lookup now needs to be done in all cases, rather\nthan only when detached. Consequently, a new boolean is needed to handle\n(b) since the lookup result itself can no longer perform that role.\n\nReported-by: Matthew K Gumbel <matthew.k.gumbel@intel.com>\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n Documentation/githooks.txt |  3 ++-\n builtin/worktree.c         | 22 ++++++++++++++++------\n t/t2025-worktree-add.sh    | 29 +++++++++++++++++++++++++++++\n 3 files changed, 47 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 0bb0042d8c..91eb297f7b 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -170,7 +170,8 @@ This hook cannot affect the outcome of 'git checkout'.\n \n It is also run after 'git clone', unless the --no-checkout (-n) option is\n used. The first parameter given to the hook is the null-ref, the second the\n-ref of the new HEAD and the flag is always 1.\n+ref of the new HEAD and the flag is always 1. Likewise for 'git worktree add'\n+unless --no-checkout is used.\n \n This hook can be used to perform repository validity checks, auto-display\n differences from the previous HEAD if different, or set working dir metadata\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex ed043d5f1c..9591f10442 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -218,20 +218,21 @@ static int add_worktree(const char *path, const char *refname,\n \tint counter = 0, len, ret;\n \tstruct strbuf symref = STRBUF_INIT;\n \tstruct commit *commit = NULL;\n+\tint is_branch = 0;\n \n \tif (file_exists(path) && !is_empty_dir(path))\n \t\tdie(_(\"'%s' already exists\"), path);\n \n \t/* is 'refname' a branch or commit? */\n \tif (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n-\t\t ref_exists(symref.buf)) { /* it's a branch */\n+\t    ref_exists(symref.buf)) {\n+\t\tis_branch = 1;\n \t\tif (!opts->force)\n \t\t\tdie_if_checked_out(symref.buf, 0);\n-\t} else { /* must be a commit */\n-\t\tcommit = lookup_commit_reference_by_name(refname);\n-\t\tif (!commit)\n-\t\t\tdie(_(\"invalid reference: %s\"), refname);\n \t}\n+\tcommit = lookup_commit_reference_by_name(refname);\n+\tif (!commit)\n+\t\tdie(_(\"invalid reference: %s\"), refname);\n \n \tname = worktree_basename(path, &len);\n \tgit_path_buf(&sb_repo, \"worktrees/%.*s\", (int)(path + len - name), name);\n@@ -296,7 +297,7 @@ static int add_worktree(const char *path, const char *refname,\n \targv_array_pushf(&child_env, \"%s=%s\", GIT_WORK_TREE_ENVIRONMENT, path);\n \tcp.git_cmd = 1;\n \n-\tif (commit)\n+\tif (!is_branch)\n \t\targv_array_pushl(&cp.args, \"update-ref\", \"HEAD\",\n \t\t\t\t oid_to_hex(&commit->object.oid), NULL);\n \telse\n@@ -327,6 +328,15 @@ static int add_worktree(const char *path, const char *refname,\n \t\tstrbuf_addf(&sb, \"%s/locked\", sb_repo.buf);\n \t\tunlink_or_warn(sb.buf);\n \t}\n+\n+\t/*\n+\t * Hook failure does not warrant worktree deletion, so run hook after\n+\t * is_junk is cleared, but do return appropriate code when hook fails.\n+\t */\n+\tif (!ret && opts->checkout)\n+\t\tret = run_hook_le(NULL, \"post-checkout\", oid_to_hex(&null_oid),\n+\t\t\t\t  oid_to_hex(&commit->object.oid), \"1\", NULL);\n+\n \targv_array_clear(&child_env);\n \tstrbuf_release(&sb);\n \tstrbuf_release(&symref);\ndiff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\nindex b5c47ac602..d6fb062f83 100755\n--- a/t/t2025-worktree-add.sh\n+++ b/t/t2025-worktree-add.sh\n@@ -314,4 +314,33 @@ test_expect_success 'rename a branch under bisect not allowed' '\n \ttest_must_fail git branch -M under-bisect bisect-with-new-name\n '\n \n+post_checkout_hook () {\n+\ttest_when_finished \"rm -f .git/hooks/post-checkout\" &&\n+\tmkdir -p .git/hooks &&\n+\twrite_script .git/hooks/post-checkout <<-\\EOF\n+\techo $* >hook.actual\n+\tEOF\n+}\n+\n+test_expect_success '\"add\" invokes post-checkout hook (branch)' '\n+\tpost_checkout_hook &&\n+\tprintf \"%s %s 1\\n\" $_z40 $(git rev-parse HEAD) >hook.expect &&\n+\tgit worktree add gumby &&\n+\ttest_cmp hook.expect hook.actual\n+'\n+\n+test_expect_success '\"add\" invokes post-checkout hook (detached)' '\n+\tpost_checkout_hook &&\n+\tprintf \"%s %s 1\\n\" $_z40 $(git rev-parse HEAD) >hook.expect &&\n+\tgit worktree add --detach grumpy &&\n+\ttest_cmp hook.expect hook.actual\n+'\n+\n+test_expect_success '\"add --no-checkout\" suppresses post-checkout hook' '\n+\tpost_checkout_hook &&\n+\trm -f hook.actual &&\n+\tgit worktree add --no-checkout gloopy &&\n+\ttest_path_is_missing hook.actual\n+'\n+\n test_done\n-- \n2.15.1.502.gccaef8de57\n\n"}]}