From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: ZheNing Hu <adlternative@gmail.com>,
Eric Sunshine <sunshine@sunshineco.com>,
Junio C Hamano <gitster@pobox.com>,
Elijah Newren <newren@gmail.com>,
Elijah Newren <newren@gmail.com>,
Elijah Newren <newren@gmail.com>
Subject: [PATCH v4 2/7] merge-resolve: abort if index does not match HEAD
Date: Fri, 22 Jul 2022 05:15:37 +0000 [thread overview]
Message-ID: <b79f44e54b9611fa2b10a4e1cb666992d006951c.1658466942.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1231.v4.git.1658466942.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
As noted in 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. One of the commits
referenced in the long tale of issues arising from lax enforcement of
this requirement was commit 55f39cf755 ("merge: fix misleading
pre-merge check documentation", 2018-06-30), which tried to document
the requirement and noted there were some exceptions. As mentioned in
that commit message, the `resolve` strategy was the one strategy that
did not have an explicit index matching HEAD check, and the reason it
didn't was that I wasn't able to discover any cases where the
implementation would fail to catch the problem and abort, and didn't
want to introduce unnecessary performance overhead of adding another
check.
Well, today I discovered a testcase where the implementation does not
catch the problem and so an explicit check is needed. Add a testcase
that previously would have failed, and update git-merge-resolve.sh to
have an explicit check. Note that the code is copied from 3ec62ad9ff
("merge-octopus: abort if index does not match HEAD", 2016-04-09), so
that we reuse the same message and avoid making translators need to
translate some new message.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
builtin/merge.c | 20 ++++++++++++++++++
git-merge-resolve.sh | 10 +++++++++
t/t6424-merge-unrelated-index-changes.sh | 26 ++++++++++++++++++++++++
3 files changed, 56 insertions(+)
diff --git a/builtin/merge.c b/builtin/merge.c
index 23170f2d2a6..13884b8e836 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1599,6 +1599,26 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
*/
refresh_cache(REFRESH_QUIET);
if (allow_trivial && fast_forward != FF_ONLY) {
+ /*
+ * Must first ensure that index matches HEAD before
+ * attempting a trivial merge.
+ */
+ struct tree *head_tree = get_commit_tree(head_commit);
+ struct strbuf sb = STRBUF_INIT;
+
+ if (repo_index_has_changes(the_repository, head_tree,
+ &sb)) {
+ struct strbuf err = STRBUF_INIT;
+ strbuf_addstr(&err, "error: ");
+ strbuf_addf(&err, _("Your local changes to the following files would be overwritten by merge:\n %s"),
+ sb.buf);
+ strbuf_addch(&err, '\n');
+ fputs(err.buf, stderr);
+ strbuf_release(&err);
+ strbuf_release(&sb);
+ return -1;
+ }
+
/* See if it is really trivial. */
git_committer_info(IDENT_STRICT);
printf(_("Trying really trivial in-index merge...\n"));
diff --git a/git-merge-resolve.sh b/git-merge-resolve.sh
index 343fe7bccd0..77e93121bf8 100755
--- a/git-merge-resolve.sh
+++ b/git-merge-resolve.sh
@@ -5,6 +5,16 @@
#
# Resolve two trees, using enhanced multi-base read-tree.
+. git-sh-setup
+
+# Abort if index does not match HEAD
+if ! git diff-index --quiet --cached HEAD --
+then
+ gettextln "Error: Your local changes to the following files would be overwritten by merge"
+ git diff-index --cached --name-only HEAD -- | sed -e 's/^/ /'
+ exit 2
+fi
+
# The first parameters up to -- are merge bases; the rest are heads.
bases= head= remotes= sep_seen=
for arg
diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh
index b6e424a427b..f35d3182b86 100755
--- a/t/t6424-merge-unrelated-index-changes.sh
+++ b/t/t6424-merge-unrelated-index-changes.sh
@@ -114,6 +114,32 @@ test_expect_success 'resolve, non-trivial' '
test_path_is_missing .git/MERGE_HEAD
'
+test_expect_success 'resolve, trivial, related file removed' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ git rm a &&
+ test_path_is_missing a &&
+
+ test_must_fail git merge -s resolve C^0 &&
+
+ test_path_is_missing a &&
+ test_path_is_missing .git/MERGE_HEAD
+'
+
+test_expect_success 'resolve, non-trivial, related file removed' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ git rm a &&
+ test_path_is_missing a &&
+
+ test_must_fail git merge -s resolve D^0 &&
+
+ test_path_is_missing a &&
+ test_path_is_missing .git/MERGE_HEAD
+'
+
test_expect_success 'recursive' '
git reset --hard &&
git checkout B^0 &&
--
gitgitgadget
next prev parent reply other threads:[~2022-07-22 5:15 UTC|newest]
Thread overview: 87+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-19 16:26 [PATCH 0/2] Fix merge restore state Elijah Newren via GitGitGadget
2022-05-19 16:26 ` [PATCH 1/2] merge: remove unused variable Elijah Newren via GitGitGadget
2022-05-19 17:45 ` Junio C Hamano
2022-05-19 16:26 ` [PATCH 2/2] merge: make restore_state() do as its name says Elijah Newren via GitGitGadget
2022-05-19 17:44 ` Junio C Hamano
2022-05-19 18:32 ` Junio C Hamano
2022-06-12 6:58 ` [PATCH 0/2] Fix merge restore state Elijah Newren
2022-06-12 8:54 ` ZheNing Hu
2022-06-19 6:50 ` [PATCH v2 0/6] " Elijah Newren via GitGitGadget
2022-06-19 6:50 ` [PATCH v2 1/6] t6424: make sure a failed merge preserves local changes Junio C Hamano via GitGitGadget
2022-06-19 6:50 ` [PATCH v2 2/6] merge: remove unused variable Elijah Newren via GitGitGadget
2022-07-19 23:14 ` Junio C Hamano
2022-06-19 6:50 ` [PATCH v2 3/6] merge: fix save_state() to work when there are racy-dirty files Elijah Newren via GitGitGadget
2022-07-17 16:28 ` ZheNing Hu
2022-07-19 22:49 ` Junio C Hamano
2022-07-21 1:09 ` Elijah Newren
2022-07-19 22:43 ` Junio C Hamano
2022-06-19 6:50 ` [PATCH v2 4/6] merge: make restore_state() restore staged state too Elijah Newren via GitGitGadget
2022-07-17 16:37 ` ZheNing Hu
2022-07-19 23:14 ` Junio C Hamano
2022-07-19 23:28 ` Junio C Hamano
2022-07-21 1:37 ` Elijah Newren
2022-06-19 6:50 ` [PATCH v2 5/6] merge: ensure we can actually restore pre-merge state Elijah Newren via GitGitGadget
2022-07-17 16:41 ` ZheNing Hu
2022-07-19 22:57 ` Junio C Hamano
2022-07-21 2:03 ` Elijah Newren
2022-06-19 6:50 ` [PATCH v2 6/6] merge: do not exit restore_state() prematurely Elijah Newren via GitGitGadget
2022-07-17 16:44 ` ZheNing Hu
2022-07-19 23:13 ` Junio C Hamano
2022-07-20 0:09 ` Eric Sunshine
2022-07-21 2:03 ` Elijah Newren
2022-07-21 3:27 ` Elijah Newren
2022-07-21 8:16 ` [PATCH v3 0/7] Fix merge restore state Elijah Newren via GitGitGadget
2022-07-21 8:16 ` [PATCH v3 1/7] merge-ort-wrappers: make printed message match the one from recursive Elijah Newren via GitGitGadget
2022-07-21 15:47 ` Junio C Hamano
2022-07-21 19:51 ` Elijah Newren
2022-07-21 20:05 ` Junio C Hamano
2022-07-21 21:14 ` Elijah Newren
2022-07-21 8:16 ` [PATCH v3 2/7] merge-resolve: abort if index does not match HEAD Elijah Newren via GitGitGadget
2022-07-21 8:16 ` [PATCH v3 3/7] merge: do not abort early if one strategy fails to handle the merge Elijah Newren via GitGitGadget
2022-07-21 16:09 ` Junio C Hamano
2022-07-25 10:38 ` Ævar Arnfjörð Bjarmason
2022-07-26 1:31 ` Elijah Newren
2022-07-26 6:54 ` Ævar Arnfjörð Bjarmason
2022-07-21 8:16 ` [PATCH v3 4/7] merge: fix save_state() to work when there are stat-dirty files Elijah Newren via GitGitGadget
2022-07-21 8:16 ` [PATCH v3 5/7] merge: make restore_state() restore staged state too Elijah Newren via GitGitGadget
2022-07-21 16:16 ` Junio C Hamano
2022-07-21 16:24 ` Junio C Hamano
2022-07-21 19:52 ` Elijah Newren
2022-07-21 8:16 ` [PATCH v3 6/7] merge: ensure we can actually restore pre-merge state Elijah Newren via GitGitGadget
2022-07-21 16:31 ` Junio C Hamano
2023-03-02 7:17 ` Ben Humphreys
2023-03-02 15:35 ` Elijah Newren
2023-03-02 16:19 ` Junio C Hamano
2023-03-04 16:18 ` Rudy Rigot
2023-03-06 22:19 ` Ben Humphreys
2022-07-21 8:16 ` [PATCH v3 7/7] merge: do not exit restore_state() prematurely Elijah Newren via GitGitGadget
2022-07-21 16:34 ` Junio C Hamano
2022-07-22 5:15 ` [PATCH v4 0/7] Fix merge restore state Elijah Newren via GitGitGadget
2022-07-22 5:15 ` [PATCH v4 1/7] merge-ort-wrappers: make printed message match the one from recursive Elijah Newren via GitGitGadget
2022-07-22 5:15 ` Elijah Newren via GitGitGadget [this message]
2022-07-22 10:27 ` [PATCH v4 2/7] merge-resolve: abort if index does not match HEAD Ævar Arnfjörð Bjarmason
2022-07-23 0:28 ` Elijah Newren
2022-07-23 5:44 ` Ævar Arnfjörð Bjarmason
2022-07-26 1:58 ` Elijah Newren
2022-07-26 6:35 ` Ævar Arnfjörð Bjarmason
2022-07-22 5:15 ` [PATCH v4 3/7] merge: do not abort early if one strategy fails to handle the merge Elijah Newren via GitGitGadget
2022-07-22 10:47 ` Ævar Arnfjörð Bjarmason
2022-07-23 0:36 ` Elijah Newren
2022-07-22 5:15 ` [PATCH v4 4/7] merge: fix save_state() to work when there are stat-dirty files Elijah Newren via GitGitGadget
2022-07-22 5:15 ` [PATCH v4 5/7] merge: make restore_state() restore staged state too Elijah Newren via GitGitGadget
2022-07-22 10:53 ` Ævar Arnfjörð Bjarmason
2022-07-23 1:56 ` Elijah Newren
2022-07-22 5:15 ` [PATCH v4 6/7] merge: ensure we can actually restore pre-merge state Elijah Newren via GitGitGadget
2022-07-22 5:15 ` [PATCH v4 7/7] merge: do not exit restore_state() prematurely Elijah Newren via GitGitGadget
2022-07-23 1:53 ` [PATCH v5 0/8] Fix merge restore state Elijah Newren via GitGitGadget
2022-07-23 1:53 ` [PATCH v5 1/8] merge-ort-wrappers: make printed message match the one from recursive Elijah Newren via GitGitGadget
2022-07-23 1:53 ` [PATCH v5 2/8] merge-resolve: abort if index does not match HEAD Elijah Newren via GitGitGadget
2022-07-23 1:53 ` [PATCH v5 3/8] merge: abort if index does not match HEAD for trivial merges Elijah Newren via GitGitGadget
2022-07-23 1:53 ` [PATCH v5 4/8] merge: do not abort early if one strategy fails to handle the merge Elijah Newren via GitGitGadget
2022-07-23 1:53 ` [PATCH v5 5/8] merge: fix save_state() to work when there are stat-dirty files Elijah Newren via GitGitGadget
2022-07-23 1:53 ` [PATCH v5 6/8] merge: make restore_state() restore staged state too Elijah Newren via GitGitGadget
2022-07-23 1:53 ` [PATCH v5 7/8] merge: ensure we can actually restore pre-merge state Elijah Newren via GitGitGadget
2022-07-23 1:53 ` [PATCH v5 8/8] merge: do not exit restore_state() prematurely Elijah Newren via GitGitGadget
2022-07-25 19:03 ` [PATCH v5 0/8] Fix merge restore state Junio C Hamano
2022-07-26 1:59 ` Elijah Newren
2022-07-26 4:03 ` ZheNing Hu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b79f44e54b9611fa2b10a4e1cb666992d006951c.1658466942.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=adlternative@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=newren@gmail.com \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).