From 9f664347ff74716dc35ff0bd9a1a87606ae29cf0 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 6 Nov 2014 14:40:21 -0500 Subject: [PATCH 1/2] checkout_index: remove conflicts when checking out new files --- src/checkout.c | 185 +++++++++++++++++++++++++++++++----------- tests/checkout/tree.c | 90 +++++++++++++++++++- 2 files changed, 224 insertions(+), 51 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index fb425bb0c..8ffd40d5d 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -37,9 +37,10 @@ enum { CHECKOUT_ACTION__UPDATE_BLOB = 2, CHECKOUT_ACTION__UPDATE_SUBMODULE = 4, CHECKOUT_ACTION__CONFLICT = 8, - CHECKOUT_ACTION__UPDATE_CONFLICT = 16, - CHECKOUT_ACTION__MAX = 16, - CHECKOUT_ACTION__DEFER_REMOVE = 32, + CHECKOUT_ACTION__REMOVE_CONFLICT = 16, + CHECKOUT_ACTION__UPDATE_CONFLICT = 32, + CHECKOUT_ACTION__MAX = 32, + CHECKOUT_ACTION__DEFER_REMOVE = 64, CHECKOUT_ACTION__REMOVE_AND_UPDATE = (CHECKOUT_ACTION__UPDATE_BLOB | CHECKOUT_ACTION__REMOVE), }; @@ -54,9 +55,10 @@ typedef struct { git_index *index; git_pool pool; git_vector removes; - git_vector conflicts; - git_vector *reuc; - git_vector *names; + git_vector remove_conflicts; + git_vector update_conflicts; + git_vector *update_reuc; + git_vector *update_names; git_buf path; size_t workdir_len; git_buf tmp; @@ -793,49 +795,60 @@ done: return error; } -static int checkout_conflicts_load(checkout_data *data, git_iterator *workdir, git_vector *pathspec) +static int checkout_conflict_append_update( + const git_index_entry *ancestor, + const git_index_entry *ours, + const git_index_entry *theirs, + void *payload) +{ + checkout_data *data = payload; + checkout_conflictdata *conflict; + int error; + + conflict = git__calloc(1, sizeof(checkout_conflictdata)); + GITERR_CHECK_ALLOC(conflict); + + conflict->ancestor = ancestor; + conflict->ours = ours; + conflict->theirs = theirs; + + if ((error = checkout_conflict_detect_submodule(conflict)) < 0 || + (error = checkout_conflict_detect_binary(data->repo, conflict)) < 0) + { + git__free(conflict); + return error; + } + + if (git_vector_insert(&data->update_conflicts, conflict)) + return -1; + + return 0; +} + +static int checkout_conflicts_foreach( + checkout_data *data, + git_index *index, + git_iterator *workdir, + git_vector *pathspec, + int (*cb)(const git_index_entry *, const git_index_entry *, const git_index_entry *, void *), + void *payload) { git_index_conflict_iterator *iterator = NULL; - git_index *index; const git_index_entry *ancestor, *ours, *theirs; - checkout_conflictdata *conflict; int error = 0; - /* Only write conficts from sources that have them: indexes. */ - if ((index = git_iterator_get_index(data->target)) == NULL) - return 0; - if ((error = git_index_conflict_iterator_new(&iterator, index)) < 0) goto done; - data->conflicts._cmp = checkout_conflictdata_cmp; - /* Collect the conflicts */ while ((error = git_index_conflict_next(&ancestor, &ours, &theirs, iterator)) == 0) { if (!conflict_pathspec_match(data, workdir, pathspec, ancestor, ours, theirs)) continue; - conflict = git__calloc(1, sizeof(checkout_conflictdata)); - GITERR_CHECK_ALLOC(conflict); - - conflict->ancestor = ancestor; - conflict->ours = ours; - conflict->theirs = theirs; - - if ((error = checkout_conflict_detect_submodule(conflict)) < 0 || - (error = checkout_conflict_detect_binary(data->repo, conflict)) < 0) - { - git__free(conflict); + if ((error = cb(ancestor, ours, theirs, payload)) < 0) goto done; - } - - git_vector_insert(&data->conflicts, conflict); } - /* Collect the REUC and NAME entries */ - data->reuc = &index->reuc; - data->names = &index->names; - if (error == GIT_ITEROVER) error = 0; @@ -845,6 +858,26 @@ done: return error; } +static int checkout_conflicts_load(checkout_data *data, git_iterator *workdir, git_vector *pathspec) +{ + git_index *index; + + /* Only write conficts from sources that have them: indexes. */ + if ((index = git_iterator_get_index(data->target)) == NULL) + return 0; + + data->update_conflicts._cmp = checkout_conflictdata_cmp; + + if (checkout_conflicts_foreach(data, index, workdir, pathspec, checkout_conflict_append_update, data) < 0) + return -1; + + /* Collect the REUC and NAME entries */ + data->update_reuc = &index->reuc; + data->update_names = &index->names; + + return 0; +} + GIT_INLINE(int) checkout_conflicts_cmp_entry( const char *path, const git_index_entry *entry) @@ -869,10 +902,10 @@ static checkout_conflictdata *checkout_conflicts_search_ancestor( { size_t pos; - if (git_vector_bsearch2(&pos, &data->conflicts, checkout_conflicts_cmp_ancestor, path) < 0) + if (git_vector_bsearch2(&pos, &data->update_conflicts, checkout_conflicts_cmp_ancestor, path) < 0) return NULL; - return git_vector_get(&data->conflicts, pos); + return git_vector_get(&data->update_conflicts, pos); } static checkout_conflictdata *checkout_conflicts_search_branch( @@ -882,7 +915,7 @@ static checkout_conflictdata *checkout_conflicts_search_branch( checkout_conflictdata *conflict; size_t i; - git_vector_foreach(&data->conflicts, i, conflict) { + git_vector_foreach(&data->update_conflicts, i, conflict) { int cmp = -1; if (conflict->ancestor) @@ -1022,7 +1055,7 @@ static int checkout_conflicts_coalesce_renames( } git_vector_remove_matching( - &data->conflicts, checkout_conflictdata_empty, NULL); + &data->update_conflicts, checkout_conflictdata_empty, NULL); done: return error; @@ -1044,7 +1077,7 @@ static int checkout_conflicts_mark_directoryfile( len = git_index_entrycount(index); /* Find d/f conflicts */ - git_vector_foreach(&data->conflicts, i, conflict) { + git_vector_foreach(&data->update_conflicts, i, conflict) { if ((conflict->ours && conflict->theirs) || (!conflict->ours && !conflict->theirs)) continue; @@ -1084,7 +1117,7 @@ done: return error; } -static int checkout_get_conflicts( +static int checkout_get_update_conflicts( checkout_data *data, git_iterator *workdir, git_vector *pathspec) @@ -1103,6 +1136,38 @@ done: return error; } +static int checkout_conflict_append_remove( + const git_index_entry *ancestor, + const git_index_entry *ours, + const git_index_entry *theirs, + void *payload) +{ + checkout_data *data = payload; + const char *name; + + if (ancestor) + name = git__strdup(ancestor->path); + else if (ours) + name = git__strdup(ours->path); + else if (theirs) + name = git__strdup(theirs->path); + + GITERR_CHECK_ALLOC(name); + + return git_vector_insert(&data->remove_conflicts, (char *)name); +} + +static int checkout_get_remove_conflicts( + checkout_data *data, + git_iterator *workdir, + git_vector *pathspec) +{ + if ((data->strategy & GIT_CHECKOUT_DONT_UPDATE_INDEX) != 0) + return 0; + + return checkout_conflicts_foreach(data, data->index, workdir, pathspec, checkout_conflict_append_remove, data); +} + static int checkout_get_actions( uint32_t **actions_ptr, size_t **counts_ptr, @@ -1170,10 +1235,12 @@ static int checkout_get_actions( } - if ((error = checkout_get_conflicts(data, workdir, &pathspec)) < 0) + if ((error = checkout_get_remove_conflicts(data, workdir, &pathspec)) < 0 || + (error = checkout_get_update_conflicts(data, workdir, &pathspec)) < 0) goto fail; - counts[CHECKOUT_ACTION__UPDATE_CONFLICT] = git_vector_length(&data->conflicts); + counts[CHECKOUT_ACTION__REMOVE_CONFLICT] = git_vector_length(&data->remove_conflicts); + counts[CHECKOUT_ACTION__UPDATE_CONFLICT] = git_vector_length(&data->update_conflicts); git_pathspec__vfree(&pathspec); git_pool_clear(&pathpool); @@ -1869,7 +1936,7 @@ static int checkout_create_conflicts(checkout_data *data) size_t i; int error = 0; - git_vector_foreach(&data->conflicts, i, conflict) { + git_vector_foreach(&data->update_conflicts, i, conflict) { /* Both deleted: nothing to do */ if (conflict->ours == NULL && conflict->theirs == NULL) @@ -1943,6 +2010,21 @@ static int checkout_create_conflicts(checkout_data *data) return error; } +static int checkout_remove_conflicts(checkout_data *data) +{ + const char *conflict; + size_t i; + + git_vector_foreach(&data->remove_conflicts, i, conflict) { + if (git_index_conflict_remove(data->index, conflict) < 0) + return -1; + + data->completed_steps++; + } + + return 0; +} + static int checkout_extensions_update_index(checkout_data *data) { const git_index_reuc_entry *reuc_entry; @@ -1953,8 +2035,8 @@ static int checkout_extensions_update_index(checkout_data *data) if ((data->strategy & GIT_CHECKOUT_UPDATE_ONLY) != 0) return 0; - if (data->reuc) { - git_vector_foreach(data->reuc, i, reuc_entry) { + if (data->update_reuc) { + git_vector_foreach(data->update_reuc, i, reuc_entry) { if ((error = git_index_reuc_add(data->index, reuc_entry->path, reuc_entry->mode[0], &reuc_entry->oid[0], reuc_entry->mode[1], &reuc_entry->oid[1], @@ -1963,8 +2045,8 @@ static int checkout_extensions_update_index(checkout_data *data) } } - if (data->names) { - git_vector_foreach(data->names, i, name_entry) { + if (data->update_names) { + git_vector_foreach(data->update_names, i, name_entry) { if ((error = git_index_name_add(data->index, name_entry->ancestor, name_entry->ours, name_entry->theirs)) < 0) goto done; @@ -1985,7 +2067,8 @@ static void checkout_data_clear(checkout_data *data) git_vector_free(&data->removes); git_pool_clear(&data->pool); - git_vector_free_deep(&data->conflicts); + git_vector_free_deep(&data->remove_conflicts); + git_vector_free_deep(&data->update_conflicts); git__free(data->pfx); data->pfx = NULL; @@ -2127,7 +2210,8 @@ static int checkout_data_init( } if ((error = git_vector_init(&data->removes, 0, git__strcmp_cb)) < 0 || - (error = git_vector_init(&data->conflicts, 0, NULL)) < 0 || + (error = git_vector_init(&data->remove_conflicts, 0, NULL)) < 0 || + (error = git_vector_init(&data->update_conflicts, 0, NULL)) < 0 || (error = git_pool_init(&data->pool, 1, 0)) < 0 || (error = git_buf_puts(&data->path, data->opts.target_directory)) < 0 || (error = git_path_to_dir(&data->path)) < 0) @@ -2206,6 +2290,7 @@ int git_checkout_iterator( goto cleanup; data.total_steps = counts[CHECKOUT_ACTION__REMOVE] + + counts[CHECKOUT_ACTION__REMOVE_CONFLICT] + counts[CHECKOUT_ACTION__UPDATE_BLOB] + counts[CHECKOUT_ACTION__UPDATE_SUBMODULE] + counts[CHECKOUT_ACTION__UPDATE_CONFLICT]; @@ -2219,6 +2304,10 @@ int git_checkout_iterator( (error = checkout_remove_the_old(actions, &data)) < 0) goto cleanup; + if (counts[CHECKOUT_ACTION__REMOVE_CONFLICT] > 0 && + (error = checkout_remove_conflicts(&data)) < 0) + goto cleanup; + if (counts[CHECKOUT_ACTION__UPDATE_BLOB] > 0 && (error = checkout_create_the_new(actions, &data)) < 0) goto cleanup; diff --git a/tests/checkout/tree.c b/tests/checkout/tree.c index 0933f64f7..3212d6503 100644 --- a/tests/checkout/tree.c +++ b/tests/checkout/tree.c @@ -882,7 +882,7 @@ void test_checkout_tree__extremely_long_file_name(void) cl_assert(!git_path_exists(path)); } -static void create_conflict(void) +static void create_conflict(const char *path) { git_index *index; git_index_entry entry; @@ -893,7 +893,7 @@ static void create_conflict(void) entry.mode = 0100644; entry.flags = 1 << GIT_IDXENTRY_STAGESHIFT; git_oid_fromstr(&entry.id, "d427e0b2e138501a3d15cc376077a3631e15bd46"); - entry.path = "conflicts.txt"; + entry.path = path; cl_git_pass(git_index_add(index, &entry)); entry.flags = 2 << GIT_IDXENTRY_STAGESHIFT; @@ -919,7 +919,7 @@ void test_checkout_tree__fails_when_conflicts_exist_in_index(void) cl_git_pass(git_reference_name_to_id(&oid, g_repo, "HEAD")); cl_git_pass(git_object_lookup(&obj, g_repo, &oid, GIT_OBJ_ANY)); - create_conflict(); + create_conflict("conflicts.txt"); cl_git_fail(git_checkout_tree(g_repo, obj, &opts)); @@ -948,3 +948,87 @@ void test_checkout_tree__filemode_preserved_in_index(void) git_commit_free(commit); git_index_free(index); } + +void test_checkout_tree__removes_conflicts(void) +{ + git_oid commit_id; + git_commit *commit; + git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT; + git_index *index; + + cl_git_pass(git_oid_fromstr(&commit_id, "afe4393b2b2a965f06acf2ca9658eaa01e0cd6b6")); + cl_git_pass(git_commit_lookup(&commit, g_repo, &commit_id)); + + opts.checkout_strategy = GIT_CHECKOUT_FORCE; + + cl_git_pass(git_checkout_tree(g_repo, (const git_object *)commit, &opts)); + + cl_git_pass(git_repository_index(&index, g_repo)); + cl_git_pass(git_index_remove(index, "executable.txt", 0)); + + create_conflict("executable.txt"); + cl_git_mkfile("testrepo/executable.txt", "This is the conflict file.\n"); + + create_conflict("other.txt"); + cl_git_mkfile("testrepo/other.txt", "This is another conflict file.\n"); + + git_index_write(index); + + cl_git_pass(git_checkout_tree(g_repo, (const git_object *)commit, &opts)); + + cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 1)); + cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 2)); + cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 3)); + + cl_assert_equal_p(NULL, git_index_get_bypath(index, "other.txt", 1)); + cl_assert_equal_p(NULL, git_index_get_bypath(index, "other.txt", 2)); + cl_assert_equal_p(NULL, git_index_get_bypath(index, "other.txt", 3)); + + cl_assert(!git_path_exists("testrepo/other.txt")); + + git_index_free(index); +} + + +void test_checkout_tree__removes_conflicts_only_by_pathscope(void) +{ + git_oid commit_id; + git_commit *commit; + git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT; + git_index *index; + const char *path = "executable.txt"; + + cl_git_pass(git_oid_fromstr(&commit_id, "afe4393b2b2a965f06acf2ca9658eaa01e0cd6b6")); + cl_git_pass(git_commit_lookup(&commit, g_repo, &commit_id)); + + opts.checkout_strategy = GIT_CHECKOUT_FORCE; + opts.paths.count = 1; + opts.paths.strings = (char **)&path; + + cl_git_pass(git_checkout_tree(g_repo, (const git_object *)commit, &opts)); + + cl_git_pass(git_repository_index(&index, g_repo)); + cl_git_pass(git_index_remove(index, "executable.txt", 0)); + + create_conflict("executable.txt"); + cl_git_mkfile("testrepo/executable.txt", "This is the conflict file.\n"); + + create_conflict("other.txt"); + cl_git_mkfile("testrepo/other.txt", "This is another conflict file.\n"); + + git_index_write(index); + + cl_git_pass(git_checkout_tree(g_repo, (const git_object *)commit, &opts)); + + cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 1)); + cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 2)); + cl_assert_equal_p(NULL, git_index_get_bypath(index, "executable.txt", 3)); + + cl_assert(git_index_get_bypath(index, "other.txt", 1) != NULL); + cl_assert(git_index_get_bypath(index, "other.txt", 2) != NULL); + cl_assert(git_index_get_bypath(index, "other.txt", 3) != NULL); + + cl_assert(git_path_exists("testrepo/other.txt")); + + git_index_free(index); +} From 2d24816b4611a7392617855f497f81f385092f34 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 6 Nov 2014 18:49:37 -0500 Subject: [PATCH 2/2] checkout_index: Remove stage 0 when checking out conflicts --- src/checkout.c | 20 ++++++++++++++--- tests/checkout/index.c | 49 +++++++++++++++++++++++++++++++++--------- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index 8ffd40d5d..8203c39ea 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -1912,6 +1912,20 @@ done: return error; } +static int checkout_conflict_add( + checkout_data *data, + const git_index_entry *conflict) +{ + int error = git_index_remove(data->index, conflict->path, 0); + + if (error == GIT_ENOTFOUND) + giterr_clear(); + else if (error < 0) + return error; + + return git_index_add(data->index, conflict); +} + static int checkout_conflict_update_index( checkout_data *data, checkout_conflictdata *conflict) @@ -1919,13 +1933,13 @@ static int checkout_conflict_update_index( int error = 0; if (conflict->ancestor) - error = git_index_add(data->index, conflict->ancestor); + error = checkout_conflict_add(data, conflict->ancestor); if (!error && conflict->ours) - error = git_index_add(data->index, conflict->ours); + error = checkout_conflict_add(data, conflict->ours); if (!error && conflict->theirs) - error = git_index_add(data->index, conflict->theirs); + error = checkout_conflict_add(data, conflict->theirs); return error; } diff --git a/tests/checkout/index.c b/tests/checkout/index.c index 3c01e2411..f94556214 100644 --- a/tests/checkout/index.c +++ b/tests/checkout/index.c @@ -619,17 +619,14 @@ void test_checkout_index__can_get_repo_from_index(void) git_index_free(index); } -static void add_conflict(void) +static void add_conflict(git_index *index, const char *path) { - git_index *index; git_index_entry entry; memset(&entry, 0, sizeof(git_index_entry)); - cl_git_pass(git_repository_index(&index, g_repo)); - entry.mode = 0100644; - entry.path = "conflicting.txt"; + entry.path = path; git_oid_fromstr(&entry.id, "d427e0b2e138501a3d15cc376077a3631e15bd46"); entry.flags = (1 << GIT_IDXENTRY_STAGESHIFT); @@ -642,17 +639,19 @@ static void add_conflict(void) git_oid_fromstr(&entry.id, "2bd0a343aeef7a2cf0d158478966a6e587ff3863"); entry.flags = (3 << GIT_IDXENTRY_STAGESHIFT); cl_git_pass(git_index_add(index, &entry)); - - cl_git_pass(git_index_write(index)); - git_index_free(index); } void test_checkout_index__writes_conflict_file(void) { + git_index *index; git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT; git_buf conflicting_buf = GIT_BUF_INIT; - add_conflict(); + cl_git_pass(git_repository_index(&index, g_repo)); + + add_conflict(index, "conflicting.txt"); + cl_git_pass(git_index_write(index)); + cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); cl_git_pass(git_futils_readbuffer(&conflicting_buf, "testrepo/conflicting.txt")); @@ -663,18 +662,46 @@ void test_checkout_index__writes_conflict_file(void) "this file is changed in branch and master\n" ">>>>>>> theirs\n") == 0); git_buf_free(&conflicting_buf); + + git_index_free(index); +} + +void test_checkout_index__adding_conflict_removes_stage_0(void) +{ + git_index *new_index, *index; + git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT; + + cl_git_pass(git_index_new(&new_index)); + + add_conflict(new_index, "new.txt"); + cl_git_pass(git_checkout_index(g_repo, new_index, &opts)); + + cl_git_pass(git_repository_index(&index, g_repo)); + + cl_assert(git_index_get_bypath(index, "new.txt", 0) == NULL); + cl_assert(git_index_get_bypath(index, "new.txt", 1) != NULL); + cl_assert(git_index_get_bypath(index, "new.txt", 2) != NULL); + cl_assert(git_index_get_bypath(index, "new.txt", 3) != NULL); + + git_index_free(index); + git_index_free(new_index); } void test_checkout_index__conflicts_honor_coreautocrlf(void) { #ifdef GIT_WIN32 + git_index *index; git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT; git_buf conflicting_buf = GIT_BUF_INIT; cl_git_pass(p_unlink("./testrepo/.gitattributes")); cl_repo_set_bool(g_repo, "core.autocrlf", true); - add_conflict(); + cl_git_pass(git_repository_index(&index, g_repo)); + + add_conflict(index, "conflicting.txt"); + cl_git_pass(git_index_write(index)); + cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); cl_git_pass(git_futils_readbuffer(&conflicting_buf, "testrepo/conflicting.txt")); @@ -685,5 +712,7 @@ void test_checkout_index__conflicts_honor_coreautocrlf(void) "this file is changed in branch and master\r\n" ">>>>>>> theirs\r\n") == 0); git_buf_free(&conflicting_buf); + + git_index_free(index); #endif }