From 45f274fbb118cc4cb00640c30b4e3069d96755a0 Mon Sep 17 00:00:00 2001 From: Hariom Verma Date: Sun, 23 Feb 2020 18:57:08 +0000 Subject: [PATCH 1/4] get_main_worktree(): allow it to be called in the Git directory When called in the Git directory of a non-bare repository, this function would not return the directory of the main worktree, but of the Git directory instead. The reason: when the Git directory is the current working directory, the absolute path of the common directory will be reported with a trailing `/.git/.`, which the code of `get_main_worktree()` does not handle correctly. Let's fix this. Helped-by: Johannes Schindelin Signed-off-by: Hariom Verma Signed-off-by: Junio C Hamano --- worktree.c | 1 + 1 file changed, 1 insertion(+) diff --git a/worktree.c b/worktree.c index 5b4793caa3..7c8cd21317 100644 --- a/worktree.c +++ b/worktree.c @@ -51,6 +51,7 @@ static struct worktree *get_main_worktree(void) struct strbuf worktree_path = STRBUF_INIT; strbuf_add_absolute_path(&worktree_path, get_git_common_dir()); + strbuf_strip_suffix(&worktree_path, "/."); if (!strbuf_strip_suffix(&worktree_path, "/.git")) strbuf_strip_suffix(&worktree_path, "/."); From f8692114dbb1b3ffe0b71871a015c632c195b784 Mon Sep 17 00:00:00 2001 From: Hariom Verma Date: Sun, 23 Feb 2020 18:57:09 +0000 Subject: [PATCH 2/4] t5509: use a bare repository for test push target `receive.denyCurrentBranch` currently has a bug where it allows pushing into non-bare repository using namespaces as long as it does not have any commits. This would cause t5509 to fail once that bug is fixed because it pushes into an unborn current branch. In t5509, no operations are performed inside `pushee`, as it is only a target for `git push` and `git ls-remote` calls. Therefore it does not need to have a worktree. So, it is safe to change `pushee` to a bare repository. Helped-by: Johannes Schindelin Signed-off-by: Hariom Verma Signed-off-by: Junio C Hamano --- t/t5509-fetch-push-namespaces.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5509-fetch-push-namespaces.sh b/t/t5509-fetch-push-namespaces.sh index 75cbfcc392..e3975bd21d 100755 --- a/t/t5509-fetch-push-namespaces.sh +++ b/t/t5509-fetch-push-namespaces.sh @@ -20,7 +20,7 @@ test_expect_success setup ' ) && commit0=$(cd original && git rev-parse HEAD^) && commit1=$(cd original && git rev-parse HEAD) && - git init pushee && + git init --bare pushee && git init puller ' From 4ef346482d6d5748861c1aa9d56712e847369b40 Mon Sep 17 00:00:00 2001 From: Hariom Verma Date: Sun, 23 Feb 2020 18:57:10 +0000 Subject: [PATCH 3/4] receive.denyCurrentBranch: respect all worktrees 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 using ref namespaces. 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 even while using ref namespaces. Helped-by: Johannes Schindelin Signed-off-by: Hariom Verma Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 36 +++++++++++++++++--------------- t/t5509-fetch-push-namespaces.sh | 11 ++++++++++ t/t5516-fetch-push.sh | 11 ++++++++++ 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 411e0b4d99..2cc18bbffd 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 "), @@ -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,26 @@ 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 +1027,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 +1039,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 +1071,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 +1120,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/t5509-fetch-push-namespaces.sh b/t/t5509-fetch-push-namespaces.sh index e3975bd21d..a67f792adf 100755 --- a/t/t5509-fetch-push-namespaces.sh +++ b/t/t5509-fetch-push-namespaces.sh @@ -152,4 +152,15 @@ test_expect_success 'clone chooses correct HEAD (v2)' ' test_cmp expect actual ' +test_expect_success 'denyCurrentBranch and unborn branch with ref namespace' ' + ( + cd original && + git init unborn && + git remote add unborn-namespaced "ext::git --namespace=namespace %s unborn" && + test_must_fail git push unborn-namespaced HEAD:master && + git -C unborn config receive.denyCurrentBranch updateInstead && + git push unborn-namespaced HEAD:master + ) +' + test_done diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index c81ca360ac..49982b0fd9 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 From 4d864895a23bbbb5403d9aa8a65d0576d2029597 Mon Sep 17 00:00:00 2001 From: Hariom Verma Date: Wed, 4 Mar 2020 07:00:00 +0000 Subject: [PATCH 4/4] t2402: test worktree path when called in .git directory The bug which reports an extra `/.git/.` in worktree path when called in '.git' directory already has been fixed. But unfortunately, the regression test to ensure this behavior has been forgotten. Here is that test. Helped-by: Johannes Schindelin Signed-off-by: Hariom Verma Acked-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t2402-worktree-list.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh index bb6fb9b12c..69ffe865b4 100755 --- a/t/t2402-worktree-list.sh +++ b/t/t2402-worktree-list.sh @@ -151,4 +151,10 @@ test_expect_success 'linked worktrees are sorted' ' test_cmp expected sorted/main/actual ' +test_expect_success 'worktree path when called in .git directory' ' + git worktree list >list1&& + git -C .git worktree list >list2 && + test_cmp list1 list2 +' + test_done