From b60149ecedab39fffc8607896f653ef7ac81b1e2 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 31 Jan 2014 18:43:50 -0800 Subject: [PATCH 1/5] Tests merge when changes exist in workdir/index --- tests/merge/workdir/dirty.c | 182 ++++++++++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 tests/merge/workdir/dirty.c diff --git a/tests/merge/workdir/dirty.c b/tests/merge/workdir/dirty.c new file mode 100644 index 000000000..67026336b --- /dev/null +++ b/tests/merge/workdir/dirty.c @@ -0,0 +1,182 @@ +#include "clar_libgit2.h" +#include "git2/merge.h" +#include "buffer.h" +#include "merge.h" +#include "../merge_helpers.h" + +#define TEST_REPO_PATH "merge-resolve" +#define MERGE_BRANCH_OID "7cb63eed597130ba4abb87b3e544b85021905520" + +static git_repository *repo; +static git_index *repo_index; + +static char *unaffected[][4] = { + { "added-in-master.txt", NULL }, + { "changed-in-master.txt", NULL }, + { "unchanged.txt", NULL }, + { "added-in-master.txt", "changed-in-master.txt", NULL }, + { "added-in-master.txt", "unchanged.txt", NULL }, + { "changed-in-master.txt", "unchanged.txt", NULL }, + { "added-in-master.txt", "changed-in-master.txt", "unchanged.txt", NULL }, + { "new_file.txt", NULL }, + { "new_file.txt", "unchanged.txt", NULL }, + { NULL }, +}; + +static char *affected[][5] = { + { "automergeable.txt", NULL }, + { "changed-in-branch.txt", NULL }, + { "conflicting.txt", NULL }, + { "removed-in-branch.txt", NULL }, + { "automergeable.txt", "changed-in-branch.txt", NULL }, + { "automergeable.txt", "conflicting.txt", NULL }, + { "automergeable.txt", "removed-in-branch.txt", NULL }, + { "changed-in-branch.txt", "conflicting.txt", NULL }, + { "changed-in-branch.txt", "removed-in-branch.txt", NULL }, + { "conflicting.txt", "removed-in-branch.txt", NULL }, + { "automergeable.txt", "changed-in-branch.txt", "conflicting.txt", NULL }, + { "automergeable.txt", "changed-in-branch.txt", "removed-in-branch.txt", NULL }, + { "automergeable.txt", "conflicting.txt", "removed-in-branch.txt", NULL }, + { "changed-in-branch.txt", "conflicting.txt", "removed-in-branch.txt", NULL }, + { "automergeable.txt", "changed-in-branch.txt", "conflicting.txt", "removed-in-branch.txt", NULL }, + { NULL }, +}; + +void test_merge_workdir_dirty__initialize(void) +{ + repo = cl_git_sandbox_init(TEST_REPO_PATH); + git_repository_index(&repo_index, repo); +} + +void test_merge_workdir_dirty__cleanup(void) +{ + git_index_free(repo_index); + cl_git_sandbox_cleanup(); +} + +static void set_core_autocrlf_to(git_repository *repo, bool value) +{ + git_config *cfg; + + cl_git_pass(git_repository_config(&cfg, repo)); + cl_git_pass(git_config_set_bool(cfg, "core.autocrlf", value)); + + git_config_free(cfg); +} + +static int merge_branch(git_merge_result **result, int merge_file_favor, int checkout_strategy) +{ + git_oid their_oids[1]; + git_merge_head *their_heads[1]; + git_merge_opts opts = GIT_MERGE_OPTS_INIT; + int error; + + cl_git_pass(git_oid_fromstr(&their_oids[0], MERGE_BRANCH_OID)); + cl_git_pass(git_merge_head_from_id(&their_heads[0], repo, &their_oids[0])); + + opts.merge_tree_opts.file_favor = merge_file_favor; + opts.checkout_opts.checkout_strategy = checkout_strategy; + error = git_merge(result, repo, (const git_merge_head **)their_heads, 1, &opts); + + git_merge_head_free(their_heads[0]); + + return error; +} + +static void write_files(char *files[]) +{ + char *filename; + git_buf path = GIT_BUF_INIT, content = GIT_BUF_INIT; + size_t i; + + for (i = 0, filename = files[i]; filename; filename = files[++i]) { + git_buf_clear(&path); + git_buf_clear(&content); + + git_buf_printf(&path, "%s/%s", TEST_REPO_PATH, filename); + git_buf_printf(&content, "This is a dirty file in the working directory!\n\n" + "It will not be staged! Its filename is %s.\n", filename); + + cl_git_mkfile(path.ptr, content.ptr); + } + + git_buf_free(&path); + git_buf_free(&content); +} + +static void stage_random_files(char *files[]) +{ + char *filename; + size_t i; + + write_files(files); + + for (i = 0, filename = files[i]; filename; filename = files[++i]) + cl_git_pass(git_index_add_bypath(repo_index, filename)); +} + +static int merge_dirty_files(char *dirty_files[]) +{ + git_reference *head; + git_object *head_object; + git_merge_result *result = NULL; + int error; + + cl_git_pass(git_repository_head(&head, repo)); + cl_git_pass(git_reference_peel(&head_object, head, GIT_OBJ_COMMIT)); + cl_git_pass(git_reset(repo, head_object, GIT_RESET_HARD)); + + write_files(dirty_files); + + error = merge_branch(&result, 0, 0); + + git_merge_result_free(result); + git_object_free(head_object); + git_reference_free(head); + + return error; +} + +static int merge_staged_files(char *staged_files[]) +{ + git_merge_result *result = NULL; + int error; + + stage_random_files(staged_files); + + error = merge_branch(&result, 0, 0); + + git_merge_result_free(result); + + return error; +} + +void test_merge_workdir_dirty__unaffected_dirty_files_allowed(void) +{ + char **files; + size_t i; + + for (i = 0, files = unaffected[i]; files[0]; files = unaffected[++i]) + cl_git_pass(merge_dirty_files(files)); +} + +void test_merge_workdir_dirty__affected_dirty_files_disallowed(void) +{ + char **files; + size_t i; + + for (i = 0, files = affected[i]; files[0]; files = affected[++i]) + cl_git_fail(merge_dirty_files(files)); +} + +void test_merge_workdir_dirty__staged_files_in_index_disallowed(void) +{ + char **files; + size_t i; + + for (i = 0, files = unaffected[i]; files[0]; files = unaffected[++i]) + cl_git_fail(merge_staged_files(files)); + + for (i = 0, files = affected[i]; files[0]; files = affected[++i]) + cl_git_fail(merge_staged_files(files)); +} From 16eb8b7c0667ba819e85b7bb4b40c2f71f73e59a Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 1 Feb 2014 12:02:55 -0800 Subject: [PATCH 2/5] Tests merging staged files identical to result --- tests/merge/workdir/dirty.c | 70 +++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tests/merge/workdir/dirty.c b/tests/merge/workdir/dirty.c index 67026336b..23840c1c3 100644 --- a/tests/merge/workdir/dirty.c +++ b/tests/merge/workdir/dirty.c @@ -7,6 +7,20 @@ #define TEST_REPO_PATH "merge-resolve" #define MERGE_BRANCH_OID "7cb63eed597130ba4abb87b3e544b85021905520" +#define AUTOMERGEABLE_MERGED_FILE \ + "this file is changed in master\n" \ + "this file is automergeable\n" \ + "this file is automergeable\n" \ + "this file is automergeable\n" \ + "this file is automergeable\n" \ + "this file is automergeable\n" \ + "this file is automergeable\n" \ + "this file is automergeable\n" \ + "this file is changed in branch\n" + +#define CHANGED_IN_BRANCH_FILE \ + "changed in branch\n" + static git_repository *repo; static git_index *repo_index; @@ -42,6 +56,13 @@ static char *affected[][5] = { { NULL }, }; +static char *result_contents[4][6] = { + { "automergeable.txt", AUTOMERGEABLE_MERGED_FILE, NULL, NULL }, + { "changed-in-branch.txt", CHANGED_IN_BRANCH_FILE, NULL, NULL }, + { "automergeable.txt", AUTOMERGEABLE_MERGED_FILE, "changed-in-branch.txt", CHANGED_IN_BRANCH_FILE, NULL, NULL }, + { NULL } +}; + void test_merge_workdir_dirty__initialize(void) { repo = cl_git_sandbox_init(TEST_REPO_PATH); @@ -115,6 +136,37 @@ static void stage_random_files(char *files[]) cl_git_pass(git_index_add_bypath(repo_index, filename)); } +static void stage_content(char *content[]) +{ + git_reference *head; + git_object *head_object; + git_merge_result *result = NULL; + git_buf path = GIT_BUF_INIT; + char *filename, *text; + size_t i; + + cl_git_pass(git_repository_head(&head, repo)); + cl_git_pass(git_reference_peel(&head_object, head, GIT_OBJ_COMMIT)); + cl_git_pass(git_reset(repo, head_object, GIT_RESET_HARD)); + + for (i = 0, filename = content[i], text = content[++i]; + filename && text; + filename = content[++i], text = content[++i]) { + + git_buf_clear(&path); + + cl_git_pass(git_buf_printf(&path, "%s/%s", TEST_REPO_PATH, filename)); + + cl_git_mkfile(path.ptr, text); + cl_git_pass(git_index_add_bypath(repo_index, filename)); + } + + git_merge_result_free(result); + git_object_free(head_object); + git_reference_free(head); + git_buf_free(&path); +} + static int merge_dirty_files(char *dirty_files[]) { git_reference *head; @@ -180,3 +232,21 @@ void test_merge_workdir_dirty__staged_files_in_index_disallowed(void) for (i = 0, files = affected[i]; files[0]; files = affected[++i]) cl_git_fail(merge_staged_files(files)); } + +void test_merge_workdir_dirty__identical_staged_files_allowed(void) +{ + git_merge_result *result; + char **content; + size_t i; + + set_core_autocrlf_to(repo, false); + + for (i = 0, content = result_contents[i]; content[0]; content = result_contents[++i]) { + stage_content(content); + + git_index_write(repo_index); + cl_git_pass(merge_branch(&result, 0, 0)); + + git_merge_result_free(result); + } +} From bb13d39162b0416585b34f18e9072fc4364d2655 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 1 Feb 2014 12:51:44 -0800 Subject: [PATCH 3/5] Test that emulates a strange filter implementation --- tests/merge/workdir/dirty.c | 70 +++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tests/merge/workdir/dirty.c b/tests/merge/workdir/dirty.c index 23840c1c3..625d3d01a 100644 --- a/tests/merge/workdir/dirty.c +++ b/tests/merge/workdir/dirty.c @@ -3,6 +3,7 @@ #include "buffer.h" #include "merge.h" #include "../merge_helpers.h" +#include "posix.h" #define TEST_REPO_PATH "merge-resolve" #define MERGE_BRANCH_OID "7cb63eed597130ba4abb87b3e544b85021905520" @@ -125,6 +126,41 @@ static void write_files(char *files[]) git_buf_free(&content); } +static void hack_index(char *files[]) +{ + char *filename; + struct stat statbuf; + git_buf path = GIT_BUF_INIT; + git_index_entry *entry; + size_t i; + + /* Update the index to suggest that checkout placed these files on + * disk, keeping the object id but updating the cache, which will + * emulate a Git implementation's different filter. + */ + for (i = 0, filename = files[i]; filename; filename = files[++i]) { + git_buf_clear(&path); + + cl_assert(entry = (git_index_entry *) + git_index_get_bypath(repo_index, filename, 0)); + + cl_git_pass(git_buf_printf(&path, "%s/%s", TEST_REPO_PATH, filename)); + cl_git_pass(p_stat(path.ptr, &statbuf)); + + entry->ctime.seconds = (git_time_t)statbuf.st_ctime; + entry->ctime.nanoseconds = 0; + entry->mtime.seconds = (git_time_t)statbuf.st_mtime; + entry->mtime.nanoseconds = 0; + entry->dev = statbuf.st_dev; + entry->ino = statbuf.st_ino; + entry->uid = statbuf.st_uid; + entry->gid = statbuf.st_gid; + entry->file_size = statbuf.st_size; + } + + git_buf_free(&path); +} + static void stage_random_files(char *files[]) { char *filename; @@ -189,6 +225,31 @@ static int merge_dirty_files(char *dirty_files[]) return error; } +static int merge_differently_filtered_files(char *files[]) +{ + git_reference *head; + git_object *head_object; + git_merge_result *result = NULL; + int error; + + cl_git_pass(git_repository_head(&head, repo)); + cl_git_pass(git_reference_peel(&head_object, head, GIT_OBJ_COMMIT)); + cl_git_pass(git_reset(repo, head_object, GIT_RESET_HARD)); + + write_files(files); + hack_index(files); + + cl_git_pass(git_index_write(repo_index)); + + error = merge_branch(&result, 0, 0); + + git_merge_result_free(result); + git_object_free(head_object); + git_reference_free(head); + + return error; +} + static int merge_staged_files(char *staged_files[]) { git_merge_result *result = NULL; @@ -250,3 +311,12 @@ void test_merge_workdir_dirty__identical_staged_files_allowed(void) git_merge_result_free(result); } } + +void test_merge_workdir_dirty__honors_cache(void) +{ + char **files; + size_t i; + + for (i = 0, files = affected[i]; files[0]; files = affected[++i]) + cl_git_pass(merge_differently_filtered_files(files)); +} From c0b10c25e02c6922276567600988fb2b85aeede1 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 1 Feb 2014 12:05:00 -0800 Subject: [PATCH 4/5] Merge wd validation tests against index not HEAD Validating the workdir should not compare HEAD to working directory - this is both inefficient (as it ignores the cache) and incorrect. If we had legitimately allowed changes in the index (identical to the merge result) then comparing HEAD to workdir would reject these changes as different. Further, this will identify files that were filtered strangely as modified, while testing with the cache would prevent this. Also, it's stupid slow. --- src/merge.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/merge.c b/src/merge.c index 20cfc0e23..2ae02e54d 100644 --- a/src/merge.c +++ b/src/merge.c @@ -2330,7 +2330,7 @@ done: static int merge_check_workdir(size_t *conflicts, git_repository *repo, git_index *index_new, git_vector *merged_paths) { - git_tree *head_tree = NULL; + git_index *index_repo = NULL; git_diff *wd_diff_list = NULL; git_diff_options opts = GIT_DIFF_OPTIONS_INIT; int error = 0; @@ -2341,9 +2341,6 @@ static int merge_check_workdir(size_t *conflicts, git_repository *repo, git_inde opts.flags |= GIT_DIFF_INCLUDE_UNTRACKED; - if ((error = git_repository_head_tree(&head_tree, repo)) < 0) - goto done; - /* Workdir changes may exist iff they do not conflict with changes that * will be applied by the merge (including conflicts). Ensure that there * are no changes in the workdir to these paths. @@ -2351,13 +2348,13 @@ static int merge_check_workdir(size_t *conflicts, git_repository *repo, git_inde opts.pathspec.count = merged_paths->length; opts.pathspec.strings = (char **)merged_paths->contents; - if ((error = git_diff_tree_to_workdir(&wd_diff_list, repo, head_tree, &opts)) < 0) + if ((error = git_diff_index_to_workdir(&wd_diff_list, repo, index_repo, &opts)) < 0) goto done; *conflicts = wd_diff_list->deltas.length; done: - git_tree_free(head_tree); + git_index_free(index_repo); git_diff_free(wd_diff_list); return error; From dbfd83bc6567763e6c363c462e9dd0628eaf3fe6 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 3 Feb 2014 19:56:13 -0800 Subject: [PATCH 5/5] Remove unused pointer assignment --- src/merge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/merge.c b/src/merge.c index 2ae02e54d..ac973efd0 100644 --- a/src/merge.c +++ b/src/merge.c @@ -2397,7 +2397,7 @@ int git_merge__indexes(git_repository *repo, git_index *index_new) /* Remove removed items from the index */ git_vector_foreach(&paths, i, path) { - if ((e = git_index_get_bypath(index_new, path, 0)) == NULL) { + if (git_index_get_bypath(index_new, path, 0) == NULL) { if ((error = git_index_remove(index_repo, path, 0)) < 0 && error != GIT_ENOTFOUND) goto done;