@matheusbsbs / matheus-bagliano / commits / e4cdfe84a0

merge: abort if index does not match HEAD for trivial merges

As noted in the last commit and the links therein (especially commit 9822175d2b ("Ensure index matches head before invoking merge machinery, round N", 2019-08-17), we have had a very long history of problems with failing to enforce the requirement that index matches HEAD when starting a merge. The "trivial merge" logic in builtin/merge.c is yet another such case we previously missed. Add a check for it to ensure it aborts if the index does not match HEAD, and add a testcase where this fix is needed. Note that the fix here would also incidentally be an alternative fix for the testcase added in the last patch, but the fix in the last patch is still needed when multiple merge strategies are in use, so tweak the testcase from the previous commit so that it continues to exercise the codepath added in the last commit. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Jul 23, 2022 at 01:53 UTC e4cdfe84a0d2ac560bf0a2ab0e9beade5f71a844
2 files changed +36 -1
builtin/merge.c
+15
@@ -1599,6 +1599,21 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1599 */
1600 refresh_cache(REFRESH_QUIET);
1601 if (allow_trivial && fast_forward != FF_ONLY) {
1602 + /*
1603 + * Must first ensure that index matches HEAD before
1604 + * attempting a trivial merge.
1605 + */
1606 + struct tree *head_tree = get_commit_tree(head_commit);
1607 + struct strbuf sb = STRBUF_INIT;
1608 +
1609 + if (repo_index_has_changes(the_repository, head_tree,
1610 + &sb)) {
1611 + error(_("Your local changes to the following files would be overwritten by merge:\n %s"),
1612 + sb.buf);
1613 + strbuf_release(&sb);
1614 + return 2;
1615 + }
1616 +
1617 /* See if it is really trivial. */
1618 git_committer_info(IDENT_STRICT);
1619 printf(_("Trying really trivial in-index merge...\n"));
t/t6424-merge-unrelated-index-changes.sh
+21 -1
@@ -114,6 +114,19 @@ test_expect_success 'resolve, non-trivial' '
114 test_path_is_missing .git/MERGE_HEAD
115 '
116
117 +test_expect_success 'resolve, trivial, related file removed' '
118 + git reset --hard &&
119 + git checkout B^0 &&
120 +
121 + git rm a &&
122 + test_path_is_missing a &&
123 +
124 + test_must_fail git merge -s resolve C^0 &&
125 +
126 + test_path_is_missing a &&
127 + test_path_is_missing .git/MERGE_HEAD
128 +'
129 +
130 test_expect_success 'resolve, non-trivial, related file removed' '
131 git reset --hard &&
132 git checkout B^0 &&
@@ -121,7 +134,14 @@ test_expect_success 'resolve, non-trivial, related file removed' '
134 git rm a &&
135 test_path_is_missing a &&
136
124 - test_must_fail git merge -s resolve D^0 &&
137 + # We also ask for recursive in order to turn off the "allow_trivial"
138 + # setting in builtin/merge.c, and ensure that resolve really does
139 + # correctly fail the merge (I guess this also tests that recursive
140 + # correctly fails the merge, but the main thing we are attempting
141 + # to test here is resolve and are just using the side effect of
142 + # adding recursive to ensure that resolve is actually tested rather
143 + # than the trivial merge codepath)
144 + test_must_fail git merge -s resolve -s recursive D^0 &&
145
146 test_path_is_missing a &&
147 test_path_is_missing .git/MERGE_HEAD