diff options
author | Edward Thomson <ethomson@microsoft.com> | 2015-01-17 23:28:53 -0600 |
---|---|---|
committer | Edward Thomson <ethomson@edwardthomson.com> | 2015-02-14 09:25:35 -0500 |
commit | 8b0ddd5dd90e1d62f32faf14d5cb4bf88f04a095 (patch) | |
tree | 5b132d61f079508ae55d8ba4d7151f1335a2ce72 | |
parent | 8639ea5f98ac53e532820e8a186f8a1d6d98f573 (diff) | |
download | libgit2-8b0ddd5dd90e1d62f32faf14d5cb4bf88f04a095.tar.gz |
merge: lock the index at the start of the merge
Always lock the index when we begin the merge, before we write
any of the metdata files. This prevents a race where another
client may run a commit after we have written the MERGE_HEAD but
before we have updated the index, which will produce a merge
commit that is treesame to one parent. The merge will finish and
update the index and the resultant commit would not be a merge at
all.
-rw-r--r-- | src/merge.c | 21 | ||||
-rw-r--r-- | tests/merge/workdir/setup.c | 49 |
2 files changed, 68 insertions, 2 deletions
diff --git a/src/merge.c b/src/merge.c index 7c38b5692..8201c0c1c 100644 --- a/src/merge.c +++ b/src/merge.c @@ -2651,7 +2651,9 @@ int git_merge( git_checkout_options checkout_opts; git_annotated_commit *ancestor_head = NULL, *our_head = NULL; git_tree *ancestor_tree = NULL, *our_tree = NULL, **their_trees = NULL; - git_index *index_new = NULL; + git_index *index = NULL, *index_new = NULL; + git_indexwriter indexwriter = GIT_INDEXWRITER_INIT; + bool write_index = false; size_t i; int error = 0; @@ -2672,6 +2674,17 @@ int git_merge( ancestor_head, our_head, their_heads_len, their_heads)) < 0) goto on_error; + write_index = (checkout_opts.checkout_strategy & GIT_CHECKOUT_DONT_WRITE_INDEX) == 0; + + if (write_index) { + /* Never let checkout update the index, we'll update it ourselves. */ + checkout_opts.checkout_strategy |= GIT_CHECKOUT_DONT_WRITE_INDEX; + + if ((error = git_repository_index(&index, repo)) < 0 || + (error = git_indexwriter_init(&indexwriter, index)) < 0) + goto on_error; + } + /* Write the merge files to the repository. */ if ((error = git_merge__setup(repo, our_head, their_heads, their_heads_len)) < 0) goto on_error; @@ -2693,7 +2706,8 @@ int git_merge( if ((error = git_merge_trees(&index_new, repo, ancestor_tree, our_tree, their_trees[0], merge_opts)) < 0 || (error = git_merge__check_result(repo, index_new)) < 0 || (error = git_merge__append_conflicts_to_merge_msg(repo, index_new)) < 0 || - (error = git_checkout_index(repo, index_new, &checkout_opts)) < 0) + (error = git_checkout_index(repo, index_new, &checkout_opts)) < 0 || + (write_index && (error = git_indexwriter_commit(&indexwriter)) < 0)) goto on_error; goto done; @@ -2702,6 +2716,9 @@ on_error: merge_state_cleanup(repo); done: + git_indexwriter_cleanup(&indexwriter); + + git_index_free(index); git_index_free(index_new); git_tree_free(ancestor_tree); diff --git a/tests/merge/workdir/setup.c b/tests/merge/workdir/setup.c index 31ffd5702..4aebf8701 100644 --- a/tests/merge/workdir/setup.c +++ b/tests/merge/workdir/setup.c @@ -1045,3 +1045,52 @@ void test_merge_workdir_setup__removed_after_failure(void) git_annotated_commit_free(our_head); git_annotated_commit_free(their_heads[0]); } + +void test_merge_workdir_setup__unlocked_after_success(void) +{ + git_oid our_oid; + git_reference *octo1_ref; + git_annotated_commit *our_head, *their_heads[1]; + + cl_git_pass(git_oid_fromstr(&our_oid, ORIG_HEAD)); + cl_git_pass(git_annotated_commit_lookup(&our_head, repo, &our_oid)); + + cl_git_pass(git_reference_lookup(&octo1_ref, repo, GIT_REFS_HEADS_DIR OCTO1_BRANCH)); + cl_git_pass(git_annotated_commit_from_ref(&their_heads[0], repo, octo1_ref)); + + cl_git_pass(git_merge( + repo, (const git_annotated_commit **)&their_heads[0], 1, NULL, NULL)); + + cl_assert(!git_path_exists("merge-resolve/.git/index.lock")); + + git_reference_free(octo1_ref); + + git_annotated_commit_free(our_head); + git_annotated_commit_free(their_heads[0]); +} + +void test_merge_workdir_setup__unlocked_after_conflict(void) +{ + git_oid our_oid; + git_reference *octo1_ref; + git_annotated_commit *our_head, *their_heads[1]; + + cl_git_pass(git_oid_fromstr(&our_oid, ORIG_HEAD)); + cl_git_pass(git_annotated_commit_lookup(&our_head, repo, &our_oid)); + + cl_git_pass(git_reference_lookup(&octo1_ref, repo, GIT_REFS_HEADS_DIR OCTO1_BRANCH)); + cl_git_pass(git_annotated_commit_from_ref(&their_heads[0], repo, octo1_ref)); + + cl_git_rewritefile("merge-resolve/new-in-octo1.txt", + "Conflicting file!\n\nMerge will fail!\n"); + + cl_git_fail(git_merge( + repo, (const git_annotated_commit **)&their_heads[0], 1, NULL, NULL)); + + cl_assert(!git_path_exists("merge-resolve/.git/index.lock")); + + git_reference_free(octo1_ref); + + git_annotated_commit_free(our_head); + git_annotated_commit_free(their_heads[0]); +} |