git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: git@vger.kernel.org
Cc: "Gumbel, Matthew K" <matthew.k.gumbel@intel.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH] worktree: invoke post-checkout hook (unless --no-checkout)
Date: Thu,  7 Dec 2017 16:20:17 -0500	[thread overview]
Message-ID: <20171207212017.53774-1-sunshine@sunshineco.com> (raw)

git-clone and git-checkout both invoke the post-checkout hook following
a successful checkout, yet git-worktree neglects to do so even though it
too "checks out" the worktree. Fix this oversight.

Implementation note: The newly-created worktree may reference a branch
or be detached. In the latter case, a commit lookup is performed, though
the result is used only in a boolean sense to (a) determine if the
commit actually exists, and (b) assign either the branch name or commit
ID to HEAD. Since the post-commit hook needs to know the ID of the
checked-out commit, the lookup now needs to be done in all cases, rather
than only when detached. Consequently, a new boolean is needed to handle
(b) since the lookup result itself can no longer perform that role.

Reported-by: Matthew K Gumbel <matthew.k.gumbel@intel.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/githooks.txt |  3 ++-
 builtin/worktree.c         | 22 ++++++++++++++++------
 t/t2025-worktree-add.sh    | 29 +++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 0bb0042d8c..91eb297f7b 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -170,7 +170,8 @@ This hook cannot affect the outcome of 'git checkout'.
 
 It is also run after 'git clone', unless the --no-checkout (-n) option is
 used. The first parameter given to the hook is the null-ref, the second the
-ref of the new HEAD and the flag is always 1.
+ref of the new HEAD and the flag is always 1. Likewise for 'git worktree add'
+unless --no-checkout is used.
 
 This hook can be used to perform repository validity checks, auto-display
 differences from the previous HEAD if different, or set working dir metadata
diff --git a/builtin/worktree.c b/builtin/worktree.c
index ed043d5f1c..9591f10442 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -218,20 +218,21 @@ static int add_worktree(const char *path, const char *refname,
 	int counter = 0, len, ret;
 	struct strbuf symref = STRBUF_INIT;
 	struct commit *commit = NULL;
+	int is_branch = 0;
 
 	if (file_exists(path) && !is_empty_dir(path))
 		die(_("'%s' already exists"), path);
 
 	/* is 'refname' a branch or commit? */
 	if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&
-		 ref_exists(symref.buf)) { /* it's a branch */
+	    ref_exists(symref.buf)) {
+		is_branch = 1;
 		if (!opts->force)
 			die_if_checked_out(symref.buf, 0);
-	} else { /* must be a commit */
-		commit = lookup_commit_reference_by_name(refname);
-		if (!commit)
-			die(_("invalid reference: %s"), refname);
 	}
+	commit = lookup_commit_reference_by_name(refname);
+	if (!commit)
+		die(_("invalid reference: %s"), refname);
 
 	name = worktree_basename(path, &len);
 	git_path_buf(&sb_repo, "worktrees/%.*s", (int)(path + len - name), name);
@@ -296,7 +297,7 @@ static int add_worktree(const char *path, const char *refname,
 	argv_array_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
 	cp.git_cmd = 1;
 
-	if (commit)
+	if (!is_branch)
 		argv_array_pushl(&cp.args, "update-ref", "HEAD",
 				 oid_to_hex(&commit->object.oid), NULL);
 	else
@@ -327,6 +328,15 @@ static int add_worktree(const char *path, const char *refname,
 		strbuf_addf(&sb, "%s/locked", sb_repo.buf);
 		unlink_or_warn(sb.buf);
 	}
+
+	/*
+	 * Hook failure does not warrant worktree deletion, so run hook after
+	 * is_junk is cleared, but do return appropriate code when hook fails.
+	 */
+	if (!ret && opts->checkout)
+		ret = run_hook_le(NULL, "post-checkout", oid_to_hex(&null_oid),
+				  oid_to_hex(&commit->object.oid), "1", NULL);
+
 	argv_array_clear(&child_env);
 	strbuf_release(&sb);
 	strbuf_release(&symref);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index b5c47ac602..d6fb062f83 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -314,4 +314,33 @@ test_expect_success 'rename a branch under bisect not allowed' '
 	test_must_fail git branch -M under-bisect bisect-with-new-name
 '
 
+post_checkout_hook () {
+	test_when_finished "rm -f .git/hooks/post-checkout" &&
+	mkdir -p .git/hooks &&
+	write_script .git/hooks/post-checkout <<-\EOF
+	echo $* >hook.actual
+	EOF
+}
+
+test_expect_success '"add" invokes post-checkout hook (branch)' '
+	post_checkout_hook &&
+	printf "%s %s 1\n" $_z40 $(git rev-parse HEAD) >hook.expect &&
+	git worktree add gumby &&
+	test_cmp hook.expect hook.actual
+'
+
+test_expect_success '"add" invokes post-checkout hook (detached)' '
+	post_checkout_hook &&
+	printf "%s %s 1\n" $_z40 $(git rev-parse HEAD) >hook.expect &&
+	git worktree add --detach grumpy &&
+	test_cmp hook.expect hook.actual
+'
+
+test_expect_success '"add --no-checkout" suppresses post-checkout hook' '
+	post_checkout_hook &&
+	rm -f hook.actual &&
+	git worktree add --no-checkout gloopy &&
+	test_path_is_missing hook.actual
+'
+
 test_done
-- 
2.15.1.502.gccaef8de57


                 reply	other threads:[~2017-12-07 21:21 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171207212017.53774-1-sunshine@sunshineco.com \
    --to=sunshine@sunshineco.com \
    --cc=git@vger.kernel.org \
    --cc=matthew.k.gumbel@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).