diff --git a/src/cherrypick.c b/src/cherrypick.c index 3754b1f77..ebc9fcdd8 100644 --- a/src/cherrypick.c +++ b/src/cherrypick.c @@ -172,9 +172,8 @@ int git_cherrypick( char commit_oidstr[GIT_OID_HEXSZ + 1]; const char *commit_msg, *commit_summary; git_buf their_label = GIT_BUF_INIT; - git_index *index = NULL, *index_new = NULL; + git_index *index = NULL; git_indexwriter indexwriter = GIT_INDEXWRITER_INIT; - bool write_index = false; int error = 0; assert(repo && commit); @@ -194,28 +193,16 @@ int git_cherrypick( if ((error = write_merge_msg(repo, commit_msg)) < 0 || (error = git_buf_printf(&their_label, "%.7s... %s", commit_oidstr, commit_summary)) < 0 || - (error = cherrypick_normalize_opts(repo, &opts, given_opts, git_buf_cstr(&their_label))) < 0) - goto on_error; - - write_index = (opts.checkout_opts.checkout_strategy & GIT_CHECKOUT_DONT_WRITE_INDEX) == 0; - - if (write_index) { - /* Never let checkout update the index, we'll update it ourselves. */ - opts.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; - } - - if ((error = write_cherrypick_head(repo, commit_oidstr)) < 0 || + (error = cherrypick_normalize_opts(repo, &opts, given_opts, git_buf_cstr(&their_label))) < 0 || + (error = git_indexwriter_init_for_operation(&indexwriter, repo, &opts.checkout_opts.checkout_strategy)) < 0 || + (error = write_cherrypick_head(repo, commit_oidstr)) < 0 || (error = git_repository_head(&our_ref, repo)) < 0 || (error = git_reference_peel((git_object **)&our_commit, our_ref, GIT_OBJ_COMMIT)) < 0 || - (error = git_cherrypick_commit(&index_new, repo, commit, our_commit, opts.mainline, &opts.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, &opts.checkout_opts)) < 0 || - (write_index && (error = git_indexwriter_commit(&indexwriter)) < 0)) + (error = git_cherrypick_commit(&index, repo, commit, our_commit, opts.mainline, &opts.merge_opts)) < 0 || + (error = git_merge__check_result(repo, index)) < 0 || + (error = git_merge__append_conflicts_to_merge_msg(repo, index)) < 0 || + (error = git_checkout_index(repo, index, &opts.checkout_opts)) < 0 || + (error = git_indexwriter_commit(&indexwriter)) < 0) goto on_error; goto done; @@ -226,7 +213,6 @@ on_error: done: git_indexwriter_cleanup(&indexwriter); git_index_free(index); - git_index_free(index_new); git_commit_free(our_commit); git_reference_free(our_ref); git_buf_free(&their_label); diff --git a/src/index.c b/src/index.c index 58575fec3..6c01b40f2 100644 --- a/src/index.c +++ b/src/index.c @@ -2669,6 +2669,8 @@ int git_indexwriter_init( { int error; + GIT_REFCOUNT_INC(index); + writer->index = index; if (!index->index_file_path) @@ -2677,12 +2679,33 @@ int git_indexwriter_init( if ((error = git_filebuf_open( &writer->file, index->index_file_path, GIT_FILEBUF_HASH_CONTENTS, GIT_INDEX_FILE_MODE)) < 0) { + if (error == GIT_ELOCKED) giterr_set(GITERR_INDEX, "The index is locked. This might be due to a concurrent or crashed process"); return error; } + writer->should_write = 1; + + return 0; +} + +int git_indexwriter_init_for_operation( + git_indexwriter *writer, + git_repository *repo, + unsigned int *checkout_strategy) +{ + git_index *index; + int error; + + if ((error = git_repository_index__weakptr(&index, repo)) < 0 || + (error = git_indexwriter_init(writer, index)) < 0) + return error; + + writer->should_write = (*checkout_strategy & GIT_CHECKOUT_DONT_WRITE_INDEX) == 0; + *checkout_strategy |= GIT_CHECKOUT_DONT_WRITE_INDEX; + return 0; } @@ -2690,6 +2713,9 @@ int git_indexwriter_commit(git_indexwriter *writer) { int error; + if (!writer->should_write) + return 0; + if (index_sort_if_needed(writer->index, true) < 0) return -1; @@ -2711,10 +2737,16 @@ int git_indexwriter_commit(git_indexwriter *writer) writer->index->on_disk = 1; + git_index_free(writer->index); + writer->index = NULL; + return 0; } void git_indexwriter_cleanup(git_indexwriter *writer) { git_filebuf_cleanup(&writer->file); + + git_index_free(writer->index); + writer->index = NULL; } diff --git a/src/index.h b/src/index.h index d151f6614..6d2904fdc 100644 --- a/src/index.h +++ b/src/index.h @@ -97,6 +97,7 @@ extern int git_index_snapshot_find( typedef struct { git_index *index; git_filebuf file; + unsigned int should_write:1; } git_indexwriter; #define GIT_INDEXWRITER_INIT { NULL, GIT_FILEBUF_INIT } @@ -104,6 +105,16 @@ typedef struct { /* Lock the index for eventual writing. */ extern int git_indexwriter_init(git_indexwriter *writer, git_index *index); +/* Lock the index for eventual writing by a repository operation: a merge, + * revert, cherry-pick or a rebase. Note that the given checkout strategy + * will be updated for the operation's use so that checkout will not write + * the index. + */ +extern int git_indexwriter_init_for_operation( + git_indexwriter *writer, + git_repository *repo, + unsigned int *checkout_strategy); + /* Write the index and unlock it. */ extern int git_indexwriter_commit(git_indexwriter *writer); diff --git a/src/merge.c b/src/merge.c index 8201c0c1c..a92e6aef9 100644 --- a/src/merge.c +++ b/src/merge.c @@ -2651,9 +2651,8 @@ 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 = NULL, *index_new = NULL; + git_index *index = NULL; git_indexwriter indexwriter = GIT_INDEXWRITER_INIT; - bool write_index = false; size_t i; int error = 0; @@ -2667,24 +2666,12 @@ int git_merge( their_trees = git__calloc(their_heads_len, sizeof(git_tree *)); GITERR_CHECK_ALLOC(their_trees); - if ((error = merge_heads(&ancestor_head, &our_head, repo, their_heads, their_heads_len)) < 0) + if ((error = merge_heads(&ancestor_head, &our_head, repo, their_heads, their_heads_len)) < 0 || + (error = merge_normalize_checkout_opts(repo, &checkout_opts, given_checkout_opts, + ancestor_head, our_head, their_heads_len, their_heads)) < 0 || + (error = git_indexwriter_init_for_operation(&indexwriter, repo, &checkout_opts.checkout_strategy)) < 0) goto on_error; - if ((error = merge_normalize_checkout_opts(repo, &checkout_opts, given_checkout_opts, - 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; @@ -2703,11 +2690,11 @@ int git_merge( /* TODO: recursive, octopus, etc... */ - 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 || - (write_index && (error = git_indexwriter_commit(&indexwriter)) < 0)) + if ((error = git_merge_trees(&index, repo, ancestor_tree, our_tree, their_trees[0], merge_opts)) < 0 || + (error = git_merge__check_result(repo, index)) < 0 || + (error = git_merge__append_conflicts_to_merge_msg(repo, index)) < 0 || + (error = git_checkout_index(repo, index, &checkout_opts)) < 0 || + (error = git_indexwriter_commit(&indexwriter)) < 0) goto on_error; goto done; @@ -2719,7 +2706,6 @@ done: git_indexwriter_cleanup(&indexwriter); git_index_free(index); - git_index_free(index_new); git_tree_free(ancestor_tree); git_tree_free(our_tree); diff --git a/src/rebase.c b/src/rebase.c index 9e6c4c1d7..b24830bd9 100644 --- a/src/rebase.c +++ b/src/rebase.c @@ -726,9 +726,8 @@ static int rebase_next_merge( git_checkout_options checkout_opts = {0}; git_commit *current_commit = NULL, *parent_commit = NULL; git_tree *current_tree = NULL, *head_tree = NULL, *parent_tree = NULL; - git_index *index = NULL, *index_new = NULL; + git_index *index = NULL; git_indexwriter indexwriter = GIT_INDEXWRITER_INIT; - bool write_index = false; git_rebase_operation *operation; char current_idstr[GIT_OID_HEXSZ]; unsigned int parent_count; @@ -758,27 +757,15 @@ static int rebase_next_merge( git_oid_fmt(current_idstr, &operation->id); - 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, rebase->repo)) < 0 || - (error = git_indexwriter_init(&indexwriter, index)) < 0) - goto done; - } - - if ((error = rebase_setupfile(rebase, MSGNUM_FILE, -1, "%d\n", rebase->current+1)) < 0 || - (error = rebase_setupfile(rebase, CURRENT_FILE, -1, "%.*s\n", GIT_OID_HEXSZ, current_idstr)) < 0) - goto done; - normalize_checkout_opts(rebase, current_commit, &checkout_opts, given_checkout_opts); - if ((error = git_merge_trees(&index, rebase->repo, parent_tree, head_tree, current_tree, NULL)) < 0 || - (error = git_merge__check_result(rebase->repo, index_new)) < 0 || - (error = git_checkout_index(rebase->repo, index_new, &checkout_opts)) < 0 || - (write_index && (error = git_indexwriter_commit(&indexwriter)) < 0)) + if ((error = git_indexwriter_init_for_operation(&indexwriter, rebase->repo, &checkout_opts.checkout_strategy)) < 0 || + (error = rebase_setupfile(rebase, MSGNUM_FILE, -1, "%d\n", rebase->current+1)) < 0 || + (error = rebase_setupfile(rebase, CURRENT_FILE, -1, "%.*s\n", GIT_OID_HEXSZ, current_idstr)) < 0 || + (error = git_merge_trees(&index, rebase->repo, parent_tree, head_tree, current_tree, NULL)) < 0 || + (error = git_merge__check_result(rebase->repo, index)) < 0 || + (error = git_checkout_index(rebase->repo, index, &checkout_opts)) < 0 || + (error = git_indexwriter_commit(&indexwriter)) < 0) goto done; *out = operation; @@ -786,7 +773,6 @@ static int rebase_next_merge( done: git_indexwriter_cleanup(&indexwriter); git_index_free(index); - git_index_free(index_new); git_tree_free(current_tree); git_tree_free(head_tree); git_tree_free(parent_tree); diff --git a/src/revert.c b/src/revert.c index f5f1a5d6f..f8a7f4333 100644 --- a/src/revert.c +++ b/src/revert.c @@ -175,9 +175,8 @@ int git_revert( char commit_oidstr[GIT_OID_HEXSZ + 1]; const char *commit_msg; git_buf their_label = GIT_BUF_INIT; - git_index *index = NULL, *index_new = NULL; + git_index *index = NULL; git_indexwriter indexwriter = GIT_INDEXWRITER_INIT; - bool write_index = false; int error; assert(repo && commit); @@ -196,29 +195,17 @@ int git_revert( } if ((error = git_buf_printf(&their_label, "parent of %.7s... %s", commit_oidstr, commit_msg)) < 0 || - (error = revert_normalize_opts(repo, &opts, given_opts, git_buf_cstr(&their_label))) < 0) - goto on_error; - - write_index = (opts.checkout_opts.checkout_strategy & GIT_CHECKOUT_DONT_WRITE_INDEX) == 0; - - if (write_index) { - /* Never let checkout update the index, we'll update it ourselves. */ - opts.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; - } - - if ((error = write_revert_head(repo, commit_oidstr)) < 0 || + (error = revert_normalize_opts(repo, &opts, given_opts, git_buf_cstr(&their_label))) < 0 || + (error = git_indexwriter_init_for_operation(&indexwriter, repo, &opts.checkout_opts.checkout_strategy)) < 0 || + (error = write_revert_head(repo, commit_oidstr)) < 0 || (error = write_merge_msg(repo, commit_oidstr, commit_msg)) < 0 || (error = git_repository_head(&our_ref, repo)) < 0 || (error = git_reference_peel((git_object **)&our_commit, our_ref, GIT_OBJ_COMMIT)) < 0 || - (error = git_revert_commit(&index_new, repo, commit, our_commit, opts.mainline, &opts.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, &opts.checkout_opts)) < 0 || - (write_index && (error = git_indexwriter_commit(&indexwriter)) < 0)) + (error = git_revert_commit(&index, repo, commit, our_commit, opts.mainline, &opts.merge_opts)) < 0 || + (error = git_merge__check_result(repo, index)) < 0 || + (error = git_merge__append_conflicts_to_merge_msg(repo, index)) < 0 || + (error = git_checkout_index(repo, index, &opts.checkout_opts)) < 0 || + (error = git_indexwriter_commit(&indexwriter)) < 0) goto on_error; goto done; @@ -229,7 +216,6 @@ on_error: done: git_indexwriter_cleanup(&indexwriter); git_index_free(index); - git_index_free(index_new); git_commit_free(our_commit); git_reference_free(our_ref); git_buf_free(&their_label);