threads / patch / 64811

patchsubtree: validate --prefix against commit in split

Subject: [PATCH] subtree: validate --prefix against commit in split

## tl;dr

19 messages between Jan 15, 2026 and Feb 5, 2026. Diffs are folded; open one to read it.

replies: 18people: 5as markdown or json

Pushkar Singh· Jan 15, 2026, 12:09 UTC · lore
---
 contrib/subtree/git-subtree.sh | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
Show changes to contrib/subtree/git-subtree.sh +13 −4
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 17106d1a72..a1b60eac8b 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -257,6 +257,9 @@ main () {
 		test -e "$arg_prefix" &&
 			die "fatal: prefix '$arg_prefix' already exists."
 		;;
+	split)
+        # checked later against the commit, not the working tree
+        ;;
 	*)
 		test -e "$arg_prefix" ||
 			die "fatal: '$arg_prefix' does not exist; use 'git subtree add'"
@@ -958,13 +961,19 @@ cmd_add_commit () {
 cmd_split () {
 	if test $# -eq 0
 	then
-		rev=$(git rev-parse HEAD)
+			rev=$(git rev-parse HEAD)
 	elif test $# -eq 1 || test $# -eq 2
 	then
-		rev=$(git rev-parse -q --verify "$1^{commit}") ||
-			die "fatal: '$1' does not refer to a commit"
+			rev=$(git rev-parse -q --verify "$1^{commit}") ||
+					die "fatal: '$1' does not refer to a commit"
 	else
-		die "fatal: you must provide exactly one revision, and optionally a repository.  Got: '$*'"
+			die "fatal: you must provide exactly one revision, and optionally a repository.  Got: '$*'"
+	fi
+
+	# Now validate prefix against the commit, not the working tree
+	if ! git ls-tree -d "$rev" -- "$dir" >/dev/null
+	then
+			die "fatal: '$dir' does not exist in commit $rev"
 	fi
 	repository=""
 	if test "$#" = 2
-- 
2.43.0
Pushkar Singh· Jan 15, 2026, 12:24 UTC · re: Pushkar Singh · lore

[PATCH v2] subtree: validate --prefix against commit in split

git subtree split currently validates --prefix against the working tree. This breaks when splitting an older commit or when the working tree does not contain the subtree, even though the commit does.

For example:
  git subtree split --prefix=pkg <commit>
fails if pkg was removed later, even though it exists in <commit>.

Fix this by validating the prefix against the specified commit using git ls-tree instead of the working tree.

Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
---
 contrib/subtree/git-subtree.sh | 9 +++++++++
 1 file changed, 9 insertions(+)
Show changes to contrib/subtree/git-subtree.sh +9 −0
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 17106d1a72..a5822b66d5 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -257,6 +257,9 @@ main () {
 		test -e "$arg_prefix" &&
 			die "fatal: prefix '$arg_prefix' already exists."
 		;;
+	split)
+        # checked later against the commit, not the working tree
+        ;;
 	*)
 		test -e "$arg_prefix" ||
 			die "fatal: '$arg_prefix' does not exist; use 'git subtree add'"
@@ -966,6 +969,12 @@ cmd_split () {
 	else
 		die "fatal: you must provide exactly one revision, and optionally a repository.  Got: '$*'"
 	fi
+
+	# Now validate prefix against the commit, not the working tree
+	if ! git ls-tree -d "$rev" -- "$dir" >/dev/null
+	then
+		die "fatal: '$dir' does not exist in commit $rev"
+	fi
 	repository=""
 	if test "$#" = 2
 	then
-- 
2.43.0
Junio C Hamano· Jan 15, 2026, 16:30 UTC · re: Pushkar Singh · lore

Re: [PATCH v2] subtree: validate --prefix against commit in split

Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:
Show 17 quoted lines
> git subtree split currently validates --prefix against the working tree.
> This breaks when splitting an older commit or when the working tree does
> not contain the subtree, even though the commit does.
>
> For example:
>
>   git subtree split --prefix=pkg <commit>
>
> fails if pkg was removed later, even though it exists in <commit>.
>
> Fix this by validating the prefix against the specified commit using
> git ls-tree instead of the working tree.
>
> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
> ---
>  contrib/subtree/git-subtree.sh | 9 +++++++++
>  1 file changed, 9 insertions(+)

Is this something you can protect from future breakage with a test, perhaps in contrib/subtree/t/t7900-subtree.sh?

Show 11 quoted lines
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 17106d1a72..a5822b66d5 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -257,6 +257,9 @@ main () {
>  		test -e "$arg_prefix" &&
>  			die "fatal: prefix '$arg_prefix' already exists."
>  		;;
> +	split)
> +        # checked later against the commit, not the working tree
> +        ;;
Funny indentation?
Pushkar Singh· Jan 15, 2026, 17:52 UTC · re: Junio C Hamano · lore

[PATCH v3] subtree: validate --prefix against commit in split

git subtree split currently validates --prefix against the working tree. This breaks when splitting an older commit or when the working tree does not contain the subtree, even though the commit does.

For example:
  git subtree split --prefix=pkg <commit>
fails if pkg was removed later, even though it exists in <commit>.

Fix this by validating the prefix against the specified commit using git ls-tree instead of the working tree.

Add a test to ensure this behavior does not regress.
Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
---
 contrib/subtree/git-subtree.sh     |  9 +++++++++
 contrib/subtree/t/t7900-subtree.sh | 22 ++++++++++++++++++++++
 2 files changed, 31 insertions(+)
Show changes to 2 files +31 −0

contrib/subtree/git-subtree.sh, contrib/subtree/t/t7900-subtree.sh

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 17106d1a72..324ed38148 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -257,6 +257,9 @@ main () {
 		test -e "$arg_prefix" &&
 			die "fatal: prefix '$arg_prefix' already exists."
 		;;
+	split)
+		# checked later against the commit, not the working tree
+		;;
 	*)
 		test -e "$arg_prefix" ||
 			die "fatal: '$arg_prefix' does not exist; use 'git subtree add'"
@@ -966,6 +969,12 @@ cmd_split () {
 	else
 		die "fatal: you must provide exactly one revision, and optionally a repository.  Got: '$*'"
 	fi
+
+	# Now validate prefix against the commit, not the working tree
+	if ! git ls-tree -d "$rev" -- "$dir" >/dev/null
+	then
+		die "fatal: '$dir' does not exist in commit $rev"
+	fi
 	repository=""
 	if test "$#" = 2
 	then
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 316dc5269e..e4f632f3af 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -368,6 +368,28 @@ test_expect_success 'split requires path given by option --prefix must exist' '
 	)
 '
 
+test_expect_success 'split works when prefix exists in commit but not in working tree' '
+	subtree_test_create_repo "$test_count" &&
+	(
+		cd "$test_count" &&
+
+		# create subtree
+		mkdir pkg &&
+		echo ok >pkg/file &&
+		git add pkg &&
+		git commit -m "add pkg" &&
+		good=$(git rev-parse HEAD) &&
+
+		# remove it from working tree in later commit
+		git rm -r pkg &&
+		git commit -m "remove pkg" &&
+
+		# must still be able to split using the old commit
+		git subtree split --prefix=pkg "$good" >out &&
+		test -s out
+	)
+'
+
 test_expect_success 'split rejects flags for add' '
 	subtree_test_create_repo "$test_count" &&
 	subtree_test_create_repo "$test_count/sub proj" &&
-- 
2.43.0
Josh Steadmon· Feb 2, 2026, 18:54 UTC · re: Pushkar Singh · lore

Re: [PATCH v3] subtree: validate --prefix against commit in split

On 2026.01.15 17:52, Pushkar Singh wrote:
Show 16 quoted lines
> git subtree split currently validates --prefix against the working tree.
> This breaks when splitting an older commit or when the working tree does
> not contain the subtree, even though the commit does.
> 
> For example:
> 
>   git subtree split --prefix=pkg <commit>
> 
> fails if pkg was removed later, even though it exists in <commit>.
> 
> Fix this by validating the prefix against the specified commit using
> git ls-tree instead of the working tree.
> 
> Add a test to ensure this behavior does not regress.
> 
> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>

Unfortunately, it seems this patch breaks the subtree tests. We noticed a failure in our build system at $WORK, and I was able to bisect the failure to this commit:

$ git bisect start 54b18a3513eed9ed5ced5c238ade55a434fd619a 66b2238f5c17644ddf15f75a53c76faeca6d9f1e $ git bisect run sh -c 'make && make -C contrib/subtree && make -C contrib/subtree test'

The tests fail on case 17 - split requires path given by option --prefix must exist.

Junio C Hamano· Feb 2, 2026, 19:10 UTC · re: Josh Steadmon · lore

Re: [PATCH v3] subtree: validate --prefix against commit in split

Josh Steadmon <steadmon@google.com> writes:
Show 9 quoted lines
> Unfortunately, it seems this patch breaks the subtree tests. We noticed
> a failure in our build system at $WORK, and I was able to bisect the
> failure to this commit:
>
> $ git bisect start 54b18a3513eed9ed5ced5c238ade55a434fd619a 66b2238f5c17644ddf15f75a53c76faeca6d9f1e
> $ git bisect run sh -c 'make && make -C contrib/subtree && make -C contrib/subtree test'
>
> The tests fail on case 17 - split requires path given by option --prefix
> must exist.

Thanks for a quick regression report while it is still in 'next', so that we can revert it out of 'next' without breaking the 'master'.

Junio C Hamano· Feb 2, 2026, 21:07 UTC · re: Josh Steadmon · lore

Re: [PATCH v3] subtree: validate --prefix against commit in split

Josh Steadmon <steadmon@google.com> writes:
Show 27 quoted lines
> On 2026.01.15 17:52, Pushkar Singh wrote:
>> git subtree split currently validates --prefix against the working tree.
>> This breaks when splitting an older commit or when the working tree does
>> not contain the subtree, even though the commit does.
>> 
>> For example:
>> 
>>   git subtree split --prefix=pkg <commit>
>> 
>> fails if pkg was removed later, even though it exists in <commit>.
>> 
>> Fix this by validating the prefix against the specified commit using
>> git ls-tree instead of the working tree.
>> 
>> Add a test to ensure this behavior does not regress.
>> 
>> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
>
> Unfortunately, it seems this patch breaks the subtree tests. We noticed
> a failure in our build system at $WORK, and I was able to bisect the
> failure to this commit:
>
> $ git bisect start 54b18a3513eed9ed5ced5c238ade55a434fd619a 66b2238f5c17644ddf15f75a53c76faeca6d9f1e
> $ git bisect run sh -c 'make && make -C contrib/subtree && make -C contrib/subtree test'
>
> The tests fail on case 17 - split requires path given by option --prefix
> must exist.

Thanks. I am tempted to propose us doing something like this, so that you guys do not have to every time you import my 'next'.

--- >8 ---
Subject: [PATCH] test: optionally test contrib in CI

Recently it was reported that a topic merged to 'next' broke build and test for contrib/subtree part of the system.

Instead of having those who run 'next' or 'master' to hit the build and test breakage and report to us, make sure we notice breakages in contrib/ area before they hit my tree at all, during their own presubmit testing.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * There should be a cleaner way to make sure any new Makefile with
   "test" target in contrib/* directores are added to the execution,
   but for now this should do.
 Makefile                  |  6 ++++++
 ci/run-build-and-tests.sh |  2 ++
 contrib/Makefile          | 10 ++++++++++
 3 files changed, 18 insertions(+)
 create mode 100644 contrib/Makefile
Show changes to 3 files +18 −0

Makefile, ci/run-build-and-tests.sh, contrib/Makefile

diff --git a/Makefile b/Makefile
index 8aa489f3b6..d0ab8fdb04 100644
--- a/Makefile
+++ b/Makefile
@@ -342,6 +342,9 @@ include shared.mak
 # If it isn't set, fallback to $LC_ALL, $LANG or use the first utf-8
 # locale returned by "locale -a".
 #
+# Define TEST_CONTRIB_TOO to make "make test" run tests in contrib/
+# directories.
+#
 # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime.
 #
 # Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC.
@@ -3369,6 +3372,9 @@ export TEST_NO_MALLOC_CHECK
 
 test: all
 	$(MAKE) -C t/ all
+ifdef TEST_CONTRIB_TOO
+	$(MAKE) -C contrib/ test
+endif
 
 perf: all
 	$(MAKE) -C t/perf/ all
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index 8bda62b921..b07b89f954 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -5,6 +5,8 @@
 
 . ${0%/*}/lib.sh
 
+export TEST_CONTRIB_TOO=yes
+
 case "$jobname" in
 fedora-breaking-changes-musl|linux-breaking-changes)
 	export WITH_BREAKING_CHANGES=YesPlease
diff --git a/contrib/Makefile b/contrib/Makefile
new file mode 100644
index 0000000000..787cd07f52
--- /dev/null
+++ b/contrib/Makefile
@@ -0,0 +1,10 @@
+all::
+
+test::
+	$(MAKE) -C diff-highlight $@
+	$(MAKE) -C subtree $@
+
+clean::
+	$(MAKE) -C contacts $@
+	$(MAKE) -C diff-highlight $@
+	$(MAKE) -C subtree $@
-- 
2.53.0-154-gaa371a4585
Junio C Hamano· Feb 3, 2026, 15:30 UTC · re: Junio C Hamano · lore

[RFH] adding test coverage for contrib/ in CI jobs

Junio C Hamano <gitster@pobox.com> writes:
Show 20 quoted lines
> Thanks.  I am tempted to propose us doing something like this, so
> that you guys do not have to every time you import my 'next'.
>
> --- >8 ---
> Subject: [PATCH] test: optionally test contrib in CI
>
> Recently it was reported that a topic merged to 'next' broke build
> and test for contrib/subtree part of the system.
>
> Instead of having those who run 'next' or 'master' to hit the build
> and test breakage and report to us, make sure we notice breakages in
> contrib/ area before they hit my tree at all, during their own
> presubmit testing.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
>  * There should be a cleaner way to make sure any new Makefile with
>    "test" target in contrib/* directores are added to the execution,
>    but for now this should do.

Unfortunately, this seems to reveal existing other problems with subtree tests (t7900), in addition to diff-highlight tests (t9400) in various configurations.

  https://github.com/git/git/actions/runs/21617099884

This Ci run is near the tip of 'seen', so there may be breakages attributable to new topics in flight, but I suspect that many of them are already in 'master', noticed by nobody because nobody ran these tests in these configurations (like "breaking changes", "sha256", "leaks", "reftable", "asan").

I didn't look into the details of any of these (yet).
Show 59 quoted lines
>  Makefile                  |  6 ++++++
>  ci/run-build-and-tests.sh |  2 ++
>  contrib/Makefile          | 10 ++++++++++
>  3 files changed, 18 insertions(+)
>  create mode 100644 contrib/Makefile
>
> diff --git a/Makefile b/Makefile
> index 8aa489f3b6..d0ab8fdb04 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -342,6 +342,9 @@ include shared.mak
>  # If it isn't set, fallback to $LC_ALL, $LANG or use the first utf-8
>  # locale returned by "locale -a".
>  #
> +# Define TEST_CONTRIB_TOO to make "make test" run tests in contrib/
> +# directories.
> +#
>  # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime.
>  #
>  # Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC.
> @@ -3369,6 +3372,9 @@ export TEST_NO_MALLOC_CHECK
>  
>  test: all
>  	$(MAKE) -C t/ all
> +ifdef TEST_CONTRIB_TOO
> +	$(MAKE) -C contrib/ test
> +endif
>  
>  perf: all
>  	$(MAKE) -C t/perf/ all
> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
> index 8bda62b921..b07b89f954 100755
> --- a/ci/run-build-and-tests.sh
> +++ b/ci/run-build-and-tests.sh
> @@ -5,6 +5,8 @@
>  
>  . ${0%/*}/lib.sh
>  
> +export TEST_CONTRIB_TOO=yes
> +
>  case "$jobname" in
>  fedora-breaking-changes-musl|linux-breaking-changes)
>  	export WITH_BREAKING_CHANGES=YesPlease
> diff --git a/contrib/Makefile b/contrib/Makefile
> new file mode 100644
> index 0000000000..787cd07f52
> --- /dev/null
> +++ b/contrib/Makefile
> @@ -0,0 +1,10 @@
> +all::
> +
> +test::
> +	$(MAKE) -C diff-highlight $@
> +	$(MAKE) -C subtree $@
> +
> +clean::
> +	$(MAKE) -C contacts $@
> +	$(MAKE) -C diff-highlight $@
> +	$(MAKE) -C subtree $@
Junio C Hamano· Feb 3, 2026, 17:06 UTC · re: Junio C Hamano · lore

Re* [RFH] adding test coverage for contrib/ in CI jobs

Junio C Hamano <gitster@pobox.com> writes:
Show 13 quoted lines
> Unfortunately, this seems to reveal existing other problems with
> subtree tests (t7900), in addition to diff-highlight tests (t9400)
> in various configurations.
>
>   https://github.com/git/git/actions/runs/21617099884
>
> This Ci run is near the tip of 'seen', so there may be breakages
> attributable to new topics in flight, but I suspect that many of
> them are already in 'master', noticed by nobody because nobody ran
> these tests in these configurations (like "breaking changes",
> "sha256", "leaks", "reftable", "asan").
>
> I didn't look into the details of any of these (yet).

I didn't look into CI failures but spotted an easy one by eyeballing. As we seem to be lacking a dedicated subsystem maintainer for this tool, I am CCing those who have touched this test file during the past 24 months, plus our resident reftable expert.

----- >8 -----
Subject: subtree: allow testing with reftable backend

"git subtree" (in contrib/) comes with its own test script, which has this line

    defaultBranch=$(sed "s,ref: refs/heads/,," "$test_count/.git/HEAD")

that assumes that you can read from .git/HEAD as a regular text file and you'd find a textual symref in reffiles backend.

Not necessarily.
    make && cd contrib/subtree && 
    GIT_TEST_DEFAULT_REF_FORMAT=reftable make test

fails due to this. Use "git symbolic-ref" instead to read the value of the symref.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/subtree/t/t7900-subtree.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Show changes to diff +2 −1
diff --git c/contrib/subtree/t/t7900-subtree.sh w/contrib/subtree/t/t7900-subtree.sh
index 316dc5269e..344956e72e 100755
--- c/contrib/subtree/t/t7900-subtree.sh
+++ w/contrib/subtree/t/t7900-subtree.sh
@@ -1597,7 +1597,8 @@ test_expect_success 'push split to subproj' '
 
 test_expect_success 'subtree descendant check' '
 	subtree_test_create_repo "$test_count" &&
-	defaultBranch=$(sed "s,ref: refs/heads/,," "$test_count/.git/HEAD") &&
+
+	defaultBranch=$(git -C "$test_count" symbolic-ref --short HEAD) &&
 	test_create_commit "$test_count" folder_subtree/a &&
 	(
 		cd "$test_count" &&
Junio C Hamano· Feb 3, 2026, 23:09 UTC · re: Junio C Hamano · lore

Re: Re* [RFH] adding test coverage for contrib/ in CI jobs

Junio C Hamano <gitster@pobox.com> writes:
Show 21 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Unfortunately, this seems to reveal existing other problems with
>> subtree tests (t7900), in addition to diff-highlight tests (t9400)
>> in various configurations.
>>
>>   https://github.com/git/git/actions/runs/21617099884
>>
>> This Ci run is near the tip of 'seen', so there may be breakages
>> attributable to new topics in flight, but I suspect that many of
>> them are already in 'master', noticed by nobody because nobody ran
>> these tests in these configurations (like "breaking changes",
>> "sha256", "leaks", "reftable", "asan").
>>
>> I didn't look into the details of any of these (yet).
>
> I didn't look into CI failures but spotted an easy one by
> eyeballing.  As we seem to be lacking a dedicated subsystem
> maintainer for this tool, I am CCing those who have touched this
> test file during the past 24 months, plus our resident reftable
> expert.

The subtree tests seems to be badly broken, so for now I've enabled the contrib tests at CI to only linux-TEST-vars job (which seems to be passing) and *-breaking-changes jobs.

Help by those who are more familiar with subtree is very much appreciated. Start by looking at failures in

    https://github.com/git/git/actions/runs/21649279837/job/62409376111
Thanks.
Colin Stagner· Feb 4, 2026, 04:38 UTC · re: Junio C Hamano · lore

Re* [RFH] adding test coverage for contrib/ in CI jobs

Junio C Hamano <gitster@pobox.com> writes:
> I didn't look into CI failures but spotted an easy one by
> eyeballing.
[snip]
Show 9 quoted lines
>--- c/contrib/subtree/t/t7900-subtree.sh
>+++ w/contrib/subtree/t/t7900-subtree.sh
>@@ -1597,7 +1597,8 @@ test_expect_success 'push split to subproj' '
> 
> test_expect_success 'subtree descendant check' '
> 	subtree_test_create_repo "$test_count" &&
>-	defaultBranch=$(sed "s,ref: refs/heads/,," "$test_count/.git/HEAD") &&
>+
>+	defaultBranch=$(git -C "$test_count" symbolic-ref --short HEAD) &&

The subtree tests set GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME, so this is one alternative.

I'll see what I can do about any remaining failures. I'd like to improve the test coverage for subtree split. split has a lot of complicated logic that needs to be preserved across updates.

-- >8 --
Subject: contrib/subtree: fix tests with reftable backend

One git-subtree test-case relies on git internals to infer the default branch name. This test fails with the new reftable backend.

    GIT_TEST_DEFAULT_REF_FORMAT=reftable \
      meson test t7900-subtree
This test script already sets
    GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main

which eliminates the need to infer a branch name at runtime. Hardcode the branch name.

Signed-off-by: Colin Stagner <ask+git@howdoi.land>
---
 contrib/subtree/t/t7900-subtree.sh | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
Show changes to contrib/subtree/t/t7900-subtree.sh +3 −4
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 316dc5269e..e7040718f2 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -1597,7 +1597,6 @@ test_expect_success 'push split to subproj' '
 
 test_expect_success 'subtree descendant check' '
 	subtree_test_create_repo "$test_count" &&
-	defaultBranch=$(sed "s,ref: refs/heads/,," "$test_count/.git/HEAD") &&
 	test_create_commit "$test_count" folder_subtree/a &&
 	(
 		cd "$test_count" &&
@@ -1614,7 +1613,7 @@ test_expect_success 'subtree descendant check' '
 	(
 		cd "$test_count" &&
 		git cherry-pick $cherry &&
-		git checkout $defaultBranch &&
+		git checkout main &&
 		git merge -m "merge should be kept on subtree" branch &&
 		git branch no_subtree_work_branch
 	) &&
@@ -1626,10 +1625,10 @@ test_expect_success 'subtree descendant check' '
 	test_create_commit "$test_count" not_a_subtree_change &&
 	(
 		cd "$test_count" &&
-		git checkout $defaultBranch &&
+		git checkout main &&
 		git merge -m "merge should be skipped on subtree" no_subtree_work_branch &&
 
-		git subtree split --prefix folder_subtree/ --branch subtree_tip $defaultBranch &&
+		git subtree split --prefix folder_subtree/ --branch subtree_tip main &&
 		git subtree split --prefix folder_subtree/ --branch subtree_branch branch &&
 		test $(git rev-list --count subtree_tip..subtree_branch) = 0
 	)
-- 
2.43.0
Junio C Hamano· Feb 4, 2026, 19:55 UTC · re: Colin Stagner · lore

Re: Re* [RFH] adding test coverage for contrib/ in CI jobs

Colin Stagner <ask+git@howdoi.land> writes:
> I'll see what I can do about any remaining failures. I'd like to
> improve the test coverage for subtree split. split has a lot of
> complicated logic that needs to be preserved across updates.
Thanks.
Show 15 quoted lines
> Subject: contrib/subtree: fix tests with reftable backend
>
> One git-subtree test-case relies on git internals to infer the
> default branch name. This test fails with the new reftable
> backend.
>
>     GIT_TEST_DEFAULT_REF_FORMAT=reftable \
>       meson test t7900-subtree
>
> This test script already sets
>
>     GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>
> which eliminates the need to infer a branch name at runtime.
> Hardcode the branch name.
Makes sense.  

I didn't read the test script carefully enough to be certain that we were on the initial branch when the defaultBranch computation happened.

Thanks.  Will replace my hack with this version.
Show 39 quoted lines
> Signed-off-by: Colin Stagner <ask+git@howdoi.land>
> ---
>  contrib/subtree/t/t7900-subtree.sh | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
> index 316dc5269e..e7040718f2 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -1597,7 +1597,6 @@ test_expect_success 'push split to subproj' '
>  
>  test_expect_success 'subtree descendant check' '
>  	subtree_test_create_repo "$test_count" &&
> -	defaultBranch=$(sed "s,ref: refs/heads/,," "$test_count/.git/HEAD") &&
>  	test_create_commit "$test_count" folder_subtree/a &&
>  	(
>  		cd "$test_count" &&
> @@ -1614,7 +1613,7 @@ test_expect_success 'subtree descendant check' '
>  	(
>  		cd "$test_count" &&
>  		git cherry-pick $cherry &&
> -		git checkout $defaultBranch &&
> +		git checkout main &&
>  		git merge -m "merge should be kept on subtree" branch &&
>  		git branch no_subtree_work_branch
>  	) &&
> @@ -1626,10 +1625,10 @@ test_expect_success 'subtree descendant check' '
>  	test_create_commit "$test_count" not_a_subtree_change &&
>  	(
>  		cd "$test_count" &&
> -		git checkout $defaultBranch &&
> +		git checkout main &&
>  		git merge -m "merge should be skipped on subtree" no_subtree_work_branch &&
>  
> -		git subtree split --prefix folder_subtree/ --branch subtree_tip $defaultBranch &&
> +		git subtree split --prefix folder_subtree/ --branch subtree_tip main &&
>  		git subtree split --prefix folder_subtree/ --branch subtree_branch branch &&
>  		test $(git rev-list --count subtree_tip..subtree_branch) = 0
>  	)
Colin Stagner· Feb 5, 2026, 06:05 UTC · re: Junio C Hamano · lore

Re: Re* [RFH] adding test coverage for contrib/ in CI jobs

Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
> Unfortunately, this seems to reveal existing other problems with
> subtree tests (t7900), in addition to diff-highlight tests (t9400)
> in various configurations.
> 
> https://github.com/git/git/actions/runs/21617099884
> 
> This Ci run is near the tip of 'seen', so there may be breakages
> attributable to new topics in flight
At least some of the subtree failures on linux-reftable, such as
<https://github.com/git/git/actions/runs/21617099884/job/62298228602#step:10:421>

are actually due to a bug in ubuntu:rolling's "dirname" implementation. This was fixed upstream in late January [1].

bug behavior demo:
     podman run --rm -it -q docker.io/library/ubuntu:questing-20251217 \
       dirname whatever/.
outputs "whatever"
     podman run --rm -it -q docker.io/library/ubuntu:25.10 \
       dirname whatever/.
outputs "."
subtree has an up-front call to
     dir="$(dirname "$arg_prefix/.")"
which encounters the defect. It then attempts to do things like
     git read-tree --prefix=. $someref
that error out with messages like "error: invalid path './sub1'"
[1]: https://github.com/uutils/coreutils/issues/10508
Junio C Hamano· Feb 5, 2026, 16:39 UTC · re: Colin Stagner · lore

Re: Re* [RFH] adding test coverage for contrib/ in CI jobs

Colin Stagner <ask+git@howdoi.land> writes:
Show 16 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Unfortunately, this seems to reveal existing other problems with
>> subtree tests (t7900), in addition to diff-highlight tests (t9400)
>> in various configurations.
>> 
>> https://github.com/git/git/actions/runs/21617099884
>> 
>> This Ci run is near the tip of 'seen', so there may be breakages
>> attributable to new topics in flight
>
> At least some of the subtree failures on linux-reftable, such as
>
> <https://github.com/git/git/actions/runs/21617099884/job/62298228602#step:10:421>
>
> are actually due to a bug in ubuntu:rolling's "dirname" implementation. 
Ahh, rust bites again?

Is there /etc/alternatives/dirname like /etc/alternatives/sudo that we used in fddb4842 (ci: fix broken jobs on Ubuntu 25.10 caused by switch to sudo-rs(1), 2025-10-10) to work a breakage around, I have to wonder...

Junio C Hamano· Feb 5, 2026, 20:54 UTC · re: Junio C Hamano · lore

Re: Re* [RFH] adding test coverage for contrib/ in CI jobs

Junio C Hamano <gitster@pobox.com> writes:
Show 14 quoted lines
> Colin Stagner <ask+git@howdoi.land> writes:
> ...
>> At least some of the subtree failures on linux-reftable, such as
>>
>> <https://github.com/git/git/actions/runs/21617099884/job/62298228602#step:10:421>
>>
>> are actually due to a bug in ubuntu:rolling's "dirname" implementation. 
>
> Ahh, rust bites again?
>
> Is there /etc/alternatives/dirname like /etc/alternatives/sudo that
> we used in fddb4842 (ci: fix broken jobs on Ubuntu 25.10 caused by
> switch to sudo-rs(1), 2025-10-10) to work a breakage around, I have
> to wonder...
So I tried a custom CI run that has
 * a temporary patch that demotes ubuntu:rolling to ubuntu:latest in
   GitHub workflows
 * your subtree test fix to use the hardcoded 'main'
 * a similar fix for diff-highlight test <xmqq7bstsemv.fsf@gitster.g>
 * a change to run "make test" in contrib/* directories <xmqqjywuyhu9.fsf@gitster.g>
on top of 'master'.  Everything seems to be happy.      
https://github.com/git/git/actions/runs/21726017981
Junio C Hamano· Feb 3, 2026, 21:26 UTC · re: Junio C Hamano · lore

Re: [RFH] adding test coverage for contrib/ in CI jobs

Junio C Hamano <gitster@pobox.com> writes:
Show 13 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> ...
> Unfortunately, this seems to reveal existing other problems with
> subtree tests (t7900), in addition to diff-highlight tests (t9400)
> in various configurations.
>
>   https://github.com/git/git/actions/runs/21617099884
>
> This CI run is near the tip of 'seen', so there may be breakages
> attributable to new topics in flight, but I suspect that many of
> them are already in 'master', noticed by nobody because nobody ran
> these tests in these configurations (like "breaking changes",
> "sha256", "leaks", "reftable", "asan").

Test that comes with diff-highlight fails WITH_BREAKING_CHANGES CI job, which has multiple ways to work around. The easiest one is to force the branch name that is documented in the comment part of the test file that illustrates the topology of the history, which is what I picked.

----- >8 -----
Subject: diff-highlight: allow testing with Git 3.0 breaking changes

The diff-highlight (in contrib/) comes with its own test script, which relies on the initial branch name being 'master'. This is not just encoded in the test logic, but in the illustration in the file that shows the topology of the history.

Force the initial branch name to 'master' to allow it pass.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 2 ++
 1 file changed, 2 insertions(+)
Show changes to diff +2 −0
diff --git c/contrib/diff-highlight/t/t9400-diff-highlight.sh w/contrib/diff-highlight/t/t9400-diff-highlight.sh
index f6f5195d00..dee296739c 100755
--- c/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ w/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -10,6 +10,8 @@ DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight
 CW="$(printf "\033[7m")"	# white
 CR="$(printf "\033[27m")"	# reset
 
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . "$TEST_DIRECTORY"/test-lib.sh
 
 if ! test_have_prereq PERL
Jeff King· Feb 3, 2026, 21:53 UTC · re: Junio C Hamano · lore

Re: [RFH] adding test coverage for contrib/ in CI jobs

On Tue, Feb 03, 2026 at 01:26:00PM -0800, Junio C Hamano wrote:
Show 11 quoted lines
> ----- >8 -----
> Subject: diff-highlight: allow testing with Git 3.0 breaking changes
> 
> The diff-highlight (in contrib/) comes with its own test script,
> which relies on the initial branch name being 'master'.  This is not
> just encoded in the test logic, but in the illustration in the file
> that shows the topology of the history.
> 
> Force the initial branch name to 'master' to allow it pass.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Thanks, I think this is a fine solution. In such cases it is sometimes nice to remove the dependence on the branch name entirely. But it looks like it would be a pain to do so in this case, and not worth the time.

Most of the other instances of GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME set it to "main". I guess one day in the future, post v3.0, we might drop all of those and decide that "main" is here to stay. In which case we might also want to drop these outliers and just switch them to "main", too. But I am content to punt that off to another day.

-Peff
PS As you might have guessed, I have not run these tests in ages. I'd
   only do so when actually changing something in diff-highlight, and
   that hasn't happened in a while. In fact, I rarely run it at all
   these days; I usually use the third-party "delta" program in its
   "--color-only" mode, as it does a better job of true intra-line
   tokenization and diffing.
   I don't know what that means for diff-highlight. I'm happy to
   continue to review patches for it, and I think it mostly Just Works
   and doesn't need active maintenance. But I'm also OK if we dropped
   it.
Pushkar Singh· Feb 3, 2026, 16:48 UTC · re: Pushkar Singh · lore

[PATCH v4] subtree: validate --prefix against commit in split

git subtree split currently validates --prefix against the working tree. This breaks when splitting an older commit or when the working tree does not contain the subtree, even though the commit does.

For example:
  git subtree split --prefix=pkg <commit>
fails if pkg was removed later, even though it exists in <commit>.

Fix this by validating the prefix against the specified commit using git cat-file instead of the working tree.

Add a test to ensure this behavior does not regress.
Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
---
Changes since v3:
- Fix regression in existing subtree tests by checking prefix existence
  with git cat-file instead of git ls-tree -d
- Preserve original error message to keep test 17 passing
 contrib/subtree/git-subtree.sh     |  9 +++++++++
 contrib/subtree/t/t7900-subtree.sh | 22 ++++++++++++++++++++++
 2 files changed, 31 insertions(+)
Show changes to 2 files +31 −0

contrib/subtree/git-subtree.sh, contrib/subtree/t/t7900-subtree.sh

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 17106d1a72..d7f9121f2f 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -257,6 +257,9 @@ main () {
 		test -e "$arg_prefix" &&
 			die "fatal: prefix '$arg_prefix' already exists."
 		;;
+	split)
+		# checked later against the commit, not the working tree
+		;;
 	*)
 		test -e "$arg_prefix" ||
 			die "fatal: '$arg_prefix' does not exist; use 'git subtree add'"
@@ -966,6 +969,12 @@ cmd_split () {
 	else
 		die "fatal: you must provide exactly one revision, and optionally a repository.  Got: '$*'"
 	fi
+
+	# Now validate prefix against the commit, not the working tree
+	if ! git cat-file -e "$rev:$dir" 2>/dev/null
+	then
+		die "fatal: '$dir' does not exist; use 'git subtree add'"
+	fi
 	repository=""
 	if test "$#" = 2
 	then
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 316dc5269e..e4f632f3af 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -368,6 +368,28 @@ test_expect_success 'split requires path given by option --prefix must exist' '
 	)
 '
 
+test_expect_success 'split works when prefix exists in commit but not in working tree' '
+	subtree_test_create_repo "$test_count" &&
+	(
+		cd "$test_count" &&
+
+		# create subtree
+		mkdir pkg &&
+		echo ok >pkg/file &&
+		git add pkg &&
+		git commit -m "add pkg" &&
+		good=$(git rev-parse HEAD) &&
+
+		# remove it from working tree in later commit
+		git rm -r pkg &&
+		git commit -m "remove pkg" &&
+
+		# must still be able to split using the old commit
+		git subtree split --prefix=pkg "$good" >out &&
+		test -s out
+	)
+'
+
 test_expect_success 'split rejects flags for add' '
 	subtree_test_create_repo "$test_count" &&
 	subtree_test_create_repo "$test_count/sub proj" &&
-- 
2.43.0
Junio C Hamano· Feb 3, 2026, 17:37 UTC · re: Pushkar Singh · lore

Re: [PATCH v4] subtree: validate --prefix against commit in split

Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:
> Changes since v3:
> - Fix regression in existing subtree tests by checking prefix existence
>   with git cat-file instead of git ls-tree -d
OK, the check used to run
    git ls-tree -d "$rev" -- "$dir"

which (if I am reading this correctly) succeeded only when "$dir" is an existing directory in "$rev". Now you use

    git cat-file -e "$rev:$dir" 2>/dev/null
and this allows "$dir" to be a non-directory but say a blob.
> - Preserve original error message to keep test 17 passing

Ahh, OK. I didn't check how the test was failing. This message is made to look like the error message we used to get much earlier before calling cmd_split, and now cmd_split detects the condition to give the error message, so it is better to match it. Makes sense.

Show 64 quoted lines
>  contrib/subtree/git-subtree.sh     |  9 +++++++++
>  contrib/subtree/t/t7900-subtree.sh | 22 ++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 17106d1a72..d7f9121f2f 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -257,6 +257,9 @@ main () {
>  		test -e "$arg_prefix" &&
>  			die "fatal: prefix '$arg_prefix' already exists."
>  		;;
> +	split)
> +		# checked later against the commit, not the working tree
> +		;;
>  	*)
>  		test -e "$arg_prefix" ||
>  			die "fatal: '$arg_prefix' does not exist; use 'git subtree add'"
> @@ -966,6 +969,12 @@ cmd_split () {
>  	else
>  		die "fatal: you must provide exactly one revision, and optionally a repository.  Got: '$*'"
>  	fi
> +
> +	# Now validate prefix against the commit, not the working tree
> +	if ! git cat-file -e "$rev:$dir" 2>/dev/null
> +	then
> +		die "fatal: '$dir' does not exist; use 'git subtree add'"
> +	fi
>  	repository=""
>  	if test "$#" = 2
>  	then
> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
> index 316dc5269e..e4f632f3af 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -368,6 +368,28 @@ test_expect_success 'split requires path given by option --prefix must exist' '
>  	)
>  '
>  
> +test_expect_success 'split works when prefix exists in commit but not in working tree' '
> +	subtree_test_create_repo "$test_count" &&
> +	(
> +		cd "$test_count" &&
> +
> +		# create subtree
> +		mkdir pkg &&
> +		echo ok >pkg/file &&
> +		git add pkg &&
> +		git commit -m "add pkg" &&
> +		good=$(git rev-parse HEAD) &&
> +
> +		# remove it from working tree in later commit
> +		git rm -r pkg &&
> +		git commit -m "remove pkg" &&
> +
> +		# must still be able to split using the old commit
> +		git subtree split --prefix=pkg "$good" >out &&
> +		test -s out
> +	)
> +'
> +
>  test_expect_success 'split rejects flags for add' '
>  	subtree_test_create_repo "$test_count" &&
>  	subtree_test_create_repo "$test_count/sub proj" &&

← back to recent threads