Commit 8b0ddd5d by Edward Thomson Committed by Edward Thomson

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.
parent 8639ea5f
...@@ -2651,7 +2651,9 @@ int git_merge( ...@@ -2651,7 +2651,9 @@ int git_merge(
git_checkout_options checkout_opts; git_checkout_options checkout_opts;
git_annotated_commit *ancestor_head = NULL, *our_head = NULL; git_annotated_commit *ancestor_head = NULL, *our_head = NULL;
git_tree *ancestor_tree = NULL, *our_tree = NULL, **their_trees = 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; size_t i;
int error = 0; int error = 0;
...@@ -2672,6 +2674,17 @@ int git_merge( ...@@ -2672,6 +2674,17 @@ int git_merge(
ancestor_head, our_head, their_heads_len, their_heads)) < 0) ancestor_head, our_head, their_heads_len, their_heads)) < 0)
goto on_error; 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. */ /* Write the merge files to the repository. */
if ((error = git_merge__setup(repo, our_head, their_heads, their_heads_len)) < 0) if ((error = git_merge__setup(repo, our_head, their_heads, their_heads_len)) < 0)
goto on_error; goto on_error;
...@@ -2693,7 +2706,8 @@ int git_merge( ...@@ -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 || 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__check_result(repo, index_new)) < 0 ||
(error = git_merge__append_conflicts_to_merge_msg(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 on_error;
goto done; goto done;
...@@ -2702,6 +2716,9 @@ on_error: ...@@ -2702,6 +2716,9 @@ on_error:
merge_state_cleanup(repo); merge_state_cleanup(repo);
done: done:
git_indexwriter_cleanup(&indexwriter);
git_index_free(index);
git_index_free(index_new); git_index_free(index_new);
git_tree_free(ancestor_tree); git_tree_free(ancestor_tree);
......
...@@ -1045,3 +1045,52 @@ void test_merge_workdir_setup__removed_after_failure(void) ...@@ -1045,3 +1045,52 @@ void test_merge_workdir_setup__removed_after_failure(void)
git_annotated_commit_free(our_head); git_annotated_commit_free(our_head);
git_annotated_commit_free(their_heads[0]); 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]);
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment