From: Kyle Zhao <kylezhao@xxxxxxxxxxx> 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. 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@xxxxxxxxxxx> --- 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-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1731/keyu98/kz/fix-merge-when-index-lock-exists-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/1731 Range-diff vs v2: 1: 86101e5b778 ! 1: 138fe0a054d merge: avoid write merge state when unable to write index @@ Metadata ## Commit message ## merge: avoid write merge state when unable to write index - Currently, when index.lock exists, if a merge occurs, the merge state - will be written and the index will be unchanged. + 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. - If the user exec "git commit" instead of "git merge --abort" after this, - a merge commit will be generated and all modifications from the source - branch will be lost. + 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@xxxxxxxxxxx> ## builtin/merge.c ## @@ builtin/merge.c: 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) + NULL) < 0) - return error(_("Unable to write index.")); -+ NULL) < 0) { -+ error(_("Unable to write index.")); -+ return 2; -+ } ++ die(_("Unable to write index.")); if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree") || !strcmp(strategy, "ort")) { @@ 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 && -+ touch .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..c1958095dcf 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