From 5f69a31f7d706aa5788ad9937391577a66e3c77d Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 24 Sep 2012 20:52:34 -0700 Subject: Initial implementation of new diff patch API Replacing the `git_iterator` object, this creates a simple API for accessing the "patch" for any file pair in a diff list and then gives indexed access to the hunks in the patch and the lines in the hunk. This is the initial implementation of this revised API - it is still broken, but at least builds cleanly. --- tests-clar/diff/diffiter.c | 275 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 208 insertions(+), 67 deletions(-) (limited to 'tests-clar/diff/diffiter.c') diff --git a/tests-clar/diff/diffiter.c b/tests-clar/diff/diffiter.c index 23071e48b..9e33d91e1 100644 --- a/tests-clar/diff/diffiter.c +++ b/tests-clar/diff/diffiter.c @@ -14,11 +14,16 @@ void test_diff_diffiter__create(void) { git_repository *repo = cl_git_sandbox_init("attr"); git_diff_list *diff; - git_diff_iterator *iter; + size_t d, num_d; cl_git_pass(git_diff_workdir_to_index(repo, NULL, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); - git_diff_iterator_free(iter); + + num_d = git_diff_num_deltas(diff); + for (d = 0; d < num_d; ++d) { + git_diff_delta *delta; + cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); + } + git_diff_list_free(diff); } @@ -26,24 +31,22 @@ void test_diff_diffiter__iterate_files(void) { git_repository *repo = cl_git_sandbox_init("attr"); git_diff_list *diff; - git_diff_iterator *iter; - git_diff_delta *delta; - int error, count = 0; + size_t d, num_d; + int count = 0; cl_git_pass(git_diff_workdir_to_index(repo, NULL, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); - while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); + num_d = git_diff_num_deltas(diff); + cl_assert_equal_i(6, num_d); + + for (d = 0; d < num_d; ++d) { + git_diff_delta *delta; + cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); cl_assert(delta != NULL); count++; } - - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert(delta == NULL); cl_assert_equal_i(6, count); - git_diff_iterator_free(iter); git_diff_list_free(diff); } @@ -51,24 +54,22 @@ void test_diff_diffiter__iterate_files_2(void) { git_repository *repo = cl_git_sandbox_init("status"); git_diff_list *diff; - git_diff_iterator *iter; - git_diff_delta *delta; - int error, count = 0; + size_t d, num_d; + int count = 0; cl_git_pass(git_diff_workdir_to_index(repo, NULL, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); - while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); + num_d = git_diff_num_deltas(diff); + cl_assert_equal_i(8, num_d); + + for (d = 0; d < num_d; ++d) { + git_diff_delta *delta; + cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); cl_assert(delta != NULL); count++; } - - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert(delta == NULL); cl_assert_equal_i(8, count); - git_diff_iterator_free(iter); git_diff_list_free(diff); } @@ -77,12 +78,8 @@ void test_diff_diffiter__iterate_files_and_hunks(void) git_repository *repo = cl_git_sandbox_init("status"); git_diff_options opts = {0}; git_diff_list *diff = NULL; - git_diff_iterator *iter; - git_diff_delta *delta; - git_diff_range *range; - const char *header; - size_t header_len; - int error, file_count = 0, hunk_count = 0; + size_t d, num_d; + int file_count = 0, hunk_count = 0; opts.context_lines = 3; opts.interhunk_lines = 1; @@ -90,28 +87,42 @@ void test_diff_diffiter__iterate_files_and_hunks(void) cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); + num_d = git_diff_num_deltas(diff); + + for (d = 0; d < num_d; ++d) { + git_diff_patch *patch; + git_diff_delta *delta; + size_t h, num_h; + + cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); - while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); cl_assert(delta); + cl_assert(patch); file_count++; - while ((error = git_diff_iterator_next_hunk( - &range, &header, &header_len, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); + num_h = git_diff_patch_num_hunks(patch); + + for (h = 0; h < num_h; h++) { + git_diff_range *range; + const char *header; + size_t header_len, num_l; + + cl_git_pass(git_diff_patch_get_hunk( + &range, &header, &header_len, &num_l, patch, h)); + cl_assert(range); + cl_assert(header); + hunk_count++; } + + git_diff_patch_free(patch); } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert(delta == NULL); cl_assert_equal_i(13, file_count); cl_assert_equal_i(8, hunk_count); - git_diff_iterator_free(iter); git_diff_list_free(diff); } @@ -120,45 +131,42 @@ void test_diff_diffiter__max_size_threshold(void) git_repository *repo = cl_git_sandbox_init("status"); git_diff_options opts = {0}; git_diff_list *diff = NULL; - git_diff_iterator *iter; - git_diff_delta *delta; - int error, file_count = 0, binary_count = 0, hunk_count = 0; + int file_count = 0, binary_count = 0, hunk_count = 0; + size_t d, num_d; opts.context_lines = 3; opts.interhunk_lines = 1; opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED; cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); + num_d = git_diff_num_deltas(diff); - while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); + for (d = 0; d < num_d; ++d) { + git_diff_patch *patch; + git_diff_delta *delta; + + cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); cl_assert(delta); + cl_assert(patch); file_count++; - - hunk_count += git_diff_iterator_num_hunks_in_file(iter); + hunk_count += git_diff_patch_num_hunks(patch); assert(delta->binary == 0 || delta->binary == 1); - binary_count += delta->binary; - } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert(delta == NULL); + git_diff_patch_free(patch); + } cl_assert_equal_i(13, file_count); cl_assert_equal_i(0, binary_count); cl_assert_equal_i(8, hunk_count); - git_diff_iterator_free(iter); git_diff_list_free(diff); /* try again with low file size threshold */ - file_count = 0; - binary_count = 0; - hunk_count = 0; + file_count = binary_count = hunk_count = 0; opts.context_lines = 3; opts.interhunk_lines = 1; @@ -166,36 +174,169 @@ void test_diff_diffiter__max_size_threshold(void) opts.max_size = 50; /* treat anything over 50 bytes as binary! */ cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff)); - cl_git_pass(git_diff_iterator_new(&iter, diff)); + num_d = git_diff_num_deltas(diff); - while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) { - cl_assert_equal_i(0, error); - cl_assert(delta); + for (d = 0; d < num_d; ++d) { + git_diff_patch *patch; + git_diff_delta *delta; - file_count++; + cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); - hunk_count += git_diff_iterator_num_hunks_in_file(iter); + file_count++; + hunk_count += git_diff_patch_num_hunks(patch); assert(delta->binary == 0 || delta->binary == 1); - binary_count += delta->binary; - } - cl_assert_equal_i(GIT_ITEROVER, error); - cl_assert(delta == NULL); + git_diff_patch_free(patch); + } cl_assert_equal_i(13, file_count); - /* Three files are over the 50 byte threshold: * - staged_changes_file_deleted * - staged_changes_modified_file * - staged_new_file_modified_file */ cl_assert_equal_i(3, binary_count); - cl_assert_equal_i(5, hunk_count); - git_diff_iterator_free(iter); git_diff_list_free(diff); +} + + +void test_diff_diffiter__iterate_all(void) +{ + git_repository *repo = cl_git_sandbox_init("status"); + git_diff_options opts = {0}; + git_diff_list *diff = NULL; + diff_expects exp = {0}; + size_t d, num_d; + + opts.context_lines = 3; + opts.interhunk_lines = 1; + opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED; + + cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff)); + + num_d = git_diff_num_deltas(diff); + for (d = 0; d < num_d; ++d) { + git_diff_patch *patch; + git_diff_delta *delta; + size_t h, num_h; + + cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); + cl_assert(patch && delta); + exp.files++; + + num_h = git_diff_patch_num_hunks(patch); + for (h = 0; h < num_h; h++) { + git_diff_range *range; + const char *header; + size_t header_len, l, num_l; + + cl_git_pass(git_diff_patch_get_hunk( + &range, &header, &header_len, &num_l, patch, h)); + cl_assert(range && header); + exp.hunks++; + + for (l = 0; l < num_l; ++l) { + char origin; + const char *content; + size_t content_len; + + cl_git_pass(git_diff_patch_get_line_in_hunk( + &origin, &content, &content_len, NULL, NULL, patch, h, l)); + cl_assert(content); + exp.lines++; + } + } + + git_diff_patch_free(patch); + } + + cl_assert_equal_i(13, exp.files); + cl_assert_equal_i(8, exp.hunks); + cl_assert_equal_i(13, exp.lines); + + git_diff_list_free(diff); +} + +static void iterate_over_patch(git_diff_patch *patch, diff_expects *exp) +{ + size_t h, num_h = git_diff_patch_num_hunks(patch); + + exp->files++; + exp->hunks += num_h; + + /* let's iterate in reverse, just because we can! */ + for (h = 1; h <= num_h; ++h) + exp->lines += git_diff_patch_num_lines_in_hunk(patch, num_h - h); +} + +#define PATCH_CACHE 5 + +void test_diff_diffiter__iterate_randomly_while_saving_state(void) +{ + git_repository *repo = cl_git_sandbox_init("status"); + git_diff_options opts = {0}; + git_diff_list *diff = NULL; + diff_expects exp = {0}; + git_diff_patch *patches[PATCH_CACHE]; + size_t p, d, num_d; + + memset(patches, 0, sizeof(patches)); + + opts.context_lines = 3; + opts.interhunk_lines = 1; + opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED; + + cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff)); + + num_d = git_diff_num_deltas(diff); + + /* To make sure that references counts work for diff and patch objects, + * this generates patches and randomly caches them. Only when the patch + * is removed from the cache are hunks and lines counted. At the end, + * there are still patches in the cache, so free the diff and try to + * process remaining patches after the diff is freed. + */ + + srand(121212); + p = rand() % PATCH_CACHE; + + for (d = 0; d < num_d; ++d) { + /* take old patch */ + git_diff_patch *patch = patches[p]; + patches[p] = NULL; + + /* cache new patch */ + cl_git_pass(git_diff_get_patch(&patches[p], NULL, diff, d)); + cl_assert(patches[p] != NULL); + + /* process old patch if non-NULL */ + if (patch != NULL) { + iterate_over_patch(patch, &exp); + git_diff_patch_free(patch); + } + + p = rand() % PATCH_CACHE; + } + + /* free diff list now - refcounts should keep things safe */ + git_diff_list_free(diff); + + /* process remaining unprocessed patches */ + for (p = 0; p < PATCH_CACHE; p++) { + git_diff_patch *patch = patches[p]; + + if (patch != NULL) { + iterate_over_patch(patch, &exp); + git_diff_patch_free(patch); + } + } + /* hopefully it all still added up right */ + cl_assert_equal_i(13, exp.files); + cl_assert_equal_i(8, exp.hunks); + cl_assert_equal_i(13, exp.lines); } -- cgit v1.2.1 From 642863086575a61b3ed0bbbe909f4f07d87ff9db Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 25 Sep 2012 10:48:50 -0700 Subject: Fix bugs in new diff patch code This fixes all the bugs in the new diff patch code. The only really interesting one is that when we merge two diffs, we now have to actually exclude diff delta records that are not supposed to be tracked, as opposed to before where they could be included because they would be skipped silently by `git_diff_foreach()`. Other than that, there are just minor errors. --- tests-clar/diff/diffiter.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'tests-clar/diff/diffiter.c') diff --git a/tests-clar/diff/diffiter.c b/tests-clar/diff/diffiter.c index 9e33d91e1..4273b16dd 100644 --- a/tests-clar/diff/diffiter.c +++ b/tests-clar/diff/diffiter.c @@ -256,21 +256,23 @@ void test_diff_diffiter__iterate_all(void) cl_assert_equal_i(13, exp.files); cl_assert_equal_i(8, exp.hunks); - cl_assert_equal_i(13, exp.lines); + cl_assert_equal_i(14, exp.lines); git_diff_list_free(diff); } static void iterate_over_patch(git_diff_patch *patch, diff_expects *exp) { - size_t h, num_h = git_diff_patch_num_hunks(patch); + size_t h, num_h = git_diff_patch_num_hunks(patch), num_l; exp->files++; exp->hunks += num_h; /* let's iterate in reverse, just because we can! */ - for (h = 1; h <= num_h; ++h) - exp->lines += git_diff_patch_num_lines_in_hunk(patch, num_h - h); + for (h = 1, num_l = 0; h <= num_h; ++h) + num_l += git_diff_patch_num_lines_in_hunk(patch, num_h - h); + + exp->lines += num_l; } #define PATCH_CACHE 5 @@ -338,5 +340,5 @@ void test_diff_diffiter__iterate_randomly_while_saving_state(void) /* hopefully it all still added up right */ cl_assert_equal_i(13, exp.files); cl_assert_equal_i(8, exp.hunks); - cl_assert_equal_i(13, exp.lines); + cl_assert_equal_i(14, exp.lines); } -- cgit v1.2.1 From bae957b95d59a840df72a725b06f00635471cfd8 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 25 Sep 2012 16:31:46 -0700 Subject: Add const to all shared pointers in diff API There are a lot of places where the diff API gives the user access to internal data structures and many of these were being exposed through non-const pointers. This replaces them all with const pointers for any object that the user can access but is still owned internally to the git_diff_list or git_diff_patch objects. This will probably break some bindings... Sorry! --- tests-clar/diff/diffiter.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'tests-clar/diff/diffiter.c') diff --git a/tests-clar/diff/diffiter.c b/tests-clar/diff/diffiter.c index 4273b16dd..78f97fc86 100644 --- a/tests-clar/diff/diffiter.c +++ b/tests-clar/diff/diffiter.c @@ -20,7 +20,7 @@ void test_diff_diffiter__create(void) num_d = git_diff_num_deltas(diff); for (d = 0; d < num_d; ++d) { - git_diff_delta *delta; + const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); } @@ -40,7 +40,7 @@ void test_diff_diffiter__iterate_files(void) cl_assert_equal_i(6, num_d); for (d = 0; d < num_d; ++d) { - git_diff_delta *delta; + const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); cl_assert(delta != NULL); count++; @@ -63,7 +63,7 @@ void test_diff_diffiter__iterate_files_2(void) cl_assert_equal_i(8, num_d); for (d = 0; d < num_d; ++d) { - git_diff_delta *delta; + const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); cl_assert(delta != NULL); count++; @@ -91,7 +91,7 @@ void test_diff_diffiter__iterate_files_and_hunks(void) for (d = 0; d < num_d; ++d) { git_diff_patch *patch; - git_diff_delta *delta; + const git_diff_delta *delta; size_t h, num_h; cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); @@ -104,7 +104,7 @@ void test_diff_diffiter__iterate_files_and_hunks(void) num_h = git_diff_patch_num_hunks(patch); for (h = 0; h < num_h; h++) { - git_diff_range *range; + const git_diff_range *range; const char *header; size_t header_len, num_l; @@ -143,7 +143,7 @@ void test_diff_diffiter__max_size_threshold(void) for (d = 0; d < num_d; ++d) { git_diff_patch *patch; - git_diff_delta *delta; + const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); cl_assert(delta); @@ -178,7 +178,7 @@ void test_diff_diffiter__max_size_threshold(void) for (d = 0; d < num_d; ++d) { git_diff_patch *patch; - git_diff_delta *delta; + const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); @@ -221,7 +221,7 @@ void test_diff_diffiter__iterate_all(void) num_d = git_diff_num_deltas(diff); for (d = 0; d < num_d; ++d) { git_diff_patch *patch; - git_diff_delta *delta; + const git_diff_delta *delta; size_t h, num_h; cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); @@ -230,7 +230,7 @@ void test_diff_diffiter__iterate_all(void) num_h = git_diff_patch_num_hunks(patch); for (h = 0; h < num_h; h++) { - git_diff_range *range; + const git_diff_range *range; const char *header; size_t header_len, l, num_l; -- cgit v1.2.1