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

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

[Pushkar Singh]

Changes in v4:
 * fix indentation in the case statement
 * add a regression test to ensure --prefix is validated against the
   target commit rather than the working tree

No functional changes beyond the above.

Pushkar Singh (1):
  subtree: validate --prefix against commit in split

 contrib/subtree/git-subtree.sh     |  9 +++++++++
 contrib/subtree/t/t7900-subtree.sh | 22 ++++++++++++++++++++++
 2 files changed, 31 insertions(+)

-- 
2.43.0

[PATCH v4 1/1] 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.

Address review feedback by fixing indentation and adding regression test.

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 v4 0/1] subtree: validate --prefix against commit in split

[Junio C Hamano]

Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:

> Changes in v4:
>  * fix indentation in the case statement
>  * add a regression test to ensure --prefix is validated against the
>    target commit rather than the working tree
>
> No functional changes beyond the above.

This seems to be identical to v3, which already had the fix to the
case and addition of tests.

https://lore.kernel.org/git/20260115175403.3971-3-pushkarkumarsingh1970@gmail.com/

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

[Pushkar Singh]

You're right, thanks for pointing that out.

On rechecking, v3 already included both the indentation fix and the
regression test, so v4 did not introduce any substantive change.

Sorry for the unnecessary reroll. I'll make sure to only resend when
there is a meaningful difference.

Thanks.