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

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

[Runxi Yu]

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(+)

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

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

[Junio C Hamano]

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?

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

[Runxi Yu]

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:

> 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.

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

[Runxi Yu]

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

Thank you!!

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

[Pablo Sabater]

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

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

[Pablo Sabater]

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(+)

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

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

[Pablo Sabater]

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(+)

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

[GSoC PATCH 3/3] receive-pack: use worktree HEAD for updateInstead

[Pablo Sabater]

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

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

[Junio C Hamano]

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 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

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

[Pablo]

Junio C Hamano (<gitster@pobox.com>) writes:
>
> 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 :)

> 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

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

[Junio C Hamano]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> 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.




>
> 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