receive-pack: use worktree HEAD for updateInstead

When a bare repo has linked worktrees, and its HEAD points to an unborn branch,
pushing to a wt branch with updateInstead fails and rejects the push, even if
the wt is clean. This happens because HEAD is checked only for the bare repo
context, instead of the wt.

Remove head_has_history and check for worktree->head_oid which does
have the correct HEAD of the wt.

Update the test added by Runxi's patch to expect success.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e34edff406959a9b72d3c2eae30c6c6849c31c49..26a3a0bcd308ea6809201abb7c7e29337ba410a2 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1380,32 +1380,16 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
        return 0;
 }
 
-/*
- * NEEDSWORK: we should consolidate various implementations of "are we
- * on an unborn branch?" test into one, and make the unified one more
- * robust. !get_sha1() based check used here and elsewhere would not
- * allow us to tell an unborn branch from corrupt ref, for example.
- * For the purpose of fixing "deploy-to-update does not work when
- * pushing into an empty repository" issue, this should suffice for
- * now.
- */
-static int head_has_history(void)
-{
-       struct object_id oid;
-
-       return !repo_get_oid(the_repository, "HEAD", &oid);
-}
-
 static const char *push_to_deploy(unsigned char *sha1,
                                  struct strvec *env,
-                                 const char *work_tree)
+                                 const struct worktree *worktree)
 {
        struct child_process child = CHILD_PROCESS_INIT;
 
        strvec_pushl(&child.args, "update-index", "-q", "--ignore-submodules",
                     "--refresh", NULL);
        strvec_pushv(&child.env, env->v);
-       child.dir = work_tree;
+       child.dir = worktree->path;
        child.no_stdin = 1;
        child.stdout_to_stderr = 1;
        child.git_cmd = 1;
@@ -1417,7 +1401,7 @@ static const char *push_to_deploy(unsigned char *sha1,
        strvec_pushl(&child.args, "diff-files", "--quiet",
                     "--ignore-submodules", "--", NULL);
        strvec_pushv(&child.env, env->v);
-       child.dir = work_tree;
+       child.dir = worktree->path;
        child.no_stdin = 1;
        child.stdout_to_stderr = 1;
        child.git_cmd = 1;
@@ -1427,9 +1411,16 @@ static const char *push_to_deploy(unsigned char *sha1,
        child_process_init(&child);
        strvec_pushl(&child.args, "diff-index", "--quiet", "--cached",
                     "--ignore-submodules",
-                    /* diff-index with either HEAD or an empty tree */
-                    head_has_history() ? "HEAD" : empty_tree_oid_hex(the_repository->hash_algo),
-                    "--", NULL);
+                    /*
+                     * diff-index with either HEAD or an empty tree
+                     *
+                     * NEEDSWORK: is_null_oid() cannot know whether it's an
+                     * unborn HEAD or a corrupt ref. It works for now because
+                     * it's only needed to know if we are comparing HEAD or an
+                     * empty tree.
+                     */
+                    !is_null_oid(&worktree->head_oid) ? "HEAD" :
+                    empty_tree_oid_hex(the_repository->hash_algo), "--", NULL);
        strvec_pushv(&child.env, env->v);
        child.no_stdin = 1;
        child.no_stdout = 1;
@@ -1442,7 +1433,7 @@ static const char *push_to_deploy(unsigned char *sha1,
        strvec_pushl(&child.args, "read-tree", "-u", "-m", hash_to_hex(sha1),
                     NULL);
        strvec_pushv(&child.env, env->v);
-       child.dir = work_tree;
+       child.dir = worktree->path;
        child.no_stdin = 1;
        child.no_stdout = 1;
        child.stdout_to_stderr = 0;
@@ -1490,7 +1481,7 @@ static const char *update_worktree(unsigned char *sha1, const struct worktree *w
 
        retval = push_to_checkout(sha1, &invoked_hook, &env, worktree->path);
        if (!invoked_hook)
-               retval = push_to_deploy(sha1, &env, worktree->path);
+               retval = push_to_deploy(sha1, &env, worktree);
 
        strvec_clear(&env);
        free(git_dir);

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index c40f2790d80f00b857701910c73a19e0cc115626..117cfa051f33e25a8e077802e5eccec56765d028 100755 (executable)
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1817,11 +1817,7 @@ test_expect_success 'denyCurrentBranch and bare repository worktrees' '
        test_must_fail git push --delete bare.git wt
 '
 
-# NEEDSWORK: updateInstead unexpectedly fails when bare HEAD points to unborn
-# branch (or probably any ref that differs from the target worktree) despite
-# the target worktree being clean. This seems to be because receive-pack.c
-# diffs the target worktree index against the bare repository HEAD.
-test_expect_failure 'updateInstead with bare repository worktree and unborn bare HEAD' '
+test_expect_success 'updateInstead with bare repository worktree and unborn bare HEAD' '
        test_when_finished "rm -fr bare.git cloned" &&
        git clone --bare . bare.git &&
        git -C bare.git worktree add wt &&