git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4] merge: avoid write merge state when unable to write index

From
Kyle Zhao via GitGitGadget <gitgitgadget@gmail.com>
Date
May 17, 2024, 04:41 UTC
Message-ID
<pull.1731.v4.git.1715920862420.gitgitgadget@gmail.com>
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
Previous: Kyle Zhao via GitGitGadgetNext: goldofzky@gmail.com
Message 5 of 11 in “merge: avoid write merge state when unable to write index”
  1. merge: avoid write merge state when unable to write indexKyle Zhao via GitGitGadget, May 16, 2024
  2. merge: avoid write merge state when unable to write indexKyle Zhao via GitGitGadget, May 16, 2024
  3. Junio C HamanoMay 16, 2024
  4. merge: avoid write merge state when unable to write indexKyle Zhao via GitGitGadget, May 17, 2024
  5. merge: avoid write merge state when unable to write indexKyle Zhao via GitGitGadget, May 17, 2024
  6. goldofzky@gmail.comJun 12, 2024
  7. merge: avoid write merge state when unable to write indexKyle Zhao via GitGitGadget, Jun 12, 2024
  8. Junio C HamanoJun 13, 2024
  9. goldofzky@gmail.comJun 17, 2024
  10. goldofzky@gmail.comJun 17, 2024
  11. merge: avoid write merge state when unable to write indexKyle Zhao via GitGitGadget, Jun 17, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.