From: "Kyle Zhao via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Kyle Zhao <kylezhao@tencent.com>, Kyle Zhao <kylezhao@tencent.com>
Subject: [PATCH v4] merge: avoid write merge state when unable to write index
Date: Fri, 17 May 2024 04:41:02 +0000 [thread overview]
Message-ID: <pull.1731.v4.git.1715920862420.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1731.v3.git.1715917639587.gitgitgadget@gmail.com>
From: Kyle Zhao <kylezhao@tencent.com>
When running a merge while the index is locked (presumably by another
process), the merge state is written, the index is not updated, and then
the merge fails. This might cause unexpected results.
i.g. if another running process is "git commit", MERGE_HEAD and other state
files we write on our side will be taken into account by them and cause them
to record a merge, even though they may have been trying to record something
entirely different.
Signed-off-by: Kyle Zhao <kylezhao@tencent.com>
---
merge: avoid write merge state when unable to write index
In some of our monorepos, code is sometimes lost after merging.
After investigation, we discovered the problem.
This happens if we perform "git pull" or "git merge" when another git
process is writing to the index, especially in a monorepo (because its
index will be larger).
How to reproduce:
git init demo
cd demo
touch 1.txt && git add . && git commit -m "1"
git checkout -b source-branch
touch 2.txt && git add . && git commit -m "2"
git checkout master
echo "1" >> 1.txt && git add . && git commit -m "3"
# another git process runnning
touch .git/index.lock
git merge source-branch
# another git process finished
rm .git/index.lock
git commit -m "4"
Then the modifications from the source branch are lost.
Regards, Kyle
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1731%2Fkeyu98%2Fkz%2Ffix-merge-when-index-lock-exists-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1731/keyu98/kz/fix-merge-when-index-lock-exists-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1731
Range-diff vs v3:
1: 138fe0a054d ! 1: 7055dfb82c7 merge: avoid write merge state when unable to write index
@@ Commit message
When running a merge while the index is locked (presumably by another
process), the merge state is written, the index is not updated, and then
- the merge fails. This can cause unexpected results.
+ the merge fails. This might cause unexpected results.
i.g. if another running process is "git commit", MERGE_HEAD and other state
files we write on our side will be taken into account by them and cause them
@@ t/t7600-merge.sh: test_expect_success 'merge c1 with c2' '
+test_expect_success 'merge c1 with c2 when index.lock exists' '
+ test_when_finished rm .git/index.lock &&
+ git reset --hard c1 &&
-+ : >.git/index.lock &&
++ >.git/index.lock &&
+ test_must_fail git merge c2 &&
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_path_is_missing .git/MERGE_MODE &&
builtin/merge.c | 2 +-
t/t7600-merge.sh | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 6a6d3798858..12c1b048fe1 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -699,7 +699,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET,
SKIP_IF_UNCHANGED, 0, NULL, NULL,
NULL) < 0)
- return error(_("Unable to write index."));
+ die(_("Unable to write index."));
if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree") ||
!strcmp(strategy, "ort")) {
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index e5ff073099a..ef54cff4faa 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -236,6 +236,16 @@ test_expect_success 'merge c1 with c2' '
verify_parents $c1 $c2
'
+test_expect_success 'merge c1 with c2 when index.lock exists' '
+ test_when_finished rm .git/index.lock &&
+ git reset --hard c1 &&
+ >.git/index.lock &&
+ test_must_fail git merge c2 &&
+ test_path_is_missing .git/MERGE_HEAD &&
+ test_path_is_missing .git/MERGE_MODE &&
+ test_path_is_missing .git/MERGE_MSG
+'
+
test_expect_success 'merge --squash c3 with c7' '
git reset --hard c3 &&
test_must_fail git merge --squash c7 &&
base-commit: 19fe900cfce8096b7645ec9611a0b981f6bbd154
--
gitgitgadget
next prev parent reply other threads:[~2024-05-17 4:41 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-16 5:08 [PATCH] merge: avoid write merge state when unable to write index Kyle Zhao via GitGitGadget
2024-05-16 5:18 ` [PATCH v2] " Kyle Zhao via GitGitGadget
2024-05-16 16:18 ` Junio C Hamano
2024-05-17 3:47 ` [PATCH v3] " Kyle Zhao via GitGitGadget
2024-05-17 4:41 ` Kyle Zhao via GitGitGadget [this message]
2024-06-12 2:40 ` [PATCH v4] " goldofzky
2024-06-12 6:27 ` [PATCH v5] " Kyle Zhao via GitGitGadget
2024-06-13 17:27 ` Junio C Hamano
2024-06-17 3:02 ` [Internet]Re: " goldofzky
2024-06-17 9:17 ` goldofzky
2024-06-17 3:08 ` [PATCH v6] " Kyle Zhao via GitGitGadget
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=pull.1731.v4.git.1715920862420.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=kylezhao@tencent.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 read-only IMAP folder(s) and NNTP newsgroup(s).