Volume XXII, number 280Wednesday, October 7, 2026Latest message 1 hour ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patcht5516: test updateInstead with worktree and unborn bare HEAD

11 messages between Feb 23, 2026 and Mar 31, 2026, from Runxi Yu, Junio C Hamano, Pablo Sabater, Pablo.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Runxi YuFeb 23, 2026, 14:12 UTC on lore

This is a regression test which should presently fail, to demonstrate the behavior I encountered that looks like a bug.

When a bare repository has a worktree checked out on a separate branch, receive.denyCurrentBranch=updateInstead should allow a push to that branch and update the linked worktree, as long as the linked worktree is clean.

But, if the bare repository's own HEAD is repointed to an unborn branch, the push is rejected with "Working directory has staged changes", even though the linked worktree itself is clean.

This test is essentially a minimal working example of what I encountered while actually using Git; it might not be the optimal way to demonstrate the underlying bug. I suspect builtin/receive-pack.c is using the bare repository's HEAD even when comparing it to the worktree's index.

Signed-off-by: Runxi Yu <me@runxiyu.org>
---
 t/t5516-fetch-push.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Show changes to t/t5516-fetch-push.sh +18 −0
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 29e2f17608..f44250c38f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1816,6 +1816,24 @@ 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_when_finished "rm -fr bare.git cloned" &&
+	git clone --bare . bare.git &&
+	git -C bare.git worktree add wt &&
+	git -C bare.git config receive.denyCurrentBranch updateInstead &&
+	git -C bare.git symbolic-ref HEAD refs/heads/unborn &&
+	test_must_fail git -C bare.git rev-parse -q --verify HEAD^{commit} &&
+	git clone . cloned &&
+	test_commit -C cloned mozzarella &&
+	git -C cloned push ../bare.git HEAD:wt &&
+	test_path_exists bare.git/wt/mozzarella.t &&
+	test "$(git -C cloned rev-parse HEAD)" = "$(git -C bare.git/wt rev-parse HEAD)"
+'
+
 test_expect_success 'refuse fetch to current branch of worktree' '
 	test_when_finished "git worktree remove --force wt && git branch -D wt" &&
 	git worktree add wt &&

-- 
2.53.0
Junio C HamanoFeb 23, 2026, 19:15 UTC in reply to Runxi Yu on lore

Re: [PATCH git] t5516: test updateInstead with worktree and unborn bare HEAD

Runxi Yu <me@runxiyu.org> writes:
> This is a regression test which should presently fail, to demonstrate
> the behavior I encountered that looks like a bug.
Did it ever worked before?  When did it regress?
Runxi YuFeb 24, 2026, 04:33 UTC in reply to Junio C Hamano on lore

Re: [PATCH git] t5516: test updateInstead with worktree and unborn bare HEAD

On Tue Feb 24, 2026 at 3:15 AM CST, Junio C Hamano wrote:
> Did it ever worked before?  When did it regress?

Not that I know of. Unfortunately it's a bit difficult to build older versions, but the same logic was present in this commit and in main:

Show 13 quoted lines
> commit 9fdf4f1db422cc259e4a3ce0023a255102c6fa3b
> Author: Anders Kaseorg <andersk@MIT.EDU>
> Date:   Wed Dec 1 14:15:46 2021 -0800
> 
>     receive-pack: protect current branch for bare repository worktree
>     
>     A bare repository won’t have a working tree at "..", but it may still
>     have separate working trees created with git worktree. We should protect
>     the current branch of such working trees from being updated or deleted,
>     according to receive.denyCurrentBranch.
>     
>     Signed-off-by: Anders Kaseorg <andersk@mit.edu>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>

update finds the destination linked worktree correctly update_worktree sets GIT_DIR push_to_deploy decides whether to diff against HEAD or the empty tree by calling head_has_history but head_has_history uses the bare repo's HEAD rather than the destination worktree

head_has_history used get_oid before vs repo_get_oid now, but they still both seem to use the bare repo instead of the worktree's HEAD.

So if the bare repo's own HEAD is unborn, in both current main and 9fdf4f1db42, we would be incorrectly choosing the empty tree for the 'has staged changes' check's base, and report failure, even though the linked worktree is clean

Well, by regression test, I don't mean that this was a previous regression; I just mean that, this was a way to reproduce this bug that sounds reasonable and could serve as a future regression test.

Runxi YuMar 3, 2026, 10:03 UTC in reply to Runxi Yu on lore

Re: [PATCH git] t5516: test updateInstead with worktree and unborn bare HEAD

I'd appreciate considering this thread again since it's been a bit more than a week.

Thank you!!
Pablo SabaterMar 30, 2026, 11:18 UTC in reply to Runxi Yu on lore

[GSoC PATCH 0/3] receive-pack: fix HEAD check 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.

This series includes Runxi's test, a cleanup of a test that messes with Runxi's test and the fix for the issue.

Pablo Sabater (2):
  t5516: clean up cloned and new-wt in denyCurrentBranch and worktrees
    test
  receive-pack: use worktree HEAD for updateInstead
Runxi Yu (1):
  t5516: test updateInstead with worktree and unborn bare HEAD
 builtin/receive-pack.c | 39 +++++++++++++++------------------------
 t/t5516-fetch-push.sh  | 15 +++++++++++++++
 2 files changed, 30 insertions(+), 24 deletions(-)
base-commit: ca1db8a0f7dc0dbea892e99f5b37c5fe5861be71
-- 
2.43.0
Pablo SabaterMar 30, 2026, 11:18 UTC in reply to Pablo Sabater on lore

[GSoC PATCH 1/3] t5516: test updateInstead with worktree and unborn bare HEAD

From: Runxi Yu <me@runxiyu.org>

This is a regression test which should presently fail, to demonstrate the behavior I encountered that looks like a bug.

When a bare repository has a worktree checked out on a separate branch, receive.denyCurrentBranch=updateInstead should allow a push to that branch and update the linked worktree, as long as the linked worktree is clean.

But, if the bare repository's own HEAD is repointed to an unborn branch, the push is rejected with "Working directory has staged changes", even though the linked worktree itself is clean.

This test is essentially a minimal working example of what I encountered while actually using Git; it might not be the optimal way to demonstrate the underlying bug. I suspect builtin/receive-pack.c is using the bare repository's HEAD even when comparing it to the worktree's index.

Signed-off-by: Runxi Yu <me@runxiyu.org>
---
 t/t5516-fetch-push.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Show changes to t/t5516-fetch-push.sh +18 −0
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 29e2f17608..f44250c38f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1816,6 +1816,24 @@ 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_when_finished "rm -fr bare.git cloned" &&
+	git clone --bare . bare.git &&
+	git -C bare.git worktree add wt &&
+	git -C bare.git config receive.denyCurrentBranch updateInstead &&
+	git -C bare.git symbolic-ref HEAD refs/heads/unborn &&
+	test_must_fail git -C bare.git rev-parse -q --verify HEAD^{commit} &&
+	git clone . cloned &&
+	test_commit -C cloned mozzarella &&
+	git -C cloned push ../bare.git HEAD:wt &&
+	test_path_exists bare.git/wt/mozzarella.t &&
+	test "$(git -C cloned rev-parse HEAD)" = "$(git -C bare.git/wt rev-parse HEAD)"
+'
+
 test_expect_success 'refuse fetch to current branch of worktree' '
 	test_when_finished "git worktree remove --force wt && git branch -D wt" &&
 	git worktree add wt &&

-- 
2.43.0
Pablo SabaterMar 30, 2026, 11:18 UTC in reply to Pablo Sabater on lore

[GSoC PATCH 2/3] t5516: clean up cloned and new-wt in denyCurrentBranch and worktrees test

The 'denyCurrentBranch and worktrees' test creates a 'cloned' and a 'new-wt' but it doesn't clean them after the test. This makes other tests that use the same name after this one to fail.

Add test_when_finished to clean them at the end.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 t/t5516-fetch-push.sh | 1 +
 1 file changed, 1 insertion(+)
Show changes to t/t5516-fetch-push.sh +1 −0
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index f44250c38f..c40f2790d8 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1792,6 +1792,7 @@ test_expect_success 'updateInstead with push-to-checkout hook' '
 '
 
 test_expect_success 'denyCurrentBranch and worktrees' '
+	test_when_finished "rm -fr cloned && git worktree remove --force new-wt" &&
 	git worktree add new-wt &&
 	git clone . cloned &&
 	test_commit -C cloned first &&

-- 
2.43.0
Pablo SabaterMar 30, 2026, 11:18 UTC in reply to Pablo Sabater on lore

[GSoC PATCH 3/3] 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>
---
 builtin/receive-pack.c | 39 +++++++++++++++------------------------
 t/t5516-fetch-push.sh  |  6 +-----
 2 files changed, 16 insertions(+), 29 deletions(-)
Show changes to 2 files +16 −29

builtin/receive-pack.c, t/t5516-fetch-push.sh

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
Junio C HamanoMar 30, 2026, 15:26 UTC in reply to Pablo Sabater on lore

Re: [GSoC PATCH 0/3] receive-pack: fix HEAD check for updateInstead

Pablo Sabater <pabloosabaterr@gmail.com> writes:
Show 9 quoted lines
> 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.
>
> This series includes Runxi's test, a cleanup of a test that messes with Runxi's
> test and the fix for the issue.

It would have made a perfect cover letter if you said in a very early paragraph what Runxi is and how it related to this issue. I am guessing (from the fact that the same word appears in the patch list below with family name) that is a name of a person who first reported the issue? If so, the missing sentence would have said something like "At https/lore.kernel.org/git/$MessageId, Runxi Yu reported ...".

I have to leave the keyboard now, so will take a look at the patches later today. Thanks.

Show 14 quoted lines
> Pablo Sabater (2):
>   t5516: clean up cloned and new-wt in denyCurrentBranch and worktrees
>     test
>   receive-pack: use worktree HEAD for updateInstead
>
> Runxi Yu (1):
>   t5516: test updateInstead with worktree and unborn bare HEAD
>
>  builtin/receive-pack.c | 39 +++++++++++++++------------------------
>  t/t5516-fetch-push.sh  | 15 +++++++++++++++
>  2 files changed, 30 insertions(+), 24 deletions(-)
>
>
> base-commit: ca1db8a0f7dc0dbea892e99f5b37c5fe5861be71
PabloMar 30, 2026, 18:49 UTC in reply to Junio C Hamano on lore

Re: [GSoC PATCH 0/3] receive-pack: fix HEAD check for updateInstead

Junio C Hamano (<gitster@pobox.com>) writes:
Show 21 quoted lines
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> > 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.
> >
> > This series includes Runxi's test, a cleanup of a test that messes with Runxi's
> > test and the fix for the issue.
>
> It would have made a perfect cover letter if you said in a very
> early paragraph what Runxi is and how it related to this issue.  I
> am guessing (from the fact that the same word appears in the patch
> list below with family name) that is a name of a person who first
> reported the issue?  If so, the missing sentence would have said
> something like "At https/lore.kernel.org/git/$MessageId, Runxi Yu
> reported ...".
>

I thought that by sending the series in-reply to the original Runxi's report it wouldn't be needed. But yeah, it isn't clear if someone would only read the cover letter.

this is Runxi's original report which is the parent of this thread:
  https://lore.kernel.org/git/20260223141236.22476-1-me@runxiyu.org/
Thanks for the feedback :)
Show 18 quoted lines
> I have to leave the keyboard now, so will take a look at the patches
> later today.  Thanks.
>
>
> > Pablo Sabater (2):
> >   t5516: clean up cloned and new-wt in denyCurrentBranch and worktrees
> >     test
> >   receive-pack: use worktree HEAD for updateInstead
> >
> > Runxi Yu (1):
> >   t5516: test updateInstead with worktree and unborn bare HEAD
> >
> >  builtin/receive-pack.c | 39 +++++++++++++++------------------------
> >  t/t5516-fetch-push.sh  | 15 +++++++++++++++
> >  2 files changed, 30 insertions(+), 24 deletions(-)
> >
> >
> > base-commit: ca1db8a0f7dc0dbea892e99f5b37c5fe5861be71
Junio C HamanoMar 31, 2026, 22:21 UTC in reply to Pablo Sabater on lore

Re: [GSoC PATCH 0/3] receive-pack: fix HEAD check for updateInstead

Pablo Sabater <pabloosabaterr@gmail.com> writes:
Show 7 quoted lines
> This series includes Runxi's test, a cleanup of a test that messes with Runxi's
> test and the fix for the issue.
>
> Pablo Sabater (2):
>   t5516: clean up cloned and new-wt in denyCurrentBranch and worktrees
>     test
>   receive-pack: use worktree HEAD for updateInstead

I re-read the main patch [3/3]; we used to check "HEAD" in the_repository but now we pass the worktree object (not just its "path" member) down the callchain, and that allows us to check if the worktree's HEAD is unborn. Which makes sense and is a surprisingly small fix.

Let me mark the topic for 'next'.  Thanks.
Show 10 quoted lines
>
> Runxi Yu (1):
>   t5516: test updateInstead with worktree and unborn bare HEAD
>
>  builtin/receive-pack.c | 39 +++++++++++++++------------------------
>  t/t5516-fetch-push.sh  | 15 +++++++++++++++
>  2 files changed, 30 insertions(+), 24 deletions(-)
>
>
> base-commit: ca1db8a0f7dc0dbea892e99f5b37c5fe5861be71

Back to recent threads