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

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

[Pushkar Singh]

---
 contrib/subtree/git-subtree.sh | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

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

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

[Pushkar Singh]

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

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

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

[Junio C Hamano]

Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:

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

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

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

[Pushkar Singh]

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

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

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

[Josh Steadmon]

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.

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

[Junio C Hamano]

Josh Steadmon <steadmon@google.com> writes:

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

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

[Junio C Hamano]

Josh Steadmon <steadmon@google.com> writes:

> 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

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

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

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



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

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

[Junio C Hamano]

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.

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

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

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

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

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

[Colin Stagner]

Junio C Hamano <gitster@pobox.com> writes:

> I didn't look into CI failures but spotted an easy one by
> eyeballing.

[snip]

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

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

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

[Junio C Hamano]

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.


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

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

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

[Colin Stagner]

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

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

[Junio C Hamano]

Colin Stagner <ask+git@howdoi.land> writes:

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

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> 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

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

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

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

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

[Jeff King]

On Tue, Feb 03, 2026 at 01:26:00PM -0800, Junio C Hamano wrote:

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

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

[Pushkar Singh]

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

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

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

[Junio C Hamano]

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.

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