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>

Philippe Blain committed Oct 21, 2022 at 15:13 UTC 0d330673d4fa9e0bea91092abb657606f4325e7e
3 files changed +86 -15
contrib/subtree/git-subtree.sh
+43 -13
@@ -371,20 +371,45 @@ try_remove_previous () {
371 fi
372 }
373
374 -# Usage: process_subtree_split_trailer SPLIT_HASH MAIN_HASH
374 +# Usage: process_subtree_split_trailer SPLIT_HASH MAIN_HASH [REPOSITORY]
375 process_subtree_split_trailer () {
376 - assert test $# = 2
376 + assert test $# = 2 -o $# = 3
377 b="$1"
378 sq="$2"
379 - sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
380 - die "fatal: could not rev-parse split hash $b from commit $sq"
379 + repository=""
380 + if test "$#" = 3
381 + then
382 + repository="$3"
383 + fi
384 + fail_msg="fatal: could not rev-parse split hash $b from commit $sq"
385 + if ! sub="$(git rev-parse --verify --quiet "$b^{commit}")"
386 + then
387 + # if 'repository' was given, try to fetch the 'git-subtree-split' hash
388 + # before 'rev-parse'-ing it again, as it might be a tag that we do not have locally
389 + if test -n "${repository}"
390 + then
391 + git fetch "$repository" "$b"
392 + sub="$(git rev-parse --verify --quiet "$b^{commit}")" ||
393 + die "$fail_msg"
394 + else
395 + hint1=$(printf "hint: hash might be a tag, try fetching it from the subtree repository:")
396 + hint2=$(printf "hint: git fetch <subtree-repository> $b")
397 + fail_msg=$(printf "$fail_msg\n$hint1\n$hint2")
398 + die "$fail_msg"
399 + fi
400 + fi
401 }
402
383 -# Usage: find_latest_squash DIR
403 +# Usage: find_latest_squash DIR [REPOSITORY]
404 find_latest_squash () {
385 - assert test $# = 1
405 + assert test $# = 1 -o $# = 2
406 dir="$1"
387 - debug "Looking for latest squash ($dir)..."
407 + repository=""
408 + if test "$#" = 2
409 + then
410 + repository="$2"
411 + fi
412 + debug "Looking for latest squash (dir=$dir, repository=$repository)..."
413 local indent=$(($indent + 1))
414
415 sq=
@@ -404,7 +429,7 @@ find_latest_squash () {
429 main="$b"
430 ;;
431 git-subtree-split:)
407 - process_subtree_split_trailer "$b" "$sq"
432 + process_subtree_split_trailer "$b" "$sq" "$repository"
433 ;;
434 END)
435 if test -n "$sub"
@@ -969,17 +994,22 @@ cmd_split () {
994 exit 0
995 }
996
972 -# Usage: cmd_merge REV
997 +# Usage: cmd_merge REV [REPOSITORY]
998 cmd_merge () {
974 - test $# -eq 1 ||
975 - die "fatal: you must provide exactly one revision. Got: '$*'"
999 + test $# -eq 1 -o $# -eq 2 ||
1000 + die "fatal: you must provide exactly one revision, and optionally a repository. Got: '$*'"
1001 rev=$(git rev-parse -q --verify "$1^{commit}") ||
1002 die "fatal: '$1' does not refer to a commit"
1003 + repository=""
1004 + if test "$#" = 2
1005 + then
1006 + repository="$2"
1007 + fi
1008 ensure_clean
1009
1010 if test -n "$arg_addmerge_squash"
1011 then
982 - first_split="$(find_latest_squash "$dir")" || exit $?
1012 + first_split="$(find_latest_squash "$dir" "$repository")" || exit $?
1013 if test -z "$first_split"
1014 then
1015 die "fatal: can't squash-merge: '$dir' was never added."
@@ -1017,7 +1047,7 @@ cmd_pull () {
1047 ensure_clean
1048 ensure_valid_ref_format "$ref"
1049 git fetch "$repository" "$ref" || exit $?
1020 - cmd_merge FETCH_HEAD
1050 + cmd_merge FETCH_HEAD "$repository"
1051 }
1052
1053 # Usage: cmd_push REPOSITORY [+][LOCALREV:]REMOTEREF
contrib/subtree/git-subtree.txt
+7 -2
@@ -11,7 +11,7 @@ SYNOPSIS
11 [verse]
12 'git subtree' [<options>] -P <prefix> add <local-commit>
13 'git subtree' [<options>] -P <prefix> add <repository> <remote-ref>
14 -'git subtree' [<options>] -P <prefix> merge <local-commit>
14 +'git subtree' [<options>] -P <prefix> merge <local-commit> [<repository>]
15 'git subtree' [<options>] -P <prefix> split [<local-commit>]
16
17 [verse]
@@ -76,7 +76,7 @@ add <repository> <remote-ref>::
76 only a single commit from the subproject, rather than its
77 entire history.
78
79 -merge <local-commit>::
79 +merge <local-commit> [<repository>]::
80 Merge recent changes up to <local-commit> into the <prefix>
81 subtree. As with normal 'git merge', this doesn't
82 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
88 forward; you can use this command to go back in time from v2.5 to v2.4,
89 for example. If your merge introduces a conflict, you can resolve it in
90 the usual ways.
91 ++
92 +When using '--squash', and the previous merge with '--squash' merged an
93 +annotated tag of the subtree repository, that tag needs to be available locally.
94 +If <repository> is given, a missing tag will automatically be fetched from that
95 +repository.
96
97 split [<local-commit>]::
98 Extract a new, synthetic project history from the
contrib/subtree/t/t7900-subtree.sh
+36
@@ -43,6 +43,30 @@ last_commit_subject () {
43 git log --pretty=format:%s -1
44 }
45
46 +# Upon 'git subtree add|merge --squash' of an annotated tag,
47 +# pre-2.32.0 versions of 'git subtree' would write the hash of the tag
48 +# (sub1 below), instead of the commit (sub1^{commit}) in the
49 +# "git-subtree-split" trailer.
50 +# We immitate this behaviour below using a replace ref.
51 +# This function creates 3 repositories:
52 +# - $1
53 +# - $1-sub (added as subtree "sub" in $1)
54 +# - $1-clone (clone of $1)
55 +test_create_pre2_32_repo () {
56 + subtree_test_create_repo "$1" &&
57 + subtree_test_create_repo "$1-sub" &&
58 + test_commit -C "$1" main1 &&
59 + test_commit -C "$1-sub" --annotate sub1 &&
60 + git -C "$1" subtree add --prefix="sub" --squash "../$1-sub" sub1 &&
61 + tag=$(git -C "$1" rev-parse FETCH_HEAD) &&
62 + commit=$(git -C "$1" rev-parse FETCH_HEAD^{commit}) &&
63 + git -C "$1" log -1 --format=%B HEAD^2 >msg &&
64 + test_commit -C "$1-sub" --annotate sub2 &&
65 + git clone --no-local "$1" "$1-clone" &&
66 + new_commit=$(cat msg | sed -e "s/$commit/$tag/" | git -C "$1-clone" commit-tree HEAD^2^{tree}) &&
67 + git -C "$1-clone" replace HEAD^2 $new_commit
68 +}
69 +
70 test_expect_success 'shows short help text for -h' '
71 test_expect_code 129 git subtree -h >out 2>err &&
72 test_must_be_empty err &&
@@ -264,6 +288,13 @@ test_expect_success 'merge new subproj history into subdir/ with a slash appende
288 )
289 '
290
291 +test_expect_success 'merge with --squash after annotated tag was added/merged with --squash pre-v2.32.0 ' '
292 + test_create_pre2_32_repo "$test_count" &&
293 + git -C "$test_count-clone" fetch "../$test_count-sub" sub2 &&
294 + test_must_fail git -C "$test_count-clone" subtree merge --prefix="sub" --squash FETCH_HEAD &&
295 + git -C "$test_count-clone" subtree merge --prefix="sub" --squash FETCH_HEAD "../$test_count-sub"
296 +'
297 +
298 #
299 # Tests for 'git subtree split'
300 #
@@ -630,6 +661,11 @@ test_expect_success 'pull rejects flags for split' '
661 )
662 '
663
664 +test_expect_success 'pull with --squash after annotated tag was added/merged with --squash pre-v2.32.0 ' '
665 + test_create_pre2_32_repo "$test_count" &&
666 + git -C "$test_count-clone" subtree -d pull --prefix="sub" --squash "../$test_count-sub" sub2
667 +'
668 +
669 #
670 # Tests for 'git subtree push'
671 #