Git development
 help / color / mirror / Atom feed
From: Pablo Sabater <pabloosabaterr@gmail.com>
To: git@vger.kernel.org
Cc: me@runxiyu.org, gitster@pobox.com, christian.couder@gmail.com,
	karthik.188@gmail.com, jltobler@gmail.com,
	ayu.chandekar@gmail.com, siddharthasthana31@gmail.com,
	chandrapratap3519@gmail.com,
	Pablo Sabater <pabloosabaterr@gmail.com>
Subject: [GSoC PATCH 3/3] receive-pack: use worktree HEAD for updateInstead
Date: Mon, 30 Mar 2026 13:18:22 +0200	[thread overview]
Message-ID: <20260330111822.165188-4-pabloosabaterr@gmail.com> (raw)
In-Reply-To: <20260330111822.165188-1-pabloosabaterr@gmail.com>

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>
---
 builtin/receive-pack.c | 39 +++++++++++++++------------------------
 t/t5516-fetch-push.sh  |  6 +-----
 2 files changed, 16 insertions(+), 29 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d6225df890..2a0fb13250 100644
--- 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 c40f2790d8..117cfa051f 100755
--- 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 &&
-- 
2.43.0


  parent reply	other threads:[~2026-03-30 11:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-23 14:12 [PATCH git] t5516: test updateInstead with worktree and unborn bare HEAD Runxi Yu
2026-02-23 19:15 ` Junio C Hamano
2026-02-24  4:33   ` Runxi Yu
2026-03-03 10:03     ` Runxi Yu
2026-03-30 11:18 ` [GSoC PATCH 0/3] receive-pack: fix HEAD check for updateInstead Pablo Sabater
2026-03-30 11:18   ` [GSoC PATCH 1/3] t5516: test updateInstead with worktree and unborn bare HEAD Pablo Sabater
2026-03-30 11:18   ` [GSoC PATCH 2/3] t5516: clean up cloned and new-wt in denyCurrentBranch and worktrees test Pablo Sabater
2026-03-30 11:18   ` Pablo Sabater [this message]
2026-03-30 15:26   ` [GSoC PATCH 0/3] receive-pack: fix HEAD check for updateInstead Junio C Hamano
2026-03-30 18:49     ` Pablo
2026-03-31 22:21   ` Junio C Hamano

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

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

  git send-email \
    --in-reply-to=20260330111822.165188-4-pabloosabaterr@gmail.com \
    --to=pabloosabaterr@gmail.com \
    --cc=ayu.chandekar@gmail.com \
    --cc=chandrapratap3519@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=karthik.188@gmail.com \
    --cc=me@runxiyu.org \
    --cc=siddharthasthana31@gmail.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.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox