[3/3] receive.denyCurrentBranch: respect all worktrees

Commit Message

Linus Arver via GitGitGadget Feb. 13, 2020, 6:59 p.m. UTC

From: Hariom Verma <[e-mail]>

The receive.denyCurrentBranch config option controls what happens if
you push to a branch that is checked out into a non-bare repository.
By default, it rejects it. It can be disabled via `ignore` or `warn`.
Another yet trickier option is `updateInstead`.

However, this setting was forgotten when the git worktree command was
introduced: only the main worktree's current branch is respected.

With this change, all worktrees are respected.

That change also leads to revealing another bug,
i.e. `receive.denyCurrentBranch = true` was ignored when pushing into a
non-bare repository's unborn current branch.  As `is_ref_checked_out()`
returns 0 which means `receive-pack` does not get into conditional
statement to switch `deny_current_branch` accordingly(ignore, warn,
refuse, unconfigured, updateInstead).

receive.denyCurrentBranch uses the function `refs_resolve_ref_unsafe()`
(called via `resolve_refdup()`) to resolve the symbolic ref HEAD, but
that function fails when HEAD does not point at a valid commit.
As we replace the call to `refs_resolve_ref_unsafe()` with
`find_shared_symref()`, which has no problem finding the worktree for a
given branch even if it is unborn yet, this bug is fixed at the same
time: receive.denyCurrentBranch now also handles worktrees with unborn
branches as intended.

Helped-by: Johannes Schindelin <[e-mail]>
Signed-off-by: Hariom Verma <[e-mail]>
---
 builtin/receive-pack.c | 37 ++++++++++++++++++++-----------------
 t/t5516-fetch-push.sh  | 11 +++++++++++
 2 files changed, 31 insertions(+), 17 deletions(-)

Patch

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 411e0b4d99..b5ca3123b7 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -27,6 +27,7 @@ 
 #include "object-store.h"
 #include "protocol.h"
 #include "commit-reach.h"
+#include "worktree.h"
 
 static const char * const receive_pack_usage[] = {
 	N_("git receive-pack <git-dir>"),
@@ -816,16 +817,6 @@  static int run_update_hook(struct command *cmd)
 	return finish_command(&proc);
 }
 
-static int is_ref_checked_out(const char *ref)
-{
-	if (is_bare_repository())
-		return 0;
-
-	if (!head_name)
-		return 0;
-	return !strcmp(head_name, ref);
-}
-
 static char *refuse_unconfigured_deny_msg =
 	N_("By default, updating the current branch in a non-bare repository\n"
 	   "is denied, because it will make the index and work tree inconsistent\n"
@@ -997,16 +988,27 @@  static const char *push_to_checkout(unsigned char *hash,
 		return NULL;
 }
 
-static const char *update_worktree(unsigned char *sha1)
+static const char *update_worktree(unsigned char *sha1, const struct worktree *worktree)
 {
-	const char *retval;
-	const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : "..";
+	const char *retval, *work_tree, *git_dir = NULL;
 	struct argv_array env = ARGV_ARRAY_INIT;
 
+	if (worktree && worktree->path)
+		work_tree = worktree->path;
+	else if (git_work_tree_cfg)
+		work_tree = git_work_tree_cfg;
+	else
+		work_tree = "..";
+
 	if (is_bare_repository())
 		return "denyCurrentBranch = updateInstead needs a worktree";
+	
+	if (worktree)
+		git_dir = get_worktree_git_dir(worktree);
+	if (!git_dir)
+		git_dir = get_git_dir();
 
-	argv_array_pushf(&env, "GIT_DIR=%s", absolute_path(get_git_dir()));
+	argv_array_pushf(&env, "GIT_DIR=%s", absolute_path(git_dir));
 
 	if (!find_hook(push_to_checkout_hook))
 		retval = push_to_deploy(sha1, &env, work_tree);
@@ -1026,6 +1028,7 @@  static const char *update(struct command *cmd, struct shallow_info *si)
 	struct object_id *old_oid = &cmd->old_oid;
 	struct object_id *new_oid = &cmd->new_oid;
 	int do_update_worktree = 0;
+	const struct worktree *worktree = is_bare_repository() ? NULL : find_shared_symref("HEAD", name);
 
 	/* only refs/... are allowed */
 	if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
@@ -1037,7 +1040,7 @@  static const char *update(struct command *cmd, struct shallow_info *si)
 	free(namespaced_name);
 	namespaced_name = strbuf_detach(&namespaced_name_buf, NULL);
 
-	if (is_ref_checked_out(namespaced_name)) {
+	if (worktree) {
 		switch (deny_current_branch) {
 		case DENY_IGNORE:
 			break;
@@ -1069,7 +1072,7 @@  static const char *update(struct command *cmd, struct shallow_info *si)
 			return "deletion prohibited";
 		}
 
-		if (head_name && !strcmp(namespaced_name, head_name)) {
+		if (worktree || (head_name && !strcmp(namespaced_name, head_name))) {
 			switch (deny_delete_current) {
 			case DENY_IGNORE:
 				break;
@@ -1118,7 +1121,7 @@  static const char *update(struct command *cmd, struct shallow_info *si)
 	}
 
 	if (do_update_worktree) {
-		ret = update_worktree(new_oid->hash);
+		ret = update_worktree(new_oid->hash, find_shared_symref("HEAD", name));
 		if (ret)
 			return ret;
 	}
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index c81ca360ac..6608e391f0 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1712,4 +1712,15 @@  test_expect_success 'updateInstead with push-to-checkout hook' '
 	)
 '
 
+test_expect_success 'denyCurrentBranch and worktrees' '
+    git worktree add new-wt &&
+	git clone . cloned &&
+	test_commit -C cloned first &&
+	test_config receive.denyCurrentBranch refuse &&
+	test_must_fail git -C cloned push origin HEAD:new-wt &&
+	test_config receive.denyCurrentBranch updateInstead &&
+	git -C cloned push origin HEAD:new-wt &&
+	test_must_fail git -C cloned push --delete origin new-wt
+'
+
 test_done