From 455f0adf5709e24712bff725005ff1a59508a054 Mon Sep 17 00:00:00 2001
From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Fri, 21 Oct 2022 15:13:31 +0000
Subject: [PATCH 1/9] test-lib-functions: mark 'test_commit' variables as
 'local'

Some variables in 'test_commit' have names that are common enough that
it is very likely that test authors might use them in a test. If they do
so and use 'test_commit' between setting such a variable and using it,
the variable value from 'test_commit' will leak back into the test and
most likely break it.

Prevent that by marking all variables in 'test_commit' as 'local'. This
allow a subsequent commit to use a 'tag' variable.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/test-lib-functions.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 527a714500..adc0fb6330 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -273,13 +273,13 @@ debug () {
 # <file>, <contents>, and <tag> all default to <message>.
 
 test_commit () {
-	notick= &&
-	echo=echo &&
-	append= &&
-	author= &&
-	signoff= &&
-	indir= &&
-	tag=light &&
+	local notick= &&
+	local echo=echo &&
+	local append= &&
+	local author= &&
+	local signoff= &&
+	local indir= &&
+	local tag=light &&
 	while test $# != 0
 	do
 		case "$1" in
@@ -322,7 +322,7 @@ test_commit () {
 		shift
 	done &&
 	indir=${indir:+"$indir"/} &&
-	file=${2:-"$1.t"} &&
+	local file=${2:-"$1.t"} &&
 	if test -n "$append"
 	then
 		$echo "${3-$1}" >>"$indir$file"

From a50fcc13dda245b39e63a2052107456e0b7e85f7 Mon Sep 17 00:00:00 2001
From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Fri, 21 Oct 2022 15:13:32 +0000
Subject: [PATCH 2/9] subtree: use 'git rev-parse --verify [--quiet]' for
 better error messages

There are three occurences of 'git rev-parse <rev>' in 'git-subtree.sh'
where the command expects a revision and the script dies or exits if the
revision can't be found. In that case, the error message from 'git
rev-parse' is:

    $ git rev-parse <bad rev>
    <bad rev>
    fatal: ambiguous argument '<bad rev>': unknown revision or path not in the working tree.
    Use '--' to separate paths from revisions, like this:
    'git <command> [<revision>...] -- [<file>...]'

This is a little confusing to the user, since this error message is
outputed by 'git subtree'.

At these points in the script, we know that we are looking for a single
revision, so be explicit by using '--verify', resulting in a little
better error message:

    $ git rev-parse --verify <bad rev>
    fatal: Needed a single revision

In the two occurences where we 'die' if 'git rev-parse' fails, 'git
subtree' outputs "could not rev-parse split hash $b from commit $sq", so
we actually do not need the supplementary error message from 'git
rev-parse'; add '--quiet' to silence it.

In the third occurence, we 'exit', so keep the error message from 'git
rev-parse'. Note that this messsage is still suboptimal since it can be
understood to mean that 'git rev-parse' did not receive a single
revision as argument, which is not the case here: the command did
receive a single revision, but the revision is not resolvable to an
available object.

The alternative would be to use '--' after the revision, as suggested by
the first error message, resulting in a clearer error message:

    $ git rev-parse <bad rev> --
    fatal: bad revision '<bad rev>'

Unfortunately we can't use that syntax because in the more common case
of the revision resolving to a known object, the command outputs the
object's hash, a newline, and the dashdash, which breaks the 'git
subtree' script.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/subtree/git-subtree.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7562a395c2..49ef493ef9 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -387,7 +387,7 @@ find_latest_squash () {
 			main="$b"
 			;;
 		git-subtree-split:)
-			sub="$(git rev-parse "$b^{commit}")" ||
+			sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
 			die "could not rev-parse split hash $b from commit $sq"
 			;;
 		END)
@@ -439,7 +439,7 @@ find_existing_splits () {
 			main="$b"
 			;;
 		git-subtree-split:)
-			sub="$(git rev-parse "$b^{commit}")" ||
+			sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
 			die "could not rev-parse split hash $b from commit $sq"
 			;;
 		END)
@@ -843,7 +843,7 @@ cmd_add_commit () {
 	git checkout -- "$dir" || exit $?
 	tree=$(git write-tree) || exit $?
 
-	headrev=$(git rev-parse HEAD) || exit $?
+	headrev=$(git rev-parse --verify HEAD) || exit $?
 	if test -n "$headrev" && test "$headrev" != "$rev"
 	then
 		headp="-p $headrev"

From 2e94339fdc3122fd2f92e865105523bc3866a65f Mon Sep 17 00:00:00 2001
From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Fri, 21 Oct 2022 15:13:33 +0000
Subject: [PATCH 3/9] subtree: add 'die_incompatible_opt' function to reduce
 duplication

9a3e3ca2ba (subtree: be stricter about validating flags, 2021-04-27)
added validation code to check that options given to 'git subtree <cmd>'
made sense with the command being used.

Refactor these checks by adding a 'die_incompatible_opt' function to
reduce code duplication.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/subtree/git-subtree.sh | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 49ef493ef9..f5eab198c8 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -102,6 +102,14 @@ assert () {
 	fi
 }
 
+# Usage: die_incompatible_opt OPTION COMMAND
+die_incompatible_opt () {
+	assert test "$#" = 2
+	opt="$1"
+	arg_command="$2"
+	die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+}
+
 main () {
 	if test $# -eq 0
 	then
@@ -176,16 +184,16 @@ main () {
 			arg_debug=1
 			;;
 		--annotate)
-			test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
 			arg_split_annotate="$1"
 			shift
 			;;
 		--no-annotate)
-			test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
 			arg_split_annotate=
 			;;
 		-b)
-			test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
 			arg_split_branch="$1"
 			shift
 			;;
@@ -194,7 +202,7 @@ main () {
 			shift
 			;;
 		-m)
-			test -n "$allow_addmerge" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_addmerge" || die_incompatible_opt "$opt" "$arg_command"
 			arg_addmerge_message="$1"
 			shift
 			;;
@@ -202,34 +210,34 @@ main () {
 			arg_prefix=
 			;;
 		--onto)
-			test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
 			arg_split_onto="$1"
 			shift
 			;;
 		--no-onto)
-			test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
 			arg_split_onto=
 			;;
 		--rejoin)
-			test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
 			;;
 		--no-rejoin)
-			test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
 			;;
 		--ignore-joins)
-			test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
 			arg_split_ignore_joins=1
 			;;
 		--no-ignore-joins)
-			test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_split" || die_incompatible_opt "$opt" "$arg_command"
 			arg_split_ignore_joins=
 			;;
 		--squash)
-			test -n "$allow_addmerge" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_addmerge" || die_incompatible_opt "$opt" "$arg_command"
 			arg_addmerge_squash=1
 			;;
 		--no-squash)
-			test -n "$allow_addmerge" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+			test -n "$allow_addmerge" || die_incompatible_opt "$opt" "$arg_command"
 			arg_addmerge_squash=
 			;;
 		--)

From 5626a9e2a93e4dd0ce13005a7fc1d74c2c3e4fd2 Mon Sep 17 00:00:00 2001
From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Fri, 21 Oct 2022 15:13:34 +0000
Subject: [PATCH 4/9] subtree: prefix die messages with 'fatal'

Just as was done in 0008d12284 (submodule: prefix die messages with
'fatal', 2021-07-10) for 'git-submodule.sh', make the 'die' messages
outputed by 'git-subtree.sh' more in line with the rest of the code base
by prefixing them with "fatal: ", and do not capitalize their first
letter.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/subtree/git-subtree.sh     | 60 +++++++++++++++---------------
 contrib/subtree/t/t7900-subtree.sh | 12 +++---
 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index f5eab198c8..89f1eb756f 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -98,7 +98,7 @@ progress () {
 assert () {
 	if ! "$@"
 	then
-		die "assertion failed: $*"
+		die "fatal: assertion failed: $*"
 	fi
 }
 
@@ -107,7 +107,7 @@ die_incompatible_opt () {
 	assert test "$#" = 2
 	opt="$1"
 	arg_command="$2"
-	die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
+	die "fatal: the '$opt' flag does not make sense with 'git subtree $arg_command'."
 }
 
 main () {
@@ -155,7 +155,7 @@ main () {
 		allow_addmerge=$arg_split_rejoin
 		;;
 	*)
-		die "Unknown command '$arg_command'"
+		die "fatal: unknown command '$arg_command'"
 		;;
 	esac
 	# Reset the arguments array for "real" flag parsing.
@@ -244,7 +244,7 @@ main () {
 			break
 			;;
 		*)
-			die "Unexpected option: $opt"
+			die "fatal: unexpected option: $opt"
 			;;
 		esac
 	done
@@ -252,17 +252,17 @@ main () {
 
 	if test -z "$arg_prefix"
 	then
-		die "You must provide the --prefix option."
+		die "fatal: you must provide the --prefix option."
 	fi
 
 	case "$arg_command" in
 	add)
 		test -e "$arg_prefix" &&
-			die "prefix '$arg_prefix' already exists."
+			die "fatal: prefix '$arg_prefix' already exists."
 		;;
 	*)
 		test -e "$arg_prefix" ||
-			die "'$arg_prefix' does not exist; use 'git subtree add'"
+			die "fatal: '$arg_prefix' does not exist; use 'git subtree add'"
 		;;
 	esac
 
@@ -282,11 +282,11 @@ cache_setup () {
 	assert test $# = 0
 	cachedir="$GIT_DIR/subtree-cache/$$"
 	rm -rf "$cachedir" ||
-		die "Can't delete old cachedir: $cachedir"
+		die "fatal: can't delete old cachedir: $cachedir"
 	mkdir -p "$cachedir" ||
-		die "Can't create new cachedir: $cachedir"
+		die "fatal: can't create new cachedir: $cachedir"
 	mkdir -p "$cachedir/notree" ||
-		die "Can't create new cachedir: $cachedir/notree"
+		die "fatal: can't create new cachedir: $cachedir/notree"
 	debug "Using cachedir: $cachedir" >&2
 }
 
@@ -342,7 +342,7 @@ cache_set () {
 		test "$oldrev" != "latest_new" &&
 		test -e "$cachedir/$oldrev"
 	then
-		die "cache for $oldrev already exists!"
+		die "fatal: cache for $oldrev already exists!"
 	fi
 	echo "$newrev" >"$cachedir/$oldrev"
 }
@@ -396,7 +396,7 @@ find_latest_squash () {
 			;;
 		git-subtree-split:)
 			sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
-			die "could not rev-parse split hash $b from commit $sq"
+			die "fatal: could not rev-parse split hash $b from commit $sq"
 			;;
 		END)
 			if test -n "$sub"
@@ -448,7 +448,7 @@ find_existing_splits () {
 			;;
 		git-subtree-split:)
 			sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
-			die "could not rev-parse split hash $b from commit $sq"
+			die "fatal: could not rev-parse split hash $b from commit $sq"
 			;;
 		END)
 			debug "Main is: '$main'"
@@ -498,7 +498,7 @@ copy_commit () {
 			cat
 		) |
 		git commit-tree "$2" $3  # reads the rest of stdin
-	) || die "Can't copy commit $1"
+	) || die "fatal: can't copy commit $1"
 }
 
 # Usage: add_msg DIR LATEST_OLD LATEST_NEW
@@ -726,11 +726,11 @@ ensure_clean () {
 	assert test $# = 0
 	if ! git diff-index HEAD --exit-code --quiet 2>&1
 	then
-		die "Working tree has modifications.  Cannot add."
+		die "fatal: working tree has modifications.  Cannot add."
 	fi
 	if ! git diff-index --cached HEAD --exit-code --quiet 2>&1
 	then
-		die "Index has modifications.  Cannot add."
+		die "fatal: index has modifications.  Cannot add."
 	fi
 }
 
@@ -738,7 +738,7 @@ ensure_clean () {
 ensure_valid_ref_format () {
 	assert test $# = 1
 	git check-ref-format "refs/heads/$1" ||
-		die "'$1' does not look like a ref"
+		die "fatal: '$1' does not look like a ref"
 }
 
 # Usage: process_split_commit REV PARENTS
@@ -804,7 +804,7 @@ cmd_add () {
 	if test $# -eq 1
 	then
 		git rev-parse -q --verify "$1^{commit}" >/dev/null ||
-			die "'$1' does not refer to a commit"
+			die "fatal: '$1' does not refer to a commit"
 
 		cmd_add_commit "$@"
 
@@ -819,7 +819,7 @@ cmd_add () {
 
 		cmd_add_repository "$@"
 	else
-		say >&2 "error: parameters were '$*'"
+		say >&2 "fatal: parameters were '$*'"
 		die "Provide either a commit or a repository and commit."
 	fi
 }
@@ -882,9 +882,9 @@ cmd_split () {
 	elif test $# -eq 1
 	then
 		rev=$(git rev-parse -q --verify "$1^{commit}") ||
-			die "'$1' does not refer to a commit"
+			die "fatal: '$1' does not refer to a commit"
 	else
-		die "You must provide exactly one revision.  Got: '$*'"
+		die "fatal: you must provide exactly one revision.  Got: '$*'"
 	fi
 
 	if test -n "$arg_split_rejoin"
@@ -927,7 +927,7 @@ cmd_split () {
 	latest_new=$(cache_get latest_new) || exit $?
 	if test -z "$latest_new"
 	then
-		die "No new revisions were found"
+		die "fatal: no new revisions were found"
 	fi
 
 	if test -n "$arg_split_rejoin"
@@ -948,7 +948,7 @@ cmd_split () {
 		then
 			if ! git merge-base --is-ancestor "$arg_split_branch" "$latest_new"
 			then
-				die "Branch '$arg_split_branch' is not an ancestor of commit '$latest_new'."
+				die "fatal: branch '$arg_split_branch' is not an ancestor of commit '$latest_new'."
 			fi
 			action='Updated'
 		else
@@ -965,9 +965,9 @@ cmd_split () {
 # Usage: cmd_merge REV
 cmd_merge () {
 	test $# -eq 1 ||
-		die "You must provide exactly one revision.  Got: '$*'"
+		die "fatal: you must provide exactly one revision.  Got: '$*'"
 	rev=$(git rev-parse -q --verify "$1^{commit}") ||
-		die "'$1' does not refer to a commit"
+		die "fatal: '$1' does not refer to a commit"
 	ensure_clean
 
 	if test -n "$arg_addmerge_squash"
@@ -975,7 +975,7 @@ cmd_merge () {
 		first_split="$(find_latest_squash "$dir")" || exit $?
 		if test -z "$first_split"
 		then
-			die "Can't squash-merge: '$dir' was never added."
+			die "fatal: can't squash-merge: '$dir' was never added."
 		fi
 		set $first_split
 		old=$1
@@ -1003,7 +1003,7 @@ cmd_merge () {
 cmd_pull () {
 	if test $# -ne 2
 	then
-		die "You must provide <repository> <ref>"
+		die "fatal: you must provide <repository> <ref>"
 	fi
 	ensure_clean
 	ensure_valid_ref_format "$2"
@@ -1015,7 +1015,7 @@ cmd_pull () {
 cmd_push () {
 	if test $# -ne 2
 	then
-		die "You must provide <repository> <refspec>"
+		die "fatal: you must provide <repository> <refspec>"
 	fi
 	if test -e "$dir"
 	then
@@ -1030,13 +1030,13 @@ cmd_push () {
 		fi
 		ensure_valid_ref_format "$remoteref"
 		localrev_presplit=$(git rev-parse -q --verify "$localrevname_presplit^{commit}") ||
-			die "'$localrevname_presplit' does not refer to a commit"
+			die "fatal: '$localrevname_presplit' does not refer to a commit"
 
 		echo "git push using: " "$repository" "$refspec"
 		localrev=$(cmd_split "$localrev_presplit") || die
 		git push "$repository" "$localrev":"refs/heads/$remoteref"
 	else
-		die "'$dir' must already exist. Try 'git subtree add'."
+		die "fatal: '$dir' must already exist. Try 'git subtree add'."
 	fi
 }
 
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 1c1f76f04a..249743ab9a 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -277,7 +277,7 @@ test_expect_success 'split requires option --prefix' '
 		cd "$test_count" &&
 		git fetch ./"sub proj" HEAD &&
 		git subtree add --prefix="sub dir" FETCH_HEAD &&
-		echo "You must provide the --prefix option." >expected &&
+		echo "fatal: you must provide the --prefix option." >expected &&
 		test_must_fail git subtree split >actual 2>&1 &&
 		test_debug "printf '"expected: "'" &&
 		test_debug "cat expected" &&
@@ -296,7 +296,7 @@ test_expect_success 'split requires path given by option --prefix must exist' '
 		cd "$test_count" &&
 		git fetch ./"sub proj" HEAD &&
 		git subtree add --prefix="sub dir" FETCH_HEAD &&
-		echo "'\''non-existent-directory'\'' does not exist; use '\''git subtree add'\''" >expected &&
+		echo "fatal: '\''non-existent-directory'\'' does not exist; use '\''git subtree add'\''" >expected &&
 		test_must_fail git subtree split --prefix=non-existent-directory >actual 2>&1 &&
 		test_debug "printf '"expected: "'" &&
 		test_debug "cat expected" &&
@@ -570,7 +570,7 @@ test_expect_success 'pull requires option --prefix' '
 		cd "$test_count" &&
 		test_must_fail git subtree pull ./"sub proj" HEAD >out 2>err &&
 
-		echo "You must provide the --prefix option." >expected &&
+		echo "fatal: you must provide the --prefix option." >expected &&
 		test_must_be_empty out &&
 		test_cmp expected err
 	)
@@ -584,7 +584,7 @@ test_expect_success 'pull requires path given by option --prefix must exist' '
 	(
 		test_must_fail git subtree pull --prefix="sub dir" ./"sub proj" HEAD >out 2>err &&
 
-		echo "'\''sub dir'\'' does not exist; use '\''git subtree add'\''" >expected &&
+		echo "fatal: '\''sub dir'\'' does not exist; use '\''git subtree add'\''" >expected &&
 		test_must_be_empty out &&
 		test_cmp expected err
 	)
@@ -643,7 +643,7 @@ test_expect_success 'push requires option --prefix' '
 		cd "$test_count" &&
 		git fetch ./"sub proj" HEAD &&
 		git subtree add --prefix="sub dir" FETCH_HEAD &&
-		echo "You must provide the --prefix option." >expected &&
+		echo "fatal: you must provide the --prefix option." >expected &&
 		test_must_fail git subtree push "./sub proj" from-mainline >actual 2>&1 &&
 		test_debug "printf '"expected: "'" &&
 		test_debug "cat expected" &&
@@ -662,7 +662,7 @@ test_expect_success 'push requires path given by option --prefix must exist' '
 		cd "$test_count" &&
 		git fetch ./"sub proj" HEAD &&
 		git subtree add --prefix="sub dir" FETCH_HEAD &&
-		echo "'\''non-existent-directory'\'' does not exist; use '\''git subtree add'\''" >expected &&
+		echo "fatal: '\''non-existent-directory'\'' does not exist; use '\''git subtree add'\''" >expected &&
 		test_must_fail git subtree push --prefix=non-existent-directory "./sub proj" from-mainline >actual 2>&1 &&
 		test_debug "printf '"expected: "'" &&
 		test_debug "cat expected" &&

From 34ab458cb1df13ca400c10bbc4ff69c75a4e217e Mon Sep 17 00:00:00 2001
From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Fri, 21 Oct 2022 15:13:35 +0000
Subject: [PATCH 5/9] subtree: define a variable before its first use in
 'find_latest_squash'

The function 'find_latest_squash' takes a single argument, 'dir', but a
debug statement uses this variable before it takes its value from $1.

This statement thus gets the value of 'dir' from the calling function,
which currently is the same as the 'dir' argument, so it works but it
is confusing.

Move the definition of 'dir' before its first use.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/subtree/git-subtree.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 89f1eb756f..d91a967907 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -374,10 +374,10 @@ try_remove_previous () {
 # Usage: find_latest_squash DIR
 find_latest_squash () {
 	assert test $# = 1
+	dir="$1"
 	debug "Looking for latest squash ($dir)..."
 	local indent=$(($indent + 1))
 
-	dir="$1"
 	sq=
 	main=
 	sub=

From 7990142eb1b6fdd60e87f63f003bc593fc105260 Mon Sep 17 00:00:00 2001
From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Fri, 21 Oct 2022 15:13:36 +0000
Subject: [PATCH 6/9] subtree: use named variables instead of "$@" in cmd_pull

'cmd_pull' already checks that only two arguments are given,
'repository' and 'ref'. Define variables with these names instead of
using the positional parameter $2 and "$@".

This will allow a subsequent commit to pass 'repository' to 'cmd_merge'.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/subtree/git-subtree.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index d91a967907..2b3c429991 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -1005,9 +1005,11 @@ cmd_pull () {
 	then
 		die "fatal: you must provide <repository> <ref>"
 	fi
+	repository="$1"
+	ref="$2"
 	ensure_clean
-	ensure_valid_ref_format "$2"
-	git fetch "$@" || exit $?
+	ensure_valid_ref_format "$ref"
+	git fetch "$repository" "$ref" || exit $?
 	cmd_merge FETCH_HEAD
 }
 

From f10d31cf2d41fb892fbda1fb2b77dd518418c1a1 Mon Sep 17 00:00:00 2001
From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Fri, 21 Oct 2022 15:13:37 +0000
Subject: [PATCH 7/9] subtree: process 'git-subtree-split' trailer in separate
 function

Both functions 'find_latest_squash' (called by 'git subtree merge
--squash' and 'git subtree split --rejoin') and 'find_existing_splits'
(called by git 'subtree split') loop through commits that have a
'git-subtree-dir' trailer, and then process the 'git-subtree-mainline'
and 'git-subtree-split' trailers for those commits.

The processing done for the 'git-subtree-split' trailer is simple: we
check if the object exists with 'rev-parse' and set the variable
'sub' to the object name, or we die if the object does not exist.

In a future commit we will add more steps to the processing of this
trailer in order to make the code more robust.

To reduce code duplication, move the processing of the
'git-subtree-split' trailer to a dedicated function,
'process_subtree_split_trailer'.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/subtree/git-subtree.sh | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 2b3c429991..b90ca0036f 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -371,6 +371,15 @@ try_remove_previous () {
 	fi
 }
 
+# Usage: process_subtree_split_trailer SPLIT_HASH MAIN_HASH
+process_subtree_split_trailer () {
+	assert test $# = 2
+	b="$1"
+	sq="$2"
+	sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
+	die "fatal: could not rev-parse split hash $b from commit $sq"
+}
+
 # Usage: find_latest_squash DIR
 find_latest_squash () {
 	assert test $# = 1
@@ -395,8 +404,7 @@ find_latest_squash () {
 			main="$b"
 			;;
 		git-subtree-split:)
-			sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
-			die "fatal: could not rev-parse split hash $b from commit $sq"
+			process_subtree_split_trailer "$b" "$sq"
 			;;
 		END)
 			if test -n "$sub"
@@ -447,8 +455,7 @@ find_existing_splits () {
 			main="$b"
 			;;
 		git-subtree-split:)
-			sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
-			die "fatal: could not rev-parse split hash $b from commit $sq"
+			process_subtree_split_trailer "$b" "$sq"
 			;;
 		END)
 			debug "Main is: '$main'"

From 0d330673d4fa9e0bea91092abb657606f4325e7e Mon Sep 17 00:00:00 2001
From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Fri, 21 Oct 2022 15:13:38 +0000
Subject: [PATCH 8/9] subtree: fix squash merging after annotated tag was
 squashed merged

When 'git subtree merge --squash $ref' is invoked, either directly or
through 'git subtree pull --squash $repo $ref', the code looks for the
latest squash merge of the subtree in order to create the new merge
commit as a child of the previous squash merge.

This search is done in function 'process_subtree_split_trailer', invoked
by 'find_latest_squash', which looks for the most recent commit with a
'git-subtree-split' trailer; that trailer's value is the object name in
the subtree repository of the ref that was last squash-merged. The
function verifies that this object is present locally with 'git
rev-parse', and aborts if it's not.

The hash referenced by the 'git-subtree-split' trailer is guaranteed to
correspond to a commit since it is the result of running 'git rev-parse
-q --verify "$1^{commit}"' on the first argument of 'cmd_merge' (this
corresponds to 'rev' in 'cmd_merge' which is passed through to
'new_squash_commit' and 'squash_msg').

But this is only the case since e4f8baa88a (subtree: parse revs in
individual cmd_ functions, 2021-04-27), which went into Git 2.32. Before
that commit, 'cmd_merge' verified the revision it was given using 'git
rev-parse --revs-only "$@"'. Such an invocation, when fed the name of an
annotated tag, would return the hash of the tag, not of the commit
referenced by the tag.

This leads to a failure in 'find_latest_squash' when squash-merging if
the most recent squash-merge merged an annotated tag of the subtree
repository, using a pre-2.32 version of 'git subtree', unless that
previous annotated tag is present locally (which is not usually the
case).

We can fix this by fetching the object directly by its hash in
'process_subtree_split_trailer' when 'git rev-parse' fails, but in order
to do so we need to know the name or URL of the subtree repository.
This is not possible in general for 'git subtree merge', but is easy
when it is invoked through 'git subtree pull' since in that case the
subtree repository is passed by the user at the command line.

Allow the 'git subtree pull' scenario to work out-of-the-box by adding
an optional 'repository' argument to functions 'cmd_merge',
'find_latest_squash' and 'process_subtree_split_trailer', and invoke
'cmd_merge' with that 'repository' argument in 'cmd_pull'.

If 'repository' is absent in 'process_subtree_split_trailer', instruct
the user to try fetching the missing object directly.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/subtree/git-subtree.sh     | 56 +++++++++++++++++++++++-------
 contrib/subtree/git-subtree.txt    |  9 +++--
 contrib/subtree/t/t7900-subtree.sh | 36 +++++++++++++++++++
 3 files changed, 86 insertions(+), 15 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index b90ca0036f..2c67989fe8 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -371,20 +371,45 @@ try_remove_previous () {
 	fi
 }
 
-# Usage: process_subtree_split_trailer SPLIT_HASH MAIN_HASH
+# Usage: process_subtree_split_trailer SPLIT_HASH MAIN_HASH [REPOSITORY]
 process_subtree_split_trailer () {
-	assert test $# = 2
+	assert test $# = 2 -o $# = 3
 	b="$1"
 	sq="$2"
-	sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
-	die "fatal: could not rev-parse split hash $b from commit $sq"
+	repository=""
+	if test "$#" = 3
+	then
+		repository="$3"
+	fi
+	fail_msg="fatal: could not rev-parse split hash $b from commit $sq"
+	if ! sub="$(git rev-parse --verify --quiet "$b^{commit}")"
+	then
+		# if 'repository' was given, try to fetch the 'git-subtree-split' hash
+		# before 'rev-parse'-ing it again, as it might be a tag that we do not have locally
+		if test -n "${repository}"
+		then
+			git fetch "$repository" "$b"
+			sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
+				die "$fail_msg"
+		else
+			hint1=$(printf "hint: hash might be a tag, try fetching it from the subtree repository:")
+			hint2=$(printf "hint:    git fetch <subtree-repository> $b")
+			fail_msg=$(printf "$fail_msg\n$hint1\n$hint2")
+			die "$fail_msg"
+		fi
+	fi
 }
 
-# Usage: find_latest_squash DIR
+# Usage: find_latest_squash DIR [REPOSITORY]
 find_latest_squash () {
-	assert test $# = 1
+	assert test $# = 1 -o $# = 2
 	dir="$1"
-	debug "Looking for latest squash ($dir)..."
+	repository=""
+	if test "$#" = 2
+	then
+		repository="$2"
+	fi
+	debug "Looking for latest squash (dir=$dir, repository=$repository)..."
 	local indent=$(($indent + 1))
 
 	sq=
@@ -404,7 +429,7 @@ find_latest_squash () {
 			main="$b"
 			;;
 		git-subtree-split:)
-			process_subtree_split_trailer "$b" "$sq"
+			process_subtree_split_trailer "$b" "$sq" "$repository"
 			;;
 		END)
 			if test -n "$sub"
@@ -969,17 +994,22 @@ cmd_split () {
 	exit 0
 }
 
-# Usage: cmd_merge REV
+# Usage: cmd_merge REV [REPOSITORY]
 cmd_merge () {
-	test $# -eq 1 ||
-		die "fatal: you must provide exactly one revision.  Got: '$*'"
+	test $# -eq 1 -o $# -eq 2 ||
+		die "fatal: you must provide exactly one revision, and optionally a repository. Got: '$*'"
 	rev=$(git rev-parse -q --verify "$1^{commit}") ||
 		die "fatal: '$1' does not refer to a commit"
+	repository=""
+	if test "$#" = 2
+	then
+		repository="$2"
+	fi
 	ensure_clean
 
 	if test -n "$arg_addmerge_squash"
 	then
-		first_split="$(find_latest_squash "$dir")" || exit $?
+		first_split="$(find_latest_squash "$dir" "$repository")" || exit $?
 		if test -z "$first_split"
 		then
 			die "fatal: can't squash-merge: '$dir' was never added."
@@ -1017,7 +1047,7 @@ cmd_pull () {
 	ensure_clean
 	ensure_valid_ref_format "$ref"
 	git fetch "$repository" "$ref" || exit $?
-	cmd_merge FETCH_HEAD
+	cmd_merge FETCH_HEAD "$repository"
 }
 
 # Usage: cmd_push REPOSITORY [+][LOCALREV:]REMOTEREF
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index 9cddfa2654..0e7524d786 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git subtree' [<options>] -P <prefix> add <local-commit>
 'git subtree' [<options>] -P <prefix> add <repository> <remote-ref>
-'git subtree' [<options>] -P <prefix> merge <local-commit>
+'git subtree' [<options>] -P <prefix> merge <local-commit> [<repository>]
 'git subtree' [<options>] -P <prefix> split [<local-commit>]
 
 [verse]
@@ -76,7 +76,7 @@ add <repository> <remote-ref>::
 	only a single commit from the subproject, rather than its
 	entire history.
 
-merge <local-commit>::
+merge <local-commit> [<repository>]::
 	Merge recent changes up to <local-commit> into the <prefix>
 	subtree.  As with normal 'git merge', this doesn't
 	remove your own local changes; it just merges those
@@ -88,6 +88,11 @@ If you use '--squash', the merge direction doesn't always have to be
 forward; you can use this command to go back in time from v2.5 to v2.4,
 for example.  If your merge introduces a conflict, you can resolve it in
 the usual ways.
++
+When using '--squash', and the previous merge with '--squash' merged an
+annotated tag of the subtree repository, that tag needs to be available locally.
+If <repository> is given, a missing tag will automatically be fetched from that
+repository.
 
 split [<local-commit>]::
 	Extract a new, synthetic project history from the
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 249743ab9a..d0671676c7 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -43,6 +43,30 @@ last_commit_subject () {
 	git log --pretty=format:%s -1
 }
 
+# Upon 'git subtree add|merge --squash' of an annotated tag,
+# pre-2.32.0 versions of 'git subtree' would write the hash of the tag
+# (sub1 below), instead of the commit (sub1^{commit}) in the
+# "git-subtree-split" trailer.
+# We immitate this behaviour below using a replace ref.
+# This function creates 3 repositories:
+# - $1
+# - $1-sub (added as subtree "sub" in $1)
+# - $1-clone (clone of $1)
+test_create_pre2_32_repo () {
+	subtree_test_create_repo "$1" &&
+	subtree_test_create_repo "$1-sub" &&
+	test_commit -C "$1" main1 &&
+	test_commit -C "$1-sub" --annotate sub1 &&
+	git -C "$1" subtree add --prefix="sub" --squash "../$1-sub" sub1 &&
+	tag=$(git -C "$1" rev-parse FETCH_HEAD) &&
+	commit=$(git -C "$1" rev-parse FETCH_HEAD^{commit}) &&
+	git -C "$1" log -1 --format=%B HEAD^2 >msg &&
+	test_commit -C "$1-sub" --annotate sub2 &&
+	git clone --no-local "$1" "$1-clone" &&
+	new_commit=$(cat msg | sed -e "s/$commit/$tag/" | git -C "$1-clone" commit-tree HEAD^2^{tree}) &&
+	git -C "$1-clone" replace HEAD^2 $new_commit
+}
+
 test_expect_success 'shows short help text for -h' '
 	test_expect_code 129 git subtree -h >out 2>err &&
 	test_must_be_empty err &&
@@ -264,6 +288,13 @@ test_expect_success 'merge new subproj history into subdir/ with a slash appende
 	)
 '
 
+test_expect_success 'merge with --squash after annotated tag was added/merged with --squash pre-v2.32.0 ' '
+	test_create_pre2_32_repo "$test_count" &&
+	git -C "$test_count-clone" fetch "../$test_count-sub" sub2  &&
+	test_must_fail git -C "$test_count-clone" subtree merge --prefix="sub" --squash FETCH_HEAD &&
+	git -C "$test_count-clone" subtree merge --prefix="sub" --squash FETCH_HEAD  "../$test_count-sub"
+'
+
 #
 # Tests for 'git subtree split'
 #
@@ -630,6 +661,11 @@ test_expect_success 'pull rejects flags for split' '
 	)
 '
 
+test_expect_success 'pull with --squash after annotated tag was added/merged with --squash pre-v2.32.0 ' '
+	test_create_pre2_32_repo "$test_count" &&
+	git -C "$test_count-clone" subtree -d pull --prefix="sub" --squash "../$test_count-sub" sub2
+'
+
 #
 # Tests for 'git subtree push'
 #

From 1762382ab19dd6d5d84dd32e35e25c2b55b651f0 Mon Sep 17 00:00:00 2001
From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Fri, 21 Oct 2022 15:13:39 +0000
Subject: [PATCH 9/9] subtree: fix split after annotated tag was squashed
 merged

The previous commit fixed a failure in 'git subtree merge --squash' when
the previous squash-merge merged an annotated tag of the subtree
repository which is missing locally.

The same failure happens in 'git subtree split', either directly or when
called by 'git subtree push', under the same circumstances: 'cmd_split'
invokes 'find_existing_splits', which loops through previous commits and
invokes 'git rev-parse' (via 'process_subtree_split_trailer') on the
value of any 'git subtree-split' trailer it finds. This fails if this
value is the hash of an annotated tag which is missing locally.

Add a new optional argument 'repository' to 'cmd_split' and
'find_existing_splits', and invoke 'cmd_split' with that argument from
'cmd_push'. This allows 'process_subtree_split_trailer' to try to fetch
the missing tag from the 'repository' if it's not available locally,
mirroring the new behaviour of 'git subtree pull' and 'git subtree
merge'.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/subtree/git-subtree.sh     | 26 ++++++++++++++++++--------
 contrib/subtree/git-subtree.txt    |  7 ++++++-
 contrib/subtree/t/t7900-subtree.sh | 12 ++++++++++++
 3 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 2c67989fe8..10c9c87839 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -453,14 +453,19 @@ find_latest_squash () {
 	done || exit $?
 }
 
-# Usage: find_existing_splits DIR REV
+# Usage: find_existing_splits DIR REV [REPOSITORY]
 find_existing_splits () {
-	assert test $# = 2
+	assert test $# = 2 -o $# = 3
 	debug "Looking for prior splits..."
 	local indent=$(($indent + 1))
 
 	dir="$1"
 	rev="$2"
+	repository=""
+	if test "$#" = 3
+	then
+		repository="$3"
+	fi
 	main=
 	sub=
 	local grep_format="^git-subtree-dir: $dir/*\$"
@@ -480,7 +485,7 @@ find_existing_splits () {
 			main="$b"
 			;;
 		git-subtree-split:)
-			process_subtree_split_trailer "$b" "$sq"
+			process_subtree_split_trailer "$b" "$sq" "$repository"
 			;;
 		END)
 			debug "Main is: '$main'"
@@ -906,17 +911,22 @@ cmd_add_commit () {
 	say >&2 "Added dir '$dir'"
 }
 
-# Usage: cmd_split [REV]
+# Usage: cmd_split [REV] [REPOSITORY]
 cmd_split () {
 	if test $# -eq 0
 	then
 		rev=$(git rev-parse HEAD)
-	elif test $# -eq 1
+	elif test $# -eq 1 -o $# -eq 2
 	then
 		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.  Got: '$*'"
+		die "fatal: you must provide exactly one revision, and optionnally a repository.  Got: '$*'"
+	fi
+	repository=""
+	if test "$#" = 2
+	then
+		repository="$2"
 	fi
 
 	if test -n "$arg_split_rejoin"
@@ -940,7 +950,7 @@ cmd_split () {
 		done || exit $?
 	fi
 
-	unrevs="$(find_existing_splits "$dir" "$rev")" || exit $?
+	unrevs="$(find_existing_splits "$dir" "$rev" "$repository")" || exit $?
 
 	# We can't restrict rev-list to only $dir here, because some of our
 	# parents have the $dir contents the root, and those won't match.
@@ -1072,7 +1082,7 @@ cmd_push () {
 			die "fatal: '$localrevname_presplit' does not refer to a commit"
 
 		echo "git push using: " "$repository" "$refspec"
-		localrev=$(cmd_split "$localrev_presplit") || die
+		localrev=$(cmd_split "$localrev_presplit" "$repository") || die
 		git push "$repository" "$localrev":"refs/heads/$remoteref"
 	else
 		die "fatal: '$dir' must already exist. Try 'git subtree add'."
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index 0e7524d786..004abf415b 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -94,7 +94,7 @@ annotated tag of the subtree repository, that tag needs to be available locally.
 If <repository> is given, a missing tag will automatically be fetched from that
 repository.
 
-split [<local-commit>]::
+split [<local-commit>] [<repository>]::
 	Extract a new, synthetic project history from the
 	history of the <prefix> subtree of <local-commit>, or of
 	HEAD if no <local-commit> is given.  The new history
@@ -114,6 +114,11 @@ settings passed to 'split' (such as '--annotate') are the same.
 Because of this, if you add new commits and then re-split, the new
 commits will be attached as commits on top of the history you
 generated last time, so 'git merge' and friends will work as expected.
++
+When a previous merge with '--squash' merged an annotated tag of the
+subtree repository, that tag needs to be available locally.
+If <repository> is given, a missing tag will automatically be fetched from that
+repository.
 
 pull <repository> <remote-ref>::
 	Exactly like 'merge', but parallels 'git pull' in that
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index d0671676c7..341c169eca 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -582,6 +582,12 @@ test_expect_success 'split "sub dir"/ with --branch for an incompatible branch'
 	)
 '
 
+test_expect_success 'split after annotated tag was added/merged with --squash pre-v2.32.0' '
+	test_create_pre2_32_repo "$test_count" &&
+	test_must_fail git -C "$test_count-clone" subtree split --prefix="sub" HEAD &&
+	git -C "$test_count-clone" subtree split --prefix="sub" HEAD "../$test_count-sub"
+'
+
 #
 # Tests for 'git subtree pull'
 #
@@ -989,6 +995,12 @@ test_expect_success 'push "sub dir"/ with a local rev' '
 	)
 '
 
+test_expect_success 'push after annotated tag was added/merged with --squash pre-v2.32.0' '
+	test_create_pre2_32_repo "$test_count" &&
+	test_create_commit "$test_count-clone" sub/main-sub1 &&
+	git -C "$test_count-clone" subtree push --prefix="sub" "../$test_count-sub" from-mainline
+'
+
 #
 # Validity checking
 #